Re: svn commit: r1909137 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_alias.c

[Graham Leggett via dev]

On 17 Apr 2023, at 12:07, Yann Ylavic  wrote:

>> The question is if we find an approach to
>> make it backportable e.g. by adding some kind of 2.4.x only configuration 
>> switch to get the behavior of r1909137.
> 
> Maybe "AliasPerDirRelative on" (default: "off", context: "server
> config, virtual host, directory").
> And then we change the default to "on" in trunk.

I called the directive AliasPreservePath which seemed a more accurate fit, and 
added it in r1911067.

Regards,
Graham
—

Re: svn commit: r1909137 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_alias.c

[Ruediger Pluem]

On 4/17/23 1:07 PM, Yann Ylavic wrote:
> On Mon, Apr 17, 2023 at 9:08 AM Ruediger Pluem  wrote:
>>
>> On 4/16/23 12:20 PM, Graham Leggett via dev wrote:
>>> On 14 Apr 2023, at 17:18, Ruediger Pluem >> > wrote:
>>>
 Would that break configs like

 

 Alias /filesystemprefix/%{HTTP:X-example-header}

 

 where the expression evaluation determines the complete filesystem path 
 without adding the remainder of the URL?
>>>
>>> It would, which alas might mean we can't backport this, which isn’t great.
>>>
 I admit that the above looks like a strange setup and is probably a bad 
 example, but the parameter to Alias could be an arbitrary
 complex expression that evaluates to the final filesystem resource (like 
 AliasMatch). Wouldn't we need a kind of way to figure out
 if the users wants the remainder of the URI added or not even if we do not 
 use a regular expression in the surrounding Location
 block?
>>>
>>> LocationMatch is the correct way to do this - the config explicitly 
>>> declares the exact URL to match, and the Alias gives the exact
>>> unambiguous mapping.
>>>
>>> Having a prefix in the “Alias /foo /bar” case and then arbitrarily not 
>>> having a prefix in the “Alias /bar” case sent me on a huge
>>> wild goose chase - what made it worse was the client was a webdav client, 
>>> so the behaviour just made no sense. I am seeing people
>>> asking why they’re getting a 405 and not getting answers, so I think this 
>>> is tripping up other people too.
>>
>> I completely agree that Alias in a Location should behave as it does after 
>> r1909137 and the behavior change is not a problem in
>> trunk, but I think due this behavior change it is not possible to backport 
>> it as is.
> 
> If we change the behaviour in trunk such that:
>   1. Alias /fake /real
> is the same as:
>   2. 
>Alias /real
>  
> I'd suggest that we treat the /real part the same too (currently /real
> is a plain path in 1. and an expr in 2).

+1, but I guess this creates further backport problems one way or the other.

> It seems that:
>   3. 
>Alias /fake
>  
> could be a thing too (though /real would never be an expr here).

Is 3. really allowed today? I don't think so.

> 
> Likewise "AliasMatch /fake /real" and LocationMatch + "AliasMatch
> /real" only should probably work the same, where /real could use
> captures from the LocationMatch (just like
> LocationMatch+ProxyPassMatch if I'm not mistaken).

I think AliasMatch inside LocationMatch does not work currently.
If we would want to make it work it could be kind of challenging to get the 
captures
without executing the regex twice. Furthermore we need to be careful how we 
intermix
capture replacements and expression execution. With the current code you could 
use
Alias in LocationMatch and if you use named captures you can use them via 
environment
variables in the expression execution which seems kind of safe.

> 
> I also find the names in alias_dir_conf quite confusing, the "alias"
> corresponds to "real" in alias_entry used by alias_server_conf, while
> "alias_fake" corresponds to "alias". ISTM that alias_dir_conf should
> embed an alias_entry rather than duplicating all/most of its fields.
> Maybe we could even get rid of alias_server_conf actually and handle
> everything as perdir merging?
> 
>> The question is if we find an approach to
>> make it backportable e.g. by adding some kind of 2.4.x only configuration 
>> switch to get the behavior of r1909137.
> 
> Maybe "AliasPerDirRelative on" (default: "off", context: "server
> config, virtual host, directory").
> And then we change the default to "on" in trunk.

I am not sure how we get out of this. All stuff sounds messy.
Probably we should create a clean solution for trunk and leave stuff in 2.4.x 
as is.
For the case where the issue was discovered there is always a way to move the 
Alias from the Location to virtual host context.
Probably we could add a warning log message in 2.4.x if the Alias in a Location 
is just a plain string, that it might not behave
as the user expects.

Regards

Rüdiger

Re: svn commit: r1909137 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_alias.c

[Yann Ylavic]

On Mon, Apr 17, 2023 at 9:08 AM Ruediger Pluem  wrote:
>
> On 4/16/23 12:20 PM, Graham Leggett via dev wrote:
> > On 14 Apr 2023, at 17:18, Ruediger Pluem  > > wrote:
> >
> >> Would that break configs like
> >>
> >> 
> >>
> >> Alias /filesystemprefix/%{HTTP:X-example-header}
> >>
> >> 
> >>
> >> where the expression evaluation determines the complete filesystem path 
> >> without adding the remainder of the URL?
> >
> > It would, which alas might mean we can't backport this, which isn’t great.
> >
> >> I admit that the above looks like a strange setup and is probably a bad 
> >> example, but the parameter to Alias could be an arbitrary
> >> complex expression that evaluates to the final filesystem resource (like 
> >> AliasMatch). Wouldn't we need a kind of way to figure out
> >> if the users wants the remainder of the URI added or not even if we do not 
> >> use a regular expression in the surrounding Location
> >> block?
> >
> > LocationMatch is the correct way to do this - the config explicitly 
> > declares the exact URL to match, and the Alias gives the exact
> > unambiguous mapping.
> >
> > Having a prefix in the “Alias /foo /bar” case and then arbitrarily not 
> > having a prefix in the “Alias /bar” case sent me on a huge
> > wild goose chase - what made it worse was the client was a webdav client, 
> > so the behaviour just made no sense. I am seeing people
> > asking why they’re getting a 405 and not getting answers, so I think this 
> > is tripping up other people too.
>
> I completely agree that Alias in a Location should behave as it does after 
> r1909137 and the behavior change is not a problem in
> trunk, but I think due this behavior change it is not possible to backport it 
> as is.

If we change the behaviour in trunk such that:
  1. Alias /fake /real
is the same as:
  2. 
   Alias /real
 
I'd suggest that we treat the /real part the same too (currently /real
is a plain path in 1. and an expr in 2).
It seems that:
  3. 
   Alias /fake
 
could be a thing too (though /real would never be an expr here).

Likewise "AliasMatch /fake /real" and LocationMatch + "AliasMatch
/real" only should probably work the same, where /real could use
captures from the LocationMatch (just like
LocationMatch+ProxyPassMatch if I'm not mistaken).

I also find the names in alias_dir_conf quite confusing, the "alias"
corresponds to "real" in alias_entry used by alias_server_conf, while
"alias_fake" corresponds to "alias". ISTM that alias_dir_conf should
embed an alias_entry rather than duplicating all/most of its fields.
Maybe we could even get rid of alias_server_conf actually and handle
everything as perdir merging?

> The question is if we find an approach to
> make it backportable e.g. by adding some kind of 2.4.x only configuration 
> switch to get the behavior of r1909137.

Maybe "AliasPerDirRelative on" (default: "off", context: "server
config, virtual host, directory").
And then we change the default to "on" in trunk.


Regards;
Yann.

Re: svn commit: r1909137 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_alias.c

[Ruediger Pluem]

On 4/16/23 12:20 PM, Graham Leggett via dev wrote:
> On 14 Apr 2023, at 17:18, Ruediger Pluem  > wrote:
> 
>> Would that break configs like
>>
>> 
>>
>> Alias /filesystemprefix/%{HTTP:X-example-header}
>>
>> 
>>
>> where the expression evaluation determines the complete filesystem path 
>> without adding the remainder of the URL?
> 
> It would, which alas might mean we can't backport this, which isn’t great.
> 
>> I admit that the above looks like a strange setup and is probably a bad 
>> example, but the parameter to Alias could be an arbitrary
>> complex expression that evaluates to the final filesystem resource (like 
>> AliasMatch). Wouldn't we need a kind of way to figure out
>> if the users wants the remainder of the URI added or not even if we do not 
>> use a regular expression in the surrounding Location
>> block?
> 
> LocationMatch is the correct way to do this - the config explicitly declares 
> the exact URL to match, and the Alias gives the exact
> unambiguous mapping.
> 
> Having a prefix in the “Alias /foo /bar” case and then arbitrarily not having 
> a prefix in the “Alias /bar” case sent me on a huge
> wild goose chase - what made it worse was the client was a webdav client, so 
> the behaviour just made no sense. I am seeing people
> asking why they’re getting a 405 and not getting answers, so I think this is 
> tripping up other people too.

I completely agree that Alias in a Location should behave as it does after 
r1909137 and the behavior change is not a problem in
trunk, but I think due this behavior change it is not possible to backport it 
as is. The question is if we find an approach to
make it backportable e.g. by adding some kind of 2.4.x only configuration 
switch to get the behavior of r1909137.


Regards

Rüdiger

Re: svn commit: r1909137 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_alias.c

[Graham Leggett via dev]

On 14 Apr 2023, at 17:18, Ruediger Pluem  wrote:

> Would that break configs like
> 
> 
> 
> Alias /filesystemprefix/%{HTTP:X-example-header}
> 
> 
> 
> where the expression evaluation determines the complete filesystem path 
> without adding the remainder of the URL?

It would, which alas might mean we can't backport this, which isn’t great.

> I admit that the above looks like a strange setup and is probably a bad 
> example, but the parameter to Alias could be an arbitrary
> complex expression that evaluates to the final filesystem resource (like 
> AliasMatch). Wouldn't we need a kind of way to figure out
> if the users wants the remainder of the URI added or not even if we do not 
> use a regular expression in the surrounding Location block?

LocationMatch is the correct way to do this - the config explicitly declares 
the exact URL to match, and the Alias gives the exact unambiguous mapping.

Having a prefix in the “Alias /foo /bar” case and then arbitrarily not having a 
prefix in the “Alias /bar” case sent me on a huge wild goose chase - what made 
it worse was the client was a webdav client, so the behaviour just made no 
sense. I am seeing people asking why they’re getting a 405 and not getting 
answers, so I think this is tripping up other people too.

Regards,
Graham
—

Re: svn commit: r1909137 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_alias.c

[Ruediger Pluem]

On 4/14/23 4:07 PM, minf...@apache.org wrote:
> Author: minfrin
> Date: Fri Apr 14 14:07:49 2023
> New Revision: 1909137
> 
> URL: http://svn.apache.org/viewvc?rev=1909137&view=rev
> Log:
> mod_alias: When an alias is declared inside a Location, make sure
> the balance of the URL is preserved to match the alias declared
> outside a location. Fixes an error where all requests are mapped
> to the root of the location.
> 
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/mappers/mod_alias.c
> 
> Modified: httpd/httpd/trunk/CHANGES
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1909137&r1=1909136&r2=1909137&view=diff
> ==
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Apr 14 14:07:49 2023
> @@ -1,6 +1,11 @@
>   -*- coding: utf-8 
> -*-
>  Changes with Apache 2.5.1
>  
> +  *) mod_alias: When an alias is declared inside a Location, make sure
> + the balance of the URL is preserved to match the alias declared
> + outside a location. Fixes an error where all requests are mapped
> + to the root of the location. [Graham Leggett]
> +
>*) core: Be explicit if an enclosing directive contains a path or a
>   regex. [Graham Leggett]
>  
> 
> Modified: httpd/httpd/trunk/modules/mappers/mod_alias.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_alias.c?rev=1909137&r1=1909136&r2=1909137&view=diff
> ==
> --- httpd/httpd/trunk/modules/mappers/mod_alias.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_alias.c Fri Apr 14 14:07:49 2023
> @@ -59,6 +59,7 @@ typedef struct {
>  unsigned int redirect_set:1;
>  apr_array_header_t *redirects;
>  const ap_expr_info_t *alias;
> +const char *alias_fake;
>  char *handler;
>  const ap_expr_info_t *redirect;
>  int redirect_status;/* 301, 302, 303, 410, etc */
> @@ -113,6 +114,7 @@ static void *merge_alias_dir_config(apr_
>  a->redirects = apr_array_append(p, overrides->redirects, 
> base->redirects);
>  
>  a->alias = (overrides->alias_set == 0) ? base->alias : overrides->alias;
> +a->alias_fake = (overrides->alias_set == 0) ? base->alias_fake : 
> overrides->alias_fake;
>  a->handler = (overrides->alias_set == 0) ? base->handler : 
> overrides->handler;
>  a->alias_set = overrides->alias_set || base->alias_set;
>  
> @@ -220,6 +222,9 @@ static const char *add_alias(cmd_parms *
>  NULL);
>  }
>  
> +if (!cmd->regex) {
> +dirconf->alias_fake = cmd->path;
> +}
>  dirconf->handler = cmd->info;
>  dirconf->alias_set = 1;
>  
> @@ -438,6 +443,17 @@ static char *try_alias(request_rec *r)
>  return PREGSUB_ERROR;
>  }
>  
> +if (dirconf->alias_fake) {
> +int l;
> +
> +l = alias_matches(r->uri, dirconf->alias_fake);
> +
> +if (l > 0) {
> +ap_set_context_info(r, dirconf->alias_fake, found);
> +found = apr_pstrcat(r->pool, found, r->uri + l, NULL);
> +}
> +}
> +

Would that break configs like



Alias /filesystemprefix/%{HTTP:X-example-header}



where the expression evaluation determines the complete filesystem path without 
adding the remainder of the URL?

I admit that the above looks like a strange setup and is probably a bad 
example, but the parameter to Alias could be an arbitrary
complex expression that evaluates to the final filesystem resource (like 
AliasMatch). Wouldn't we need a kind of way to figure out
if the users wants the remainder of the URI added or not even if we do not use 
a regular expression in the surrounding Location block?

Regards

Rüdiger