You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2006/04/10 22:21:57 UTC

Re: svn commit: r393037 - in /httpd/httpd/trunk: CHANGES server/protocol.c

-1 Veto.

You aren't checking the scheme (other than that we've dumped CONNECT earlier
in the code) so you don't know that this transform is appropriate.  If you
would like to test that scheme applies, you still have the question of unusual
methods (eg non-GET/non-POST)

rpluem@apache.org wrote:
> Author: rpluem
> Date: Mon Apr 10 12:59:33 2006
> New Revision: 393037
> 
> URL: http://svn.apache.org/viewcvs?rev=393037&view=rev
> Log:
> * Prevent r->parsed_uri.path from being NULL as this can cause segmentation
>   faults e.g. in mod_cache. Set it to "/" in this case.
> 
> PR: 39259
> Submitted by: Davi Arnaut <davi haxent.com.br>
> Reviewed by: rpluem
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/server/protocol.c
> 
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/CHANGES?rev=393037&r1=393036&r2=393037&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Mon Apr 10 12:59:33 2006
> @@ -2,6 +2,9 @@
>  Changes with Apache 2.3.0
>    [Remove entries to the current 2.0 and 2.2 section below, when backported]
>  
> +  *) core: Set the path of a parsed uri in a request to "/" if the path is
> +     NULL. PR 39259. [Davi Arnaut <davi haxent.com.br>]
> +
>    *) htdbm: Warn the user when adding a plaintext password on a platform
>       where it wouldn't work with the server (i.e., anywhere that has
>       crypt()).  [Jeff Trawick]
> 
> Modified: httpd/httpd/trunk/server/protocol.c
> URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/server/protocol.c?rev=393037&r1=393036&r2=393037&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/protocol.c (original)
> +++ httpd/httpd/trunk/server/protocol.c Mon Apr 10 12:59:33 2006
> @@ -518,9 +518,11 @@
>              r->hostname = r->parsed_uri.hostname;
>          }
>  
> +        if (r->parsed_uri.path == NULL)
> +            r->parsed_uri.path = apr_pstrdup(r->pool, "/");
> +
>          r->args = r->parsed_uri.query;
> -        r->uri = r->parsed_uri.path ? r->parsed_uri.path
> -                 : apr_pstrdup(r->pool, "/");
> +        r->uri = r->parsed_uri.path;
>  
>  #if defined(OS2) || defined(WIN32)
>          /* Handle path translations for OS/2 and plug security hole.
> 
> 
> 
> 


Re: svn commit: r393037 - in /httpd/httpd/trunk: CHANGES server/protocol.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 04/11/2006 12:09 AM, Ruediger Pluem wrote:
> 
> On 04/10/2006 10:21 PM, William A. Rowe, Jr. wrote:
> 
>>-1 Veto.
> 
> 
> Rolled back by r393088.

Of course it must be r393087 :-).

Regards

Rüdiger

Re: svn commit: r393037 - in /httpd/httpd/trunk: CHANGES server/protocol.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 04/10/2006 10:21 PM, William A. Rowe, Jr. wrote:
> -1 Veto.

Rolled back by r393088.

Regards

Rüdiger


Re: svn commit: r393037 - in /httpd/httpd/trunk: CHANGES server/protocol.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 04/10/2006 11:39 PM, Ruediger Pluem wrote:
> 

> 
> 
> No arguing about your veto. I just try to understand it better to address it.
> 
> So some questions:
> 
> 1. As I just learned from RFC3986 an empty path is allowed (I was not aware of this
>    before). But why do we set r->uri to "/" in this case? Shouldn't it be NULL too
>    in this case?

Ok. Forget about this. This was just answered.

Regards

Rüdiger

Re: svn commit: r393037 - in /httpd/httpd/trunk: CHANGES server/protocol.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 04/10/2006 10:21 PM, William A. Rowe, Jr. wrote:
> -1 Veto.
> 
> You aren't checking the scheme (other than that we've dumped CONNECT
> earlier
> in the code) so you don't know that this transform is appropriate.  If you
> would like to test that scheme applies, you still have the question of
> unusual
> methods (eg non-GET/non-POST)

No arguing about your veto. I just try to understand it better to address it.

So some questions:

1. As I just learned from RFC3986 an empty path is allowed (I was not aware of this
   before). But why do we set r->uri to "/" in this case? Shouldn't it be NULL too
   in this case?

2. Do you know any http method where this transformation is wrong or even better
   for which ones this is correct?

3. Provided the list from 2. is present. Would it address your veto to do this
   transformation only if r->method_number is one of these methods?


Regards

Rüdiger