httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rainer Jung <rainer.j...@kippdata.de>
Subject Re: mod_speling changes in trunk and backport
Date Wed, 02 Mar 2016 09:59:49 GMT
Am 02.03.2016 um 08:03 schrieb Christophe JAILLET:
> Le 23/02/2016 20:24, Rainer Jung a écrit :
>> There were two changes to mod_speling in trunk in 2013/2014. They were
>> motivated by PR 44221: although the docs claim that using
>> "CheckCaseOnly On" "limits the action of the spelling correction to
>> lower/upper case changes. Other potential corrections are not
>> performed." In fact mod_speling still checks for content files which
>> only differ in the file name suffix form the request URI. That part of
>> the code was contained in an #ifdef but active by default. The changes
>> introduce a new directive that allows to control that part of the
>> behavior by configuration.
>>
>> Now the question: it seems to me that due to the change being done in
>> two steps, now the directive CheckCaseOnly and the new one
>> CheckBasenameMatch are not independent of each other. Setting
>> CheckBasenameMatch  to "On" only enables the suffix handling if
>> CheckCaseOnly if "Off". For improved compatibility and for better
>> orthogonality I would suggest to keep the features separate from each
>> other, ie.:
>>
>> Index: modules/mappers/mod_speling.c
>> ===================================================================
>> --- modules/mappers/mod_speling.c       (revision 1731929)
>> +++ modules/mappers/mod_speling.c       (working copy)
>> @@ -326,8 +326,7 @@
>>           * because it won't find anything matching that spelling.
>>           * With the extension-munging, it would locate "foobar.html".
>>           */
>> -        else if ((cfg->check_case_only == 0) &&
>> -                 (cfg->check_basename_match == 1)) {
>> +        else if (cfg->check_basename_match == 1) {
>>              /*
>>               * Okay... we didn't find anything. Now we take out the
>> hard-core
>>               * power tools. There are several cases here. Someone
>> might have
>>
>>
>> That way the old changes could be ported back to 2.4 with the only
>> change that the default for CheckBasenameMatch would be "On" in 2.4
>> for compatibility reasons and "Off" in trunk to satisfy the principle
>> of least surprise.
>>
>> Regards,
>>
>> Rainer
>>
> Hi,
>
> One other remark from Eric at the time being [1] was that is also
> changed the legacy behavior.
> Having CheckBasenameMatch default to 1 (at least for 2.4.x) would keep
> compatibility with existing config.
>
> Keeping the 2 options independent would also require a tweak in the doc.
>
> CJ
>
> [1]
> http://mail-archives.apache.org/mod_mbox/httpd-cvs/201401.mbox/%3C20140128052621.21F91238899C@eris.apache.org%3E

I already tweaked the trunk docs (hopefully understandable) and indeed 
plan to backport with a different default for CheckBasenameMatch.

I haven't yet proposed the backport, because I don't like that 
mod_speling has no merge code for the config. IMHO that means if a 
config has a hierarchy of mod_speling settings, non-set values will 
always be reset to their defaults in the non-merging. I plan to add 
merging using a "set" flag.

Regards,

Rainer


Mime
View raw message