* Do not retry a request in the case that we either failed to sent a part of the

  request body or if the request is not idempotent.

PR: 44334

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

diff --git a/CHANGES b/CHANGES
index f45004441db20b60567c6979e408b25cb5fb4d77..bc28dce7ca9a8a3db00a97411084b447a571ea47 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,10 @@
 Changes with Apache 2.3.0
 [ When backported to 2.2.x, remove entry from this file ]
 
+  *) mod_proxy_ajp: Do not retry request in the case that we either failed to
+     sent a part of the request body or if the request is not idempotent.
+     PR 44334 [Ruediger Pluem]
+
   *) mod_proxy_http: Fix processing of chunked responses if
      Connection: Transfer-Encoding is set in the response of the proxied
      system. PR 44311 [Ruediger Pluem]

diff --git a/modules/proxy/mod_proxy_ajp.c b/modules/proxy/mod_proxy_ajp.c
index 99d9cf14321d3ede3cddc8f440b2c45bfad4d72d..fa09ae895c1b390a7a2ae6ce6315ef10307541b9 100644 (file)
--- a/modules/proxy/mod_proxy_ajp.c
+++ b/modules/proxy/mod_proxy_ajp.c
@@ -89,6 +89,32 @@ static int proxy_ajp_canon(request_rec *r, char *url)
     return OK;
 }
 
+static int is_idempotent(request_rec *r)
+{
+    /*
+     * If the request has arguments it might not be idempotent as it might have
+     * side-effects.
+     */
+    if (r->args) {
+        return 0;
+    }
+    /*
+     * RFC2616 (9.1.2): GET, HEAD, PUT, DELETE, OPTIONS, TRACE are considered
+     * idempotent. Hint: HEAD requests use M_GET as method number as well.
+     */
+    switch (r->method_number) {
+        case M_GET:
+        case M_DELETE:
+        case M_PUT:
+        case M_OPTIONS:
+        case M_TRACE:
+            return 1;
+        /* Everything else is not considered idempotent. */
+        default:
+            return 0;
+    }
+}
+
 /*
  * XXX: AJP Auto Flushing
  *
@@ -122,7 +148,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
     apr_bucket_brigade *input_brigade;
     apr_bucket_brigade *output_brigade;
     ajp_msg_t *msg;
-    apr_size_t bufsiz;
+    apr_size_t bufsiz = 0;
     char *buff;
     apr_uint16_t size;
     const char *tenc;
@@ -161,8 +187,17 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
                      conn->worker->hostname);
         if (status == AJP_EOVERFLOW)
             return HTTP_BAD_REQUEST;
-        else
-            return HTTP_SERVICE_UNAVAILABLE;
+        else {
+            /*
+             * This is only non fatal when the method is idempotent. In this
+             * case we can dare to retry it with a different worker if we are
+             * a balancer member.
+             */
+            if (is_idempotent(r)) {
+                return HTTP_SERVICE_UNAVAILABLE;
+            }
+            return HTTP_INTERNAL_SERVER_ERROR;
+        }
     }
 
     /* allocate an AJP message to store the data of the buckets */
@@ -231,7 +266,11 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
                              "proxy: send failed to %pI (%s)",
                              conn->worker->cp->addr,
                              conn->worker->hostname);
-                return HTTP_SERVICE_UNAVAILABLE;
+                /*
+                 * It is fatal when we failed to send a (part) of the request
+                 * body.
+                 */
+                return HTTP_INTERNAL_SERVER_ERROR;
             }
             conn->worker->s->transferred += bufsiz;
         }
@@ -249,7 +288,16 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
                      "proxy: read response failed from %pI (%s)",
                      conn->worker->cp->addr,
                      conn->worker->hostname);
-        return HTTP_SERVICE_UNAVAILABLE;
+        /*
+         * This is only non fatal when we have not sent (parts) of a possible
+         * request body so far (we do not store it and thus cannot sent it
+         * again) and the method is idempotent. In this case we can dare to
+         * retry it with a different worker if we are a balancer member.
+         */
+        if ((bufsiz == 0) && is_idempotent(r)) {
+            return HTTP_SERVICE_UNAVAILABLE;
+        }
+        return HTTP_INTERNAL_SERVER_ERROR;
     }
     /* parse the reponse */
     result = ajp_parse_type(r, conn->data);