mod_proxy: Avoid confusion of prefix/regex matching workers at loading. PR 65429.

ap_proxy_get_worker() needs to know whether it should lookup for prefix or
match or both matching workers, depending on the context.

For instance <Proxy[Match]> or ProxyPass[Match] directives need to lookup for
an existing worker with the same type as the directive (*Match or not), because
they will define one with that matching type if none exists.

On the contrary, "ProxySet <url>" at load time or ap_proxy_pre_request() at run
time need to find a worker matching an url whether it's by prefix or by regex.

So this commit adds ap_proxy_get_worker_ex() which takes a bitmask for the
matching type and calls it appropriately where needed.

For consistency, ap_proxy_define_worker_ex() is also added, using the same
bitmask flags, deprecating ap_proxy_define_match_worker().

Follow up to r1891206.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1891284 13f79535-47bb-0310-9956-ffa450edef68

include/ap_mmn.h

@@ -675,6 +675,8 @@
  * 20210531.0 (2.5.1-dev)  add conn_rec->outgoing and ap_ssl_bind_outgoing()
  * 20210531.1 (2.5.1-dev)  Add ap_bucket_type_wc, ap_bucket_wc_make() and
  *                         ap_bucket_wc_create() to util_filter.h
+ * 20210531.2 (2.5.1-dev)  Add ap_proxy_get_worker_ex() and
+ *                         ap_proxy_define_worker_ex() to mod_proxy.h
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -682,7 +684,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20210531
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 1             /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 2             /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

modules/proxy/mod_proxy.c

@@ -2046,7 +2046,8 @@ static const char *
     const apr_array_header_t *arr;
     const apr_table_entry_t *elts;
     int i;
-    int use_regex = is_regex;
+    unsigned int worker_type = (is_regex) ? AP_PROXY_WORKER_IS_MATCH
+                                          : AP_PROXY_WORKER_IS_PREFIX;
     unsigned int flags = 0;
     const char *err;
 
@@ -2062,7 +2063,7 @@ static const char *
                 if (is_regex) {
                     return "ProxyPassMatch invalid syntax ('~' usage).";
                 }
-                use_regex = 1;
+                worker_type = AP_PROXY_WORKER_IS_MATCH;
                 continue;
             }
             f = word;
@@ -2133,7 +2134,7 @@ static const char *
         dconf->alias_set = 1;
         new = dconf->alias;
         if (apr_fnmatch_test(f)) {
-            use_regex = 1;
+            worker_type = AP_PROXY_WORKER_IS_MATCH;
         }
     }
     /* if per server, add to the alias array */
@@ -2144,7 +2145,7 @@ static const char *
     new->fake = apr_pstrdup(cmd->pool, f);
     new->real = apr_pstrdup(cmd->pool, ap_proxy_de_socketfy(cmd->pool, r));
     new->flags = flags;
-    if (use_regex) {
+    if (worker_type & AP_PROXY_WORKER_IS_MATCH) {
         new->regex = ap_pregcomp(cmd->pool, f, AP_REG_EXTENDED);
         if (new->regex == NULL)
             return "Regular expression could not be compiled.";
@@ -2168,7 +2169,7 @@ static const char *
          * cannot be parsed anyway with apr_uri_parse later on in
          * ap_proxy_define_balancer / ap_proxy_update_balancer
          */
-        if (use_regex) {
+        if (worker_type & AP_PROXY_WORKER_IS_MATCH) {
             fake_copy = NULL;
         }
         else {
@@ -2191,29 +2192,19 @@ static const char *
         new->balancer = balancer;
     }
     else {
-        proxy_worker *worker = ap_proxy_get_worker(cmd->temp_pool, NULL, conf, new->real);
         int reuse = 0;
+        proxy_worker *worker = ap_proxy_get_worker_ex(cmd->temp_pool, NULL,
+                                                      conf, new->real,
+                                                      worker_type);
         if (!worker) {
             const char *err;
-            if (use_regex) {
+            err = ap_proxy_define_worker_ex(cmd->pool, &worker, NULL,
-                err = ap_proxy_define_match_worker(cmd->pool, &worker, NULL,
+                                            conf, r, worker_type);
-                                                   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 if ((use_regex != 0) ^ (worker->s->is_name_matchable != 0)) {
-            return apr_pstrcat(cmd->temp_pool, "ProxyPass/<Proxy> and "
-                               "ProxyPassMatch/<ProxyMatch> can't be used "
-                               "together with the same worker name ",
-                               "(", worker->s->name, ")", NULL);
-        }
         else {
             reuse = 1;
             ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd->server, APLOGNO(01145)
@@ -2757,7 +2748,8 @@ static const char *add_member(cmd_parms *cmd, void *dummy, const char *arg)
     }
 
     /* Try to find existing worker */
-    worker = ap_proxy_get_worker(cmd->temp_pool, balancer, conf, ap_proxy_de_socketfy(cmd->temp_pool, name));
+    worker = ap_proxy_get_worker(cmd->temp_pool, balancer, conf,
+                                 ap_proxy_de_socketfy(cmd->temp_pool, name));
     if (!worker) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd->server, APLOGNO(01147)
                      "Defining worker '%s' for balancer '%s'",
@@ -2806,6 +2798,7 @@ static const char *
     char *word, *val;
     proxy_balancer *balancer = NULL;
     proxy_worker *worker = NULL;
+    unsigned int worker_type = 0;
     int in_proxy_section = 0;
     /* XXX: Should this be NOT_IN_DIRECTORY|NOT_IN_FILES? */
     const char *err = ap_check_cmd_context(cmd, NOT_IN_HTACCESS);
@@ -2822,6 +2815,13 @@ static const char *
         name = ap_getword_conf(cmd->temp_pool, &pargs);
         if ((word = ap_strchr(name, '>')))
             *word = '\0';
+        if (strncasecmp(cmd->directive->parent->directive + 6,
+                        "Match", 5) == 0) {
+            worker_type = AP_PROXY_WORKER_IS_MATCH;
+        }
+        else {
+            worker_type = AP_PROXY_WORKER_IS_PREFIX;
+        }
         in_proxy_section = 1;
     }
     else {
@@ -2846,11 +2846,13 @@ static const char *
         }
     }
     else {
-        worker = ap_proxy_get_worker(cmd->temp_pool, NULL, conf, ap_proxy_de_socketfy(cmd->temp_pool, name));
+        worker = ap_proxy_get_worker_ex(cmd->temp_pool, NULL, conf,
+                                        ap_proxy_de_socketfy(cmd->temp_pool, name),
+                                        worker_type);
         if (!worker) {
             if (in_proxy_section) {
-                err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
+                err = ap_proxy_define_worker_ex(cmd->pool, &worker, NULL,
-                                             conf, name, 0);
+                                                conf, name, worker_type);
                 if (err)
                     return apr_pstrcat(cmd->temp_pool, "ProxySet ",
                                        err, NULL);
@@ -2904,8 +2906,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;
+    unsigned int worker_type = AP_PROXY_WORKER_IS_PREFIX;
-
     const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_CONTEXT);
     proxy_server_conf *sconf =
     (proxy_server_conf *) ap_get_module_config(cmd->server->module_config, &proxy_module);
@@ -2943,7 +2944,7 @@ static const char *proxysection(cmd_parms *cmd, void *mconfig, const char *arg)
         if (!r) {
             return "Regex could not be compiled";
         }
-        use_regex = 1;
+        worker_type = AP_PROXY_WORKER_IS_MATCH;
     }
 
     /* initialize our config and fetch it */
@@ -2990,27 +2991,16 @@ static const char *proxysection(cmd_parms *cmd, void *mconfig, const char *arg)
             }
         }
         else {
-            worker = ap_proxy_get_worker(cmd->temp_pool, NULL, sconf,
+            worker = ap_proxy_get_worker_ex(cmd->temp_pool, NULL, sconf,
-                                         ap_proxy_de_socketfy(cmd->temp_pool, (char*)conf->p));
+                                            ap_proxy_de_socketfy(cmd->temp_pool, conf->p),
+                                            worker_type);
             if (!worker) {
-                if (use_regex) {
+                err = ap_proxy_define_worker_ex(cmd->pool, &worker, NULL, sconf,
-                    err = ap_proxy_define_match_worker(cmd->pool, &worker, NULL,
+                                                conf->p, worker_type);
-                                                       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)) {
-                return apr_pstrcat(cmd->temp_pool, "ProxyPass/<Proxy> and "
-                                   "ProxyPassMatch/<ProxyMatch> can't be used "
-                                   "altogether with the same worker name ",
-                                   "(", worker->s->name, ")", NULL);
-            }
             if (!worker->section_config) {
                 worker->section_config = new_dir_conf;
             }

modules/proxy/mod_proxy.h

@@ -764,8 +764,29 @@ PROXY_DECLARE(int) ap_proxy_worker_can_upgrade(apr_pool_t *p,
                                                const char *upgrade,
                                                const char *dflt);
 
+/* Bitmask for ap_proxy_{define,get}_worker_ex(). */
+#define AP_PROXY_WORKER_IS_PREFIX   (1u << 0)
+#define AP_PROXY_WORKER_IS_MATCH    (1u << 1)
+#define AP_PROXY_WORKER_IS_MALLOCED (1u << 2)
+
 /**
- * Get the worker from proxy configuration
+ * Get the worker from proxy configuration, looking for either PREFIXED or
+ * MATCHED or both types of workers according to given mask
+ * @param p        memory pool used for finding worker
+ * @param balancer the balancer that the worker belongs to
+ * @param conf     current proxy server configuration
+ * @param url      url to find the worker from
+ * @param mask     bitmask of AP_PROXY_WORKER_IS_*
+ * @return         proxy_worker or NULL if not found
+ */
+PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker_ex(apr_pool_t *p,
+                                                     proxy_balancer *balancer,
+                                                     proxy_server_conf *conf,
+                                                     const char *url,
+                                                     unsigned int mask);
+
+/**
+ * Get the worker from proxy configuration, both types
  * @param p        memory pool used for finding worker
  * @param balancer the balancer that the worker belongs to
  * @param conf     current proxy server configuration
@@ -776,6 +797,25 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
                                                   proxy_balancer *balancer,
                                                   proxy_server_conf *conf,
                                                   const char *url);
+
+/**
+ * Define and Allocate space for the worker to proxy configuration, of either
+ * PREFIXED or MATCHED type according to given mask
+ * @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
+ * @param mask      bitmask of AP_PROXY_WORKER_IS_*
+ * @return          error message or NULL if successful (*worker is new worker)
+ */
+PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p,
+                                             proxy_worker **worker,
+                                             proxy_balancer *balancer,
+                                             proxy_server_conf *conf,
+                                             const char *url,
+                                             unsigned int mask);
+
  /**
  * Define and Allocate space for the worker to proxy configuration
  * @param p         memory pool to allocate worker from
@@ -803,6 +843,7 @@ PROXY_DECLARE(char *) ap_proxy_define_worker(apr_pool_t *p,
  * @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)
+ * @deprecated Replaced by ap_proxy_define_worker_ex()
  */
 PROXY_DECLARE(char *) ap_proxy_define_match_worker(apr_pool_t *p,
                                              proxy_worker **worker,

modules/proxy/proxy_util.c

@@ -1714,10 +1714,11 @@ static int ap_proxy_strcmp_ematch(const char *str, const char *expected)
     return 0;
 }
 
-PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
+PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker_ex(apr_pool_t *p,
                                                      proxy_balancer *balancer,
                                                      proxy_server_conf *conf,
-                                                  const char *url)
+                                                     const char *url,
+                                                     unsigned int mask)
 {
     proxy_worker *worker;
     proxy_worker *max_worker = NULL;
@@ -1743,6 +1744,11 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
     url_length = strlen(url);
     url_copy = apr_pstrmemdup(p, url, url_length);
 
+    /* Default to lookup for both _PREFIX and _MATCH workers */
+    if (!(mask & (AP_PROXY_WORKER_IS_PREFIX | AP_PROXY_WORKER_IS_MATCH))) {
+        mask |= AP_PROXY_WORKER_IS_PREFIX | AP_PROXY_WORKER_IS_MATCH;
+    }
+
     /*
      * We need to find the start of the path and
      * therefore we know the length of the scheme://hostname/
@@ -1777,11 +1783,13 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
                 && (worker_name_length >= min_match)
                 && (worker_name_length > max_match)
                 && (worker->s->is_name_matchable
-                    || strncmp(url_copy, worker->s->name,
+                    || ((mask & AP_PROXY_WORKER_IS_PREFIX)
-                               worker_name_length) == 0)
+                        && strncmp(url_copy, worker->s->name,
+                                   worker_name_length) == 0))
                 && (!worker->s->is_name_matchable
-                    || ap_proxy_strcmp_ematch(url_copy,
+                    || ((mask & AP_PROXY_WORKER_IS_MATCH)
-                                              worker->s->name) == 0) ) {
+                        && ap_proxy_strcmp_ematch(url_copy,
+                                                  worker->s->name) == 0)) ) {
                 max_worker = worker;
                 max_match = worker_name_length;
             }
@@ -1793,11 +1801,13 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
                 && (worker_name_length >= min_match)
                 && (worker_name_length > max_match)
                 && (worker->s->is_name_matchable
-                    || strncmp(url_copy, worker->s->name,
+                    || ((mask & AP_PROXY_WORKER_IS_PREFIX)
-                               worker_name_length) == 0)
+                        && strncmp(url_copy, worker->s->name,
+                                   worker_name_length) == 0))
                 && (!worker->s->is_name_matchable
-                    || ap_proxy_strcmp_ematch(url_copy,
+                    || ((mask & AP_PROXY_WORKER_IS_MATCH)
-                                              worker->s->name) == 0) ) {
+                        && ap_proxy_strcmp_ematch(url_copy,
+                                                  worker->s->name) == 0)) ) {
                 max_worker = worker;
                 max_match = worker_name_length;
             }
@@ -1807,6 +1817,14 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
     return max_worker;
 }
 
+PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
+                                                  proxy_balancer *balancer,
+                                                  proxy_server_conf *conf,
+                                                  const char *url)
+{
+    return ap_proxy_get_worker_ex(p, balancer, conf, url, 0);
+}
+
 /*
  * To create a worker from scratch first we define the
  * specifics of the worker; this is all local data.
@@ -1814,11 +1832,12 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
  * shared. This allows for dynamic addition during
  * config and runtime.
  */
-static char *proxy_define_worker(apr_pool_t *p,
+PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p,
                                              proxy_worker **worker,
                                              proxy_balancer *balancer,
-                                 proxy_server_conf *conf, const char *url,
+                                             proxy_server_conf *conf,
-                                 int do_malloc, int matchable)
+                                             const char *url,
+                                             unsigned int mask)
 {
     apr_status_t rv;
     proxy_worker_shared *wshared;
@@ -1846,7 +1865,7 @@ static char *proxy_define_worker(apr_pool_t *p,
         ptr = url;
     }
 
-    if (matchable) {
+    if (mask & AP_PROXY_WORKER_IS_MATCH) {
         /* apr_uri_parse() will accept the '$' sign anywhere in the URL but
          * in the :port part, and we don't want scheme://host:port$1$2/path
          * to fail (e.g. "ProxyPassMatch ^/(a|b)(/.*)? http://host:port$2").
@@ -1942,7 +1961,7 @@ static char *proxy_define_worker(apr_pool_t *p,
     /* right here we just want to tuck away the worker info.
      * if called during config, we don't have shm setup yet,
      * so just note the info for later. */
-    if (do_malloc)
+    if (mask & AP_PROXY_WORKER_IS_MALLOCED)
         wshared = ap_malloc(sizeof(proxy_worker_shared));  /* will be freed ap_proxy_share_worker */
     else
         wshared = apr_palloc(p, sizeof(proxy_worker_shared));
@@ -1975,7 +1994,7 @@ static char *proxy_define_worker(apr_pool_t *p,
     wshared->smax = -1;
     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->was_malloced = (mask & AP_PROXY_WORKER_IS_MALLOCED) != 0;
     wshared->is_name_matchable = 0;
     if (sockpath) {
         if (PROXY_STRNCPY(wshared->uds_path, sockpath) != APR_SUCCESS) {
@@ -1996,6 +2015,20 @@ static char *proxy_define_worker(apr_pool_t *p,
     (*worker)->balancer = balancer;
     (*worker)->s = wshared;
 
+    if (mask & AP_PROXY_WORKER_IS_MATCH) {
+        (*worker)->s->is_name_matchable = 1;
+        if (ap_strchr_c((*worker)->s->name, '$')) {
+            /* Before AP_PROXY_WORKER_IS_MATCH (< 2.4.47), 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;
 }
 
@@ -2006,9 +2039,13 @@ PROXY_DECLARE(char *) ap_proxy_define_worker(apr_pool_t *p,
                                              const char *url,
                                              int do_malloc)
 {
-    return proxy_define_worker(p, worker, balancer, conf, url, do_malloc, 0);
+    return ap_proxy_define_worker_ex(p, worker, balancer, conf, url,
+                                     AP_PROXY_WORKER_IS_PREFIX |
+                                     (do_malloc ? AP_PROXY_WORKER_IS_MALLOCED
+                                                : 0));
 }
 
+/* DEPRECATED */
 PROXY_DECLARE(char *) ap_proxy_define_match_worker(apr_pool_t *p,
                                              proxy_worker **worker,
                                              proxy_balancer *balancer,
@@ -2016,25 +2053,10 @@ PROXY_DECLARE(char *) ap_proxy_define_match_worker(apr_pool_t *p,
                                              const char *url,
                                              int do_malloc)
 {
-    char *err;
+    return ap_proxy_define_worker_ex(p, worker, balancer, conf, url,
-
+                                     AP_PROXY_WORKER_IS_MATCH |
-    err = proxy_define_worker(p, worker, balancer, conf, url, do_malloc, 1);
+                                     (do_malloc ? AP_PROXY_WORKER_IS_MALLOCED
-    if (err) {
+                                                : 0));
-        return err;
-    }
-
-    (*worker)->s->is_name_matchable = 1;
-    if (ap_strchr_c((*worker)->s->name, '$')) {
-        /* 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;
 }
 
 /*