httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From d...@ast.cam.ac.uk (David Robinson)
Subject Re: One more patch for 1.0.0
Date Wed, 13 Dec 1995 10:17:00 GMT
> I have a patch for 1.0.0 that I would like to try to get in before the next
> vote.
> 
> This patch fixes a bug in mod_dir.c which causes query portions of a request
> on a directory to be lost via internal redirects.  This can be inconveient
> when redirecting the directory to a script(such as with mod_actions.c) or
> with SSIs. The following patch to mod_dir.c makes sure that the query
> is preserved before the redirect occurs.
> 
> I am including the patch here as I am not sure how else to submit it.
> 
> Thank you,
> 
> Adam Sussman
> Vidya Media Ventures

You patch is ok, but it does not fix two bugs that became apparent to me
on reading the original source. Could you update it to fix these bugs?


>From: asussman@vidya.com (Adam Sussman)
>Subject: Preserve query portion of URL in mod_dir.c redirects
>Affects: mod_dir.c
>ChangeLog: Internal redirects which occur in mod_dir.c do not preserve the
>        query portion of a request (the bit after the question mark).  If
>        the directory index is redirected to a script or has an include,
>        the query string is lost before invocation.
>
>*** mod_dir.c.old       Fri Nov 17 13:33:16 1995
>--- mod_dir.c   Tue Dec 12 22:35:14 1995
>***************
>*** 768,774 ****
>      if (r->method_number != M_GET) return NOT_IMPLEMENTED;
>      
>      if (r->uri[0] == '\0' || r->uri[strlen(r->uri)-1] != '/') {
>!         char* ifile = pstrcat (r->pool, r->uri, "/", NULL);
>        table_set (r->headers_out, "Location",
>                   construct_url(r->pool, ifile, r->server));
>        return REDIRECT;
>--- 768,780 ----
...
Bug: r->uri does not have escape_uri called on it; it should do.
Symptom: if I have a directory /foo%bar and I request an index with
http://server/foo%25bar
I get
Location: http://server/foo%bar/
which is wrong.
(Of course, you should not re-escape the query string as it has not be
de-escaped.)

...
>***************
>*** 787,794 ****
>        request_rec *rr = sub_req_lookup_uri (name_ptr, r);
>             
>        if (rr->status == 200 && rr->finfo.st_mode != 0) {
>!             char *new_uri = escape_uri(r->pool, rr->uri);
>            destroy_sub_req (rr);
>            internal_redirect (new_uri, r);
>            return OK;
>        }

Bug: the original code did not append any query string information that
was given in the DirectoryIndex command, i.e. it should have appended
"?" rr->args to the new URI.

>--- 793,807 ----
>        request_rec *rr = sub_req_lookup_uri (name_ptr, r);
>             
>        if (rr->status == 200 && rr->finfo.st_mode != 0) {
>!           char* new_uri = escape_uri(r->pool, rr->uri);
>            destroy_sub_req (rr);
>+ 
>+           if (r->args!=NULL)
>+           {
>+               char* f = new_uri;
>+               new_uri = pstrcat(r->pool, f, "?", r->args, NULL);
>+           }
>+ 
>            internal_redirect (new_uri, r);
>            return OK;
>        }

Could your new code only append r->args to the new URI if rr->args was empty?
i.e. take the query string from the request if the DirectoryIndex did
not specifiy any query string.

Also, you don't need to take a temporary copy of new_uri; 
   new_uri = pstrcat(r->pool, new_uri, "?", r->args, NULL);
is valid.

 David.

Mime
View raw message