Good point... forward proxies should just pass through as is, good or bad. Will revert the patch and fix Nick's impl tomorrow when I get back in the office.
On Wed, Oct 10, 2007 at 12:39:56PM -0700, Roy T. Fielding wrote: > The whole idea behind this routine is just wrong. > That set of characters is insufficient (RFC 3986) and, in > any case, a proxy is not responsible for checking valid > characters in a URI. Both the original and this new function > should be deleted. > > ....Roy > > On Oct 10, 2007, at 6:16 AM, [EMAIL PROTECTED] wrote: > > >Author: jim > >Date: Wed Oct 10 06:16:56 2007 > >New Revision: 583466 > > > >URL: http://svn.apache.org/viewvc?rev=583466&view=rev > >Log: > >Abstract out "verification of valid encoding" via > >ap_proxy_isvalidenc(). Now we can use it in other > >proxy protocols. > > > >Modified: > > httpd/httpd/trunk/include/ap_mmn.h > > httpd/httpd/trunk/modules/proxy/mod_proxy.h > > httpd/httpd/trunk/modules/proxy/mod_proxy_http.c > > httpd/httpd/trunk/modules/proxy/proxy_util.c > > > >Modified: httpd/httpd/trunk/include/ap_mmn.h > >URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ > >ap_mmn.h?rev=583466&r1=583465&r2=583466&view=diff > >====================================================================== > >======== > >--- httpd/httpd/trunk/include/ap_mmn.h (original) > >+++ httpd/httpd/trunk/include/ap_mmn.h Wed Oct 10 06:16:56 2007 > >@@ -133,6 +133,7 @@ > > * 20070823.0 (2.3.0-dev) Removed ap_all_available_mutexes_string, > > * ap_available_mutexes_string for macros > > * 20070823.1 (2.3.0-dev) add ap_send_interim_response() > >+ * 20070823.2 (2.3.0-dev) add ap_proxy_isvalidenc() > > * > > */ > > > > > >Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h > >URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/ > >mod_proxy.h?rev=583466&r1=583465&r2=583466&view=diff > >====================================================================== > >======== > >--- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original) > >+++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Wed Oct 10 06:16:56 > >2007 > >@@ -460,6 +460,7 @@ > > PROXY_DECLARE(request_rec *)ap_proxy_make_fake_req(conn_rec *c, > >request_rec *r); > > PROXY_DECLARE(int) ap_proxy_hex2c(const char *x); > > PROXY_DECLARE(void) ap_proxy_c2hex(int ch, char *x); > >+PROXY_DECLARE(apr_status_t)ap_proxy_isvalidenc(const char *url, > >const char *allowed); > > PROXY_DECLARE(char *)ap_proxy_canonenc(apr_pool_t *p, const char > >*x, int len, enum enctype t, > > int forcedec, int proxyreq); > > PROXY_DECLARE(char *)ap_proxy_canon_netloc(apr_pool_t *p, char > >**const urlp, char **userp, > > > >Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c > >URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/ > >mod_proxy_http.c?rev=583466&r1=583465&r2=583466&view=diff > >====================================================================== > >======== > >--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original) > >+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Wed Oct 10 > >06:16:56 2007 > >@@ -37,9 +37,6 @@ > > const char *err; > > const char *scheme; > > apr_port_t port, def_port; > >- const char *p; > >- const char *allowed = "~$-_.+!*'(),;:@&=/"; /* allowed > >+reserved from > >- > >ap_proxy_canonenc */ > > > > /* ap_port_of_scheme() */ > > if (strncasecmp(url, "http:", 5) == 0) { > >@@ -94,15 +91,8 @@ > > path = ap_proxy_canonenc(r->pool, url, strlen(url), > >enc_path, 0, r->proxyreq); > > break; > > case PROXYREQ_PROXY: > >- for (p = url; *p; ++p) { > >- if (!apr_isalnum(*p) && !strchr(allowed, *p)) { > >- if (*p == '%' && apr_isxdigit(p[1]) && apr_isxdigit > >(p[2])) { > >- p += 2; /* an encoded char */ > >- } > >- else { > >- return HTTP_BAD_REQUEST; /* reject bad char in > >URL */ > >- } > >- } > >+ if (ap_proxy_isvalidenc(url, NULL) != APR_SUCCESS) { > >+ return HTTP_BAD_REQUEST; > > } > > path = url; > > break; > > > >Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c > >URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/ > >proxy_util.c?rev=583466&r1=583465&r2=583466&view=diff > >====================================================================== > >======== > >--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) > >+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Wed Oct 10 > >06:16:56 2007 > >@@ -135,6 +135,33 @@ > > } > > > > /* > >+ * Confirm that a URL-encoded string only contains > >+ * valid encoding, valid chars are passed in allowed. > >+ * If allowed is NULL, we use useful default. > >+ */ > >+PROXY_DECLARE(apr_status_t)ap_proxy_isvalidenc(const char *url, > >+ const char *allowed) > >+ > >+{ > >+ if (!allowed) { > >+ allowed = "~$-_.+!*'(),;:@&=/"; /* allowed+reserved from > >+ ap_proxy_canonenc */ > >+ } > >+ > >+ for ( ; *url; ++url) { > >+ if (!apr_isalnum(*url) && !ap_strchr_c(allowed, *url)) { > >+ if (*url == '%' && apr_isxdigit(url[1]) && apr_isxdigit > >(url[2])) { > >+ url += 2; /* an encoded char */ > >+ } > >+ else { > >+ return APR_EGENERAL; /* reject bad char in URL */ > >+ } > >+ } > >+ } > >+ return APR_SUCCESS; > >+} > >+ > >+/* > > * canonicalise a URL-encoded string > > */ > > > > > > -- =========================================================================== Jim Jagielski [|] [EMAIL PROTECTED] [|] http://www.jaguNET.com/ "If you can dodge a wrench, you can dodge a ball."
