httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@devsys.jaguNET.com>
Subject Re: svn commit: r583466 - in /httpd/httpd/trunk: include/ap_mmn.h modules/proxy/mod_proxy.h modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c
Date Wed, 10 Oct 2007 19:50:19 GMT
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, jim@apache.org 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   [|]   jim@jaguNET.com   [|]   http://www.jaguNET.com/
	    "If you can dodge a wrench, you can dodge a ball."

Mime
View raw message