Re: svn commit: r1909135 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h server/core.c

2023-04-22 Thread Graham Leggett via dev
On 19 Apr 2023, at 19:08, Yann Ylavic  wrote:

> Currently all the tests (framework) are broken due to cmd->regex not
> being properly stacked/scoped.
> If cmd->regex is to be used to store the enclosing section's regex, it
> should be saved and restored by *all* the the sections (much like
> cmd->override is saved/restored using the old_overrides stack variable
> in most sections). It should also be set to NULL when parsing
> (sub-)sections with no possible regex, unless we want to inherit
> cmd->regex there.

This is done in r1909356.

> 
> But for instance some like:
>   
>  
># use /fake from 
>Alias /real
>  
>   
> won't work for Alias because  overwrites cmd->path with "*If".

I just found that - that’s definitely broken, it means you can’t do this:


  
DAV svn
SVNPath /home/trac/svn/extra
  


Looks like the same effect as the Alias bug, where every url is mapped to “*If”.

> Also, what about:
>   
>  
># which regex here?
>  
>   

In the above “which regex here” would be FilesMatch, which wouldn’t change.

> Or:
>   
>  
># 's regex usable here?
>  
>   
> ?

Combining regexes is probably a step too far. Right now DirectoryMatch’s regex 
isn’t usable at the point as the DirectoryMatch regex is not passed across 
Files. This behaviour doesn't change.

> Or third-party sections (unaware of cmd->regex) which contain
> directives that depend on cmd->regex?

Again there would be no change to behaviour that I can see, the regex was 
hidden before and the raw path (containing the regex) would be used, now 
cmd->regex would be set to NULL and the raw path (containing the regex) would 
be used.

> I'm not saying it's a bad idea but it needs more changes than this
> commit, changes that spread all over the code base it seems (modules
> can have their sections too), something like:
> $ grep -r 'AP_INIT\w\+("<' server/ modules/ 2>/dev/null
> server/core.c:AP_INIT_RAW_ARGS(" server/core.c:AP_INIT_RAW_ARGS(" server/core.c:AP_INIT_RAW_ARGS(" NULL, RSRC_CONF,
[snip]

cmd->regex is a function of cmd->path, so only places that touch cmd->path need 
touch cmd->regex, and this seems to be directory, location and file.

 seems broken right now, need to fix that separately.

> Maybe we only want to check that the parent directive is a 
> with Alias for now?

Right now it’s limited to not-in-directory:

https://github.com/apache/httpd/blob/trunk/modules/mappers/mod_alias.c#L146

If this is too difficult to backport it isn't the end of the world, the main 
thing is that it's fixed for the future.

Regards,
Graham
—



Re: svn commit: r1909135 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h server/core.c

2023-04-21 Thread Joe Orton
On Wed, Apr 19, 2023 at 08:08:49PM +0200, Yann Ylavic wrote:
> On Fri, Apr 14, 2023 at 4:02 PM  wrote:
> >
> > Author: minfrin
> > Date: Fri Apr 14 14:02:11 2023
> > New Revision: 1909135
> >
> > URL: http://svn.apache.org/viewvc?rev=1909135=rev
> > Log:
> > core: Be explicit if an enclosing directive contains a path or a
> > regex.
> 
> Currently all the tests (framework) are broken due to cmd->regex not
> being properly stacked/scoped.

Trunk has been broken for a week, Graham could you move this work to a 
branch or PR until the regressions are fixed?

Regards, Joe



Re: svn commit: r1909135 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h server/core.c

2023-04-19 Thread Yann Ylavic
On Fri, Apr 14, 2023 at 4:02 PM  wrote:
>
> Author: minfrin
> Date: Fri Apr 14 14:02:11 2023
> New Revision: 1909135
>
> URL: http://svn.apache.org/viewvc?rev=1909135=rev
> Log:
> core: Be explicit if an enclosing directive contains a path or a
> regex.

Currently all the tests (framework) are broken due to cmd->regex not
being properly stacked/scoped.
If cmd->regex is to be used to store the enclosing section's regex, it
should be saved and restored by *all* the the sections (much like
cmd->override is saved/restored using the old_overrides stack variable
in most sections). It should also be set to NULL when parsing
(sub-)sections with no possible regex, unless we want to inherit
cmd->regex there.

But for instance some like:
   
  
# use /fake from 
Alias /real
  
   
won't work for Alias because  overwrites cmd->path with "*If".

Also, what about:
   
  
# which regex here?
  
   
Or:
   
  
# 's regex usable here?
  
   
?

Or third-party sections (unaware of cmd->regex) which contain
directives that depend on cmd->regex?

I'm not saying it's a bad idea but it needs more changes than this
commit, changes that spread all over the code base it seems (modules
can have their sections too), something like:
$ grep -r 'AP_INIT\w\+("<' server/ modules/ 2>/dev/null
server/core.c:AP_INIT_RAW_ARGS("
with Alias for now?


Regards;
Yann.