Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
On Sun, Mar 30, 2014 at 2:20 PM, 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
cove...@gmail.com
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
On 30.03.2014 20:29, Eric Covener wrote:
> On Sun, Mar 30, 2014 at 2:20 PM, 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
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
On Sun, Mar 30, 2014 at 4:13 PM, Rainer Jung 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 cove...@gmail.com
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
On 30.03.2014 22:52, Eric Covener wrote:
> On Sun, Mar 30, 2014 at 4:13 PM, Rainer Jung 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
> 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
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 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 cove...@gmail.com
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
!me. On May 5, 2014, at 9:34 AM, 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? > > On Mon, Mar 31, 2014 at 1:49 PM, Eric Covener 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 > cove...@gmail.com >
Re: svn commit: r1583175 - /httpd/httpd/trunk/modules/mappers/mod_alias.c
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 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.
