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 2014/03/30 20:29:59 UTC
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
On Sun, Mar 30, 2014 at 2:20 PM, <rj...@apache.org> wrote:
> Author: rjung
> Date: Sun Mar 30 18:20:09 2014
> New Revision: 1583175
>
> URL: http://svn.apache.org/r1583175
> Log:
> Fix segfault in mod_alias introduced in r1132494.
>
> AliasMatch does not append unmatched parts of the
> original URI to the new URI. So no need to subtract
> anything from the new URI length.
>
> The existing code crashed when using
> "AliasMatch / /some/thing" and sending a request
> with a long URI.
>
> Modified:
> httpd/httpd/trunk/modules/mappers/mod_alias.c
>
> Modified: httpd/httpd/trunk/modules/mappers/mod_alias.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_alias.c?rev=1583175&r1=1583174&r2=1583175&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/mappers/mod_alias.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_alias.c Sun Mar 30 18:20:09 2014
> @@ -371,15 +371,11 @@ static char *try_alias_list(request_rec
> }
> }
> else {
> - int pathlen = strlen(found) -
> - (strlen(r->uri + regm[0].rm_eo));
> - AP_DEBUG_ASSERT(pathlen >= 0);
> - AP_DEBUG_ASSERT(pathlen <= strlen(found));
> ap_set_context_info(r,
> apr_pstrmemdup(r->pool, r->uri,
> regm[0].rm_eo),
> apr_pstrmemdup(r->pool, found,
> - pathlen));
> + strlen(found)));
> }
> }
> else {
>
>
AFAICT {
In as much that it was ever useful, this breaks people relying on the
"context info" for aliasmatches structured the way this code was
originally biased to expecting:
AliasMatch ^(/foo/bar/)(.*) /var/www/baz/$1
IMO might be best to just never set context info here, or not set it
when the first capture is not starting at offset 0.
}
--
Eric Covener
covener@gmail.com
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
!me.
On May 5, 2014, at 9:34 AM, Eric Covener <co...@gmail.com> wrote:
> I don't want to churn in SVN too much, does anyone have an issue with
> dropping the context info stuff for the regex case completely?
>
> On Mon, Mar 31, 2014 at 1:49 PM, Eric Covener <co...@gmail.com> wrote:
>>> So you suggest to remove ap_set_context_info() from the AliasMatch handling?
>>
>> Yes, I just don't think we can pick appropriate values safely.
>
>
>
> --
> Eric Covener
> covener@gmail.com
>
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 05.05.2014 15:34, Eric Covener wrote:
> I don't want to churn in SVN too much, does anyone have an issue with
> dropping the context info stuff for the regex case completely?
Thanks for asking again and agreed here.
Regards,
Rainer
> On Mon, Mar 31, 2014 at 1:49 PM, Eric Covener <co...@gmail.com> wrote:
>>> So you suggest to remove ap_set_context_info() from the AliasMatch handling?
>>
>> Yes, I just don't think we can pick appropriate values safely.
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
Posted by Eric Covener <co...@gmail.com>.
I don't want to churn in SVN too much, does anyone have an issue with
dropping the context info stuff for the regex case completely?
On Mon, Mar 31, 2014 at 1:49 PM, Eric Covener <co...@gmail.com> wrote:
>> So you suggest to remove ap_set_context_info() from the AliasMatch handling?
>
> Yes, I just don't think we can pick appropriate values safely.
--
Eric Covener
covener@gmail.com
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
Posted by Eric Covener <co...@gmail.com>.
> So you suggest to remove ap_set_context_info() from the AliasMatch handling?
Yes, I just don't think we can pick appropriate values safely.
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 30.03.2014 22:52, Eric Covener wrote:
> On Sun, Mar 30, 2014 at 4:13 PM, Rainer Jung <ra...@kippdata.de> wrote:
>>> AFAICT {
>>> In as much that it was ever useful, this breaks people relying on the
>>> "context info" for aliasmatches structured the way this code was
>>> originally biased to expecting:
>>>
>>> AliasMatch ^(/foo/bar/)(.*) /var/www/baz/$1
>>>
>>> IMO might be best to just never set context info here, or not set it
>>> when the first capture is not starting at offset 0.
>>> }
>> What would have been the expected prefix value in this case?
>
> Sorry, got myself mixed up with regm[0], it's the entire match not a capture.
>
> Once you have a trailing wildcard like that, this code will think the
> entire request path was the prefix.
Still I don't understand what a correct "prefix" would be. I don't get
the idea behind the prefix.
>> What in the AliasMatch / /some/thing case?
>
> For "AliasMatch / /some/thing", a prefix of "/" makes sense but
> nothing will ever be meaningful for the context document root.
>
> Is there any AliasMatch that does produce a meaningul prefix and
> context document root? It makes sense that it could never know.
So you suggest to remove ap_set_context_info() from the AliasMatch handling?
Regards,
Rainer
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
Posted by Eric Covener <co...@gmail.com>.
On Sun, Mar 30, 2014 at 4:13 PM, Rainer Jung <ra...@kippdata.de> wrote:
> What would have been the expected prefix value in this case?
Sorry, got myself mixed up with regm[0], it's the entire match not a capture.
Once you have a trailing wildcard like that, this code will think the
entire request path was the prefix.
> What in the AliasMatch / /some/thing case?
For "AliasMatch / /some/thing", a prefix of "/" makes sense but
nothing will ever be meaningful for the context document root.
Is there any AliasMatch that does produce a meaningul prefix and
context document root? It makes sense that it could never know.
--
Eric Covener
covener@gmail.com
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 30.03.2014 20:29, Eric Covener wrote:
> On Sun, Mar 30, 2014 at 2:20 PM, <rj...@apache.org> wrote:
>> Author: rjung
>> Date: Sun Mar 30 18:20:09 2014
>> New Revision: 1583175
>>
>> URL: http://svn.apache.org/r1583175
>> Log:
>> Fix segfault in mod_alias introduced in r1132494.
>>
>> AliasMatch does not append unmatched parts of the
>> original URI to the new URI. So no need to subtract
>> anything from the new URI length.
>>
>> The existing code crashed when using
>> "AliasMatch / /some/thing" and sending a request
>> with a long URI.
>>
>> Modified:
>> httpd/httpd/trunk/modules/mappers/mod_alias.c
>>
>> Modified: httpd/httpd/trunk/modules/mappers/mod_alias.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_alias.c?rev=1583175&r1=1583174&r2=1583175&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/mappers/mod_alias.c (original)
>> +++ httpd/httpd/trunk/modules/mappers/mod_alias.c Sun Mar 30 18:20:09 2014
>> @@ -371,15 +371,11 @@ static char *try_alias_list(request_rec
>> }
>> }
>> else {
>> - int pathlen = strlen(found) -
>> - (strlen(r->uri + regm[0].rm_eo));
>> - AP_DEBUG_ASSERT(pathlen >= 0);
>> - AP_DEBUG_ASSERT(pathlen <= strlen(found));
>> ap_set_context_info(r,
>> apr_pstrmemdup(r->pool, r->uri,
>> regm[0].rm_eo),
>> apr_pstrmemdup(r->pool, found,
>> - pathlen));
>> + strlen(found)));
>> }
>> }
>> else {
>>
>>
>
> AFAICT {
> In as much that it was ever useful, this breaks people relying on the
> "context info" for aliasmatches structured the way this code was
> originally biased to expecting:
>
> AliasMatch ^(/foo/bar/)(.*) /var/www/baz/$1
>
> IMO might be best to just never set context info here, or not set it
> when the first capture is not starting at offset 0.
> }
What would have been the expected prefix value in this case?
What in the AliasMatch / /some/thing case?
Regards,
Rainer