You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2007/10/10 15:16:57 UTC
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
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
*/
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
Posted by Jim Jagielski <ji...@devsys.jaguNET.com>.
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."
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
Posted by "Roy T. Fielding" <ro...@gmail.com>.
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
> */
>
>
>