mod_proxy_fcgi: Don't re-encode SCRIPT_FILENAME. PR 69203

Before r1918550 (r1918559 in 2.4.60), "SetHandler proxy:..." configurations
did not pass through proxy_fixup() hence the proxy_canon_handler hooks, leaving
fcgi's SCRIPT_FILENAME environment variable (from r->filename) decoded, or more
exactly not re-encoded.

We still want to call ap_proxy_canon_url() for "fcgi:" to handle/strip the UDS
"unix:" case and check that r->filename is valid and contains no controls, but
proxy_fcgi_canon() will not ap_proxy_canonenc_ex() thus re-encode anymore.

Note that this will do the same for "ProxyPass fcgi:...", there is no reason
that using SetHandler or ProxyPass don't result in the same thing. If an opt
in/out makes sense we should probably look at ProxyFCGIBackendType.

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

diff --git a/changes-entries/bz69203.txt b/changes-entries/bz69203.txt
new file mode 100644 (file)
index 0000000..3613dad
--- /dev/null
+++ b/changes-entries/bz69203.txt
@@ -0,0 +1 @@
+mod_proxy_fcgi: Don't re-encode SCRIPT_FILENAME. PR 69203. [Yann Ylavic]
\ No newline at end of file

diff --git a/modules/proxy/mod_proxy_fcgi.c b/modules/proxy/mod_proxy_fcgi.c
index bb56cf0a45b137239e911c05b0b0ad75bcec17af..44327c70e33f7c7d7301fc0ac8ff1e43b28bb8cb 100644 (file)
--- a/modules/proxy/mod_proxy_fcgi.c
+++ b/modules/proxy/mod_proxy_fcgi.c
@@ -59,7 +59,7 @@ static int proxy_fcgi_canon(request_rec *r, char *url)
 {
     char *host, sport[7];
     const char *err;
-    char *path;
+    char *path = url, *pc;
     apr_port_t port, def_port;
     fcgi_req_config_t *rconf = NULL;
     const char *pathinfo_type = NULL;
@@ -75,7 +75,7 @@ static int proxy_fcgi_canon(request_rec *r, char *url)
 
     ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
                  "canonicalising URL %s", url);
-    err = ap_proxy_canon_netloc(r->pool, &url, NULL, NULL, &host, &port);
+    err = ap_proxy_canon_netloc(r->pool, &path, NULL, NULL, &host, &port);
     if (err) {
         ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01059)
                       "error parsing URL %s: %s", url, err);
@@ -92,29 +92,19 @@ static int proxy_fcgi_canon(request_rec *r, char *url)
         host = apr_pstrcat(r->pool, "[", host, "]", NULL);
     }
 
-    if (apr_table_get(r->notes, "proxy-nocanon")
-        || apr_table_get(r->notes, "proxy-noencode")) {
-        path = url;   /* this is the raw/encoded path */
-    }
-    else {
-        core_dir_config *d = ap_get_core_module_config(r->per_dir_config);
-        int flags = d->allow_encoded_slashes && !d->decode_encoded_slashes ? PROXY_CANONENC_NOENCODEDSLASHENCODING : 0;
-
-        path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path, flags,
-                                    r->proxyreq);
-        if (!path) {
-            return HTTP_BAD_REQUEST;
-        }
-    }
-    /*
-     * If we have a raw control character or a ' ' in nocanon path,
-     * correct encoding was missed.
+    /* We do not call ap_proxy_canonenc_ex() on the path here because the CGI
+     * environment variable SCRIPT_FILENAME based on it want the decoded/local
+     * path, don't let control characters pass still.
+     *
+     * XXX: should we encode based on dconf->backend_type though?
      */
-    if (path == url && *ap_scan_vchar_obstext(path)) {
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10414)
-                      "To be forwarded path contains control "
-                      "characters or spaces");
-        return HTTP_FORBIDDEN;
+    for (pc = path; *pc; pc++) {
+        if (apr_iscntrl(*pc)) {
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10414)
+                          "To be forwarded path contains control "
+                          "characters");
+            return HTTP_FORBIDDEN;
+        }
     }
 
     r->filename = apr_pstrcat(r->pool, "proxy:fcgi://", host, sport, "/",