diff --git a/CHANGES b/CHANGES index 97a1aa6c53..8aadcc6822 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,16 @@ Changes with Apache 2.0.34-dev + *) New directive ProxyIOBufferSize. Sets the size of the buffer used + when reading from a remote HTTP server in proxy. [Graham Leggett] + + *) Modify receive/send loop in proxy_http and proxy_ftp so that + should it be necessary, the remote server socket is closed before + transmitting the last buffer (set by ProxyIOBufferSize) to the + client. This prevents the backend server from being forced to hang + around while the last few bytes are transmitted to a slow client. + Fix the case where no error checking was performed on the final + brigade in the loop. [Graham Leggett] + *) Scrap CacheMaxExpireMin and CacheDefaultExpireMin. Change CacheMaxExpire and CacheDefaultExpire to use seconds rather than hours. [Graham Leggett, Bill Stoddard] diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c index e6e8f72a67..c5ac9b9468 100644 --- a/modules/proxy/mod_proxy.c +++ b/modules/proxy/mod_proxy.c @@ -489,6 +489,8 @@ static void * create_proxy_config(apr_pool_t *p, server_rec *s) ps->req_set = 0; ps->recv_buffer_size = 0; /* this default was left unset for some reason */ ps->recv_buffer_size_set = 0; + ps->io_buffer_size = AP_IOBUFSIZE; + ps->io_buffer_size_set = 0; ps->maxfwd = DEFAULT_MAX_FORWARDS; ps->maxfwd_set = 0; ps->error_override = 0; @@ -518,6 +520,7 @@ static void * merge_proxy_config(apr_pool_t *p, void *basev, void *overridesv) ps->viaopt = (overrides->viaopt_set == 0) ? base->viaopt : overrides->viaopt; ps->req = (overrides->req_set == 0) ? base->req : overrides->req; ps->recv_buffer_size = (overrides->recv_buffer_size_set == 0) ? base->recv_buffer_size : overrides->recv_buffer_size; + ps->io_buffer_size = (overrides->io_buffer_size_set == 0) ? base->io_buffer_size : overrides->io_buffer_size; ps->maxfwd = (overrides->maxfwd_set == 0) ? base->maxfwd : overrides->maxfwd; ps->error_override = (overrides->error_override_set == 0) ? base->error_override : overrides->error_override; ps->preserve_host = (overrides->preserve_host_set == 0) ? base->preserve_host : overrides->preserve_host; @@ -841,6 +844,18 @@ static const char * return NULL; } +static const char * + set_io_buffer_size(cmd_parms *parms, void *dummy, const char *arg) +{ + proxy_server_conf *psf = + ap_get_module_config(parms->server->module_config, &proxy_module); + long s = atol(arg); + + psf->io_buffer_size = MAX(s, AP_IOBUFSIZE); + psf->io_buffer_size_set = 1; + return NULL; +} + static const char * set_max_forwards(cmd_parms *parms, void *dummy, const char *arg) { @@ -1004,6 +1019,8 @@ static const command_rec proxy_cmds[] = "A list of names, hosts or domains to which the proxy will not connect"), AP_INIT_TAKE1("ProxyReceiveBufferSize", set_recv_buffer_size, NULL, RSRC_CONF, "Receive buffer size for outgoing HTTP and FTP connections in bytes"), + AP_INIT_TAKE1("ProxyIOBufferSize", set_io_buffer_size, NULL, RSRC_CONF, + "IO buffer size for outgoing HTTP and FTP connections in bytes"), AP_INIT_TAKE1("ProxyMaxForwards", set_max_forwards, NULL, RSRC_CONF, "The maximum number of proxies a request may be forwarded through."), AP_INIT_ITERATE("NoProxy", set_proxy_dirconn, NULL, RSRC_CONF, diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index 9047ebea67..2f7d8c5fbd 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -175,8 +175,10 @@ typedef struct { via_full } viaopt; /* how to deal with proxy Via: headers */ char viaopt_set; - size_t recv_buffer_size; + apr_size_t recv_buffer_size; char recv_buffer_size_set; + apr_size_t io_buffer_size; + char io_buffer_size_set; long maxfwd; char maxfwd_set; /** diff --git a/modules/proxy/proxy_ftp.c b/modules/proxy/proxy_ftp.c index d8d5380246..d3c2f1fe3b 100644 --- a/modules/proxy/proxy_ftp.c +++ b/modules/proxy/proxy_ftp.c @@ -1804,28 +1804,79 @@ int ap_proxy_ftp_handler(request_rec *r, proxy_server_conf *conf, /* send body */ if (!r->header_only) { + apr_bucket *e; + int finish = FALSE; ap_log_error(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r->server, "proxy: FTP: start body send"); /* read the body, pass it to the output filters */ - while (ap_get_brigade(data->input_filters, bb, AP_MODE_EXHAUSTIVE, - APR_BLOCK_READ, 0) == APR_SUCCESS) { - if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) { - ap_pass_brigade(r->output_filters, bb); + while (ap_get_brigade(data->input_filters, + bb, + AP_MODE_READBYTES, + APR_BLOCK_READ, + conf->io_buffer_size) == APR_SUCCESS) { +#if DEBUGGING + { + apr_off_t readbytes; + apr_brigade_length(bb, 0, &readbytes); + ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, + r->server, "proxy (PID %d): readbytes: %#x", + getpid(), readbytes); + } +#endif + /* sanity check */ + if (APR_BRIGADE_EMPTY(bb)) { + apr_brigade_cleanup(bb); break; } + + /* found the last brigade? */ + if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) { + /* if this is the last brigade, cleanup the + * backend connection first to prevent the + * backend server from hanging around waiting + * for a slow client to eat these bytes + */ + ap_flush_conn(data); + apr_socket_close(data_sock); + data_sock = NULL; + ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server, + "proxy: FTP: data connection closed"); + /* signal that we must leave */ + finish = TRUE; + } + + /* if no EOS yet, then we must flush */ + if (FALSE == finish) { + e = apr_bucket_flush_create(); + APR_BRIGADE_INSERT_TAIL(bb, e); + } + + /* try send what we read */ if (ap_pass_brigade(r->output_filters, bb) != APR_SUCCESS) { /* Ack! Phbtt! Die! User aborted! */ + finish = TRUE; + } + + /* make sure we always clean up after ourselves */ + apr_brigade_cleanup(bb); + + /* if we are done, leave */ + if (TRUE == finish) { break; } - apr_brigade_cleanup(bb); } + ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server, + "proxy: FTP: end body send"); + + } + if (data_sock) { + ap_flush_conn(data); + apr_socket_close(data_sock); + ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server, + "proxy: FTP: data connection closed"); } - ap_flush_conn(data); - apr_socket_close(data_sock); - ap_log_error(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r->server, - "proxy: FTP: Closing Data connection."); /* Retrieve the final response for the RETR or LIST commands */ rc = proxy_ftp_command(NULL, r, origin, bb, &ftpmessage); diff --git a/modules/proxy/proxy_http.c b/modules/proxy/proxy_http.c index 214967ebaf..754b809bef 100644 --- a/modules/proxy/proxy_http.c +++ b/modules/proxy/proxy_http.c @@ -75,6 +75,10 @@ typedef struct { int close; } proxy_http_conn_t; +static apr_status_t ap_proxy_http_cleanup(request_rec *r, + proxy_http_conn_t *p_conn, + proxy_conn_rec *backend); + /* * Canonicalise http-like URLs. * scheme is the scheme for the URL @@ -813,17 +817,19 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, "proxy: start body send"); /* - * if we are overriding the errors, we cant put the content of the - * page into the brigade + * if we are overriding the errors, we can't put the content + * of the page into the brigade */ if ( (conf->error_override ==0) || r->status < 400 ) { - /* read the body, pass it to the output filters */ + + /* read the body, pass it to the output filters */ apr_bucket *e; + int finish = FALSE; while (ap_get_brigade(rp->input_filters, bb, AP_MODE_READBYTES, APR_BLOCK_READ, - AP_IOBUFSIZE) == APR_SUCCESS) { + conf->io_buffer_size) == APR_SUCCESS) { #if DEBUGGING { apr_off_t readbytes; @@ -833,21 +839,44 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, getpid(), readbytes); } #endif + /* sanity check */ if (APR_BRIGADE_EMPTY(bb)) { + apr_brigade_cleanup(bb); break; } + + /* found the last brigade? */ if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) { - ap_pass_brigade(r->output_filters, bb); - break; + /* if this is the last brigade, cleanup the + * backend connection first to prevent the + * backend server from hanging around waiting + * for a slow client to eat these bytes + */ + ap_proxy_http_cleanup(r, p_conn, backend); + /* signal that we must leave */ + finish = TRUE; } - e = apr_bucket_flush_create(); - APR_BRIGADE_INSERT_TAIL(bb, e); + + /* if no EOS yet, then we must flush */ + if (FALSE == finish) { + e = apr_bucket_flush_create(); + APR_BRIGADE_INSERT_TAIL(bb, e); + } + + /* try send what we read */ if (ap_pass_brigade(r->output_filters, bb) != APR_SUCCESS) { /* Ack! Phbtt! Die! User aborted! */ p_conn->close = 1; /* this causes socket close below */ + finish = TRUE; + } + + /* make sure we always clean up after ourselves */ + apr_brigade_cleanup(bb); + + /* if we are done, leave */ + if (TRUE == finish) { break; } - apr_brigade_cleanup(bb); } } ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server, @@ -886,8 +915,11 @@ apr_status_t ap_proxy_http_cleanup(request_rec *r, proxy_http_conn_t *p_conn, * we close the socket, otherwise we leave it open for KeepAlive support */ if (p_conn->close || (r->proto_num < HTTP_VERSION(1,1))) { - apr_socket_close(p_conn->sock); - backend->connection = NULL; + if (p_conn->sock) { + apr_socket_close(p_conn->sock); + p_conn->sock = NULL; + backend->connection = NULL; + } } return OK; }