You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2010/08/16 20:36:19 UTC
svn commit: r986090 - in /httpd/httpd/trunk: CHANGES
docs/manual/mod/mod_proxy.xml modules/proxy/mod_proxy_http.c
Author: jim
Date: Mon Aug 16 18:36:19 2010
New Revision: 986090
URL: http://svn.apache.org/viewvc?rev=986090&view=rev
Log:
For backends which are HTTP/1.1, do a quick test (ping)
of the "connection" via 100-Continue for reverse
proxies...
ACO and Filip Hanik also helped out with the idea...
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml
httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=986090&r1=986089&r2=986090&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Mon Aug 16 18:36:19 2010
@@ -6,6 +6,10 @@ Changes with Apache 2.3.7
mod_dav, mod_cache, mod_session: Fix Handling of requests without a path
segment. PR: 49246 [Mark Drayton, Jeff Trawick]
+ *) mod_proxy_http: Support the 'ping' property for backend HTTP/1.1 servers
+ via leveraging 100-Continue as the initial "request".
+ [Jim Jagielski]
+
*) core/mod_authz_core: Introduce new access_checker_ex hook that enables
mod_authz_core to bypass authentication if access should be allowed by
IP address/env var/... [Stefan Fritsch]
Modified: httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml?rev=986090&r1=986089&r2=986090&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml (original)
+++ httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml Mon Aug 16 18:36:19 2010
@@ -743,17 +743,20 @@ expressions</description>
</td></tr>
<tr><td>ping</td>
<td>0</td>
- <td>Ping property tells webserver to send a <code>CPING</code>
- request on ajp13 connection before forwarding a request.
- The parameter is the delay in seconds to wait for the
- <code>CPONG</code> reply.
- This features has been added to avoid problem with hung and
- busy Tomcat's and require ajp13 ping/pong support which has
- been implemented on Tomcat 3.3.2+, 4.1.28+ and 5.0.13+.
+ <td>Ping property tells the webserver to "test" the connection to
+ the backend before forwarding the request. For AJP, it causes
+ <module>mod_proxy_ajp</module>to send a <code>CPING</code>
+ request on the ajp13 connection (implemented on Tomcat 3.3.2+, 4.1.28+
+ and 5.0.13+). For HTTP, it causes <module>mod_proxy_http</module>
+ to send a <code>100-Continue</code> to the backend (only valid for
+ HTTP/1.1 - for non HTTP/1.1 backends, this property has no
+ effect). In both cases the parameter is the delay in seconds to wait
+ for the reply.
+ This feature has been added to avoid problems with hung and
+ busy backends.
This will increase the network traffic during the normal operation
which could be an issue, but it will lower the
traffic in case some of the cluster nodes are down or busy.
- Currently this has an effect only for AJP.
By adding a postfix of ms the delay can be also set in
milliseconds.
</td></tr>
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=986090&r1=986089&r2=986090&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Mon Aug 16 18:36:19 2010
@@ -669,7 +669,7 @@ static int spool_reqbody_cl(apr_pool_t *
static
int ap_proxy_http_request(apr_pool_t *p, request_rec *r,
- proxy_conn_rec *p_conn, conn_rec *origin,
+ proxy_conn_rec *p_conn, proxy_worker *worker,
proxy_server_conf *conf,
apr_uri_t *uri,
char *url, char *server_portstr)
@@ -694,6 +694,8 @@ int ap_proxy_http_request(apr_pool_t *p,
int force10, rv;
apr_table_t *headers_in_copy;
proxy_dir_conf *dconf;
+ conn_rec *origin = p_conn->connection;
+ int do_100_continue;
dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
header_brigade = apr_brigade_create(p, origin->bucket_alloc);
@@ -702,6 +704,11 @@ int ap_proxy_http_request(apr_pool_t *p,
* Send the HTTP/1.1 request to the remote server
*/
+ do_100_continue = (worker->ping_timeout_set
+ && !r->header_only
+ && (PROXYREQ_REVERSE == r->proxyreq)
+ && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
+
if (apr_table_get(r->subprocess_env, "force-proxy-request-1.0")) {
/*
* According to RFC 2616 8.2.3 we are not allowed to forward an
@@ -791,6 +798,14 @@ int ap_proxy_http_request(apr_pool_t *p,
);
}
+ /* Use HTTP/1.1 100-Continue as quick "HTTP ping" test
+ * to backend
+ */
+ if (do_100_continue) {
+ apr_table_mergen(r->headers_in, "Expect", "100-Continue");
+ r->expecting_100 = 1;
+ }
+
/* X-Forwarded-*: handling
*
* XXX Privacy Note:
@@ -1350,7 +1365,7 @@ apr_status_t ap_proxygetline(apr_bucket_
static
apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
proxy_conn_rec *backend,
- conn_rec *origin,
+ proxy_worker *worker,
proxy_server_conf *conf,
char *server_portstr) {
conn_rec *c = r->connection;
@@ -1375,9 +1390,32 @@ apr_status_t ap_proxy_http_process_respo
int proxy_status = OK;
const char *original_status_line = r->status_line;
const char *proxy_status_line = NULL;
+ conn_rec *origin = backend->connection;
+ apr_interval_time_t old_timeout = 0;
+ int do_100_continue;
+
+ do_100_continue = (worker->ping_timeout_set
+ && !r->header_only
+ && (PROXYREQ_REVERSE == r->proxyreq)
+ && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
+
+
bb = apr_brigade_create(p, c->bucket_alloc);
pass_bb = apr_brigade_create(p, c->bucket_alloc);
+
+ /* Setup for 100-Continue timeout if appropriate */
+ if (do_100_continue) {
+ apr_socket_timeout_get(backend->sock, &old_timeout);
+ if (worker->ping_timeout != old_timeout) {
+ apr_status_t rc;
+ rc = apr_socket_timeout_set(backend->sock, worker->ping_timeout);
+ if (rc != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, rc, r->server,
+ "proxy: could not set 100-Continue timeout");
+ }
+ }
+ }
/* Get response from the remote server, and pass it up the
* filter chain
@@ -1406,6 +1444,9 @@ apr_status_t ap_proxy_http_process_respo
if (APR_STATUS_IS_TIMEUP(rc)) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"proxy: read timeout");
+ if (do_100_continue) {
+ return ap_proxyerror(r, HTTP_SERVICE_UNAVAILABLE, "Timeout on 100-Continue");
+ }
}
/*
* If we are a reverse proxy request shutdown the connection
@@ -1641,6 +1682,12 @@ apr_status_t ap_proxy_http_process_respo
if (ap_is_HTTP_INFO(proxy_status)) {
interim_response++;
+ /* Reset to old timeout iff we've adjusted it */
+ if (do_100_continue
+ && (r->status == HTTP_CONTINUE)
+ && (worker->ping_timeout != old_timeout)) {
+ apr_socket_timeout_set(backend->sock, old_timeout);
+ }
}
else {
interim_response = 0;
@@ -1923,6 +1970,7 @@ static int proxy_http_handler(request_re
proxy_conn_rec *backend = NULL;
int is_ssl = 0;
conn_rec *c = r->connection;
+ int retry = 0;
/*
* Use a shorter-lived pool to reduce memory usage
* and avoid a memory leak
@@ -1990,48 +2038,68 @@ static int proxy_http_handler(request_re
backend->close = 1;
}
- /* Step One: Determine Who To Connect To */
- if ((status = ap_proxy_determine_connection(p, r, conf, worker, backend,
- uri, &url, proxyname,
+ while (retry < 2) {
+ char *locurl = url;
+
+ /* Step One: Determine Who To Connect To */
+ if ((status = ap_proxy_determine_connection(p, r, conf, worker, backend,
+ uri, &locurl, proxyname,
proxyport, server_portstr,
sizeof(server_portstr))) != OK)
- goto cleanup;
-
- /* Step Two: Make the Connection */
- if (ap_proxy_connect_backend(proxy_function, backend, worker, r->server)) {
- status = HTTP_SERVICE_UNAVAILABLE;
- goto cleanup;
- }
+ break;
- /* Step Three: Create conn_rec */
- if (!backend->connection) {
- if ((status = ap_proxy_connection_create(proxy_function, backend,
- c, r->server)) != OK)
- goto cleanup;
- /*
- * On SSL connections set a note on the connection what CN is
- * requested, such that mod_ssl can check if it is requested to do
- * so.
+ /* Step Two: Make the Connection */
+ if (ap_proxy_connect_backend(proxy_function, backend, worker, r->server)) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
+ "proxy: HTTP: failed to make connection to backend: %s",
+ backend->hostname);
+ status = HTTP_SERVICE_UNAVAILABLE;
+ break;
+ }
+
+ /* Step Three: Create conn_rec */
+ if (!backend->connection) {
+ if ((status = ap_proxy_connection_create(proxy_function, backend,
+ c, r->server)) != OK)
+ break;
+ /*
+ * On SSL connections set a note on the connection what CN is
+ * requested, such that mod_ssl can check if it is requested to do
+ * so.
+ */
+ if (is_ssl) {
+ apr_table_set(backend->connection->notes, "proxy-request-hostname",
+ uri->hostname);
+ }
+ }
+
+ /* Step Four: Send the Request
+ * On the off-chance that we forced a 100-Continue as a
+ * kinda HTTP ping test, allow for retries
*/
- if (is_ssl) {
- apr_table_set(backend->connection->notes, "proxy-request-hostname",
- uri->hostname);
+ if ((status = ap_proxy_http_request(p, r, backend, worker,
+ conf, uri, locurl, server_portstr)) != OK) {
+ if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->ping_timeout_set) {
+ backend->close = 1;
+ ap_log_error(APLOG_MARK, APLOG_INFO, status, r->server,
+ "proxy: HTTP: 100-Continue failed to %pI (%s)",
+ worker->cp->addr, worker->hostname);
+ retry++;
+ continue;
+ } else {
+ break;
+ }
+
}
- }
- /* Step Four: Send the Request */
- if ((status = ap_proxy_http_request(p, r, backend, backend->connection,
- conf, uri, url, server_portstr)) != OK)
- goto cleanup;
+ /* Step Five: Receive the Response... Fall thru to cleanup */
+ status = ap_proxy_http_process_response(p, r, backend, worker,
+ conf, server_portstr);
- /* Step Five: Receive the Response */
- if ((status = ap_proxy_http_process_response(p, r, backend,
- backend->connection,
- conf, server_portstr)) != OK)
- goto cleanup;
+ break;
+ }
/* Step Six: Clean Up */
-
cleanup:
if (backend) {
if (status != OK)
Re: svn commit: r986090 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_proxy.xml
modules/proxy/mod_proxy_http.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 08/17/2010 09:08 AM, Ruediger Pluem wrote:
>
> On 08/16/2010 08:36 PM, jim@apache.org wrote:
>> Author: jim
>> Date: Mon Aug 16 18:36:19 2010
>> New Revision: 986090
>>
>> URL: http://svn.apache.org/viewvc?rev=986090&view=rev
>> Log:
>> For backends which are HTTP/1.1, do a quick test (ping)
>> of the "connection" via 100-Continue for reverse
>> proxies...
>>
>> ACO and Filip Hanik also helped out with the idea...
>>
>> Modified:
>> httpd/httpd/trunk/CHANGES
>> httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml
>> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>>
>
> General remark: You added a lot of tabs with your commit. Could you please
> clean this up an detab?
>
>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=986090&r1=986089&r2=986090&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Mon Aug 16 18:36:19 2010
>> @@ -669,7 +669,7 @@ static int spool_reqbody_cl(apr_pool_t *
>>
>> static
>> int ap_proxy_http_request(apr_pool_t *p, request_rec *r,
>> - proxy_conn_rec *p_conn, conn_rec *origin,
>> + proxy_conn_rec *p_conn, proxy_worker *worker,
>> proxy_server_conf *conf,
>> apr_uri_t *uri,
>> char *url, char *server_portstr)
>> @@ -694,6 +694,8 @@ int ap_proxy_http_request(apr_pool_t *p,
>> int force10, rv;
>> apr_table_t *headers_in_copy;
>> proxy_dir_conf *dconf;
>> + conn_rec *origin = p_conn->connection;
>> + int do_100_continue;
>>
>> dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
>> header_brigade = apr_brigade_create(p, origin->bucket_alloc);
>> @@ -702,6 +704,11 @@ int ap_proxy_http_request(apr_pool_t *p,
>> * Send the HTTP/1.1 request to the remote server
>> */
>>
>> + do_100_continue = (worker->ping_timeout_set
>> + && !r->header_only
>> + && (PROXYREQ_REVERSE == r->proxyreq)
>> + && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
>> +
>
>
> Unless I miss something important I do not see a check whether the request to the proxy
> by the client has a request body. According to 8.2.3 we MUST NOT add the Expect header
> if we do not intend to sent a body. So this would be an RFC violation.
> My prior understanding was that we are liberal of what we accept but strict in what we
> generate.
Further update: If sending a request with
Expect: 100-continue
to an httpd 2.3.5 and up webserver with no request body, the connection will be closed by
the server after this request.
See PR47087 (https://issues.apache.org/bugzilla/show_bug.cgi?id=47087) and
r888310 (http://svn.apache.org/viewvc?view=revision&revision=888310).
As most requests to the origin server will IMHO look like this (Expect: 100-continue with
no request body) this seems to make this feature pretty much useless as it effectively
disables keepalives with the origin server.
Regards
Rüdiger
Re: svn commit: r986090 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_proxy.xml
modules/proxy/mod_proxy_http.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 08/16/2010 08:36 PM, jim@apache.org wrote:
> Author: jim
> Date: Mon Aug 16 18:36:19 2010
> New Revision: 986090
>
> URL: http://svn.apache.org/viewvc?rev=986090&view=rev
> Log:
> For backends which are HTTP/1.1, do a quick test (ping)
> of the "connection" via 100-Continue for reverse
> proxies...
>
> ACO and Filip Hanik also helped out with the idea...
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml
> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>
General remark: You added a lot of tabs with your commit. Could you please
clean this up an detab?
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=986090&r1=986089&r2=986090&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Mon Aug 16 18:36:19 2010
> @@ -669,7 +669,7 @@ static int spool_reqbody_cl(apr_pool_t *
>
> static
> int ap_proxy_http_request(apr_pool_t *p, request_rec *r,
> - proxy_conn_rec *p_conn, conn_rec *origin,
> + proxy_conn_rec *p_conn, proxy_worker *worker,
> proxy_server_conf *conf,
> apr_uri_t *uri,
> char *url, char *server_portstr)
> @@ -694,6 +694,8 @@ int ap_proxy_http_request(apr_pool_t *p,
> int force10, rv;
> apr_table_t *headers_in_copy;
> proxy_dir_conf *dconf;
> + conn_rec *origin = p_conn->connection;
> + int do_100_continue;
>
> dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
> header_brigade = apr_brigade_create(p, origin->bucket_alloc);
> @@ -702,6 +704,11 @@ int ap_proxy_http_request(apr_pool_t *p,
> * Send the HTTP/1.1 request to the remote server
> */
>
> + do_100_continue = (worker->ping_timeout_set
> + && !r->header_only
> + && (PROXYREQ_REVERSE == r->proxyreq)
> + && !(apr_table_get(r->subprocess_env, "force-proxy-request-1.0")));
> +
Unless I miss something important I do not see a check whether the request to the proxy
by the client has a request body. According to 8.2.3 we MUST NOT add the Expect header
if we do not intend to sent a body. So this would be an RFC violation.
My prior understanding was that we are liberal of what we accept but strict in what we
generate.
> if (apr_table_get(r->subprocess_env, "force-proxy-request-1.0")) {
> /*
> * According to RFC 2616 8.2.3 we are not allowed to forward an
Regards
Rüdiger