Merge r1753594 from trunk:

* Prevent redirect loops between workers within a balancer by limiting the
  number of redirects to the number balancer members.

PR: 59864

Submitted by: rpluem
Reviewed by: rpluem, jim, ylavic

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

diff --git a/CHANGES b/CHANGES
index ed50f1571f2225a7d3d70b93afc36118d35513dd..4cfc8ec78c2a116c52d064467fa26854e41d6bd2 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -5,6 +5,10 @@ Changes with Apache 2.4.24
   *) core: CVE-2016-5387: Mitigate [f]cgi "httpoxy" issues.
      [Dominic Scheirlinck <dominic vendhq.com>, Yann Ylavic]
 
+  *) mod_proxy_balancer: Prevent redirect loops between workers within a
+     balancer by limiting the number of redirects to the number balancer
+     members. PR 59864 [Ruediger Pluem]
+
   *) mod_proxy: Correctly consider error response codes by the backend when
      processing failonstatus. PR 59869 [Ruediger Pluem]
 

diff --git a/STATUS b/STATUS
index 9958a4f5923b18cd0732c6eb7c850f36e0c7dd46..a745a3d342a6a47fadc74898f97ea5aa1fdc36f2 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -117,15 +117,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_proxy_balancer: Prevent redirect loops between workers within a
-     balancer by limiting the number of redirects to the number balancer
-     members. PR 59864
-      Trunk version of patch:
-         http://svn.apache.org/r1753594
-      Backport version for 2.4.x of patch:
-         Trunk version of patch works (modulo CHANGES)
-      +1: rpluem, jim, ylavic
-
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c
index 69ad5dcefd1f2714ce32088b3a27a516d0e86cdb..ec9f103adb47796421db5211252a07ee3adff4c2 100644 (file)
--- a/modules/proxy/mod_proxy_balancer.c
+++ b/modules/proxy/mod_proxy_balancer.c
@@ -212,7 +212,8 @@ static char *get_cookie_param(request_rec *r, const char *name)
 /* Find the worker that has the 'route' defined
  */
 static proxy_worker *find_route_worker(proxy_balancer *balancer,
-                                       const char *route, request_rec *r)
+                                       const char *route, request_rec *r,
+                                       int recursion)
 {
     int i;
     int checking_standby;
@@ -249,10 +250,15 @@ static proxy_worker *find_route_worker(proxy_balancer *balancer,
                          * This enables to safely remove the member from the
                          * balancer. Of course you will need some kind of
                          * session replication between those two remote.
+                         * Also check that we haven't gone thru all the
+                         * balancer members by means of redirects.
+                         * This should avoid redirect cycles.
                          */
-                        if (*worker->s->redirect) {
+                        if ((*worker->s->redirect)
+                            && (recursion < balancer->workers->nelts)) {
                             proxy_worker *rworker = NULL;
-                            rworker = find_route_worker(balancer, worker->s->redirect, r);
+                            rworker = find_route_worker(balancer, worker->s->redirect,
+                                                        r, recursion + 1);
                             /* Check if the redirect worker is usable */
                             if (rworker && !PROXY_WORKER_IS_USABLE(rworker)) {
                                 /*
@@ -315,7 +321,7 @@ static proxy_worker *find_session_route(proxy_balancer *balancer,
         /* We have a route in path or in cookie
          * Find the worker that has this route defined.
          */
-        worker = find_route_worker(balancer, *route, r);
+        worker = find_route_worker(balancer, *route, r, 1);
         if (worker && strcmp(*route, worker->s->route)) {
             /*
              * Notice that the route of the worker chosen is different from