httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r649239 - in /httpd/httpd/trunk/modules/proxy: mod_proxy_ajp.c mod_proxy_http.c
Date Sat, 19 Apr 2008 10:11:50 GMT


On 04/18/2008 03:04 PM, Jim Jagielski wrote:
> 
> On Apr 18, 2008, at 8:55 AM, Plüm, Rüdiger, VF-Group wrote:
>>
>> IMHO it cannot include the query args except for the nocanon
>> case as in all other cases r->uri was used which does not contain
>> query args.
>> This is different from the situation in mod_proxy_http where we
>> can act as real (forward) proxy. mod_proxy_ajp always acts as a gateway.
>>
> 
> Oh... I wasn't aware you were referring *only* to the AJP impl
> (I saw that the patch was against that, but assumed you meant
> its m_p_h analog as well)...

In fact I meant both. But even in the mod_proxy_http case is does not
matter because we have the switch (r->proxyreq) there which lets us
distinguish between forward and reverse proxy requests. I assume that
the comparison between r->uri and r->unparsed_uri is legacy code that
was used before we had r->proxyreq.

> 
> I'm all for good looking code ;)
> 
> 

So what about the following patch (it effectively reverts yours and adds in
mine)?

Index: modules/proxy/mod_proxy_ajp.c
===================================================================
--- modules/proxy/mod_proxy_ajp.c       (Revision 649787)
+++ modules/proxy/mod_proxy_ajp.c       (Arbeitskopie)
@@ -31,7 +31,6 @@
  {
      char *host, *path, *search, sport[7];
      const char *err;
-    const char *pnocanon;
      apr_port_t port = AJP13_DEF_PORT;

      /* ap_port_of_scheme() */
@@ -59,27 +58,17 @@

      /*
       * now parse path/search args, according to rfc1738
-     *
-     * N.B. if this isn't a true proxy request, then the URL _path_
-     * has already been decoded.  True proxy requests have
-     * r->uri == r->unparsed_uri, and no others have that property.
       */
-    pnocanon = apr_table_get(r->notes, "proxy-nocanon");
-    if ((r->uri == r->unparsed_uri) ||  pnocanon) {
-        search = strchr(url, '?');
-        if (search != NULL)
-            *(search++) = '\0';
-    }
-    else
-        search = r->args;
+    search = NULL;

      /* process path */
-    if (pnocanon) {
+    if (apr_table_get(r->notes, "proxy-nocanon")) {
          path = url;   /* this is the raw path */
      }
      else {
          path = ap_proxy_canonenc(r->pool, url, strlen(url), enc_path, 0,
                                   r->proxyreq);
+        search = r->args;
      }
      if (path == NULL)
          return HTTP_BAD_REQUEST;
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c      (Revision 649787)
+++ modules/proxy/mod_proxy_http.c      (Arbeitskopie)
@@ -36,7 +36,6 @@
      char *host, *path, *search, sport[7];
      const char *err;
      const char *scheme;
-    const char *pnocanon;
      apr_port_t port, def_port;

      /* ap_port_of_scheme() */
@@ -69,19 +68,7 @@
      }

      /* now parse path/search args, according to rfc1738 */
-    /* N.B. if this isn't a true proxy request, then the URL _path_
-     * has already been decoded.  True proxy requests have r->uri
-     * == r->unparsed_uri, and no others have that property.
-     */
-    pnocanon = apr_table_get(r->notes, "proxy-nocanon");
-    if ((r->uri == r->unparsed_uri) ||
-        ((r->proxyreq == PROXYREQ_REVERSE) && pnocanon)) {
-        search = strchr(url, '?');
-        if (search != NULL)
-            *(search++) = '\0';
-    }
-    else
-        search = r->args;
+    search = NULL;

      /* process path */
      /* In a reverse proxy, our URL has been processed, so canonicalise
@@ -91,12 +78,13 @@
      switch (r->proxyreq) {
      default: /* wtf are we doing here? */
      case PROXYREQ_REVERSE:
-        if (pnocanon) {
+        if (apr_table_get(r->notes, "proxy-nocanon")) {
              path = url;   /* this is the raw path */
          }
          else {
              path = ap_proxy_canonenc(r->pool, url, strlen(url),
                                       enc_path, 0, r->proxyreq);
+            search = r->args;
          }
          break;
      case PROXYREQ_PROXY:


If you have no further objections I would commit it.

Regards

Rüdiger

Mime
View raw message