Mailing List Archive

Re: svn commit: r1909137 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_alias.c
On 4/14/23 4:07 PM, minfrin@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

<Location /someplace>

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

</Location>

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
Re: svn commit: r1909137 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_alias.c [ In reply to ]
On 14 Apr 2023, at 17:18, Ruediger Pluem <rpluem@apache.org> wrote:

> Would that break configs like
>
> <Location /someplace>
>
> Alias /filesystemprefix/%{HTTP:X-example-header}
>
> </Location>
>
> 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 [ In reply to ]
On 4/16/23 12:20 PM, Graham Leggett via dev wrote:
> On 14 Apr 2023, at 17:18, Ruediger Pluem <rpluem@apache.org <mailto:rpluem@apache.org>> wrote:
>
>> Would that break configs like
>>
>> <Location /someplace>
>>
>> Alias /filesystemprefix/%{HTTP:X-example-header}
>>
>> </Location>
>>
>> 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 [ In reply to ]
On Mon, Apr 17, 2023 at 9:08?AM Ruediger Pluem <rpluem@apache.org> wrote:
>
> On 4/16/23 12:20 PM, Graham Leggett via dev wrote:
> > On 14 Apr 2023, at 17:18, Ruediger Pluem <rpluem@apache.org <mailto:rpluem@apache.org>> wrote:
> >
> >> Would that break configs like
> >>
> >> <Location /someplace>
> >>
> >> Alias /filesystemprefix/%{HTTP:X-example-header}
> >>
> >> </Location>
> >>
> >> 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. <Location /fake>
Alias /real
</Location>
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. <Directory /real>
Alias /fake
</Directory>
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 [ In reply to ]
On 4/17/23 1:07 PM, Yann Ylavic wrote:
> On Mon, Apr 17, 2023 at 9:08?AM Ruediger Pluem <rpluem@apache.org> wrote:
>>
>> On 4/16/23 12:20 PM, Graham Leggett via dev wrote:
>>> On 14 Apr 2023, at 17:18, Ruediger Pluem <rpluem@apache.org <mailto:rpluem@apache.org>> wrote:
>>>
>>>> Would that break configs like
>>>>
>>>> <Location /someplace>
>>>>
>>>> Alias /filesystemprefix/%{HTTP:X-example-header}
>>>>
>>>> </Location>
>>>>
>>>> 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. <Location /fake>
> Alias /real
> </Location>
> 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. <Directory /real>
> Alias /fake
> </Directory>
> 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