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.