From cbf81b46440fc7759205ea715ebc452bdab7d937 Mon Sep 17 00:00:00 2001 From: Eric Covener Date: Fri, 27 Sep 2024 13:06:46 +0000 Subject: [PATCH] Merge r1919532, r1919533 from trunk: *) mod_proxy: Avoid AH01059 parsing error for SetHandler "unix:" URLs in (incomplete fix in 2.4.62). PR 69160. When SetHandler "unix:..." is used in a 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 --- CHANGES | 3 +++ modules/proxy/mod_proxy.c | 9 ++++++--- modules/proxy/proxy_util.c | 40 +++++++++++++++++++++++++++----------- 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/CHANGES b/CHANGES index 0b224034530..db727016a5f 100644 --- 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 (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/' is messed diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c index 8f13e686f97..756c41c4a1d 100644 --- 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 7c0d3150c3c..07621daed1a 100644 --- 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 , 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;