URL decoding bugs in apache 0.8.2

[David Robinson <dr...@ast.cam.ac.uk>]

There's a couple of places where unescape_url() is inappropriately called.

1. PATH_INFO gets URL decoded _again_ before PATH_TRANSLATED is found;
   e.g.
   http://server.com/cgi-bin/test-cgi/hello%2524

   produces
   PATH_INFO = /hello%24
   PATH_TRANSLATED = /opt/httpd/htdocs/hello$

   fix: new routine sub_req_lookup_path() for this case.

2. For internal redirects, the URL gets decoded twice.

   e.g.
   redir contains:
   #!/bin/sh
   echo 'Location: /cgi-bin/test-cgi/byee%2524'
   echo

   then http://server.com/cgi-bin/redir
   produces
   PATH_INFO = /byee$
   PATH_TRANSLATED = /opt/httpd/htdocs/byee$

   fix: remove unescape_url() and friends from internal_redirect().

3. for virtual path redirects, the URL might not be decoded, or parts of
   it might be decoded twice.

   The call to unescape_url() in sub_req_lookup_uri() is in the wrong place;
   the new path should be decoded straight away.

4. (May not be a bug) sub_req_lookup_file wasn't setting the args of the
    new request.

David.


---------------------------- begin patch -------------------------------
*** http_request.h.orig	Wed Jul 19 01:50:25 1995
--- http_request.h	Tue Jul 25 14:56:51 1995
***************
*** 78,83 ****
--- 78,84 ----
   */
  
  request_rec *sub_req_lookup_uri (char *new_file, request_rec *r);
+ extern request_rec *sub_req_lookup_path(char *new_file, request_rec *r);
  request_rec *sub_req_lookup_file (char *new_file, request_rec *r);
  int run_sub_req (request_rec *r);
  void destroy_sub_req (request_rec *r);
*** http_request.c.orig	Tue Jul 25 11:21:41 1995
--- http_request.c	Tue Jul 25 15:13:44 1995
***************
*** 319,326 ****
  {
      request_rec *rnew;
      int res;
!     char *udir;
      
      /* Check for a special case... if there are no '/' characters in new_file
       * at all, then we are looking at a relative lookup in the same directory.
       * That means we don't have to redo any access checks.
--- 319,330 ----
  {
      request_rec *rnew;
      int res;
!     char *udir, *uri;
      
+ /* firstly, decode uri */
+     uri = pstrdup(r->pool, new_file);
+     unescape_url(uri);
+ 
      /* Check for a special case... if there are no '/' characters in new_file
       * at all, then we are looking at a relative lookup in the same directory.
       * That means we don't have to redo any access checks.
***************
*** 343,349 ****
  		      new_file :
  		      make_full_path (rnew->pool, udir, new_file)));
  	
-     unescape_url (rnew->uri);
      getparents (rnew->uri);
      no2slash (rnew->uri);
  	
--- 347,352 ----
***************
*** 362,367 ****
--- 365,404 ----
      return rnew;
  }
  
+ /*
+  * This one is for building a new request based on an absolute URL path,
+  * inteneded for the PATH_INFO -> PATH_INFO_TRANSLATED mapping.
+  */
+ request_rec *sub_req_lookup_path (char *new_file, request_rec *r)
+ {
+     request_rec *rnew;
+     int res;
+     
+     rnew = make_sub_request (r);
+     
+     rnew->connection = r->connection; /* For now... */
+     rnew->server = r->server;
+     rnew->request_config = create_request_config (rnew->pool);
+     set_sub_req_protocol (rnew, r);
+ 	
+     rnew->uri = new_file;
+     rnew->args = NULL;
+ 	
+     if ((res = translate_name (rnew))
+ 	|| (res = directory_walk (rnew))
+ 	|| (res = check_access (rnew))
+ 	|| (!auth_type (rnew) ? 0 :
+ 	     ((res = check_user_id (rnew)) && (res = check_auth (rnew))))
+ 	|| (res = find_types (rnew))
+ 	|| (res = run_fixups (rnew))
+ 	)
+     {
+         rnew->status = res;
+     }
+ 
+     return rnew;
+ }
+ 
  request_rec *sub_req_lookup_file (char *new_file, request_rec *r)
  {
      request_rec *rnew;
***************
*** 385,390 ****
--- 422,428 ----
      set_sub_req_protocol (rnew, r);
  	
      rnew->uri = "INTERNALLY GENERATED file-relative req";
+     rnew->args = NULL;
      rnew->filename = ((new_file[0] == '/') ?
  		      new_file :
  		      make_full_path (rnew->pool, fdir, new_file));
***************
*** 598,607 ****
      
      new->prev = r;
      r->next = new;
-     
-     unescape_url(new->uri);
-     getparents (new->uri);
-     no2slash (new->uri);
      
      /* We are redirecting.  Treat the internally generated transaction
       * as a GET, since there is not a chance of its getting POST-style
--- 636,641 ----
*** mod_cgi.c.orig	Mon Jul 24 16:45:41 1995
--- mod_cgi.c	Tue Jul 25 14:57:02 1995
***************
*** 190,196 ****
      }
  	
      if (r->path_info && r->path_info[0]) {
! 	request_rec *pa_req = sub_req_lookup_uri (r->path_info, r);
        
          table_set (e, "PATH_INFO", r->path_info);
  
--- 190,196 ----
      }
  	
      if (r->path_info && r->path_info[0]) {
! 	request_rec *pa_req = sub_req_lookup_path(r->path_info, r);
        
          table_set (e, "PATH_INFO", r->path_info);
  

---------------------------- end patch -------------------------------