This bug was present in 0.8.2, but doesn't seem to have been fixed for 0.8.3
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:
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 file enc.patch -----------------------------
*** http_request.h.orig Wed Jul 19 01:50:25 1995
--- http_request.h Thu Jul 27 12:21:38 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 02:12:45 1995
--- http_request.c Thu Jul 27 12:22:12 1995
***************
*** 327,334 ****
{
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.
--- 327,338 ----
{
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.
***************
*** 351,357 ****
new_file :
make_full_path (rnew->pool, udir, new_file)));
- unescape_url (rnew->uri);
getparents (rnew->uri);
if ((res = translate_name (rnew))
--- 355,360 ----
***************
*** 369,374 ****
--- 372,411 ----
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;
***************
*** 392,397 ****
--- 429,435 ----
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));
***************
*** 604,612 ****
new->prev = r;
r->next = new;
-
- unescape_url(new->uri);
- getparents (new->uri);
/* We are redirecting. Treat the internally generated transaction
* as a GET, since there is not a chance of its getting POST-style
--- 642,647 ----
*** mod_cgi.c.orig Wed Jul 19 01:38:43 1995
--- mod_cgi.c Thu Jul 27 12:21:39 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 file enc.patch -----------------------------
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:
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 file enc.patch -----------------------------
*** http_request.h.orig Wed Jul 19 01:50:25 1995
--- http_request.h Thu Jul 27 12:21:38 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 02:12:45 1995
--- http_request.c Thu Jul 27 12:22:12 1995
***************
*** 327,334 ****
{
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.
--- 327,338 ----
{
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.
***************
*** 351,357 ****
new_file :
make_full_path (rnew->pool, udir, new_file)));
- unescape_url (rnew->uri);
getparents (rnew->uri);
if ((res = translate_name (rnew))
--- 355,360 ----
***************
*** 369,374 ****
--- 372,411 ----
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;
***************
*** 392,397 ****
--- 429,435 ----
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));
***************
*** 604,612 ****
new->prev = r;
r->next = new;
-
- unescape_url(new->uri);
- getparents (new->uri);
/* We are redirecting. Treat the internally generated transaction
* as a GET, since there is not a chance of its getting POST-style
--- 642,647 ----
*** mod_cgi.c.orig Wed Jul 19 01:38:43 1995
--- mod_cgi.c Thu Jul 27 12:21:39 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 file enc.patch -----------------------------