Merge r1838684, r1920570, r1920571, r1920572 from trunk:

When a rewrite to proxy is configured in the server config, a check is made to make sure mod_proxy is active.  But the same is not done if a rewrite to proxy is configured in an .htaccess file. 

Basically this patch is the block of code from hook_uri2file that does the proxy check, copied to hook_fixup.

Patch provided by Michael Streeter [mstreeter1 gmail.com], slightly modified to use a new APLOGNO
PR 56264


mod_rewrite, mod_proxy: mod_proxy to cononicalize rewritten [P] URLs. PR 69235.

When mod_rewrite sets a "proxy:" URL with [P], it should be canonicalized by
mod_proxy still, notably to handle any "unix:" local socket part.

To avoid double encoding in perdir context, a follow up commit should remove the
ap_escape_uri() done in mod_rewrite since it's now on mod_proxy to canonicalize,
per PR 69260.



* Leave the proper escaping of the URL and the adding of r->args to the
  proxy module which runs after us after r1920570.
  Just take care to add r->args in case the proxy rule has the
  [NE] flag set and tell the proxy module to not escape in this case.


* Mention the additional bug

Submitted by: jailletc36, ylavic, rpluem
Reviewed by: rpluem, ylavic, covener

Github: closes #484


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

CHANGES

@@ -1,6 +1,13 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.63
 
+  *) mod_rewrite, mod_proxy: mod_proxy to canonicalize rewritten [P] URLs,
+     including "unix:" ones.  PR 69235, PR 69260.  [Yann Ylavic, Ruediger Pluem]
+
+  *) mod_rewrite: Error out in case a RewriteRule in directory context uses the
+     proxy, but mod_proxy is not loaded. PR 56264.
+     [Christophe Jaillet, Michael Streeter <mstreeter1@gmail.com>]
+
   *) http: Remove support for Request-Range header sent by Navigator 2-3 and
      MSIE 3. [Stefan Fritsch]
 

STATUS

@@ -165,18 +165,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_rewrite, mod_proxy: mod_proxy to cononicalize rewritten [P] URLs,
-     including "unix:" ones.  PR 69235, PR 69260, PR 56264
-     Trunk version of patch:
-        https://svn.apache.org/r1838684
-        https://svn.apache.org/r1920570
-        https://svn.apache.org/r1920571
-        https://svn.apache.org/r1920572
-     Backport version for 2.4.x of patch:
-       https://patch-diff.githubusercontent.com/raw/apache/httpd/pull/484.diff
-     Can be applied via apply_backport_pr.sh 484
-     +1: rpluem, ylavic, covener
-
   *) mod_ssl: Fix regression in PKCS#11 handling which should work without
      SSLCryptoDevice configured
      trunk patch: https://svn.apache.org/r1920597

modules/mappers/mod_rewrite.c

@@ -4515,20 +4515,6 @@ static rule_return_type apply_rewrite_rule(rewriterule_entry *p,
      * ourself).
      */
     if (p->flags & RULEFLAG_PROXY) {
-        /* For rules evaluated in server context, the mod_proxy fixup
-         * hook can be relied upon to escape the URI as and when
-         * necessary, since it occurs later.  If in directory context,
-         * the ordering of the fixup hooks is forced such that
-         * mod_proxy comes first, so the URI must be escaped here
-         * instead.  See PR 39746, 46428, and other headaches. */
-        if (ctx->perdir && (p->flags & RULEFLAG_NOESCAPE) == 0) {
-            char *old_filename = r->filename;
-
-            r->filename = ap_escape_uri(r->pool, r->filename);
-            rewritelog(r, 2, ctx->perdir, "escaped URI in per-dir context "
-                       "for proxy, %s -> %s", old_filename, r->filename);
-        }
-
         fully_qualify_uri(r);
 
         rewritelog(r, 2, ctx->perdir, "forcing proxy-throughput with %s",
@@ -5051,7 +5037,7 @@ static int hook_uri2file(request_rec *r)
             }
             if ((r->args != NULL)
                 && ((r->proxyreq == PROXYREQ_PROXY)
-                    || (rulestatus == ACTION_NOESCAPE))) {
+                    || apr_table_get(r->notes, "proxy-nocanon"))) {
                 /* see proxy_http:proxy_http_canon() */
                 r->filename = apr_pstrcat(r->pool, r->filename,
                                           "?", r->args, NULL);
@@ -5342,13 +5328,28 @@ static int hook_fixup(request_rec *r)
         if (to_proxyreq) {
             /* it should go on as an internal proxy request */
 
-            /* make sure the QUERY_STRING and
-             * PATH_INFO parts get incorporated
+            /* check if the proxy module is enabled, so
+             * we can actually use it!
+             */
+            if (!proxy_available) {
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10160)
+                              "attempt to make remote request from mod_rewrite "
+                              "without proxy enabled: %s", r->filename);
+                return HTTP_FORBIDDEN;
+            }
+
+            if (rulestatus == ACTION_NOESCAPE) {
+                apr_table_setn(r->notes, "proxy-nocanon", "1");
+            }
+
+            /* make sure the QUERY_STRING gets incorporated in the case
+             * [NE] was specified on the Proxy rule. We are preventing
+             * mod_proxy canon handler from incorporating r->args as well
+             * as escaping the URL.
              * (r->path_info was already appended by the
              * rewriting engine because of the per-dir context!)
              */
-            if (r->args != NULL) {
-                /* see proxy_http:proxy_http_canon() */
+            if ((r->args != NULL) && apr_table_get(r->notes, "proxy-nocanon")) {
                 r->filename = apr_pstrcat(r->pool, r->filename,
                                           "?", r->args, NULL);
             }
@@ -5648,10 +5649,7 @@ static void ap_register_rewrite_mapfunc(char *name, rewrite_mapfunc_t *func)
 
 static void register_hooks(apr_pool_t *p)
 {
-    /* fixup after mod_proxy, so that the proxied url will not
-     * escaped accidentally by mod_proxy's fixup.
-     */
-    static const char * const aszPre[]={ "mod_proxy.c", NULL };
+    static const char * const aszModProxy[] = { "mod_proxy.c", NULL };
 
     /* make the hashtable before registering the function, so that
      * other modules are prevented from accessing uninitialized memory.
@@ -5663,10 +5661,12 @@ static void register_hooks(apr_pool_t *p)
     ap_hook_pre_config(pre_config, NULL, NULL, APR_HOOK_MIDDLE);
     ap_hook_post_config(post_config, NULL, NULL, APR_HOOK_MIDDLE);
     ap_hook_child_init(init_child, NULL, NULL, APR_HOOK_MIDDLE);
-
-    ap_hook_fixups(hook_fixup, aszPre, NULL, APR_HOOK_FIRST);
+
+    /* allow to change the uri before mod_proxy takes over it */
+    ap_hook_translate_name(hook_uri2file, NULL, aszModProxy, APR_HOOK_FIRST);
+    /* fixup before mod_proxy so that a [P] URL gets fixed up there */
+    ap_hook_fixups(hook_fixup, NULL, aszModProxy, APR_HOOK_FIRST);
     ap_hook_fixups(hook_mimetype, NULL, NULL, APR_HOOK_LAST);
-    ap_hook_translate_name(hook_uri2file, NULL, NULL, APR_HOOK_FIRST);
 }
 
     /* the main config structure */

modules/proxy/mod_proxy.c

@@ -3347,27 +3347,26 @@ static int proxy_pre_config(apr_pool_t *pconf, apr_pool_t *plog,
 }
 static void register_hooks(apr_pool_t *p)
 {
-    /* fixup before mod_rewrite, so that the proxied url will not
-     * escaped accidentally by our fixup.
-     */
-    static const char * const aszSucc[] = { "mod_rewrite.c", NULL};
     /* Only the mpm_winnt has child init hook handler.
      * make sure that we are called after the mpm
      * initializes.
      */
     static const char *const aszPred[] = { "mpm_winnt.c", "mod_proxy_balancer.c",
                                            "mod_proxy_hcheck.c", NULL};
+    static const char * const aszModRewrite[] = { "mod_rewrite.c", NULL };
+
     /* handler */
     ap_hook_handler(proxy_handler, NULL, NULL, APR_HOOK_FIRST);
     /* filename-to-URI translation */
     ap_hook_pre_translate_name(proxy_pre_translate_name, NULL, NULL,
                                APR_HOOK_MIDDLE);
-    ap_hook_translate_name(proxy_translate_name, aszSucc, NULL,
+    /* mod_rewrite has a say on the uri before proxy translation */
+    ap_hook_translate_name(proxy_translate_name, aszModRewrite, NULL,
                            APR_HOOK_FIRST);
     /* walk <Proxy > entries and suppress default TRACE behavior */
     ap_hook_map_to_storage(proxy_map_location, NULL,NULL, APR_HOOK_FIRST);
-    /* fixups */
-    ap_hook_fixups(proxy_fixup, NULL, aszSucc, APR_HOOK_FIRST);
+    /* fixup after mod_rewrite so that a [P] URL from there gets fixed up */
+    ap_hook_fixups(proxy_fixup, aszModRewrite, NULL, APR_HOOK_FIRST);
     /* post read_request handling */
     ap_hook_post_read_request(proxy_detect, NULL, NULL, APR_HOOK_FIRST);
     /* pre config handling */