httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j..@apache.org
Subject svn commit: r660580 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS modules/proxy/mod_proxy_ajp.c
Date Tue, 27 May 2008 16:19:26 GMT
Author: jim
Date: Tue May 27 09:19:22 2008
New Revision: 660580

URL: http://svn.apache.org/viewvc?rev=660580&view=rev
Log:
Merge r617822, r627097, r660207 from trunk:

* 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


* As per niq's comment, better destinct the types of idempotence.


* Introduce a flag to decide whether we sent an body to the backend or not.

Submitted by: rpluem
Reviewed by: jim

Modified:
    httpd/httpd/branches/2.2.x/CHANGES
    httpd/httpd/branches/2.2.x/STATUS
    httpd/httpd/branches/2.2.x/modules/proxy/mod_proxy_ajp.c

Modified: httpd/httpd/branches/2.2.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?rev=660580&r1=660579&r2=660580&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Tue May 27 09:19:22 2008
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.2.9
 
+  *) 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_rewrite: Initialize hash needed by ap_register_rewrite_mapfunc early
      enough. PR 44641 [Daniel Lescohier <daniel.lescohier cnet.com>]
 

Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=660580&r1=660579&r2=660580&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Tue May 27 09:19:22 2008
@@ -90,16 +90,6 @@
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
- * 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. PR44334
-   Trunk version of patch:
-         http://svn.apache.org/viewvc?rev=617822&view=rev
-         http://svn.apache.org/viewvc?rev=627097&view=rev
-         http://svn.apache.org/viewvc?rev=660207&view=rev
-   Backport version for 2.2.x of patch:
-         Trunk version of patch works
-   +1: rpluem, niq, jim
-
  * mod_proxy: Do not try a direct connection if the connection via a
    remote proxy failed before and the request has a request body.
    Trunk version of patch:

Modified: httpd/httpd/branches/2.2.x/modules/proxy/mod_proxy_ajp.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/proxy/mod_proxy_ajp.c?rev=660580&r1=660579&r2=660580&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/proxy/mod_proxy_ajp.c (original)
+++ httpd/httpd/branches/2.2.x/modules/proxy/mod_proxy_ajp.c Tue May 27 09:19:22 2008
@@ -89,6 +89,37 @@
     return OK;
 }
 
+#define METHOD_NON_IDEMPOTENT       0
+#define METHOD_IDEMPOTENT           1
+#define METHOD_IDEMPOTENT_WITH_ARGS 2
+
+static int is_idempotent(request_rec *r)
+{
+    /*
+     * 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:
+            /*
+             * If the request has arguments it might have side-effects and thus
+             * it might be undesirable to resent it to a backend again
+             * automatically.
+             */
+            if (r->args) {
+                return METHOD_IDEMPOTENT_WITH_ARGS;
+            }
+            return METHOD_IDEMPOTENT;
+        /* Everything else is not considered idempotent. */
+        default:
+            return METHOD_NON_IDEMPOTENT;
+    }
+}
+
 /*
  * XXX: AJP Auto Flushing
  *
@@ -122,7 +153,7 @@
     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;
@@ -138,6 +169,7 @@
     proxy_server_conf *psf =
     ap_get_module_config(r->server->module_config, &proxy_module);
     apr_size_t maxsize = AJP_MSG_BUFFER_SZ;
+    int send_body = 0;
 
     if (psf->io_buffer_size_set)
        maxsize = psf->io_buffer_size;
@@ -161,8 +193,17 @@
                      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) == METHOD_IDEMPOTENT) {
+                return HTTP_SERVICE_UNAVAILABLE;
+            }
+            return HTTP_INTERNAL_SERVER_ERROR;
+        }
     }
 
     /* allocate an AJP message to store the data of the buckets */
@@ -231,9 +272,14 @@
                              "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;
+            send_body = 1;
         }
     }
 
@@ -249,7 +295,16 @@
                      "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 (!send_body && (is_idempotent(r) == METHOD_IDEMPOTENT)) {
+            return HTTP_SERVICE_UNAVAILABLE;
+        }
+        return HTTP_INTERNAL_SERVER_ERROR;
     }
     /* parse the reponse */
     result = ajp_parse_type(r, conn->data);



Mime
View raw message