httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Plüm, Rüdiger, Vodafone Group <ruediger.pl...@vodafone.com>
Subject RE: ap_proxy_location_reverse_map()
Date Tue, 26 Nov 2013 16:14:54 GMT
IMHO this should be fixed in the configuration with an additional mapping that has the port
in. In many cases the port matters.

Regards

Rüdiger

From: Thomas Eckert [mailto:thomas.r.w.eckert@gmail.com]
Sent: Dienstag, 26. November 2013 17:11
To: dev@httpd.apache.org
Subject: ap_proxy_location_reverse_map()

I've been debugging some problems with incorrectly reverse mapped Location headers and found
some backend servers (e.g. OWA for Exchange 2013) to give headers like

  Location: https://myserver:443/path/file?query
which I think are perfectly fine. mod proxy fails to do the trick because

        else {
            const char *part = url;
            l2 = strlen(real);
            if (real[0] == '/') {
                part = ap_strstr_c(url, "://");
                if (part) {
                    part = ap_strchr_c(part+3, '/');
                    if (part) {
                        l1 = strlen(part);
                    }
                    else {
                        part = url;
                    }
                }
                else {
                    part = url;
                }
            }
>          if (l1 >= l2 && strncasecmp(real, part, l2) == 0) {
                u = apr_pstrcat(r->pool, ent[i].fake, &part[l2], NULL);
                return ap_is_url(u) ? u : ap_construct_url(r->pool, u, r);
            }
        }
which does not take the port behind the domain name into consideration (note: simple example
setup, fake path is just '/' obviously). I looked over the code and got the feeling the same
problem applies to the whole section, not just that one strncasecmp() call. Since the port
given by the backend server is not much use to the reverse proxy at that point, we can just
drop it on the floor and continue, e.g. like this

--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -894,11 +894,17 @@ PROXY_DECLARE(const char *) ap_proxy_location_reverse_map(request_rec
*r,
                     }
                 }
                 else if (l1 >= l2 && strncasecmp((*worker)->s->name, url,
l2) == 0) {
+                    const char* tmp_pchar = url + l2;
+                    if (url[l2] == ':') {
+                        tmp_pchar = ap_strchr_c(tmp_pchar, '/');
+                    }
+
                     /* edge case where fake is just "/"... avoid double slash */
-                    if ((ent[i].fake[0] == '/') && (ent[i].fake[1] == 0) &&
(url[l2] == '/')) {
-                        u = apr_pstrdup(r->pool, &url[l2]);
+                    if ((ent[i].fake[0] == '/') && (ent[i].fake[1] == 0) &&
+                        (tmp_pchar != NULL) && (tmp_pchar[0] == '/')) {
+                        u = apr_pstrdup(r->pool, tmp_pchar);
                     } else {
-                        u = apr_pstrcat(r->pool, ent[i].fake, &url[l2], NULL);
+                        u = apr_pstrcat(r->pool, ent[i].fake, tmp_pchar + 1, NULL);
                     }
                     return ap_is_url(u) ? u : ap_construct_url(r->pool, u, r);

 As said above this most likely needs to be spread to the other cases in that section as well.
Anyone see problems with this ?

Mime
View raw message