You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Eric Covener <co...@gmail.com> on 2012/10/04 18:32:42 UTC
Re: svn commit: r1032431 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_rewrite.c
On Sun, Nov 7, 2010 at 7:41 PM, <mi...@apache.org> wrote:
> Author: minfrin
> Date: Mon Nov 8 00:41:35 2010
> New Revision: 1032431
>
> URL: http://svn.apache.org/viewvc?rev=1032431&view=rev
> Log:
> mod_rewrite: Fix the RewriteEngine directive to work within a
> location. Previously, once RewriteEngine was switched on globally,
> it was impossible to switch off.
>
> + a->baseurl = (overrides->baseurl_set == 0) ? base->baseurl : overrides->baseurl;
> + a->baseurl_set = overrides->baseurl_set || base->baseurl_set;
PR https://issues.apache.org/bugzilla/show_bug.cgi?id=53963 points out
that mergeing the base is probably not very useful and actually harms
configs where no rewritebase is required in some subdirs but present
in parent dirs.
Any issue with dropping this part of the change?
(rewritebase is needed when a relative substitution is made and the
current URL and the current URL was reached via e.g. alias and not
under the documentroot)
Re: svn commit: r1032431 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_rewrite.c
Posted by Eric Covener <co...@gmail.com>.
On Thu, Oct 4, 2012 at 7:18 PM, Graham Leggett <mi...@sharp.fm> wrote:
> On 04 Oct 2012, at 9:18 PM, André Malo <nd...@perlig.de> wrote:
>
>> Hmm. The very sole concept of RewriteBase is "base for this location and
>> this location only".
>
> Exactly, which is why merging is the only sensible behaviour. "/foo/bar" is below location "/foo" at all times, regardless of whether someone has added <Location /foo/bar> to their config or not. Until someone has explicitly stated otherwise, /foo/bar should have the same RewriteBase as /foo.
No, it's not sensible. The default base URL has always been relative
to the context the rules appear in and can be overridden by the
directive. The directive should not always be set and should not be
copied from higher scopes. The copying breaks that even for containers
of Location /foo/ and Location /foo/bar/, much less traditional
Directory and htacess where RewtiteBase is generally more useful.
> Not merging properly means that the addition of a completely unrelated directive such as RewriteRule to a Location that previously had only inherited RewriteRules suddenly makes the RewriteBase vanish completely without warning.
With this particular directive, no RewriteBase is better than a bad
guess, because the default prefix of relative substitution tracks to
the context the rule is defined in.
Re: svn commit: r1032431 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_rewrite.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 04 Oct 2012, at 9:18 PM, André Malo <nd...@perlig.de> wrote:
> Hmm. The very sole concept of RewriteBase is "base for this location and
> this location only".
Exactly, which is why merging is the only sensible behaviour. "/foo/bar" is below location "/foo" at all times, regardless of whether someone has added <Location /foo/bar> to their config or not. Until someone has explicitly stated otherwise, /foo/bar should have the same RewriteBase as /foo.
Not merging properly means that the addition of a completely unrelated directive such as RewriteRule to a Location that previously had only inherited RewriteRules suddenly makes the RewriteBase vanish completely without warning.
We should not try and second guess what the user is trying to achieve. Have a sensible consistent behaviour and let the end user make up their own mind as to how to configure it.
> So merging is most certainly a nogo. It *will* break
> stuff. mod_rewrite already has enough bad guessings built in ;)
The idea is to take out the bad guessing, not maintain it.
Regards,
Graham
--
Re: svn commit: r1032431 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_rewrite.c
Posted by André Malo <nd...@perlig.de>.
* Eric Covener wrote:
> On Thu, Oct 4, 2012 at 2:05 PM, Graham Leggett <mi...@sharp.fm> wrote:
> > On 4 Oct 2012, at 17:32, Eric Covener <co...@gmail.com> wrote:
> >>> URL: http://svn.apache.org/viewvc?rev=1032431&view=rev
> >>> Log:
> >>> mod_rewrite: Fix the RewriteEngine directive to work within a
> >>> location. Previously, once RewriteEngine was switched on globally,
> >>> it was impossible to switch off.
> >>>
> >>> + a->baseurl = (overrides->baseurl_set == 0) ? base->baseurl :
> >>> overrides->baseurl; + a->baseurl_set = overrides->baseurl_set ||
> >>> base->baseurl_set;
> >>
> >> PR https://issues.apache.org/bugzilla/show_bug.cgi?id=53963 points out
> >> that mergeing the base is probably not very useful and actually harms
> >> configs where no rewritebase is required in some subdirs but present
> >> in parent dirs.
> >>
> >> Any issue with dropping this part of the change?
> >
> > Having an arbitrary inconsistent behaviour from a directive in a module
> > already noted for its complexity is a really bad thing. What we should
> > support instead is the ability to turn RewriteBase off if needed, with
> > something like "RewriteBase off".
>
> "Rewritebase off" to undo the copy of base URL from the parent to the
> subdir seems wrong. I'll try to merge in a value less likely to be
> wrong.
Hmm. The very sole concept of RewriteBase is "base for this location and
this location only". So merging is most certainly a nogo. It *will* break
stuff. mod_rewrite already has enough bad guessings built in ;)
nd
--
Da fällt mir ein, wieso gibt es eigentlich in Unicode kein
"i" mit einem Herzchen als Tüpfelchen? Das wär sooo süüss!
-- Björn Höhrmann in darw
Re: svn commit: r1032431 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_rewrite.c
Posted by Eric Covener <co...@gmail.com>.
On Thu, Oct 4, 2012 at 2:05 PM, Graham Leggett <mi...@sharp.fm> wrote:
> On 4 Oct 2012, at 17:32, Eric Covener <co...@gmail.com> wrote:
>
>>> URL: http://svn.apache.org/viewvc?rev=1032431&view=rev
>>> Log:
>>> mod_rewrite: Fix the RewriteEngine directive to work within a
>>> location. Previously, once RewriteEngine was switched on globally,
>>> it was impossible to switch off.
>>>
>>> + a->baseurl = (overrides->baseurl_set == 0) ? base->baseurl : overrides->baseurl;
>>> + a->baseurl_set = overrides->baseurl_set || base->baseurl_set;
>>
>>
>> PR https://issues.apache.org/bugzilla/show_bug.cgi?id=53963 points out
>> that mergeing the base is probably not very useful and actually harms
>> configs where no rewritebase is required in some subdirs but present
>> in parent dirs.
>>
>> Any issue with dropping this part of the change?
>
> Having an arbitrary inconsistent behaviour from a directive in a module already noted for its complexity is a really bad thing. What we should support instead is the ability to turn RewriteBase off if needed, with something like "RewriteBase off".
"Rewritebase off" to undo the copy of base URL from the parent to the
subdir seems wrong. I'll try to merge in a value less likely to be
wrong.
Re: svn commit: r1032431 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_rewrite.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 4 Oct 2012, at 17:32, Eric Covener <co...@gmail.com> wrote:
>> URL: http://svn.apache.org/viewvc?rev=1032431&view=rev
>> Log:
>> mod_rewrite: Fix the RewriteEngine directive to work within a
>> location. Previously, once RewriteEngine was switched on globally,
>> it was impossible to switch off.
>>
>> + a->baseurl = (overrides->baseurl_set == 0) ? base->baseurl : overrides->baseurl;
>> + a->baseurl_set = overrides->baseurl_set || base->baseurl_set;
>
>
> PR https://issues.apache.org/bugzilla/show_bug.cgi?id=53963 points out
> that mergeing the base is probably not very useful and actually harms
> configs where no rewritebase is required in some subdirs but present
> in parent dirs.
>
> Any issue with dropping this part of the change?
Having an arbitrary inconsistent behaviour from a directive in a module already noted for its complexity is a really bad thing. What we should support instead is the ability to turn RewriteBase off if needed, with something like "RewriteBase off".
Regards,
Graham
--