]> git.ipfire.org Git - thirdparty/apache/httpd.git/commitdiff
Merge r1609680, r1609688, r1641381, r1826289, r1826313, r1878467, r1878994, r1879000...
authorJim Jagielski <jim@apache.org>
Tue, 11 Aug 2020 12:05:11 +0000 (12:05 +0000)
committerJim Jagielski <jim@apache.org>
Tue, 11 Aug 2020 12:05:11 +0000 (12:05 +0000)
mod_proxy: add ap_proxy_define_match_worker() and use it for ProxyPassMatch
and ProxyMatch section to distinguish between normal workers and workers
with regex substitutions in the name. Implement handling of such workers
in ap_proxy_get_worker(). PR 43513

mod_proxy: better check for worker->s->is_name_matchable

Return a match whenever we get to the end of the worker name, regardless
of whether there is URL left.

ProxyPassMatch had been using the default worker in trunk.

Follow up to r1609680: simpler/faster ap_proxy_strcmp_ematch().

No functional change.

Follow up to r1609680: further simplify/optimize ap_proxy_strcmp_ematch().

While at it, same treatment for its mother ap_strcmp_match().

make sure the $n of the regular expressions is not included the name of the worker.
for example,  the example:
ProxyPassMatch "^(/.*\.gif)$" "http://backend.example.com:8000$1"
was giving:
AH00526: Syntax error on line nnn of bla/conf/httpd.conf:
ProxyPass Unable to parse URL: http://backend.example.com:8000$1

ap_proxy_define_match_worker: don't copy the url unnecessarily.

And save a few cycles, when the duplication is needed, by not copying
the ignored part.

ap_proxy_define_match_worker: disable connection reuse by default.

To avoid compat issues with dns/connection reuse now that a worker with
dollar substitution can be elected.

CHANGES entry for ap_proxy_define_match_worker().

Oups, axe spurious copypasta.

mod_proxy: unfail mixed ProxyPass/<Proxy> and ProxyPassMatch/<ProxyMatch>.

It is not a failure in current 2.4.x, so to ease backport and to avoid compat
breakage simply warn about the second directive being ignored.

This commit can be reverted in trunk if we want next versions to fail in this
case.

[Reverted by r1879363]

Submitted by: jkaluza, covener, ylavic, ylavic, jfclere, ylavic, ylavic, ylavic, ylavic, ylavic
Reviewed by: ylavic, minfrin (with an MMN bump), jim (agree w/ minfrin)

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

CHANGES
STATUS
include/ap_mmn.h
modules/proxy/mod_proxy.c
modules/proxy/mod_proxy.h
modules/proxy/proxy_util.c
server/util.c

diff --git a/CHANGES b/CHANGES
index 0cff27ed02e81813098b23f8f063571aabfb03ed..6706ff7d1640a2d2fb959fdaf4d646d505d4ef39 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,11 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.47
 
+  *) mod_proxy: recognize parameters from ProxyPassMatch workers with dollar
+     substitution, such that they apply to the backend connection.  Note that
+     connection reuse is disabled by default to avoid compatibility issues.
+     [Takashi Sato, Jan Kaluza, Eric Covener, Yann Ylavic, Jean-Frederic Clere]
+
 Changes with Apache 2.4.46
 
   *) SECURITY: CVE-2020-11984 (cve.mitre.org)
diff --git a/STATUS b/STATUS
index 8829465ce21b933bcb74812b5f37edcaa313d118..75425c266507fd8e892c0520fa0b638097c7e421 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -138,25 +138,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_proxy: Add ap_proxy_define_match_worker() and use it for ProxyPassMatch
-     and ProxyMatch section to distinguish between normal workers and workers
-     with regex substitutions in the name. Implement handling of such workers
-     in ap_proxy_get_worker(). Fixes the bug when regex workers were not
-     matched and used for request. PR 43513 and 64537.
-     trunk patch: http://svn.apache.org/r1609680
-                  http://svn.apache.org/r1609688
-                  http://svn.apache.org/r1641381
-                  http://svn.apache.org/r1826289
-                  http://svn.apache.org/r1826313
-                  http://svn.apache.org/r1878467
-                  http://svn.apache.org/r1878994
-                  http://svn.apache.org/r1879000
-                  http://svn.apache.org/r1879001
-                  http://svn.apache.org/r1879002
-                  http://svn.apache.org/r1879361
-     2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-ap_proxy_define_match_worker-v6.patch
-     +1: ylavic, minfrin (with an MMN bump), jim (agree w/ minfrin)
-
   *) mod_proxy_uwsgi: Avoid NULL pointer dereferences for empty environment variable values
       http://svn.apache.org/r1879878
    Backport version for 2.4.x of patch:
index 805dc0c780d55941b3e57b461988faca99992322..47d15dff0b414289abc37645d249468f2dfd8138 100644 (file)
  * 20120211.91 (2.4.42-dev) Add ap_is_chunked() in httpd.h
  * 20120211.92 (2.4.42-dev) AP_REG_NO_DEFAULT macro in ap_regex.h
  * 20120211.93 (2.4.44-dev) Add ap_parse_strict_length()
+ * 20120211.94 (2.4.47-dev) Add ap_proxy_define_match_worker()
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20120211
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 93                  /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 94                  /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
index e599515d5953e7fe507e11551a8a7dbdad7e9cb3..d55aec1f7b5ac1cbb4d661eaf7c5defc47995e99 100644 (file)
@@ -1864,15 +1864,41 @@ static const char *
         new->balancer = balancer;
     }
     else {
-        proxy_worker *worker = ap_proxy_get_worker(cmd->temp_pool, NULL, conf, ap_proxy_de_socketfy(cmd->pool, r));
+        proxy_worker *worker = ap_proxy_get_worker(cmd->temp_pool, NULL, conf, new->real);
         int reuse = 0;
         if (!worker) {
-            const char *err = ap_proxy_define_worker(cmd->pool, &worker, NULL, conf, r, 0);
+            const char *err;
+            if (use_regex) {
+                err = ap_proxy_define_match_worker(cmd->pool, &worker, NULL,
+                                                   conf, r, 0);
+            }
+            else {
+                err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
+                                             conf, r, 0);
+            }
             if (err)
                 return apr_pstrcat(cmd->temp_pool, "ProxyPass ", err, NULL);
 
             PROXY_COPY_CONF_PARAMS(worker, conf);
-        } else {
+        }
+        else if ((use_regex != 0) ^ (worker->s->is_name_matchable != 0)) {
+            ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server, APLOGNO(10249)
+                         "ProxyPass/<Proxy> and ProxyPassMatch/<ProxyMatch> "
+                         "can't be used altogether with the same worker "
+                         "name (%s); ignoring ProxyPass%s",
+                         worker->s->name, use_regex ? "Match" : "");
+            /* Rollback new alias */
+            if (cmd->path) {
+                dconf->alias = NULL;
+                dconf->alias_set = 0;
+            }
+            else {
+                memset(new, 0, sizeof(*new));
+                apr_array_pop(conf->aliases);
+            }
+            return NULL;
+        }
+        else {
             reuse = 1;
             ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd->server, APLOGNO(01145)
                          "Sharing worker '%s' instead of creating new worker '%s'",
@@ -2499,6 +2525,7 @@ static const char *proxysection(cmd_parms *cmd, void *mconfig, const char *arg)
     char *word, *val;
     proxy_balancer *balancer = NULL;
     proxy_worker *worker = NULL;
+    int use_regex = 0;
 
     const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_CONTEXT);
     proxy_server_conf *sconf =
@@ -2537,6 +2564,7 @@ static const char *proxysection(cmd_parms *cmd, void *mconfig, const char *arg)
         if (!r) {
             return "Regex could not be compiled";
         }
+        use_regex = 1;
     }
 
     /* initialize our config and fetch it */
@@ -2586,12 +2614,29 @@ static const char *proxysection(cmd_parms *cmd, void *mconfig, const char *arg)
             worker = ap_proxy_get_worker(cmd->temp_pool, NULL, sconf,
                                          ap_proxy_de_socketfy(cmd->temp_pool, (char*)conf->p));
             if (!worker) {
-                err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
-                                          sconf, conf->p, 0);
+                if (use_regex) {
+                    err = ap_proxy_define_match_worker(cmd->pool, &worker, NULL,
+                                                       sconf, conf->p, 0);
+                }
+                else {
+                    err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
+                                                 sconf, conf->p, 0);
+                }
                 if (err)
                     return apr_pstrcat(cmd->temp_pool, thiscmd->name,
                                        " ", err, NULL);
             }
+            else if ((use_regex != 0) ^ (worker->s->is_name_matchable != 0)) {
+                ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server, APLOGNO(10250)
+                             "ProxyPass/<Proxy> and ProxyPassMatch/<ProxyMatch> "
+                             "can't be used altogether with the same worker "
+                             "name (%s); ignoring <Proxy%s>",
+                             worker->s->name, use_regex ? "Match" : "");
+                /* Rollback new section */
+                ((void **)sconf->sec_proxy->elts)[sconf->sec_proxy->nelts - 1] = NULL;
+                apr_array_pop(sconf->sec_proxy);
+                goto cleanup;
+            }
             if (!worker->section_config) {
                 worker->section_config = new_dir_conf;
             }
@@ -2621,6 +2666,7 @@ static const char *proxysection(cmd_parms *cmd, void *mconfig, const char *arg)
         }
     }
 
+cleanup:
     cmd->path = old_path;
     cmd->override = old_overrides;
 
index 895b93708824b45634d6eeb39f2931a3a797406e..fa2f730d799250040d48da6245e11233b45446ac 100644 (file)
@@ -453,6 +453,7 @@ typedef struct {
     unsigned int     keepalive_set:1;
     unsigned int     disablereuse_set:1;
     unsigned int     was_malloced:1;
+    unsigned int     is_name_matchable:1;
     char      hcuri[PROXY_WORKER_MAX_ROUTE_SIZE];     /* health check uri */
     char      hcexpr[PROXY_WORKER_MAX_SCHEME_SIZE];   /* name of condition expr for health check */
     int             passes;     /* number of successes for check to pass */
@@ -753,6 +754,24 @@ PROXY_DECLARE(char *) ap_proxy_define_worker(apr_pool_t *p,
                                              const char *url,
                                              int do_malloc);
 
+/**
+ * Define and Allocate space for the ap_strcmp_match()able worker to proxy
+ * configuration.
+ * @param p         memory pool to allocate worker from
+ * @param worker    the new worker
+ * @param balancer  the balancer that the worker belongs to
+ * @param conf      current proxy server configuration
+ * @param url       url containing worker name (produces match pattern)
+ * @param do_malloc true if shared struct should be malloced
+ * @return          error message or NULL if successful (*worker is new worker)
+ */
+PROXY_DECLARE(char *) ap_proxy_define_match_worker(apr_pool_t *p,
+                                             proxy_worker **worker,
+                                             proxy_balancer *balancer,
+                                             proxy_server_conf *conf,
+                                             const char *url,
+                                             int do_malloc);
+
 /**
  * Share a defined proxy worker via shm
  * @param worker  worker to be shared
index e7ffe33f21988a61f6a4cd07dc782d8c2f79947a..18fd93c06f8381811435f0d36b71a13c616acd44 100644 (file)
@@ -1658,6 +1658,46 @@ PROXY_DECLARE(char *) ap_proxy_worker_name(apr_pool_t *p,
     return apr_pstrcat(p, "unix:", worker->s->uds_path, "|", worker->s->name, NULL);
 }
 
+/*
+ * Taken from ap_strcmp_match() :
+ * Match = 0, NoMatch = 1, Abort = -1, Inval = -2
+ * Based loosely on sections of wildmat.c by Rich Salz
+ * Hmmm... shouldn't this really go component by component?
+ *
+ * Adds handling of the "\<any>" => "<any>" unescaping.
+ */
+static int ap_proxy_strcmp_ematch(const char *str, const char *expected)
+{
+    apr_size_t x, y;
+
+    for (x = 0, y = 0; expected[y]; ++y, ++x) {
+        if (expected[y] == '$' && apr_isdigit(expected[y + 1])) {
+            do {
+                y += 2;
+            } while (expected[y] == '$' && apr_isdigit(expected[y + 1]));
+            if (!expected[y])
+                return 0;
+            while (str[x]) {
+                int ret;
+                if ((ret = ap_proxy_strcmp_ematch(&str[x++], &expected[y])) != 1)
+                    return ret;
+            }
+            return -1;
+        }
+        else if (!str[x]) {
+            return -1;
+        }
+        else if (expected[y] == '\\' && !expected[++y]) {
+            /* NUL is an invalid char! */
+            return -2;
+        }
+        if (str[x] != expected[y])
+            return 1;
+    }
+    /* We got all the way through the worker path without a difference */
+    return 0;
+}
+
 PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
                                                   proxy_balancer *balancer,
                                                   proxy_server_conf *conf,
@@ -1720,11 +1760,15 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
             if ( ((worker_name_length = strlen(worker->s->name)) <= url_length)
                 && (worker_name_length >= min_match)
                 && (worker_name_length > max_match)
-                && (strncmp(url_copy, worker->s->name, worker_name_length) == 0) ) {
+                && (worker->s->is_name_matchable
+                    || strncmp(url_copy, worker->s->name,
+                               worker_name_length) == 0)
+                && (!worker->s->is_name_matchable
+                    || ap_proxy_strcmp_ematch(url_copy,
+                                              worker->s->name) == 0) ) {
                 max_worker = worker;
                 max_match = worker_name_length;
             }
-
         }
     } else {
         worker = (proxy_worker *)conf->workers->elts;
@@ -1732,7 +1776,12 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
             if ( ((worker_name_length = strlen(worker->s->name)) <= url_length)
                 && (worker_name_length >= min_match)
                 && (worker_name_length > max_match)
-                && (strncmp(url_copy, worker->s->name, worker_name_length) == 0) ) {
+                && (worker->s->is_name_matchable
+                    || strncmp(url_copy, worker->s->name,
+                               worker_name_length) == 0)
+                && (!worker->s->is_name_matchable
+                    || ap_proxy_strcmp_ematch(url_copy,
+                                              worker->s->name) == 0) ) {
                 max_worker = worker;
                 max_match = worker_name_length;
             }
@@ -1866,6 +1915,7 @@ PROXY_DECLARE(char *) ap_proxy_define_worker(apr_pool_t *p,
     wshared->hash.def = ap_proxy_hashfunc(wshared->name, PROXY_HASHFUNC_DEFAULT);
     wshared->hash.fnv = ap_proxy_hashfunc(wshared->name, PROXY_HASHFUNC_FNV);
     wshared->was_malloced = (do_malloc != 0);
+    wshared->is_name_matchable = 0;
     if (sockpath) {
         if (PROXY_STRNCPY(wshared->uds_path, sockpath) != APR_SUCCESS) {
             return apr_psprintf(p, "worker uds path (%s) too long", sockpath);
@@ -1888,6 +1938,38 @@ PROXY_DECLARE(char *) ap_proxy_define_worker(apr_pool_t *p,
     return NULL;
 }
 
+PROXY_DECLARE(char *) ap_proxy_define_match_worker(apr_pool_t *p,
+                                             proxy_worker **worker,
+                                             proxy_balancer *balancer,
+                                             proxy_server_conf *conf,
+                                             const char *url,
+                                             int do_malloc)
+{
+    char *err;
+    const char *pdollar = ap_strchr_c(url, '$');
+
+    if (pdollar != NULL) {
+        url = apr_pstrmemdup(p, url, pdollar - url);
+    }
+    err = ap_proxy_define_worker(p, worker, balancer, conf, url, do_malloc);
+    if (err) {
+        return err;
+    }
+
+    (*worker)->s->is_name_matchable = 1;
+    if (pdollar) {
+        /* Before ap_proxy_define_match_worker() existed, a regex worker
+         * with dollar substitution was never matched against the actual
+         * URL thus the request fell through the generic worker. To avoid
+         * dns and connection reuse compat issues, let's disable connection
+         * reuse by default, it can still be overwritten by an explicit
+         * enablereuse=on.
+         */
+        (*worker)->s->disablereuse = 1;
+    }
+    return NULL;
+}
+
 /*
  * Create an already defined worker and free up memory
  */
index 44c61c8709db16a2a667f83689be261d939cf448..0248b3753900aa4ae5726aef622d35a59cddd171 100644 (file)
@@ -188,8 +188,6 @@ AP_DECLARE(int) ap_strcmp_match(const char *str, const char *expected)
     int x, y;
 
     for (x = 0, y = 0; expected[y]; ++y, ++x) {
-        if ((!str[x]) && (expected[y] != '*'))
-            return -1;
         if (expected[y] == '*') {
             while (expected[++y] == '*');
             if (!expected[y])
@@ -201,6 +199,8 @@ AP_DECLARE(int) ap_strcmp_match(const char *str, const char *expected)
             }
             return -1;
         }
+        else if (!str[x])
+            return -1;
         else if ((expected[y] != '?') && (str[x] != expected[y]))
             return 1;
     }