From 660853c5a2f0d70c6f73683f95084ce01af90ebf Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Thu, 21 Sep 2023 13:34:33 +0000 Subject: [PATCH 1/3] mod_proxy: Simplify ap_proxy_get_worker_ex() Factorize duplicated code in the balancer and non-balancer cases by adding a new worker_matches() helper. No functional change intended. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1912462 13f79535-47bb-0310-9956-ffa450edef68 --- modules/proxy/proxy_util.c | 76 +++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 7c0d3150c3c..005346acfa9 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -1826,18 +1826,38 @@ static int ap_proxy_strcmp_ematch(const char *str, const char *expected) return 0; } +static APR_INLINE +int worker_matches(proxy_worker *worker, + const char *url, apr_size_t url_len, + apr_size_t min_match, apr_size_t *max_match, + unsigned int mask) +{ + apr_size_t name_len = strlen(worker->s->name_ex); + int name_match = worker->s->is_name_matchable; + if (name_len <= url_len + && name_len >= min_match + && name_len > *max_match + && ((name_match + && (mask & AP_PROXY_WORKER_IS_MATCH) + && !ap_proxy_strcmp_ematch(url, worker->s->name_ex)) + || (!name_match + && (mask & AP_PROXY_WORKER_IS_PREFIX) + && !strncmp(url, worker->s->name_ex, name_len)))) { + *max_match = name_len; + return 1; + } + return 0; +} + 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) { - proxy_worker *worker; proxy_worker *max_worker = NULL; - int max_match = 0; - int url_length; - int min_match; - int worker_name_length; + apr_size_t min_match, max_match = 0; + apr_size_t url_len; const char *c; char *url_copy; int i; @@ -1858,8 +1878,8 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker_ex(apr_pool_t *p, return NULL; } - url_length = strlen(url); - url_copy = apr_pstrmemdup(p, url, url_length); + url_len = strlen(url); + url_copy = apr_pstrmemdup(p, url, url_len); /* Default to lookup for both _PREFIX and _MATCH workers */ if (!(mask & (AP_PROXY_WORKER_IS_PREFIX | AP_PROXY_WORKER_IS_MATCH))) { @@ -1885,48 +1905,28 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker_ex(apr_pool_t *p, ap_str_tolower(url_copy); min_match = strlen(url_copy); } + /* * Do a "longest match" on the worker name to find the worker that * fits best to the URL, but keep in mind that we must have at least * a minimum matching of length min_match such that * scheme://hostname[:port] matches between worker and url. */ - if (balancer) { - proxy_worker **workers = (proxy_worker **)balancer->workers->elts; - for (i = 0; i < balancer->workers->nelts; i++, workers++) { - worker = *workers; - if ( ((worker_name_length = strlen(worker->s->name_ex)) <= url_length) - && (worker_name_length >= min_match) - && (worker_name_length > max_match) - && (worker->s->is_name_matchable - || ((mask & AP_PROXY_WORKER_IS_PREFIX) - && strncmp(url_copy, worker->s->name_ex, - worker_name_length) == 0)) - && (!worker->s->is_name_matchable - || ((mask & AP_PROXY_WORKER_IS_MATCH) - && ap_proxy_strcmp_ematch(url_copy, - worker->s->name_ex) == 0)) ) { - max_worker = worker; - max_match = worker_name_length; + proxy_worker **worker = (proxy_worker **)balancer->workers->elts; + for (i = 0; i < balancer->workers->nelts; i++, worker++) { + if (worker_matches(*worker, url_copy, url_len, + min_match, &max_match, mask)) { + max_worker = *worker; } } - } else { - worker = (proxy_worker *)conf->workers->elts; + } + else { + proxy_worker *worker = (proxy_worker *)conf->workers->elts; for (i = 0; i < conf->workers->nelts; i++, worker++) { - if ( ((worker_name_length = strlen(worker->s->name_ex)) <= url_length) - && (worker_name_length >= min_match) - && (worker_name_length > max_match) - && (worker->s->is_name_matchable - || ((mask & AP_PROXY_WORKER_IS_PREFIX) - && strncmp(url_copy, worker->s->name_ex, - worker_name_length) == 0)) - && (!worker->s->is_name_matchable - || ((mask & AP_PROXY_WORKER_IS_MATCH) - && ap_proxy_strcmp_ematch(url_copy, - worker->s->name_ex) == 0)) ) { + if (worker_matches(worker, url_copy, url_len, + min_match, &max_match, mask)) { max_worker = worker; - max_match = worker_name_length; } } } From 069d70693dce73e4903ce9c5ff10c01ba158b28b Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Thu, 1 Aug 2024 11:35:26 +0000 Subject: [PATCH 2/3] mod_proxy: Fix selection of ProxyPassMatch workers with host/port substitution. PR 69233. With "ProxyPassMatch ^/([^/]+)/(.*)$ https://$1/$2", ap_proxy_get_worker_ex() should not consider the length of scheme://host part of the given URL because of the globbing match on the host part. Fix it by setting worker->s>is_host_matchable when creating a worker with host substitution and avoiding the min_match check in worker_matches() in this case. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1919617 13f79535-47bb-0310-9956-ffa450edef68 --- changes-entries/bz69233.txt | 2 ++ include/ap_mmn.h | 3 ++- modules/proxy/mod_proxy.h | 1 + modules/proxy/proxy_util.c | 30 +++++++++++++++++------------- 4 files changed, 22 insertions(+), 14 deletions(-) create mode 100644 changes-entries/bz69233.txt diff --git a/changes-entries/bz69233.txt b/changes-entries/bz69233.txt new file mode 100644 index 00000000000..9e1e43bfd08 --- /dev/null +++ b/changes-entries/bz69233.txt @@ -0,0 +1,2 @@ + *) mod_proxy: Fix selection of ProxyPassMatch workers with substitution + in the host name or port. PR 69233. [Yann Ylavic] \ No newline at end of file diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 2e63617271e..c124eb5370e 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -607,6 +607,7 @@ * and CONN_STATE_PROCESSING * 20120211.136 (2.4.63-dev) Add wait_io field to struct process_score * 20120211.137 (2.4.63-dev) Add AP_MPMQ_CAN_WAITIO + * 20120211.138 (2.4.63-dev) Add is_host_matchable to proxy_worker_shared */ #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */ @@ -614,7 +615,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20120211 #endif -#define MODULE_MAGIC_NUMBER_MINOR 137 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 138 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index cd388898c14..329d10fdaa9 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -493,6 +493,7 @@ typedef struct { unsigned int address_ttl_set:1; apr_int32_t address_ttl; /* backend address' TTL (seconds) */ apr_uint32_t address_expiry; /* backend address' next expiry time */ + unsigned int is_host_matchable:1; } proxy_worker_shared; #define ALIGNED_PROXY_WORKER_SHARED_SIZE (APR_ALIGN_DEFAULT(sizeof(proxy_worker_shared))) diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 005346acfa9..993533be6be 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -1826,23 +1826,26 @@ static int ap_proxy_strcmp_ematch(const char *str, const char *expected) return 0; } -static APR_INLINE -int worker_matches(proxy_worker *worker, - const char *url, apr_size_t url_len, - apr_size_t min_match, apr_size_t *max_match, - unsigned int mask) +static int worker_matches(proxy_worker *worker, + const char *url, apr_size_t url_len, + apr_size_t min_match, apr_size_t *max_match, + unsigned int mask) { apr_size_t name_len = strlen(worker->s->name_ex); - int name_match = worker->s->is_name_matchable; if (name_len <= url_len - && name_len >= min_match && name_len > *max_match - && ((name_match - && (mask & AP_PROXY_WORKER_IS_MATCH) - && !ap_proxy_strcmp_ematch(url, worker->s->name_ex)) - || (!name_match - && (mask & AP_PROXY_WORKER_IS_PREFIX) - && !strncmp(url, worker->s->name_ex, name_len)))) { + /* min_match is the length of the scheme://host part only of url, + * so it's used as a fast path to avoid the match when url is too + * small, but it's irrelevant when the worker host contains globs + * (i.e. ->is_host_matchable). + */ + && (worker->s->is_name_matchable + ? ((mask & AP_PROXY_WORKER_IS_MATCH) + && (worker->s->is_host_matchable || name_len >= min_match) + && !ap_proxy_strcmp_ematch(url, worker->s->name_ex)) + : ((mask & AP_PROXY_WORKER_IS_PREFIX) + && (name_len >= min_match) + && !strncmp(url, worker->s->name_ex, name_len)))) { *max_match = name_len; return 1; } @@ -2132,6 +2135,7 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p, wshared->was_malloced = (mask & AP_PROXY_WORKER_IS_MALLOCED) != 0; if (mask & AP_PROXY_WORKER_IS_MATCH) { wshared->is_name_matchable = 1; + wshared->is_host_matchable = (address_not_reusable != 0); /* Before AP_PROXY_WORKER_IS_MATCH (< 2.4.47), a regex worker with * dollar substitution was never matched against any actual URL, thus From 00cd80e114cf365f4e19ba94405b4591c1f993fb Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Thu, 1 Aug 2024 13:00:35 +0000 Subject: [PATCH 3/3] Follow up to r1919617: Better CHANGES entry per Eric. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1919619 13f79535-47bb-0310-9956-ffa450edef68 --- changes-entries/bz69233.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes-entries/bz69233.txt b/changes-entries/bz69233.txt index 9e1e43bfd08..ea1f7c72413 100644 --- a/changes-entries/bz69233.txt +++ b/changes-entries/bz69233.txt @@ -1,2 +1,2 @@ - *) mod_proxy: Fix selection of ProxyPassMatch workers with substitution + *) mod_proxy: Honor parameters of ProxyPassMatch workers with substitution in the host name or port. PR 69233. [Yann Ylavic] \ No newline at end of file