Merge r1919532, r1919533 from trunk:

*) mod_proxy: Avoid AH01059 parsing error for SetHandler "unix:" URLs
   in <Location> (incomplete fix in 2.4.62). PR 69160.

When SetHandler "unix:..." is used in a <Location "/path"> block, the path
gets appended (including $DOCUMENT_ROOT somehow) to r->filename hence the
current checks in fixup_uds_filename() to add "localhost" when missing don't
work. Fix them.

mod_proxy: Allow for empty UDS URL hostname in ProxyPass workers too.

Using "unix:/udspath|scheme:" or "unix:/udspath|scheme://" for a ProxyPass URL
does not work currently, while it works for SetHandler "proxy:unix:...".

Submitted by: ylavic
Reviewed by: ylavic, covener, rpluem

Github: closes #467

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1920977 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 0b224034530c7ddadd10570ffda27c303787b677..db727016a5fd5f9dcc4c1b7c5181fc4f72550e05 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.63
 
+  *) mod_proxy: Avoid AH01059 parsing error for SetHandler "unix:" URLs
+     in <Location> (incomplete fix in 2.4.62). PR 69160. [Yann Ylavic]
+
   *) mod_md: update to version 2.4.28
      - When the server starts, it looks for new, staged certificates to
        activate. If the staged set of files in 'md/staging/<domain>' is messed

diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c
index 8f13e686f976556f25e1cd42b77d6b3b37035d94..756c41c4a1dee880e27750b40e35c72234ae615d 100644 (file)
--- a/modules/proxy/mod_proxy.c
+++ b/modules/proxy/mod_proxy.c
@@ -1948,9 +1948,9 @@ PROXY_DECLARE(const char *) ap_proxy_de_socketfy(apr_pool_t *p, const char *url)
         const char *ret, *c;
 
         ret = ptr + 1;
-        /* special case: "unix:....|scheme:" is OK, expand
-         * to "unix:....|scheme://localhost"
-         * */
+        /* special cases: "unix:...|scheme:" ind "unix:...|scheme://" are OK,
+         * expand to "unix:....|scheme://localhost"
+         */
         c = ap_strchr_c(ret, ':');
         if (c == NULL) {
             return NULL;
@@ -1958,6 +1958,9 @@ PROXY_DECLARE(const char *) ap_proxy_de_socketfy(apr_pool_t *p, const char *url)
         if (c[1] == '\0') {
             return apr_pstrcat(p, ret, "//localhost", NULL);
         }
+        else if (c[1] == '/' && c[2] == '/' && !c[3]) {
+            return apr_pstrcat(p, ret, "localhost", NULL);
+        }
         else {
             return ret;
         }

diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index 7c0d3150c3c2a54acdbd613283316dce29b0789f..07621daed1a972e6ef8d001b31971756f451ffc6 100644 (file)
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -1972,7 +1972,7 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p,
             && (ptr = ap_strchr_c(url + 5, '|'))) {
         rv = apr_uri_parse(p, apr_pstrmemdup(p, url, ptr - url), &uri);
         if (rv == APR_SUCCESS) {
-            sockpath = ap_runtime_dir_relative(p, uri.path);;
+            sockpath = ap_runtime_dir_relative(p, uri.path);
             ptr++;    /* so we get the scheme for the uds */
         }
         else {
@@ -2038,7 +2038,7 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p,
     if (!uri.scheme) {
         return apr_pstrcat(p, "URL must be absolute!: ", url, NULL);
     }
-    if (!uri.hostname) {
+    if (!uri.hostname || !*uri.hostname) {
         if (sockpath) {
             /* allow for unix:/path|http: */
             uri.hostname = "localhost";
@@ -2434,7 +2434,7 @@ static int fixup_uds_filename(request_rec *r)
     if (!strncmp(r->filename, "proxy:", 6) &&
             !ap_cstr_casecmpn(uds_url, "unix:", 5) &&
             (origin_url = ap_strchr(uds_url + 5, '|'))) {
-        char *uds_path = NULL, *end;
+        char *uds_path = NULL, *col;
         apr_uri_t urisock;
         apr_status_t rv;
 
@@ -2446,7 +2446,7 @@ static int fixup_uds_filename(request_rec *r)
                                                   || !urisock.hostname[0])) {
             uds_path = ap_runtime_dir_relative(r->pool, urisock.path);
         }
-        if (!uds_path || !(end = ap_strchr(origin_url, ':'))) {
+        if (!uds_path || !(col = ap_strchr(origin_url, ':'))) {
             ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10292)
                     "Invalid proxy UDS filename (%s)", r->filename);
             apr_table_unset(r->notes, "uds_path");
@@ -2459,21 +2459,39 @@ static int fixup_uds_filename(request_rec *r)
                 r->filename, origin_url, uds_path);
 
         /* The hostname part of the URL is not mandated for UDS though
-         * the canon_handler hooks will require it, so add "localhost"
-         * if it's missing (won't be used anyway for an AF_UNIX socket).
+         * the canon_handler hooks will require it. ProxyPass URLs are
+         * fixed at load time by adding "localhost" automatically in the
+         * worker URL, but SetHandler "proxy:unix:/udspath|scheme:[//]"
+         * URLs are not so we have to fix it here the same way.
          */
-        if (!end[1]) {
+        if (!col[1]) {
+            /* origin_url is "scheme:" */
             r->filename = apr_pstrcat(r->pool, "proxy:",
                                       origin_url, "//localhost",
                                       NULL);
         }
-        else if (end[1] == '/' && end[2] == '/' && !end[3]) {
+        /* For a SetHandler "proxy:..." in a <Location "/path">, the "/path"
+         * is appended to r->filename, hence the below origin_url cases too:
+         */
+        else if (col[1] == '/' && (col[2] != '/'    /* "scheme:/path" */
+                                   || col[3] == '/' /* "scheme:///path" */
+                                   || !col[3])) {   /* "scheme://" */
+            char *scheme = origin_url;
+            *col = '\0'; /* nul terminate scheme */
+            if (col[2] != '/') {
+                origin_url = col + 1;
+            }
+            else {
+                origin_url = col + 3;
+            }
             r->filename = apr_pstrcat(r->pool, "proxy:",
-                                      origin_url, "localhost",
-                                      NULL);
+                                      scheme, "://localhost",
+                                      origin_url, NULL);
         }
         else {
-            /* Overwrite the UDS part of r->filename in place */
+            /* origin_url is normal "scheme://host/path", can overwrite
+             * the UDS part of r->filename in place.
+             */
             memmove(uds_url, origin_url, strlen(origin_url) + 1);
         }
         return OK;