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 2013/02/18 19:02:21 UTC

2.4 rewritebase

I am on the road right now, but a user reported that my fix to make
rewritebase merging opt-in is busted and breaks normal override by default.
Can someone look/repair/propose for 24?

Re: 2.4 rewritebase

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On Mon, 18 Feb 2013 15:36:17 -0500
Eric Covener <co...@gmail.com> wrote:

> > Ok, so 2.2.23 had not suffered this regression yet?  If not, we
> > should just move ahead and then can consider any improved behavior
> > in 2.2.x. There are no changes to mod_rewrite in 2.2.x since 2.2.23
> > was tagged.
> 
> 2.2.23 has _a_ rewritebase regression reported a number of places. My
> recent fire drill was a regression in the fix for that original
> regression.
> 
> I'd like it in, but appreciate tough spot as RM -- your call.

What version was this introduced?  Is there a good reference PR?

Re: 2.4 rewritebase

Posted by Rainer Jung <ra...@kippdata.de>.
On 18.02.2013 22:16, William A. Rowe Jr. wrote:
> On Mon, 18 Feb 2013 21:53:16 +0100
> Rainer Jung <ra...@kippdata.de> wrote:
> 
>> On 18.02.2013 21:47, Rainer Jung wrote:
>>> On 18.02.2013 21:36, Eric Covener wrote:
>>>
>>> The regression in 2.2.23 is that RewriteBase started to get merged
>>> in 2.2.23. It wasn't before. This seems to have broken certain
>>> setups. PR 53963.
>>>
>>> Eric has provided a "RewriteOptions MergeBase" which is off by
>>> default (no more merging) and allows to switch the bahvior on if
>>> wanted. The first impl of that had a bug, which is why we had a
>>> last minute change to it today.
>>
>> Two more data points for 2.2:
>>
>> - the merging of RewriteBase was a side effect of
>>
>>   *) mod_rewrite: Fix the RewriteEngine directive to work within a
>>      location. Previously, once RewriteEngine was switched on
>> globally, it was impossible to switch off.
>>
>> - the commit was r1375113
> 
> Technically the fix appears correct, when compared to the initial
> patch.  I would rather not race through this without one more folk
> to test out this change, needs not be a committer, I just added a
> beg on PR 53963 for review (there seem to be three watchers right
> now).  I'll tag and roll an 1.25 hrs from now so you are welcome
> to do the backport honors Rainer... then hopefully nobody raises 
> the red flag before I lay down that tag.

I committed it now with a slightly clarified docs section about
MergeBase. The OP in the PR has responded positively although I think he
might have tested the previous, broken version. One would only notice
the difference if one would actually set a RewriteBase in a dir and also
in a sub dir. The original problem was I think just having one set in a
dir and not wanting it inherited in the sub dirs. So in this case the
broken fix would have kind of worked as well.

Will hang around for another 1-2 hours before it's getting late here.

Rainer


Re: 2.4 rewritebase

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On Mon, 18 Feb 2013 21:53:16 +0100
Rainer Jung <ra...@kippdata.de> wrote:

> On 18.02.2013 21:47, Rainer Jung wrote:
> > On 18.02.2013 21:36, Eric Covener wrote:
> > 
> > The regression in 2.2.23 is that RewriteBase started to get merged
> > in 2.2.23. It wasn't before. This seems to have broken certain
> > setups. PR 53963.
> > 
> > Eric has provided a "RewriteOptions MergeBase" which is off by
> > default (no more merging) and allows to switch the bahvior on if
> > wanted. The first impl of that had a bug, which is why we had a
> > last minute change to it today.
> 
> Two more data points for 2.2:
> 
> - the merging of RewriteBase was a side effect of
> 
>   *) mod_rewrite: Fix the RewriteEngine directive to work within a
>      location. Previously, once RewriteEngine was switched on
> globally, it was impossible to switch off.
> 
> - the commit was r1375113

Technically the fix appears correct, when compared to the initial
patch.  I would rather not race through this without one more folk
to test out this change, needs not be a committer, I just added a
beg on PR 53963 for review (there seem to be three watchers right
now).  I'll tag and roll an 1.25 hrs from now so you are welcome
to do the backport honors Rainer... then hopefully nobody raises 
the red flag before I lay down that tag.




Re: 2.4 rewritebase

Posted by Rainer Jung <ra...@kippdata.de>.
On 18.02.2013 21:47, Rainer Jung wrote:
> On 18.02.2013 21:36, Eric Covener wrote:
>>> Ok, so 2.2.23 had not suffered this regression yet?  If not, we should
>>> just move ahead and then can consider any improved behavior in 2.2.x.
>>> There are no changes to mod_rewrite in 2.2.x since 2.2.23 was tagged.
>>
>> 2.2.23 has _a_ rewritebase regression reported a number of places. My
>> recent fire drill was a regression in the fix for that original
>> regression.
>>
>> I'd like it in, but appreciate tough spot as RM -- your call.
> 
> Same here.
> 
> The regression in 2.2.23 is that RewriteBase started to get merged in
> 2.2.23. It wasn't before. This seems to have broken certain setups. PR
> 53963.
> 
> Eric has provided a "RewriteOptions MergeBase" which is off by default
> (no more merging) and allows to switch the bahvior on if wanted. The
> first impl of that had a bug, which is why we had a last minute change
> to it today.

Two more data points for 2.2:

- the merging of RewriteBase was a side effect of

  *) mod_rewrite: Fix the RewriteEngine directive to work within a
     location. Previously, once RewriteEngine was switched on globally,
     it was impossible to switch off.

- the commit was r1375113

Regards,

Rainer



Re: 2.4 rewritebase

Posted by Rainer Jung <ra...@kippdata.de>.
On 18.02.2013 21:36, Eric Covener wrote:
>> Ok, so 2.2.23 had not suffered this regression yet?  If not, we should
>> just move ahead and then can consider any improved behavior in 2.2.x.
>> There are no changes to mod_rewrite in 2.2.x since 2.2.23 was tagged.
> 
> 2.2.23 has _a_ rewritebase regression reported a number of places. My
> recent fire drill was a regression in the fix for that original
> regression.
> 
> I'd like it in, but appreciate tough spot as RM -- your call.

Same here.

The regression in 2.2.23 is that RewriteBase started to get merged in
2.2.23. It wasn't before. This seems to have broken certain setups. PR
53963.

Eric has provided a "RewriteOptions MergeBase" which is off by default
(no more merging) and allows to switch the bahvior on if wanted. The
first impl of that had a bug, which is why we had a last minute change
to it today.

Regards,

Rainer


Re: 2.4 rewritebase

Posted by Eric Covener <co...@gmail.com>.
> Ok, so 2.2.23 had not suffered this regression yet?  If not, we should
> just move ahead and then can consider any improved behavior in 2.2.x.
> There are no changes to mod_rewrite in 2.2.x since 2.2.23 was tagged.

2.2.23 has _a_ rewritebase regression reported a number of places. My
recent fire drill was a regression in the fix for that original
regression.

I'd like it in, but appreciate tough spot as RM -- your call.

Re: 2.4 rewritebase

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On Mon, 18 Feb 2013 14:15:45 -0500
Eric Covener <co...@gmail.com> wrote:

> On Mon, Feb 18, 2013 at 2:00 PM, Rainer Jung
> <ra...@kippdata.de> wrote:
> > On 18.02.2013 19:02, Eric Covener wrote:
> >> I am on the road right now, but a user reported that my fix to make
> >> rewritebase merging opt-in is busted and breaks normal override by
> >> default. Can someone look/repair/propose for 24?
> >
> > You beat me to it.
> >
> > If we think that the merging/inheritance of RewriteBase by default
> > is wrong, we should also backport your RewriteOption MergeBase
> > (r1418954) and the tiny fix for it you just now added to the 2.4
> > STATUS to 2.2 as well? At least we have PR 53963 open about the new
> > merge behavior in 2.2.23.
> >
> > I guess I will add a backport suggestion to the 2.2.x STATUS.
> 
> Yes I agree, that original report actually has no relief queued up in
> 2.2.24.
> 
> The reporter contacted me off-list and was running a personal build
> with the trunk change in 2.2 and realized it was broken.

Ok, so 2.2.23 had not suffered this regression yet?  If not, we should
just move ahead and then can consider any improved behavior in 2.2.x.
There are no changes to mod_rewrite in 2.2.x since 2.2.23 was tagged.

Re: 2.4 rewritebase

Posted by Eric Covener <co...@gmail.com>.
On Mon, Feb 18, 2013 at 2:00 PM, Rainer Jung <ra...@kippdata.de> wrote:
> On 18.02.2013 19:02, Eric Covener wrote:
>> I am on the road right now, but a user reported that my fix to make
>> rewritebase merging opt-in is busted and breaks normal override by
>> default. Can someone look/repair/propose for 24?
>
> You beat me to it.
>
> If we think that the merging/inheritance of RewriteBase by default is
> wrong, we should also backport your RewriteOption MergeBase (r1418954)
> and the tiny fix for it you just now added to the 2.4 STATUS to 2.2 as
> well? At least we have PR 53963 open about the new merge behavior in 2.2.23.
>
> I guess I will add a backport suggestion to the 2.2.x STATUS.

Yes I agree, that original report actually has no relief queued up in 2.2.24.

The reporter contacted me off-list and was running a personal build
with the trunk change in 2.2 and realized it was broken.

Re: 2.4 rewritebase

Posted by Rainer Jung <ra...@kippdata.de>.
On 18.02.2013 19:02, Eric Covener wrote:
> I am on the road right now, but a user reported that my fix to make
> rewritebase merging opt-in is busted and breaks normal override by
> default. Can someone look/repair/propose for 24?

You beat me to it.

If we think that the merging/inheritance of RewriteBase by default is
wrong, we should also backport your RewriteOption MergeBase (r1418954)
and the tiny fix for it you just now added to the 2.4 STATUS to 2.2 as
well? At least we have PR 53963 open about the new merge behavior in 2.2.23.

I guess I will add a backport suggestion to the 2.2.x STATUS.

Regards,

Rainer

Re: 2.4 rewritebase

Posted by Eric Covener <co...@gmail.com>.
Clue finally sunk in, now proposed.  Thanks Rainer for the quick attn.

On Mon, Feb 18, 2013 at 1:02 PM, Eric Covener <co...@gmail.com> wrote:
> I am on the road right now, but a user reported that my fix to make
> rewritebase merging opt-in is busted and breaks normal override by default.
> Can someone look/repair/propose for 24?



--
Eric Covener
covener@gmail.com