From: Ruediger Pluem <rpluem@apache.org>
Date: Sat, 2 Feb 2008 16:35:40 +0000 (+0000)
Subject: * Do not retry a request in the case that we either failed to sent a part of the
X-Git-Tag: 2.3.0~1009
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9af3a4cdfd60210ed73d3eefc22f6581e37e17c8;p=apache

* 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 f45004441d..bc28dce7ca 100644
--- 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 99d9cf1432..fa09ae895c 100644
--- 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);