From: Jim Jagielski Date: Tue, 27 May 2008 16:19:22 +0000 (+0000) Subject: Merge r617822, r627097, r660207 from trunk: X-Git-Tag: 2.2.9~114 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=863774cf229e9f0362dc8a0aa3c27c1eece07efd;p=thirdparty%2Fapache%2Fhttpd.git 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 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@660580 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 8f28b8eb5eb..969cd0b9979 100644 --- a/CHANGES +++ b/CHANGES @@ -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 ] diff --git a/STATUS b/STATUS index 47779cd68c8..cb2b88c2282 100644 --- a/STATUS +++ b/STATUS @@ -90,16 +90,6 @@ RELEASE SHOWSTOPPERS: 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: diff --git a/modules/proxy/mod_proxy_ajp.c b/modules/proxy/mod_proxy_ajp.c index bad2b26e8c0..ff83cdd181a 100644 --- a/modules/proxy/mod_proxy_ajp.c +++ b/modules/proxy/mod_proxy_ajp.c @@ -89,6 +89,37 @@ static int proxy_ajp_canon(request_rec *r, char *url) 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 @@ 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; @@ -138,6 +169,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, 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 @@ 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) == 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 @@ 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; + send_body = 1; } } @@ -249,7 +295,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 (!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);