Created attachment 24165 [details]
Just make an else into else if... Simplest of fixes.

Done.  This should fix the issue.

I just noticed that this problem is fixed in the current version, but still having status NEW.

I'm testing this on a apache 2.2.16 Unix fresh install but I'm still running into exact the same problems as submitted in 2008.

It's unexpected (and not wanted) behaviour when apache lists a few "multiple choices" (status code 300) when the checkCaseOnly directive is on!!

I have this in my httpd.conf:
CheckSpelling on
CheckCaseOnly on

first directive to use the mod_speling
second directive to limit only to case corrections

I looked at code, nothing has been changed to address the issue. Rainer, not sure what you saw as being "fixed".

My patch would change what some are  used to, but bring it to what I consider expected behavior.

If I remember correctly, Rob, I thought that your fix was an official one.

I'm experiencing this issue in version 2.2.17.

My config:
<IfModule mod_speling.c>
CheckCaseOnly On
CheckSpelling On
</IfModule>


Example:
1. Navigate to non-existing resource - https://[URL]/js/jquery.ui.monthpicker.js

2. Results in 300 Multiple Choices - Similar documents found with a single folder found -  /js/jQuery (common basename) 


I have reviewed the source of mod_speling.c from version 2.2.15 up to 2.3-trunk and there has been no change/patch implemented; all versions are affected.

If CheckCaseOnly was 'Off' I'd expect this result; however with it set to 'On' this is incorrect behavior and goes against what CheckCaseOnly implies.

In addition, I like Rainer's suggestion of making WANT_BASENAME_MATCH a configuration directive with support for context; as this would be a great addition to this module. It could be defaulted to On/true for backward compatibility and allow for those experiencing issues to set it accordingly.

I was just about filing a new bug report about this, when I found it was already discovered four years ago. Cmon, this should be really easy to fix, and it can also be considered a bit of security related, as it may lead to disclosure of information.

Fixed in trunk r1505279.


I will also propose a new option CheckBasenameMatch as suggested by Rainer Perske.

I'm still seeing this problem in 2.2.22 on Ubuntu.  I don't see any mention of mod_speling fixes in the change log up through 2.2.27 with the last mention being back in 2.2.9.

Did (or will) this patch make it into the 2.2 branch?  Or am I missing something?

It has been proposed to 2.4 but refused becasue it changed the default behaviour of the module, which is not what is expected on a stable branch.

It has not been proposed for backport for 2.2, but it would have been refused for the same reason.

I have planned to rework the patch to avoid the change of default behaviour but have not done it yet.


Anyway, I doubt it would be backported in 2.2, even reworked.

To clarify the issue, it's expected that apache gives a "Multiple Choices" list with CheckCaseOnly directive on, if there is "sOmE.file" and "SoMe.file" in the same directory. I believe most people didn't expect a list with "foo.gif" and "foo.jpg" when requesting "foo.bar", but that's the current behaviour and the proposed patch breaks it.

The proposed alternative, make WANT_BASENAME_MATCH a configuration directive, is a better solution. It is a controversial option, and the only way to solve it and make everyone happy is let each one choose.

Created attachment 32307 [details]
Make "WANT_BASENAME_MATCH" configurable

Here is a patch to add the proposed "CheckBasenameMatch" config directive.

It replaces the hard-coded WANT_BASENAME_MATCH and defaults to "on" so it maintains compatibility.

Note that this patch is not compatible (and obsoletes/replaces) the one contributed by Rob on 2009-08-25. Setting "CheckBasenameMatch" to "off" will have the same effect.

The new patch is actually against 2.4.10 code but that shouldn't be an issue, I presume.

I'm willing to provide a patch against the docs as well, if there is a reasonable chance that the source code patch will be accepted.

As this bug is also present in 2.4.10, I changed the version number accordingly. It seems at 2.2.x there is nobody home anymore.

The patch by apache@skopos.be seems to be totally adequate to fix this issue once and for all in a backwards compatible manner. Any reason why it hasn't been applied yet? The current behavior is simply broken and counterintuitive.

Hi!

Once patch is applied the issue could be verified by this:

( https://github.com/melezhik/apache-swat  - is easy way to verify some existed apache issues against your environment ).

At my environment tests suite fails ( which is expected ? )

vagrant@Debian-jessie-amd64-netboot:~/my/apache-swat$ swat -t 44221
/home/vagrant/.swat/.cache/9836/prove/44221/FOO.bar/00.GET.t ...
ok 1 - GET 127.0.0.1/44221/FOO.bar succeeded
# http headers saved to /home/vagrant/.swat/.cache/9836/prove/xeu0b4EQHn.hdr
# body saved to /home/vagrant/.swat/.cache/9836/prove/xeu0b4EQHn
ok 2 - output match '200 OK'
ok 3 - output match /Location: \S+/
ok 4 - 'Location: http://127.0.0.1/44221/foo.bar' match 'foo.bar'
1..4
ok
/home/vagrant/.swat/.cache/9836/prove/44221/foo.html/00.GET.t ..
ok 1 - GET 127.0.0.1/44221/foo.html succeeded
# http headers saved to /home/vagrant/.swat/.cache/9836/prove/ipywRk4nnn.hdr
# body saved to /home/vagrant/.swat/.cache/9836/prove/ipywRk4nnn
ok 2 - output match /HTTP\/(\S+) (\d+) \S+/
not ok 3 - 'HTTP/1.1 300 Multiple Choices' match /404 /

#   Failed test ''HTTP/1.1 300 Multiple Choices' match /404 /'
#   at /usr/local/share/perl/5.20.2/swat.pm line 218.
not ok 4 - output match 'Not Found'

#   Failed test 'output match 'Not Found''
#   at /usr/local/share/perl/5.20.2/swat.pm line 218.
1..4
# Looks like you failed 2 tests of 4.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/4 subtests
/home/vagrant/.swat/.cache/9836/prove/44221/foo.bar/00.GET.t ...
ok 1 - GET 127.0.0.1/44221/foo.bar succeeded
# http headers saved to /home/vagrant/.swat/.cache/9836/prove/MuDUZpFoR3.hdr
# body saved to /home/vagrant/.swat/.cache/9836/prove/MuDUZpFoR3
ok 2 - output match '200 OK'
1..2
ok
/home/vagrant/.swat/.cache/9836/prove/44221/foo.baz/00.GET.t ...
ok 1 - GET 127.0.0.1/44221/foo.baz succeeded
# http headers saved to /home/vagrant/.swat/.cache/9836/prove/jLZttcgKYZ.hdr
# body saved to /home/vagrant/.swat/.cache/9836/prove/jLZttcgKYZ
ok 2 - output match '200 OK'
1..2
ok
/home/vagrant/.swat/.cache/9836/prove/44221/foo.BAR/00.GET.t ...
ok 1 - GET 127.0.0.1/44221/foo.BAR succeeded
# http headers saved to /home/vagrant/.swat/.cache/9836/prove/J91YGHyu9m.hdr
# body saved to /home/vagrant/.swat/.cache/9836/prove/J91YGHyu9m
ok 2 - output match '200 OK'
ok 3 - output match /Location: \S+/
ok 4 - 'Location: http://127.0.0.1/44221/foo.bar' match 'foo.bar'
1..4
ok

Test Summary Report
-------------------
/home/vagrant/.swat/.cache/9836/prove/44221/foo.html/00.GET.t (Wstat: 512 Tests: 4 Failed: 2)
  Failed tests:  3-4
  Non-zero exit status: 2
Files=5, Tests=16,  1 wallclock secs ( 0.03 usr  0.01 sys +  0.30 cusr  0.01 csys =  0.35 CPU)
Result: FAIL



My apache build info:


vagrant@Debian-jessie-amd64-netboot:~/my/apache-swat$ sudo ~/apache/bin/apachectl -V
Server version: Apache/2.4.18 (Unix)
Server built:   Jan 30 2016 10:17:37
Server's Module Magic Number: 20120211:52
Server loaded:  APR 1.5.1, APR-UTIL 1.5.4
Compiled using: APR 1.5.1, APR-UTIL 1.5.4
Architecture:   64-bit
Server MPM:     event
  threaded:     yes (fixed thread count)
    forked:     yes (variable process count)
Server compiled with....
 -D APR_HAS_SENDFILE
 -D APR_HAS_MMAP
 -D APR_HAVE_IPV6 (IPv4-mapped addresses enabled)
 -D APR_USE_SYSVSEM_SERIALIZE
 -D APR_USE_PTHREAD_SERIALIZE
 -D SINGLE_LISTEN_UNSERIALIZED_ACCEPT
 -D APR_HAS_OTHER_CHILD
 -D AP_HAVE_RELIABLE_PIPED_LOGS
 -D DYNAMIC_MODULE_LIMIT=256
 -D HTTPD_ROOT="/home/vagrant/apache/"
 -D SUEXEC_BIN="/home/vagrant/apache//bin/suexec"
 -D DEFAULT_PIDLOG="logs/httpd.pid"
 -D DEFAULT_SCOREBOARD="logs/apache_runtime_status"
 -D DEFAULT_ERRORLOG="logs/error_log"
 -D AP_TYPES_CONFIG_FILE="conf/mime.types"
 -D SERVER_CONFIG_FILE="conf/httpd.conf"

(In reply to Rui Baptista from comment #11)
> To clarify the issue, it's expected that apache gives a "Multiple Choices"
> list with CheckCaseOnly directive on, if there is "sOmE.file" and
> "SoMe.file" in the same directory. I believe most people didn't expect a
> list with "foo.gif" and "foo.jpg" when requesting "foo.bar", but that's the
> current behaviour and the proposed patch breaks it.

CheckCaseOnly means check case ONLY. That's clearly what the directive's name says, and ".bar! is clearly not a capitalization of ".gif" or ".jpg", which means this is simply an ugly, security related bug, nothing else. In no way this can be expected behavior.

Heh, I've been waiting 8 years for this patch to go through... I'm eternally hopeful, but Apache is unlikely to introduce a change that breaks existing behaviour, even if it does bring the code closer to expected.

But there already is an option to achieve what it currently does: CheckCaseOnly = Off ! >>:[

(In reply to Bachsau from comment #18)
> But there already is an option to achieve what it currently does:
> CheckCaseOnly = Off ! >>:[

PS: Another way to fix this could be to split it up: mod_spelling and mod_casing

I'm also still waiting for that patch to land. I doubt it is not going because of any compatibility concerns. With the attached patch there simply shouldn't be any. It defaults to the old behavior and simply made the previous compile time option to get the correct behavior available to the user at runtime.

Is anyone ever going to fix this? This module has seen some additions since I opened this bug. If you won't implement "a change that breaks existing behaviour", you can close the whole bug tracker, because every bug is existing behaviour

here seems to be some confusion here.

There are TWO patches that have been proposed to tackle this bug.

The first one, proposed by Rob in comment #1, 12 years ago, DOES break
current behavior.

The second one, proposed in comment 12, does NOT break it.

It simply replaces a hard-coded #define with a user configurable
parameter with a default ("on") that does nothing different from the current code.

To change current behavior, the user would have to go in and edit the
config file to set the "CheckBasenameMatch" to "off".


It is not clear (to me) how to read Alexey's test results in comment
#15. 

It would be good to know for sure which of the 2 patches were applied for this
test run, and - most important - if the tests also fail with an unpatched version.

If they do fail, he's testing for the expected result but he's seeing the buggy behavior instead, i.e. where the unpatched server wrongly replies to a request for a non-existing "foo.bar" with a Multiple Choices for "foo.gif" and "foo.jpg", instead of a "Not Found".

In any case, the issue here is "to break or not to break", so the proper way to evaluate the second patch is to compare a patched server with an unpatched one. They should both succeed (if testing for the *current* behavior) or both fail (if testing for the *expected* behavior).

And if both test runs fail, the next step would then to run them again with the (second) patch applied, but after setting "CheckBasenameMatch=off" in httpd.conf. If they now pass, the patch does indeed fix this bug.

Of course I could try and do that myself, but the proposed patch has met eight years of dead silence, so my motivation to try and fight it in is, well, rather low. I am applying it on my local build and that's OK for me.

Because nothing has changed since the bug report was submitted, the bug still exists in 2.4.48.

And because the patch, as proposed in comment #12, also applies fine against that source code, I have bumped the version accordingly.

So who is responsible for applying patches and integrating them into production?

It looks like a similar/conflicting patch is already in trunk but not backported to 2.4:

The initial rev was but there are followups:

r1732273 | rjung | 2016-02-25 05:09:09 -0500 (Thu, 25 Feb 2016) | 5 lines

mod_speling: make behavior of CheckCaseOnly and
the new CheckBasenameMatch orthogonal, so one
can combine them and they don't influence each
other.

(In reply to Bachsau from comment #24)
> So who is responsible for applying patches and integrating them into
> production?

Well, committers but we are all volontonteers and welcome new comers ;)
As for "production", I suppose it will be on you once it lands in a release.

(In reply to Eric Covener from comment #25)
> It looks like a similar/conflicting patch is already in trunk but not
> backported to 2.4:
> 
> The initial rev was but there are followups:
> 
> r1732273 | rjung | 2016-02-25 05:09:09 -0500 (Thu, 25 Feb 2016) | 5 lines
> 
> mod_speling: make behavior of CheckCaseOnly and
> the new CheckBasenameMatch orthogonal, so one
> can combine them and they don't influence each
> other.


whoops, wrong revision as that is obviously a followup.

r1557580 | jailletc36 | 2014-01-12 13:11:04 -0500 (Sun, 12 Jan 2014) | 3 lines

- Rename variable
- Remove #define WANT_BASENAME_MATCH and define a new option 'CheckBasenameMatch' to control this behaviour
- Remove outdated comments

I've proposed it for 2.4.x but we missed the bus for 2.4.49

Backported to 2.4.50.

Even if it took 13 years 8 months to solve the problem I reported:
Thank you very much.

(In reply to Rainer Perske from comment #30)
> Even if it took 13 years 8 months to solve the problem I reported:
> Thank you very much.

I've been following for 1 year less, and I'm also really glad it's been fixed. Happy to have gone on the ride with you Rainer :)

Cheers all