Index: modules/proxy/proxy_util.c =================================================================== --- modules/proxy/proxy_util.c (revision 1596845) +++ modules/proxy/proxy_util.c (working copy) @@ -3230,7 +3230,9 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.1" CRLF, NULL); } if (apr_table_get(r->subprocess_env, "proxy-nokeepalive")) { - origin->keepalive = AP_CONN_CLOSE; + if (origin) { + origin->keepalive = AP_CONN_CLOSE; + } p_conn->close = 1; } ap_xlate_proto_to_ascii(buf, strlen(buf)); Index: modules/proxy/mod_proxy_http.c =================================================================== --- modules/proxy/mod_proxy_http.c (revision 1596845) +++ modules/proxy/mod_proxy_http.c (working copy) @@ -234,7 +234,8 @@ static int stream_reqbody_chunked(apr_pool_t *p, proxy_conn_rec *p_conn, conn_rec *origin, apr_bucket_brigade *header_brigade, - apr_bucket_brigade *input_brigade) + apr_bucket_brigade *input_brigade, + int flushall) { int seen_eos = 0, rv = OK; apr_size_t hdr_len; @@ -247,14 +248,21 @@ static int stream_reqbody_chunked(apr_pool_t *p, add_te_chunked(p, bucket_alloc, header_brigade); terminate_headers(bucket_alloc, header_brigade); - while (!APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade))) + while (APR_BRIGADE_EMPTY(input_brigade) || + !APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade))) { + int flush = flushall; + + if (!APR_BRIGADE_EMPTY(input_brigade)) { char chunk_hdr[20]; /* must be here due to transient bucket. */ /* If this brigade contains EOS, either stop or remove it. */ if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) { seen_eos = 1; + /* The request is flushed below this loop with the EOS chunk */ + flush = 0; + /* We can't pass this EOS to the output_filters. */ e = APR_BRIGADE_LAST(input_brigade); apr_bucket_delete(e); @@ -276,6 +284,7 @@ static int stream_reqbody_chunked(apr_pool_t *p, */ e = apr_bucket_immortal_create(ASCII_CRLF, 2, bucket_alloc); APR_BRIGADE_INSERT_TAIL(input_brigade, e); + } if (header_brigade) { /* we never sent the header brigade, so go ahead and @@ -283,6 +292,12 @@ static int stream_reqbody_chunked(apr_pool_t *p, */ bb = header_brigade; + /* Flush the prefeched data now to minimize the delay between + * connect (or ap_proxy_is_socket_connected) and the first bytes + * sent, unless it is done below this loop with the EOS chunk. + */ + flush = !seen_eos; + /* * Save input_brigade in bb brigade. (At least) in the SSL case * input_brigade contains transient buckets whose data would get @@ -303,8 +318,7 @@ static int stream_reqbody_chunked(apr_pool_t *p, bb = input_brigade; } - /* The request is flushed below this loop with chunk EOS header */ - rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb, 0); + rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb, flush); if (rv != OK) { return rv; } @@ -366,7 +380,7 @@ static int stream_reqbody_cl(apr_pool_t *p, conn_rec *origin, apr_bucket_brigade *header_brigade, apr_bucket_brigade *input_brigade, - char *old_cl_val) + char *old_cl_val, int flushall) { int seen_eos = 0, rv = 0; apr_status_t status = APR_SUCCESS; @@ -392,8 +406,12 @@ static int stream_reqbody_cl(apr_pool_t *p, } terminate_headers(bucket_alloc, header_brigade); - while (!APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade))) + while (APR_BRIGADE_EMPTY(input_brigade) || + !APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade))) { + int flush = flushall; + + if (!APR_BRIGADE_EMPTY(input_brigade)) { apr_brigade_length(input_brigade, 1, &bytes); bytes_streamed += bytes; @@ -401,6 +419,9 @@ static int stream_reqbody_cl(apr_pool_t *p, if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) { seen_eos = 1; + /* Once we hit EOS, we are ready to flush. */ + flush = 1; + /* We can't pass this EOS to the output_filters. */ e = APR_BRIGADE_LAST(input_brigade); apr_bucket_delete(e); @@ -427,6 +448,7 @@ static int stream_reqbody_cl(apr_pool_t *p, bytes_streamed, cl_val); return HTTP_INTERNAL_SERVER_ERROR; } + } if (header_brigade) { /* we never sent the header brigade, so go ahead and @@ -434,6 +456,11 @@ static int stream_reqbody_cl(apr_pool_t *p, */ bb = header_brigade; + /* Flush prefeched data now to minimize the delay between connect, + * or ap_proxy_is_socket_connected, and the first bytes sent. + */ + flush = 1; + /* * Save input_brigade in bb brigade. (At least) in the SSL case * input_brigade contains transient buckets whose data would get @@ -454,10 +481,9 @@ static int stream_reqbody_cl(apr_pool_t *p, bb = input_brigade; } - /* Once we hit EOS, we are ready to flush. */ - rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb, seen_eos); + rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb, flush); if (rv != OK) { - return rv ; + return rv; } if (seen_eos) { @@ -518,8 +544,10 @@ static int spool_reqbody_cl(apr_pool_t *p, limit = ap_get_limit_req_body(r); - while (!APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade))) + while (APR_BRIGADE_EMPTY(input_brigade) || + !APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade))) { + if (!APR_BRIGADE_EMPTY(input_brigade)) { /* If this brigade contains EOS, either stop or remove it. */ if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) { seen_eos = 1; @@ -612,6 +640,7 @@ static int spool_reqbody_cl(apr_pool_t *p, if (seen_eos) { break; } + } status = ap_get_brigade(r->input_filters, input_brigade, AP_MODE_READBYTES, APR_BLOCK_READ, @@ -694,28 +723,33 @@ static apr_status_t proxy_buckets_lifetime_transfo return rv; } -static -int ap_proxy_http_request(apr_pool_t *p, request_rec *r, - proxy_conn_rec *p_conn, proxy_worker *worker, - proxy_server_conf *conf, - apr_uri_t *uri, - char *url, char *server_portstr) +enum rb_methods { + RB_INIT, + RB_STREAM_CL, + RB_STREAM_CHUNKED, + RB_SPOOL_CL +}; + +static int ap_proxy_http_prefetch(apr_pool_t *p, request_rec *r, + proxy_conn_rec *p_conn, proxy_worker *worker, + proxy_server_conf *conf, + apr_uri_t *uri, + char *url, char *server_portstr, + apr_bucket_brigade *header_brigade, + apr_bucket_brigade *input_brigade, + char **old_cl_val, char **old_te_val, + enum rb_methods *rb_method, int flushall) { conn_rec *c = r->connection; apr_bucket_alloc_t *bucket_alloc = c->bucket_alloc; - apr_bucket_brigade *header_brigade; - apr_bucket_brigade *input_brigade; apr_bucket_brigade *temp_brigade; apr_bucket *e; char *buf; apr_status_t status; - enum rb_methods {RB_INIT, RB_STREAM_CL, RB_STREAM_CHUNKED, RB_SPOOL_CL}; - enum rb_methods rb_method = RB_INIT; - char *old_cl_val = NULL; - char *old_te_val = NULL; apr_off_t bytes_read = 0; apr_off_t bytes; int force10, rv; + apr_read_type_e block; conn_rec *origin = p_conn->connection; if (apr_table_get(r->subprocess_env, "force-proxy-request-1.0")) { @@ -727,17 +761,13 @@ static apr_status_t proxy_buckets_lifetime_transfo force10 = 0; } - header_brigade = apr_brigade_create(p, bucket_alloc); rv = ap_proxy_create_hdrbrgd(p, header_brigade, r, p_conn, worker, conf, uri, url, server_portstr, - &old_cl_val, &old_te_val); + old_cl_val, old_te_val); if (rv != OK) { return rv; } - /* We have headers, let's figure out our request body... */ - input_brigade = apr_brigade_create(p, bucket_alloc); - /* sub-requests never use keepalives, and mustn't pass request bodies. * Because the new logic looks at input_brigade, we will self-terminate * input_brigade and jump past all of the request body logic... @@ -750,15 +780,9 @@ static apr_status_t proxy_buckets_lifetime_transfo if (!r->kept_body && r->main) { /* XXX: Why DON'T sub-requests use keepalives? */ p_conn->close = 1; - if (old_cl_val) { - old_cl_val = NULL; - apr_table_unset(r->headers_in, "Content-Length"); - } - if (old_te_val) { - old_te_val = NULL; - apr_table_unset(r->headers_in, "Transfer-Encoding"); - } - rb_method = RB_STREAM_CL; + *old_cl_val = NULL; + *old_te_val = NULL; + *rb_method = RB_STREAM_CL; e = apr_bucket_eos_create(input_brigade->bucket_alloc); APR_BRIGADE_INSERT_TAIL(input_brigade, e); goto skip_body; @@ -772,19 +796,18 @@ static apr_status_t proxy_buckets_lifetime_transfo * encoding has been done by the extensions' handler, and * do not modify add_te_chunked's logic */ - if (old_te_val && strcasecmp(old_te_val, "chunked") != 0) { + if (*old_te_val && strcasecmp(*old_te_val, "chunked") != 0) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01093) - "%s Transfer-Encoding is not supported", old_te_val); + "%s Transfer-Encoding is not supported", *old_te_val); return HTTP_INTERNAL_SERVER_ERROR; } - if (old_cl_val && old_te_val) { + if (*old_cl_val && *old_te_val) { ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01094) "client %s (%s) requested Transfer-Encoding " "chunked body with Content-Length (C-L ignored)", c->client_ip, c->remote_host ? c->remote_host: ""); - apr_table_unset(r->headers_in, "Content-Length"); - old_cl_val = NULL; + *old_cl_val = NULL; origin->keepalive = AP_CONN_CLOSE; p_conn->close = 1; } @@ -798,10 +821,20 @@ static apr_status_t proxy_buckets_lifetime_transfo * reasonable size. */ temp_brigade = apr_brigade_create(p, bucket_alloc); + block = (flushall) ? APR_NONBLOCK_READ : APR_BLOCK_READ; do { status = ap_get_brigade(r->input_filters, temp_brigade, - AP_MODE_READBYTES, APR_BLOCK_READ, + AP_MODE_READBYTES, block, MAX_MEM_SPOOL - bytes_read); + /* ap_get_brigade will return success with an empty brigade + * for a non-blocking read which would block + */ + if (block == APR_NONBLOCK_READ + && (APR_STATUS_IS_EAGAIN(rv) + || (rv == APR_SUCCESS && APR_BRIGADE_EMPTY(temp_brigade)))) { + status = APR_SUCCESS; + break; + } if (status != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(01095) "prefetch request body failed to %pI (%s)" @@ -876,7 +909,8 @@ static apr_status_t proxy_buckets_lifetime_transfo * is absent, and the filters are unchanged (the body won't * be resized by another content filter). */ - if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) { + if (!APR_BRIGADE_EMPTY(input_brigade) && + APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) { /* The whole thing fit, so our decision is trivial, use * the filtered bytes read from the client for the request * body Content-Length. @@ -884,42 +918,44 @@ static apr_status_t proxy_buckets_lifetime_transfo * If we expected no body, and read no body, do not set * the Content-Length. */ - if (old_cl_val || old_te_val || bytes_read) { - old_cl_val = apr_off_t_toa(r->pool, bytes_read); + if (*old_cl_val || *old_te_val || bytes_read) { + *old_cl_val = apr_off_t_toa(r->pool, bytes_read); } - rb_method = RB_STREAM_CL; + *rb_method = RB_STREAM_CL; } - else if (old_te_val) { + else if (*old_te_val) { if (force10 - || (apr_table_get(r->subprocess_env, "proxy-sendcl") - && !apr_table_get(r->subprocess_env, "proxy-sendchunks") - && !apr_table_get(r->subprocess_env, "proxy-sendchunked"))) { - rb_method = RB_SPOOL_CL; + || (!flushall + && apr_table_get(r->subprocess_env, "proxy-sendcl") + && !apr_table_get(r->subprocess_env, "proxy-sendchunks") + && !apr_table_get(r->subprocess_env, "proxy-sendchunked"))) { + *rb_method = RB_SPOOL_CL; } else { - rb_method = RB_STREAM_CHUNKED; + *rb_method = RB_STREAM_CHUNKED; } } - else if (old_cl_val) { + else if (*old_cl_val) { if (r->input_filters == r->proto_input_filters) { - rb_method = RB_STREAM_CL; + *rb_method = RB_STREAM_CL; } else if (!force10 - && (apr_table_get(r->subprocess_env, "proxy-sendchunks") + && (flushall + || apr_table_get(r->subprocess_env, "proxy-sendchunks") || apr_table_get(r->subprocess_env, "proxy-sendchunked")) && !apr_table_get(r->subprocess_env, "proxy-sendcl")) { - rb_method = RB_STREAM_CHUNKED; + *rb_method = RB_STREAM_CHUNKED; } else { - rb_method = RB_SPOOL_CL; + *rb_method = RB_SPOOL_CL; } } else { /* This is an appropriate default; very efficient for no-body * requests, and has the behavior that it will not add any C-L - * when the old_cl_val is NULL. + * when the *old_cl_val is NULL. */ - rb_method = RB_SPOOL_CL; + *rb_method = RB_SPOOL_CL; } /* Yes I hate gotos. This is the subrequest shortcut */ @@ -941,17 +977,33 @@ skip_body: APR_BRIGADE_INSERT_TAIL(header_brigade, e); } + return OK; +} + +static +int ap_proxy_http_request(apr_pool_t *p, request_rec *r, + proxy_conn_rec *p_conn, + apr_bucket_brigade *header_brigade, + apr_bucket_brigade *input_brigade, + char *old_cl_val, char *old_te_val, + enum rb_methods rb_method, int flushall) +{ + int rv; + apr_off_t bytes_read = 0; + conn_rec *origin = p_conn->connection; + /* send the request body, if any. */ switch(rb_method) { case RB_STREAM_CHUNKED: rv = stream_reqbody_chunked(p, r, p_conn, origin, header_brigade, - input_brigade); + input_brigade, flushall); break; case RB_STREAM_CL: rv = stream_reqbody_cl(p, r, p_conn, origin, header_brigade, - input_brigade, old_cl_val); + input_brigade, old_cl_val, flushall); break; case RB_SPOOL_CL: + apr_brigade_length(input_brigade, 1, &bytes_read); rv = spool_reqbody_cl(p, r, p_conn, origin, header_brigade, input_brigade, (old_cl_val != NULL) || (old_te_val != NULL) @@ -964,6 +1016,7 @@ skip_body: } if (rv != OK) { + conn_rec *c = r->connection; /* apr_status_t value has been logged in lower level method */ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01097) "pass request body failed to %pI (%s) from %s (%s)", @@ -1878,10 +1931,17 @@ static int proxy_http_handler(request_rec *r, prox char *scheme; const char *proxy_function; const char *u; + apr_bucket_brigade *header_brigade, *header_bb; + apr_bucket_brigade *input_brigade, *input_bb; proxy_conn_rec *backend = NULL; int is_ssl = 0; conn_rec *c = r->connection; int retry = 0; + char *old_cl_val = NULL, *old_te_val = NULL; + enum rb_methods rb_method = RB_INIT; + char *locurl = url; + int flushall = 0; + int toclose = 0; /* * Use a shorter-lived pool to reduce memory usage * and avoid a memory leak @@ -1948,16 +2008,60 @@ static int proxy_http_handler(request_rec *r, prox backend->close = 1; } + if (apr_table_get(r->subprocess_env, "proxy-flushall")) { + flushall = 1; + } + + /* 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 Once: Prefetch (partially) the request body so to increase the + * chances to get whole (or enough) body and determine Content-Length vs + * chunked or spool. By doing this before connecting or reusing a backend + * connection, minimize the delay between checking whether this connection + * is still alive and the first packet sent, should the link be slow or + * some input filter retain the data. + */ + input_brigade = apr_brigade_create(p, c->bucket_alloc); + header_brigade = apr_brigade_create(p, c->bucket_alloc); + if ((status = ap_proxy_http_prefetch(p, r, backend, worker, conf, uri, + locurl, server_portstr, header_brigade, input_brigade, + &old_cl_val, &old_te_val, &rb_method, flushall)) != OK) { + goto cleanup; + } + + /* XXX: Reset backend->close now, since ap_proxy_http_prefetch() sets it to + * disable the reuse of the connection after this request (no keep-alive), + * not to close any reusable connection before this request. However assure + * what is expected later by using a local flag and do the right thing when + * ap_proxy_connect_backend (below) provides the connection to close. + */ + toclose = backend->close; + backend->close = 0; + while (retry < 2) { - char *locurl = url; + conn_rec *backconn; - /* 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) - break; + if (retry) { + char *newurl = url; + /* Step One-Retry: Redetermine Who To Connect To */ + if ((status = ap_proxy_determine_connection(p, r, conf, worker, + backend, uri, &newurl, proxyname, proxyport, + server_portstr, sizeof(server_portstr))) != OK) + break; + + /* XXX: the code assumes locurl is not changed during the loop, + * or ap_proxy_http_prefetch() would have to be called every time, + * and header_brigade be changed accordingly... + */ + AP_DEBUG_ASSERT(strcmp(newurl, locurl) == 0); + } + /* Step Two: Make the Connection */ if (ap_proxy_connect_backend(proxy_function, backend, worker, r->server)) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01114) @@ -1968,10 +2072,13 @@ static int proxy_http_handler(request_rec *r, prox } /* Step Three: Create conn_rec */ - if (!backend->connection) { + backconn = backend->connection; + if (!backconn) { if ((status = ap_proxy_connection_create(proxy_function, backend, c, r->server)) != OK) break; + backconn = backend->connection; + /* * 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 @@ -1984,12 +2091,25 @@ static int proxy_http_handler(request_rec *r, prox } } + /* Don't recycle the connection if prefetch (above) told not to do so */ + if (toclose) { + backend->close = 1; + backconn->keepalive = AP_CONN_CLOSE; + } + + /* Preserve the header/input brigades since they may be retried. */ + header_bb = apr_brigade_create(p, backconn->bucket_alloc); + proxy_buckets_lifetime_transform(r, header_brigade, header_bb); + input_bb = apr_brigade_create(p, backconn->bucket_alloc); + proxy_buckets_lifetime_transform(r, input_brigade, input_bb); + /* 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 ((status = ap_proxy_http_request(p, r, backend, worker, - conf, uri, locurl, server_portstr)) != OK) { + if ((status = ap_proxy_http_request(p, r, backend, header_bb, input_bb, + old_cl_val, old_te_val, rb_method, + flushall)) != OK) { if ((status == HTTP_SERVICE_UNAVAILABLE) && worker->s->ping_timeout_set) { backend->close = 1; ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(01115) @@ -2000,7 +2120,6 @@ static int proxy_http_handler(request_rec *r, prox } else { break; } - } /* Step Five: Receive the Response... Fall thru to cleanup */