From: Yann Ylavic Date: Tue, 9 Feb 2016 23:38:59 +0000 (+0000) Subject: mod_proxy: axe negative "ping" parameter setting and handling. X-Git-Tag: 2.5.0-alpha~2154 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=447582e5f41fce01869948f281b3c17793c3d34a;p=thirdparty%2Fapache%2Fhttpd.git mod_proxy: axe negative "ping" parameter setting and handling. This used to check for the backend connection readability only (instead of the full ping/100-continue round-trip), but the case is already handled by ap_proxy_connect_backend() which is always called. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1729507 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/modules/http2/mod_proxy_http2.c b/modules/http2/mod_proxy_http2.c index 1b26a11542a..879a63ae021 100644 --- a/modules/http2/mod_proxy_http2.c +++ b/modules/http2/mod_proxy_http2.c @@ -211,9 +211,9 @@ static int proxy_http2_handler(request_rec *r, { const char *proxy_function; proxy_conn_rec *backend; - char *locurl = url, *u, *firsturl; + char *locurl = url, *u; apr_size_t slen; - int is_ssl = 0, retry = 0; + int is_ssl = 0; int flushall = 0; int status; char server_portstr[32]; @@ -268,7 +268,7 @@ static int proxy_http2_handler(request_rec *r, ap_proxy_ssl_connection_cleanup(backend, r); } - while (retry < 2) { + do { /* while (0): break out */ conn_rec *backconn; /* Step One: Determine the URL to connect to (might be a proxy), @@ -288,18 +288,6 @@ static int proxy_http2_handler(request_rec *r, ssl_hostname = apr_pstrdup(r->pool, backend->ssl_hostname); } - if (!retry) { - firsturl = locurl; - /* http does a prefetch here, so that it immediately can start sending - * when the backend connection comes online. This minimizes the risk of - * reusing a connection only to experience a keepalive close. - */ - } - else { - /* On a retry, we'd expect to see the same url again */ - AP_DEBUG_ASSERT(strcmp(firsturl, locurl) == 0); - } - /* Step Two: Make the Connection (or check that an already existing * socket is still usable). On success, we have a socket connected to * backend->hostname. */ @@ -339,21 +327,6 @@ static int proxy_http2_handler(request_rec *r, apr_table_setn(backend->connection->notes, "proxy-request-alpn-protos", "h2"); } - - /* Step Three-and-a-Half: See if the socket is still connected (if - * desired). Note: Since ap_proxy_connect_backend just above does - * the same check (unconditionally), this step is not required when - * backend's socket/connection is reused (ie. no Step Three). - */ - if (worker->s->ping_timeout_set && worker->s->ping_timeout < 0 && - !ap_proxy_is_socket_connected(backend->sock)) { - backend->close = 1; - ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO() - "socket check failed to %pI (%s)", - worker->cp->addr, worker->s->hostname); - retry++; - continue; - } } /* Step Four: Send the Request in a new HTTP/2 stream and @@ -370,8 +343,7 @@ static int proxy_http2_handler(request_rec *r, proxy_run_detach_backend(r, backend); } } - break; - } + } while (0); /* clean up before return */ cleanup: diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c index 15948790745..dcd185367ff 100644 --- a/modules/proxy/mod_proxy.c +++ b/modules/proxy/mod_proxy.c @@ -280,7 +280,7 @@ static const char *set_worker_param(apr_pool_t *p, */ if (ap_timeout_parameter_parse(val, &timeout, "s") != APR_SUCCESS) return "Ping/Pong timeout has wrong format"; - if (timeout < 1000 && timeout >= 0) + if (timeout < 1000) return "Ping/Pong timeout must be at least one millisecond"; worker->s->ping_timeout = timeout; worker->s->ping_timeout_set = 1; diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index 84be23749f0..c9eac3fdade 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -365,7 +365,6 @@ do { \ #define PROXY_DO_100_CONTINUE(w, r) \ ((w)->s->ping_timeout_set \ - && ((w)->s->ping_timeout >= 0) \ && (PROXYREQ_REVERSE == (r)->proxyreq) \ && !(apr_table_get((r)->subprocess_env, "force-proxy-request-1.0")) \ && ap_request_has_body((r))) diff --git a/modules/proxy/mod_proxy_ajp.c b/modules/proxy/mod_proxy_ajp.c index 0a6ea595a1f..d57fef0eab0 100644 --- a/modules/proxy/mod_proxy_ajp.c +++ b/modules/proxy/mod_proxy_ajp.c @@ -342,8 +342,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, * but doesn't affect the whole worker. */ if (APR_STATUS_IS_TIMEUP(status) && - conn->worker->s->ping_timeout_set && - conn->worker->s->ping_timeout >= 0) { + conn->worker->s->ping_timeout_set) { return HTTP_GATEWAY_TIME_OUT; } @@ -680,8 +679,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, * but doesn't affect the whole worker. */ if (APR_STATUS_IS_TIMEUP(status) && - conn->worker->s->ping_timeout_set && - conn->worker->s->ping_timeout >= 0) { + conn->worker->s->ping_timeout_set) { apr_table_setn(r->notes, "proxy_timedout", "1"); rv = HTTP_GATEWAY_TIME_OUT; } @@ -791,35 +789,22 @@ static int proxy_ajp_handler(request_rec *r, proxy_worker *worker, /* Handle CPING/CPONG */ if (worker->s->ping_timeout_set) { - if (worker->s->ping_timeout < 0) { - if (!ap_proxy_is_socket_connected(backend->sock)) { - backend->close = 1; - ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(02534) - "socket check failed to %pI (%s)", - worker->cp->addr, worker->s->hostname); - status = HTTP_SERVICE_UNAVAILABLE; - retry++; - continue; - } - } - else { - status = ajp_handle_cping_cpong(backend->sock, r, - worker->s->ping_timeout); - /* - * In case the CPING / CPONG failed for the first time we might be - * just out of luck and got a faulty backend connection, but the - * backend might be healthy nevertheless. So ensure that the backend - * TCP connection gets closed and try it once again. - */ - if (status != APR_SUCCESS) { - backend->close = 1; - ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00897) - "cping/cpong failed to %pI (%s)", - worker->cp->addr, worker->s->hostname); - status = HTTP_SERVICE_UNAVAILABLE; - retry++; - continue; - } + status = ajp_handle_cping_cpong(backend->sock, r, + worker->s->ping_timeout); + /* + * In case the CPING / CPONG failed for the first time we might be + * just out of luck and got a faulty backend connection, but the + * backend might be healthy nevertheless. So ensure that the backend + * TCP connection gets closed and try it once again. + */ + if (status != APR_SUCCESS) { + backend->close = 1; + ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00897) + "cping/cpong failed to %pI (%s)", + worker->cp->addr, worker->s->hostname); + status = HTTP_SERVICE_UNAVAILABLE; + retry++; + continue; } } /* Step Three: Process the Request */ diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index 0418b08af0d..056c283f395 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -2097,21 +2097,6 @@ static int proxy_http_handler(request_rec *r, proxy_worker *worker, "proxy-request-hostname", backend->ssl_hostname); } - - /* Step Three-and-a-Half: See if the socket is still connected (if - * desired). Note: Since ap_proxy_connect_backend just above does - * the same check (unconditionally), this step is not required when - * backend's socket/connection is reused (ie. no Step Three). - */ - if (worker->s->ping_timeout_set && worker->s->ping_timeout < 0 && - !ap_proxy_is_socket_connected(backend->sock)) { - backend->close = 1; - ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(02535) - "socket check failed to %pI (%s)", - worker->cp->addr, worker->s->hostname); - retry++; - continue; - } } /* Don't recycle the connection if prefetch (above) told not to do so */ @@ -2130,8 +2115,7 @@ static int proxy_http_handler(request_rec *r, proxy_worker *worker, flushall)) != OK) { proxy_run_detach_backend(r, backend); if ((status == HTTP_SERVICE_UNAVAILABLE) && - worker->s->ping_timeout_set && - worker->s->ping_timeout >= 0) { + worker->s->ping_timeout_set) { backend->close = 1; ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(01115) "HTTP: 100-Continue failed to %pI (%s)", diff --git a/modules/proxy/mod_proxy_wstunnel.c b/modules/proxy/mod_proxy_wstunnel.c index a2fd50563a9..fb88a534cfe 100644 --- a/modules/proxy/mod_proxy_wstunnel.c +++ b/modules/proxy/mod_proxy_wstunnel.c @@ -421,7 +421,6 @@ static int proxy_wstunnel_handler(request_rec *r, proxy_worker *worker, proxy_conn_rec *backend = NULL; const char *upgrade; char *scheme; - int retry; conn_rec *c = r->connection; apr_pool_t *p = r->pool; apr_uri_t *uri; @@ -463,15 +462,13 @@ static int proxy_wstunnel_handler(request_rec *r, proxy_worker *worker, backend->is_ssl = is_ssl; backend->close = 0; - retry = 0; - while (retry < 2) { + do { /* while (0): break out */ char *locurl = url; /* Step One: Determine Who To Connect To */ status = ap_proxy_determine_connection(p, r, conf, worker, backend, uri, &locurl, proxyname, proxyport, server_portstr, sizeof(server_portstr)); - if (status != OK) break; @@ -495,8 +492,7 @@ static int proxy_wstunnel_handler(request_rec *r, proxy_worker *worker, /* Step Three: Process the Request */ status = proxy_wstunnel_request(p, r, backend, worker, conf, uri, locurl, server_portstr, scheme); - break; - } + } while (0); /* Do not close the socket */ if (status != SUSPENDED) {