From 5a91fd4b9032e65ee4f6ebe1ee51e82db6b90a15 Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Wed, 25 May 2016 16:45:09 -0400 Subject: [PATCH 01/11] [core] buffer large responses to tempfiles (fixes #758, fixes #760, fixes #933, fixes #1387, #1283, fixes #2083) This replaces buffering entire response in memory which might lead to huge memory footprint and possibly to memory exhaustion. use tempfiles of fixed size so disk space is freed as each file sent update callers of http_chunk_append_mem() and http_chunk_append_buffer() to handle failures when writing to tempfile. x-ref: "memory fragmentation leads to high memory usage after peaks" https://redmine.lighttpd.net/issues/758 "Random crashing on FreeBSD 6.1" https://redmine.lighttpd.net/issues/760 "lighty should buffer responses (after it grows above certain size) on disk" https://redmine.lighttpd.net/issues/933 "Memory usage increases when proxy+ssl+large file" https://redmine.lighttpd.net/issues/1283 "lighttpd+fastcgi memory problem" https://redmine.lighttpd.net/issues/1387 "Excessive Memory usage with streamed files from PHP" https://redmine.lighttpd.net/issues/2083 --- src/chunk.c | 4 +-- src/chunk.h | 3 +++ src/http_chunk.c | 69 ++++++++++++++++++++++++++++++++++------------- src/http_chunk.h | 4 +-- src/mod_cgi.c | 17 +++++++++--- src/mod_fastcgi.c | 6 ++++- src/mod_proxy.c | 15 +++++++++-- src/mod_scgi.c | 18 ++++++++++--- 8 files changed, 104 insertions(+), 32 deletions(-) diff --git a/src/chunk.c b/src/chunk.c index 78abdd68..0cb4842f 100644 --- a/src/chunk.c +++ b/src/chunk.c @@ -461,7 +461,7 @@ static chunk *chunkqueue_get_append_tempfile(chunkqueue *cq) { static void chunkqueue_remove_empty_chunks(chunkqueue *cq); -static int chunkqueue_append_to_tempfile(server *srv, chunkqueue *dest, const char *mem, size_t len) { +int chunkqueue_append_mem_to_tempfile(server *srv, chunkqueue *dest, const char *mem, size_t len) { chunk *dst_c; ssize_t written; @@ -599,7 +599,7 @@ int chunkqueue_steal_with_tempfiles(server *srv, chunkqueue *dest, chunkqueue *s case MEM_CHUNK: /* store "use" bytes from memory chunk in tempfile */ - if (0 != chunkqueue_append_to_tempfile(srv, dest, c->mem->ptr + c->offset, use)) { + if (0 != chunkqueue_append_mem_to_tempfile(srv, dest, c->mem->ptr + c->offset, use)) { return -1; } diff --git a/src/chunk.h b/src/chunk.h index 98fad2ef..3cb1fc3e 100644 --- a/src/chunk.h +++ b/src/chunk.h @@ -57,6 +57,9 @@ void chunkqueue_append_mem(chunkqueue *cq, const char *mem, size_t len); /* copi void chunkqueue_append_buffer(chunkqueue *cq, buffer *mem); /* may reset "mem" */ void chunkqueue_prepend_buffer(chunkqueue *cq, buffer *mem); /* may reset "mem" */ +struct server; /*(declaration)*/ +int chunkqueue_append_mem_to_tempfile(struct server *srv, chunkqueue *cq, const char *mem, size_t len); + /* functions to handle buffers to read into: */ /* return a pointer to a buffer in *mem with size *len; * it should be at least min_size big, and use alloc_size if diff --git a/src/http_chunk.c b/src/http_chunk.c index 32d9eef1..d895181f 100644 --- a/src/http_chunk.c +++ b/src/http_chunk.c @@ -94,45 +94,78 @@ int http_chunk_append_file(server *srv, connection *con, buffer *fn) { return 0; } -void http_chunk_append_buffer(server *srv, connection *con, buffer *mem) { - chunkqueue *cq; - - force_assert(NULL != con); - - if (buffer_string_is_empty(mem)) return; - - cq = con->write_queue; +static int http_chunk_append_to_tempfile(server *srv, connection *con, const char * mem, size_t len) { + chunkqueue * const cq = con->write_queue; if (con->response.transfer_encoding & HTTP_TRANSFER_ENCODING_CHUNKED) { - http_chunk_append_len(srv, con, buffer_string_length(mem)); + /*http_chunk_append_len(srv, con, len);*/ + buffer *b = srv->tmp_chunk_len; + + buffer_string_set_length(b, 0); + buffer_append_uint_hex(b, len); + buffer_append_string_len(b, CONST_STR_LEN("\r\n")); + + if (0 != chunkqueue_append_mem_to_tempfile(srv, cq, CONST_BUF_LEN(b))) { + return -1; + } } - chunkqueue_append_buffer(cq, mem); + if (0 != chunkqueue_append_mem_to_tempfile(srv, cq, mem, len)) { + return -1; + } if (con->response.transfer_encoding & HTTP_TRANSFER_ENCODING_CHUNKED) { - chunkqueue_append_mem(cq, CONST_STR_LEN("\r\n")); + if (0 != chunkqueue_append_mem_to_tempfile(srv, cq, CONST_STR_LEN("\r\n"))) { + return -1; + } } + + return 0; } -void http_chunk_append_mem(server *srv, connection *con, const char * mem, size_t len) { - chunkqueue *cq; +static int http_chunk_append_data(server *srv, connection *con, buffer *b, const char * mem, size_t len) { - force_assert(NULL != con); - force_assert(NULL != mem || 0 == len); + chunkqueue * const cq = con->write_queue; + chunk *c = cq->last; + if (0 == len) return 0; - if (NULL == mem || 0 == len) return; + /* current usage does not append_mem or append_buffer after appending + * file, so not checking if users of this interface have appended large + * (references to) files to chunkqueue, which would not be in memory */ - cq = con->write_queue; + if ((c && c->type == FILE_CHUNK && c->file.is_temp) + || cq->bytes_in - cq->bytes_out + len > 64 * 1024) { + return http_chunk_append_to_tempfile(srv, con, b ? b->ptr : mem, len); + } + + /* not appending to prior mem chunk just in case using openssl + * and need to resubmit same args as prior call to openssl (required?)*/ if (con->response.transfer_encoding & HTTP_TRANSFER_ENCODING_CHUNKED) { http_chunk_append_len(srv, con, len); } - chunkqueue_append_mem(cq, mem, len); + /*(chunkqueue_append_buffer() might steal buffer contents)*/ + b ? chunkqueue_append_buffer(cq, b) : chunkqueue_append_mem(cq, mem, len); if (con->response.transfer_encoding & HTTP_TRANSFER_ENCODING_CHUNKED) { chunkqueue_append_mem(cq, CONST_STR_LEN("\r\n")); } + + return 0; +} + +int http_chunk_append_buffer(server *srv, connection *con, buffer *mem) { + force_assert(NULL != con); + + return http_chunk_append_data(srv, con, mem, NULL, buffer_string_length(mem)); +} + +int http_chunk_append_mem(server *srv, connection *con, const char * mem, size_t len) { + force_assert(NULL != con); + force_assert(NULL != mem || 0 == len); + + return http_chunk_append_data(srv, con, NULL, mem, len); } void http_chunk_close(server *srv, connection *con) { diff --git a/src/http_chunk.h b/src/http_chunk.h index 266e24ce..791b7634 100644 --- a/src/http_chunk.h +++ b/src/http_chunk.h @@ -5,8 +5,8 @@ #include "server.h" #include -void http_chunk_append_mem(server *srv, connection *con, const char * mem, size_t len); /* copies memory */ -void http_chunk_append_buffer(server *srv, connection *con, buffer *mem); /* may reset "mem" */ +int http_chunk_append_mem(server *srv, connection *con, const char * mem, size_t len); /* copies memory */ +int http_chunk_append_buffer(server *srv, connection *con, buffer *mem); /* may reset "mem" */ int http_chunk_append_file(server *srv, connection *con, buffer *fn); /* copies "fn" */ int http_chunk_append_file_range(server *srv, connection *con, buffer *fn, off_t offset, off_t len); /* copies "fn" */ void http_chunk_close(server *srv, connection *con); diff --git a/src/mod_cgi.c b/src/mod_cgi.c index 31f6fe1a..236cfc01 100644 --- a/src/mod_cgi.c +++ b/src/mod_cgi.c @@ -500,7 +500,9 @@ static int cgi_demux_response(server *srv, handler_ctx *hctx) { con->response.transfer_encoding = HTTP_TRANSFER_ENCODING_CHUNKED; } - http_chunk_append_buffer(srv, con, hctx->response_header); + if (0 != http_chunk_append_buffer(srv, con, hctx->response_header)) { + return FDEVENT_HANDLED_ERROR; + } } else { const char *bstart; size_t blen; @@ -541,14 +543,18 @@ static int cgi_demux_response(server *srv, handler_ctx *hctx) { } if (blen > 0) { - http_chunk_append_mem(srv, con, bstart, blen); + if (0 != http_chunk_append_mem(srv, con, bstart, blen)) { + return FDEVENT_HANDLED_ERROR; + } } } con->file_started = 1; } } else { - http_chunk_append_buffer(srv, con, hctx->response); + if (0 != http_chunk_append_buffer(srv, con, hctx->response)) { + return FDEVENT_HANDLED_ERROR; + } } #if 0 @@ -756,7 +762,10 @@ static handler_t cgi_handle_fdevent(server *srv, void *ctx, int revents) { /* check if we still have a unfinished header package which is a body in reality */ if (con->file_started == 0 && !buffer_string_is_empty(hctx->response_header)) { con->file_started = 1; - http_chunk_append_buffer(srv, con, hctx->response_header); + if (0 != http_chunk_append_buffer(srv, con, hctx->response_header)) { + cgi_connection_close(srv, hctx); + return HANDLER_ERROR; + } } # if 0 diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c index e92040a7..56f63dcc 100644 --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -2627,7 +2627,11 @@ static int fcgi_demux_response(server *srv, handler_ctx *hctx) { } if (hctx->send_content_body && !buffer_string_is_empty(packet.b)) { - http_chunk_append_buffer(srv, con, packet.b); + if (0 != http_chunk_append_buffer(srv, con, packet.b)) { + /* error writing to tempfile; + * truncate response or send 500 if nothing sent yet */ + fin = 1; + } } break; case FCGI_STDERR: diff --git a/src/mod_proxy.c b/src/mod_proxy.c index 45d98fb4..216eccff 100644 --- a/src/mod_proxy.c +++ b/src/mod_proxy.c @@ -700,11 +700,22 @@ static int proxy_demux_response(server *srv, handler_ctx *hctx) { } con->file_started = 1; - if (blen > 0) http_chunk_append_mem(srv, con, c + 4, blen); + if (blen > 0) { + if (0 != http_chunk_append_mem(srv, con, c + 4, blen)) { + /* error writing to tempfile; + * truncate response or send 500 if nothing sent yet */ + fin = 1; + con->file_started = 0; + } + } buffer_reset(hctx->response); } } else { - http_chunk_append_buffer(srv, con, hctx->response); + if (0 != http_chunk_append_buffer(srv, con, hctx->response)) { + /* error writing to tempfile; + * truncate response or send 500 if nothing sent yet */ + fin = 1; + } buffer_reset(hctx->response); } diff --git a/src/mod_scgi.c b/src/mod_scgi.c index b47096a4..37bfd995 100644 --- a/src/mod_scgi.c +++ b/src/mod_scgi.c @@ -1968,7 +1968,11 @@ static int scgi_demux_response(server *srv, handler_ctx *hctx) { con->response.transfer_encoding = HTTP_TRANSFER_ENCODING_CHUNKED; } - http_chunk_append_buffer(srv, con, hctx->response_header); + if (0 != http_chunk_append_buffer(srv, con, hctx->response_header)) { + /* error writing to tempfile; + * truncate response or send 500 if nothing sent yet */ + return 1; + } } else { size_t blen = buffer_string_length(hctx->response_header) - hlen; @@ -1993,14 +1997,22 @@ static int scgi_demux_response(server *srv, handler_ctx *hctx) { } if (blen > 0) { - http_chunk_append_mem(srv, con, hctx->response_header->ptr + hlen, blen); + if (0 != http_chunk_append_mem(srv, con, hctx->response_header->ptr + hlen, blen)) { + /* error writing to tempfile; + * truncate response or send 500 if nothing sent yet */ + return 1; + } } } con->file_started = 1; } } else { - http_chunk_append_buffer(srv, con, hctx->response); + if (0 != http_chunk_append_buffer(srv, con, hctx->response)) { + /* error writing to tempfile; + * truncate response or send 500 if nothing sent yet */ + return 1; + } } #if 0 From 53f550b290adcaa9d73a199655655837344bbc2c Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Fri, 27 May 2016 00:24:33 -0400 Subject: [PATCH 02/11] [core] stream response to client (#949) This replaces buffering entire response prior to sending data to client x-ref: "fastcgi, cgi, flush, php5 problem." https://redmine.lighttpd.net/issues/949 --- src/connections.c | 51 +++++++++++++++++++++++++++++++----------- src/http-header-glue.c | 23 +++++++++++++++++++ src/mod_cgi.c | 17 +++----------- src/mod_fastcgi.c | 18 +++------------ src/mod_proxy.c | 18 +++------------ src/mod_scgi.c | 18 +++------------ src/response.h | 1 + 7 files changed, 74 insertions(+), 72 deletions(-) diff --git a/src/connections.c b/src/connections.c index 82808d41..7946ae38 100644 --- a/src/connections.c +++ b/src/connections.c @@ -1036,6 +1036,9 @@ int connection_state_machine(server *srv, connection *con) { } switch (r = http_response_prepare(srv, con)) { + case HANDLER_WAIT_FOR_EVENT: + if (!con->file_started) break; /* come back here */ + /* response headers received from backend; fall through to start response */ case HANDLER_FINISHED: if (con->error_handler_saved_status > 0) { con->request.http_method = con->error_handler_saved_method; @@ -1126,9 +1129,6 @@ int connection_state_machine(server *srv, connection *con) { break; case HANDLER_COMEBACK: done = -1; - /* fallthrough */ - case HANDLER_WAIT_FOR_EVENT: - /* come back here */ break; case HANDLER_ERROR: /* something went wrong */ @@ -1278,19 +1278,44 @@ int connection_state_machine(server *srv, connection *con) { "state for fd", con->fd, connection_get_state(con->state)); } - /* only try to write if we have something in the queue */ - if (!chunkqueue_is_empty(con->write_queue)) { - if (con->is_writable) { - if (-1 == connection_handle_write(srv, con)) { - log_error_write(srv, __FILE__, __LINE__, "ds", - con->fd, - "handle write failed."); + do { + /* only try to write if we have something in the queue */ + if (!chunkqueue_is_empty(con->write_queue)) { + if (con->is_writable) { + if (-1 == connection_handle_write(srv, con)) { + log_error_write(srv, __FILE__, __LINE__, "ds", + con->fd, + "handle write failed."); + connection_set_state(srv, con, CON_STATE_ERROR); + break; + } + if (con->state != CON_STATE_WRITE) break; + } + } else if (con->file_finished) { + connection_set_state(srv, con, CON_STATE_RESPONSE_END); + break; + } + + if (con->mode != DIRECT && !con->file_finished) { + switch(r = plugins_call_handle_subrequest(srv, con)) { + case HANDLER_WAIT_FOR_EVENT: + case HANDLER_FINISHED: + case HANDLER_GO_ON: + break; + case HANDLER_WAIT_FOR_FD: + srv->want_fds++; + fdwaitqueue_append(srv, con); + break; + case HANDLER_COMEBACK: + default: + log_error_write(srv, __FILE__, __LINE__, "sdd", "unexpected subrequest handler ret-value: ", con->fd, r); + /* fall through */ + case HANDLER_ERROR: connection_set_state(srv, con, CON_STATE_ERROR); + break; } } - } else if (con->file_finished) { - connection_set_state(srv, con, CON_STATE_RESPONSE_END); - } + } while (con->state == CON_STATE_WRITE && (!chunkqueue_is_empty(con->write_queue) ? con->is_writable : con->file_finished)); break; case CON_STATE_ERROR: /* transient */ diff --git a/src/http-header-glue.c b/src/http-header-glue.c index 976e4584..f72fcf9a 100644 --- a/src/http-header-glue.c +++ b/src/http-header-glue.c @@ -726,3 +726,26 @@ void http_response_xsendfile (server *srv, connection *con, buffer *path, const con->http_status = status; } } + +void http_response_backend_done (server *srv, connection *con) { + /* (not CON_STATE_ERROR and not CON_STATE_RESPONSE_END, + * i.e. not called from handle_connection_close or connection_reset + * hooks, except maybe from errdoc handler, which later resets state)*/ + switch (con->state) { + case CON_STATE_HANDLE_REQUEST: + case CON_STATE_READ_POST: + if (!con->file_started) { + /* Send an error if we haven't sent any data yet */ + con->http_status = 500; + con->mode = DIRECT; + break; + } /* else fall through */ + case CON_STATE_WRITE: + if (!con->file_finished) { + http_chunk_close(srv, con); + con->file_finished = 1; + } + default: + break; + } +} diff --git a/src/mod_cgi.c b/src/mod_cgi.c index 236cfc01..b55e7366 100644 --- a/src/mod_cgi.c +++ b/src/mod_cgi.c @@ -662,20 +662,9 @@ static void cgi_connection_close(server *srv, handler_ctx *hctx) { } #endif - /* finish response (if not already finished) */ - if (con->mode == p->id - && (con->state == CON_STATE_HANDLE_REQUEST || con->state == CON_STATE_READ_POST)) { - /* (not CON_STATE_ERROR and not CON_STATE_RESPONSE_END, - * i.e. not called from cgi_connection_close_callback()) */ - - /* Send an error if we haven't sent any data yet */ - if (0 == con->file_started) { - con->http_status = 500; - con->mode = DIRECT; - } else if (0 == con->file_finished) { - http_chunk_close(srv, con); - con->file_finished = 1; - } + /* finish response (if not already con->file_started, con->file_finished) */ + if (con->mode == p->id) { + http_response_backend_done(srv, con); } } diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c index 56f63dcc..0e5dea1f 100644 --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -1585,21 +1585,9 @@ static void fcgi_connection_close(server *srv, handler_ctx *hctx) { handler_ctx_free(srv, hctx); con->plugin_ctx[p->id] = NULL; - /* finish response (if not already finished) */ - if (con->mode == p->id - && (con->state == CON_STATE_HANDLE_REQUEST || con->state == CON_STATE_READ_POST)) { - /* (not CON_STATE_ERROR and not CON_STATE_RESPONSE_END, - * i.e. not called from fcgi_connection_reset()) */ - - /* Send an error if we haven't sent any data yet */ - if (0 == con->file_started) { - con->http_status = 500; - con->mode = DIRECT; - } - else if (!con->file_finished) { - http_chunk_close(srv, con); - con->file_finished = 1; - } + /* finish response (if not already con->file_started, con->file_finished) */ + if (con->mode == p->id) { + http_response_backend_done(srv, con); } } diff --git a/src/mod_proxy.c b/src/mod_proxy.c index 216eccff..8f3403b6 100644 --- a/src/mod_proxy.c +++ b/src/mod_proxy.c @@ -351,21 +351,9 @@ static void proxy_connection_close(server *srv, handler_ctx *hctx) { handler_ctx_free(hctx); con->plugin_ctx[p->id] = NULL; - /* finish response (if not already finished) */ - if (con->mode == p->id - && (con->state == CON_STATE_HANDLE_REQUEST || con->state == CON_STATE_READ_POST)) { - /* (not CON_STATE_ERROR and not CON_STATE_RESPONSE_END, - * i.e. not called from proxy_connection_reset()) */ - - /* Send an error if we haven't sent any data yet */ - if (0 == con->file_started) { - con->http_status = 500; - con->mode = DIRECT; - } - else if (!con->file_finished) { - http_chunk_close(srv, con); - con->file_finished = 1; - } + /* finish response (if not already con->file_started, con->file_finished) */ + if (con->mode == p->id) { + http_response_backend_done(srv, con); } } diff --git a/src/mod_scgi.c b/src/mod_scgi.c index 37bfd995..3e7cd10b 100644 --- a/src/mod_scgi.c +++ b/src/mod_scgi.c @@ -1336,21 +1336,9 @@ static void scgi_connection_close(server *srv, handler_ctx *hctx) { handler_ctx_free(hctx); con->plugin_ctx[p->id] = NULL; - /* finish response (if not already finished) */ - if (con->mode == p->id - && (con->state == CON_STATE_HANDLE_REQUEST || con->state == CON_STATE_READ_POST)) { - /* (not CON_STATE_ERROR and not CON_STATE_RESPONSE_END, - * i.e. not called from scgi_connection_reset()) */ - - /* Send an error if we haven't sent any data yet */ - if (0 == con->file_started) { - con->http_status = 500; - con->mode = DIRECT; - } - else if (!con->file_finished) { - http_chunk_close(srv, con); - con->file_finished = 1; - } + /* finish response (if not already con->file_started, con->file_finished) */ + if (con->mode == p->id) { + http_response_backend_done(srv, con); } } diff --git a/src/response.h b/src/response.h index 6ccb87a8..bf88d2bb 100644 --- a/src/response.h +++ b/src/response.h @@ -18,6 +18,7 @@ int http_response_redirect_to_directory(server *srv, connection *con); int http_response_handle_cachable(server *srv, connection *con, buffer * mtime); void http_response_send_file (server *srv, connection *con, buffer *path); void http_response_xsendfile (server *srv, connection *con, buffer *path, const array *xdocroot); +void http_response_backend_done (server *srv, connection *con); buffer * strftime_cache_get(server *srv, time_t last_mod); #endif From 5ab7944d3439f8efcd20d177d94ccdccc760881d Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Sat, 28 May 2016 16:58:59 -0400 Subject: [PATCH 03/11] [TLS] release openssl buffers as used (fixes #1265, fixes #1283, #881) use SSL_MODE_RELEASE_BUFFERS (OpenSSL >= 1.0.0) to free buffers as they are used, to potentially reduce memory footprint of idle SSL connections x-ref: "memory usage when ssl.engine used and large data uploaded through CGI" https://redmine.lighttpd.net/issues/881 "SSL + file upload = lots of memory" https://redmine.lighttpd.net/issues/1265 "Memory usage increases when proxy+ssl+large file" https://redmine.lighttpd.net/issues/1283 --- src/network.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/network.c b/src/network.c index f59b8bbe..18b8b47e 100644 --- a/src/network.c +++ b/src/network.c @@ -714,6 +714,9 @@ int network_init(server *srv) { specific_config *s = srv->config_storage[i]; #ifndef SSL_OP_NO_COMPRESSION # define SSL_OP_NO_COMPRESSION 0 +#endif +#ifndef SSL_MODE_RELEASE_BUFFERS /* OpenSSL >= 1.0.0 */ +#define SSL_MODE_RELEASE_BUFFERS 0 #endif long ssloptions = SSL_OP_ALL | SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION | SSL_OP_NO_COMPRESSION; @@ -937,7 +940,10 @@ int network_init(server *srv) { return -1; } SSL_CTX_set_default_read_ahead(s->ssl_ctx, 1); - SSL_CTX_set_mode(s->ssl_ctx, SSL_CTX_get_mode(s->ssl_ctx) | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); + SSL_CTX_set_mode(s->ssl_ctx, SSL_CTX_get_mode(s->ssl_ctx) + | SSL_MODE_ENABLE_PARTIAL_WRITE + | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER + | SSL_MODE_RELEASE_BUFFERS); # ifndef OPENSSL_NO_TLSEXT if (!SSL_CTX_set_tlsext_servername_callback(s->ssl_ctx, network_ssl_servername_callback) || From 695c8f4e070b290d98b2ea253a76897c7b86a0d6 Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Sat, 4 Jun 2016 10:56:06 -0400 Subject: [PATCH 04/11] [config] config options to stream request/response (#949, #376) This allows admin to configure if response is collected in entirety prior to sending data to client For compatibility with existing configs, default is existing behavior: buffer entire response prior to sending data to client The following are config options, though not all implemented yet // default: buffer entire request body before connecting to backend server.stream-request-body = 0 // stream request body to backend; buffer to temp files server.stream-request-body = 1 // stream request body to backend; minimal buffering might block upload server.stream-request-body = 2 // default: buffer entire response body before sending to client server.stream-request-body = 0 // stream response body to client; buffer to temp files server.stream-request-body = 1 // stream response body to client; minimal buffering might block backend server.stream-request-body = 2 x-ref: "fastcgi, cgi, flush, php5 problem." https://redmine.lighttpd.net/issues/949 "Reimplement upload (POST) handling to match apache/zeus/thttpd/boa functionality" https://redmine.lighttpd.net/issues/376 --- src/base.h | 2 ++ src/configfile.c | 20 ++++++++++++++++++++ src/connections-glue.c | 2 ++ src/connections.c | 41 ++++++++++++++++++++++++++++------------- src/fdevent.h | 9 +++++++++ src/server.c | 15 +++++++++------ 6 files changed, 70 insertions(+), 19 deletions(-) diff --git a/src/base.h b/src/base.h index ccec58dc..6798a662 100644 --- a/src/base.h +++ b/src/base.h @@ -263,6 +263,8 @@ typedef struct { unsigned short use_xattr; unsigned short follow_symlink; unsigned short range_requests; + unsigned short stream_request_body; + unsigned short stream_response_body; /* debug */ diff --git a/src/configfile.c b/src/configfile.c index b1d5a235..ffcc7a52 100644 --- a/src/configfile.c +++ b/src/configfile.c @@ -119,6 +119,8 @@ static int config_insert(server *srv) { { "server.http-parseopt-host-strict", NULL, T_CONFIG_BOOLEAN, T_CONFIG_SCOPE_SERVER }, /* 73 */ { "server.http-parseopt-host-normalize",NULL,T_CONFIG_BOOLEAN, T_CONFIG_SCOPE_SERVER }, /* 74 */ { "server.bsd-accept-filter", NULL, T_CONFIG_STRING, T_CONFIG_SCOPE_CONNECTION }, /* 75 */ + { "server.stream-request-body", NULL, T_CONFIG_SHORT, T_CONFIG_SCOPE_CONNECTION }, /* 76 */ + { "server.stream-response-body", NULL, T_CONFIG_SHORT, T_CONFIG_SCOPE_CONNECTION }, /* 77 */ { "server.host", "use server.bind instead", @@ -248,6 +250,8 @@ static int config_insert(server *srv) { s->ssl_verifyclient_export_cert = 0; s->ssl_disable_client_renegotiation = 1; s->listen_backlog = (0 == i ? 1024 : srv->config_storage[0]->listen_backlog); + s->stream_request_body = 0; + s->stream_response_body = 0; /* all T_CONFIG_SCOPE_CONNECTION options */ cv[2].destination = s->errorfile_prefix; @@ -310,12 +314,21 @@ static int config_insert(server *srv) { || defined(__OpenBSD__) || defined(__DragonflyBSD__) cv[75].destination = s->bsd_accept_filter; #endif + cv[76].destination = &(s->stream_request_body); + cv[77].destination = &(s->stream_response_body); srv->config_storage[i] = s; if (0 != (ret = config_insert_values_global(srv, config->value, cv, i == 0 ? T_CONFIG_SCOPE_SERVER : T_CONFIG_SCOPE_CONNECTION))) { break; } + + if (s->stream_request_body & FDEVENT_STREAM_REQUEST_BUFMIN) { + s->stream_request_body |= FDEVENT_STREAM_REQUEST; + } + if (s->stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN) { + s->stream_response_body |= FDEVENT_STREAM_RESPONSE; + } } { @@ -453,6 +466,9 @@ int config_setup_connection(server *srv, connection *con) { PATCH(range_requests); PATCH(force_lowercase_filenames); /*PATCH(listen_backlog);*//*(not necessary; used only at startup)*/ + PATCH(stream_request_body); + PATCH(stream_response_body); + PATCH(ssl_enabled); PATCH(ssl_pemfile); @@ -563,6 +579,10 @@ int config_patch_connection(server *srv, connection *con) { buffer_copy_buffer(con->server_name, s->server_name); } else if (buffer_is_equal_string(du->key, CONST_STR_LEN("server.tag"))) { PATCH(server_tag); + } else if (buffer_is_equal_string(du->key, CONST_STR_LEN("server.stream-request-body"))) { + PATCH(stream_request_body); + } else if (buffer_is_equal_string(du->key, CONST_STR_LEN("server.stream-response-body"))) { + PATCH(stream_response_body); } else if (buffer_is_equal_string(du->key, CONST_STR_LEN("connection.kbytes-per-second"))) { PATCH(kbytes_per_second); } else if (buffer_is_equal_string(du->key, CONST_STR_LEN("debug.log-request-handling"))) { diff --git a/src/connections-glue.c b/src/connections-glue.c index a5be6d68..21b28088 100644 --- a/src/connections-glue.c +++ b/src/connections-glue.c @@ -359,6 +359,7 @@ handler_t connection_handle_read_post_state(server *srv, connection *con) { if (dst_cq->bytes_in == (off_t)con->request.content_length) { /* Content is ready */ + con->conf.stream_request_body &= ~FDEVENT_STREAM_REQUEST_POLLIN; connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST); return HANDLER_GO_ON; } else if (is_closed) { @@ -372,6 +373,7 @@ handler_t connection_handle_read_post_state(server *srv, connection *con) { #endif return HANDLER_ERROR; } else { + con->conf.stream_request_body |= FDEVENT_STREAM_REQUEST_POLLIN; return HANDLER_WAIT_FOR_EVENT; } } diff --git a/src/connections.c b/src/connections.c index 7946ae38..e1414179 100644 --- a/src/connections.c +++ b/src/connections.c @@ -361,12 +361,6 @@ static int connection_handle_write_prepare(server *srv, connection *con) { } } - if (con->request.content_length - && (off_t)con->request.content_length > con->request_content_queue->bytes_in) { - /* request body is present and has not been read completely */ - con->keep_alive = 0; - } - if (con->request.http_method == HTTP_METHOD_HEAD) { /** * a HEAD request has the same as a GET @@ -1037,7 +1031,7 @@ int connection_state_machine(server *srv, connection *con) { switch (r = http_response_prepare(srv, con)) { case HANDLER_WAIT_FOR_EVENT: - if (!con->file_started) break; /* come back here */ + if (!con->file_started || 0 == con->conf.stream_response_body) break; /* come back here */ /* response headers received from backend; fall through to start response */ case HANDLER_FINISHED: if (con->error_handler_saved_status > 0) { @@ -1168,6 +1162,12 @@ int connection_state_machine(server *srv, connection *con) { "state for fd", con->fd, connection_get_state(con->state)); } + if (con->request.content_length + && (off_t)con->request.content_length > con->request_content_queue->bytes_in) { + /* request body is present and has not been read completely */ + con->keep_alive = 0; + } + plugins_call_handle_request_done(srv, con); srv->con_written++; @@ -1441,11 +1441,11 @@ int connection_state_machine(server *srv, connection *con) { connection_get_state(con->state)); } + r = 0; switch(con->state) { - case CON_STATE_READ_POST: case CON_STATE_READ: case CON_STATE_CLOSE: - fdevent_event_set(srv->ev, &(con->fde_ndx), con->fd, FDEVENT_IN); + r = FDEVENT_IN; break; case CON_STATE_WRITE: /* request write-fdevent only if we really need it @@ -1455,15 +1455,30 @@ int connection_state_machine(server *srv, connection *con) { if (!chunkqueue_is_empty(con->write_queue) && (con->is_writable == 0) && (con->traffic_limit_reached == 0)) { - fdevent_event_set(srv->ev, &(con->fde_ndx), con->fd, FDEVENT_OUT); - } else { - fdevent_event_set(srv->ev, &(con->fde_ndx), con->fd, 0); + r |= FDEVENT_OUT; + } + /* fall through */ + case CON_STATE_READ_POST: + if (con->conf.stream_request_body & FDEVENT_STREAM_REQUEST_POLLIN) { + r |= FDEVENT_IN; } break; default: - fdevent_event_set(srv->ev, &(con->fde_ndx), con->fd, 0); break; } + if (-1 != con->fd) { + const int events = fdevent_event_get_interest(srv->ev, con->fd); + if (r != events) { + /* update timestamps when enabling interest in events */ + if ((r & FDEVENT_IN) && !(events & FDEVENT_IN)) { + con->read_idle_ts = srv->cur_ts; + } + if ((r & FDEVENT_OUT) && !(events & FDEVENT_OUT)) { + con->write_request_ts = srv->cur_ts; + } + fdevent_event_set(srv->ev, &con->fde_ndx, con->fd, r); + } + } return 0; } diff --git a/src/fdevent.h b/src/fdevent.h index 6ee4d92b..27a46aaa 100644 --- a/src/fdevent.h +++ b/src/fdevent.h @@ -65,6 +65,13 @@ typedef handler_t (*fdevent_handler)(struct server *srv, void *ctx, int revents) #define FDEVENT_HUP BV(4) #define FDEVENT_NVAL BV(5) +#define FDEVENT_STREAM_REQUEST BV(0) +#define FDEVENT_STREAM_REQUEST_BUFMIN BV(1) +#define FDEVENT_STREAM_REQUEST_POLLIN BV(15) + +#define FDEVENT_STREAM_RESPONSE BV(0) +#define FDEVENT_STREAM_RESPONSE_BUFMIN BV(1) + typedef enum { FD_EVENT_TYPE_UNSET = -1, FD_EVENT_TYPE_CONNECTION, FD_EVENT_TYPE_FCGI_CONNECTION, @@ -173,6 +180,8 @@ fdevents *fdevent_init(struct server *srv, size_t maxfds, fdevent_handler_t type int fdevent_reset(fdevents *ev); /* "init" after fork() */ void fdevent_free(fdevents *ev); +#define fdevent_event_get_interest(ev, fd) \ + (-1 != (fd) ? (ev)->fdarray[(fd)]->events : 0) void fdevent_event_set(fdevents *ev, int *fde_ndx, int fd, int events); /* events can be FDEVENT_IN, FDEVENT_OUT or FDEVENT_IN | FDEVENT_OUT */ void fdevent_event_del(fdevents *ev, int *fde_ndx, int fd); int fdevent_event_get_revent(fdevents *ev, size_t ndx); diff --git a/src/server.c b/src/server.c index b3f36a75..d0c14f99 100644 --- a/src/server.c +++ b/src/server.c @@ -1576,15 +1576,13 @@ int main (int argc, char **argv) { * */ for (ndx = 0; ndx < conns->used; ndx++) { + connection * const con = conns->ptr[ndx]; + const int waitevents = fdevent_event_get_interest(srv->ev, con->fd); int changed = 0; - connection *con; int t_diff; - con = conns->ptr[ndx]; - - if (con->state == CON_STATE_READ || - con->state == CON_STATE_READ_POST) { - if (con->request_count == 1 || con->state == CON_STATE_READ_POST) { + if (waitevents & FDEVENT_IN) { + if (con->request_count == 1 || con->state != CON_STATE_READ) { /* e.g. CON_STATE_READ_POST || CON_STATE_WRITE */ if (srv->cur_ts - con->read_idle_ts > con->conf.max_read_idle) { /* time - out */ if (con->conf.log_request_handling) { @@ -1609,6 +1607,11 @@ int main (int argc, char **argv) { } } + /* max_write_idle timeout currently functions as backend timeout, + * too, after response has been started. + * future: have separate backend timeout, and then change this + * to check for write interest before checking for timeout */ + /*if (waitevents & FDEVENT_OUT)*/ if ((con->state == CON_STATE_WRITE) && (con->write_request_ts != 0)) { #if 0 From f69f209e6d7113f95f10f82c2cf6079fcaca12fe Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Fri, 10 Jun 2016 00:04:10 -0400 Subject: [PATCH 05/11] [core] option to stream request body to backend (fixes #376) Set server.stream-request-body = 1 or server.stream-request-body = 2 to have lighttpd connect to backend (CGI, FastCGI, SCGI, proxy) immediately after parsing request headers, and to stream request body as it arrives. default: buffer entire request body before connecting to backend, in order to avoid tying up (limited) backend resources which are often implemented using libraries which wait for entire request body before proceeding. x-ref: "Reimplement upload (POST) handling to match apache/zeus/thttpd/boa functionality" https://redmine.lighttpd.net/issues/376 --- src/connections-glue.c | 8 ++- src/mod_cgi.c | 61 ++++++++++++++---- src/mod_fastcgi.c | 142 ++++++++++++++++++++++++----------------- src/mod_proxy.c | 63 ++++++++++++------ src/mod_scgi.c | 97 ++++++++++++++++------------ src/mod_webdav.c | 1 + 6 files changed, 236 insertions(+), 136 deletions(-) diff --git a/src/connections-glue.c b/src/connections-glue.c index 21b28088..3f4b4738 100644 --- a/src/connections-glue.c +++ b/src/connections-glue.c @@ -360,7 +360,9 @@ handler_t connection_handle_read_post_state(server *srv, connection *con) { if (dst_cq->bytes_in == (off_t)con->request.content_length) { /* Content is ready */ con->conf.stream_request_body &= ~FDEVENT_STREAM_REQUEST_POLLIN; - connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST); + if (con->state == CON_STATE_READ_POST) { + connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST); + } return HANDLER_GO_ON; } else if (is_closed) { #if 0 @@ -374,6 +376,8 @@ handler_t connection_handle_read_post_state(server *srv, connection *con) { return HANDLER_ERROR; } else { con->conf.stream_request_body |= FDEVENT_STREAM_REQUEST_POLLIN; - return HANDLER_WAIT_FOR_EVENT; + return (con->conf.stream_request_body & FDEVENT_STREAM_REQUEST) + ? HANDLER_GO_ON + : HANDLER_WAIT_FOR_EVENT; } } diff --git a/src/mod_cgi.c b/src/mod_cgi.c index b55e7366..8f630fa5 100644 --- a/src/mod_cgi.c +++ b/src/mod_cgi.c @@ -697,8 +697,13 @@ static handler_t cgi_handle_fdevent_send (server *srv, void *ctx, int revents) { if (revents & FDEVENT_HUP) { /* skip sending remaining data to CGI */ - chunkqueue *cq = con->request_content_queue; - chunkqueue_mark_written(cq, chunkqueue_length(cq)); + if (con->request.content_length) { + chunkqueue *cq = con->request_content_queue; + chunkqueue_mark_written(cq, chunkqueue_length(cq)); + if (cq->bytes_in != (off_t)con->request.content_length) { + con->keep_alive = 0; + } + } cgi_connection_close_fdtocgi(srv, hctx); /*(closes only hctx->fdtocgi)*/ } else if (revents & FDEVENT_ERR) { @@ -742,10 +747,6 @@ static handler_t cgi_handle_fdevent(server *srv, void *ctx, int revents) { } } - if (revents & FDEVENT_OUT) { - /* nothing to do */ - } - /* perhaps this issue is already handled */ if (revents & FDEVENT_HUP) { /* check if we still have a unfinished header package which is a body in reality */ @@ -960,10 +961,10 @@ static int cgi_write_request(server *srv, handler_ctx *hctx, int fd) { } } - if (chunkqueue_is_empty(cq)) { + if (cq->bytes_out == (off_t)con->request.content_length) { /* sent all request body input */ /* close connection to the cgi-script */ - if (-1 == hctx->fdtocgi) { /*(entire request body sent in initial send to pipe buffer)*/ + if (-1 == hctx->fdtocgi) { /*(received request body sent in initial send to pipe buffer)*/ if (close(fd)) { log_error_write(srv, __FILE__, __LINE__, "sds", "cgi stdin close failed ", fd, strerror(errno)); } @@ -971,11 +972,25 @@ static int cgi_write_request(server *srv, handler_ctx *hctx, int fd) { cgi_connection_close_fdtocgi(srv, hctx); /*(closes only hctx->fdtocgi)*/ } } else { - /* more request body remains to be sent to CGI so register for fdevents */ + off_t cqlen = cq->bytes_in - cq->bytes_out; + if (cq->bytes_in < (off_t)con->request.content_length && cqlen < 65536 - 16384) { + /*(con->conf.stream_request_body & FDEVENT_STREAM_REQUEST)*/ + if (!(con->conf.stream_request_body & FDEVENT_STREAM_REQUEST_POLLIN)) { + con->conf.stream_request_body |= FDEVENT_STREAM_REQUEST_POLLIN; + con->is_readable = 1; /* trigger optimistic read from client */ + } + } if (-1 == hctx->fdtocgi) { /*(not registered yet)*/ hctx->fdtocgi = fd; hctx->fde_ndx_tocgi = -1; fdevent_register(srv->ev, hctx->fdtocgi, cgi_handle_fdevent_send, hctx); + } + if (0 == cqlen) { /*(chunkqueue_is_empty(cq))*/ + if ((fdevent_event_get_interest(srv->ev, hctx->fdtocgi) & FDEVENT_OUT)) { + fdevent_event_set(srv->ev, &(hctx->fde_ndx_tocgi), hctx->fdtocgi, 0); + } + } else { + /* more request body remains to be sent to CGI so register for fdevents */ fdevent_event_set(srv->ev, &(hctx->fde_ndx_tocgi), hctx->fdtocgi, FDEVENT_OUT); } } @@ -1482,13 +1497,27 @@ TRIGGER_FUNC(cgi_trigger) { SUBREQUEST_FUNC(mod_cgi_handle_subrequest) { plugin_data *p = p_d; handler_ctx *hctx = con->plugin_ctx[p->id]; + chunkqueue *cq = con->request_content_queue; if (con->mode != p->id) return HANDLER_GO_ON; if (NULL == hctx) return HANDLER_GO_ON; - if (con->state == CON_STATE_READ_POST) { - handler_t r = connection_handle_read_post_state(srv, con); - if (r != HANDLER_GO_ON) return r; + if (cq->bytes_in != (off_t)con->request.content_length) { + /*(64k - 4k to attempt to avoid temporary files + * in conjunction with FDEVENT_STREAM_REQUEST_BUFMIN)*/ + if (cq->bytes_in - cq->bytes_out > 65536 - 4096 + && (con->conf.stream_request_body & FDEVENT_STREAM_REQUEST_BUFMIN)){ + con->conf.stream_request_body &= ~FDEVENT_STREAM_REQUEST_POLLIN; + if (-1 != hctx->fd) return HANDLER_WAIT_FOR_EVENT; + } else { + handler_t r = connection_handle_read_post_state(srv, con); + if (!chunkqueue_is_empty(cq)) { + if (fdevent_event_get_interest(srv->ev, hctx->fdtocgi) & FDEVENT_OUT) { + return (r == HANDLER_GO_ON) ? HANDLER_WAIT_FOR_EVENT : r; + } + } + if (r != HANDLER_GO_ON) return r; + } } if (-1 == hctx->fd) { @@ -1500,11 +1529,15 @@ SUBREQUEST_FUNC(mod_cgi_handle_subrequest) { return HANDLER_FINISHED; } - } - #if 0 log_error_write(srv, __FILE__, __LINE__, "sdd", "subrequest, pid =", hctx, hctx->pid); #endif + } else if (!chunkqueue_is_empty(con->request_content_queue)) { + if (0 != cgi_write_request(srv, hctx, hctx->fdtocgi)) { + cgi_connection_close(srv, hctx); + return HANDLER_ERROR; + } + } /* if not done, wait for CGI to close stdout, so we read EOF on pipe */ return con->file_finished ? HANDLER_FINISHED : HANDLER_WAIT_FOR_EVENT; diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c index 0e5dea1f..874e357b 100644 --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -338,20 +338,20 @@ typedef struct { fcgi_connection_state_t state; time_t state_timestamp; - int reconnects; /* number of reconnect attempts */ - chunkqueue *rb; /* read queue */ chunkqueue *wb; /* write queue */ + off_t wb_reqlen; buffer *response_header; - size_t request_id; int fd; /* fd to the fastcgi process */ int fde_ndx; /* index into the fd-event buffer */ pid_t pid; int got_proc; + int reconnects; /* number of reconnect attempts */ + int request_id; int send_content_body; plugin_config conf; @@ -497,6 +497,7 @@ static handler_ctx * handler_ctx_init(void) { hctx->rb = chunkqueue_init(); hctx->wb = chunkqueue_init(); + hctx->wb_reqlen = 0; return hctx; } @@ -1712,7 +1713,7 @@ static int fcgi_env_add(buffer *env, const char *key, size_t key_len, const char return 0; } -static int fcgi_header(FCGI_Header * header, unsigned char type, size_t request_id, int contentLength, unsigned char paddingLength) { +static int fcgi_header(FCGI_Header * header, unsigned char type, int request_id, int contentLength, unsigned char paddingLength) { force_assert(contentLength <= FCGI_MAX_LENGTH); header->version = FCGI_VERSION_1; @@ -1895,7 +1896,42 @@ static int fcgi_env_add_request_headers(server *srv, connection *con, plugin_dat return 0; } -static int fcgi_create_env(server *srv, handler_ctx *hctx, size_t request_id) { +static void fcgi_stdin_append(server *srv, connection *con, handler_ctx *hctx, int request_id) { + FCGI_Header header; + chunkqueue *req_cq = con->request_content_queue; + plugin_data *p = hctx->plugin_data; + off_t offset, weWant; + const off_t req_cqlen = req_cq->bytes_in - req_cq->bytes_out; + + /* something to send ? */ + for (offset = 0; offset != req_cqlen; offset += weWant) { + weWant = req_cqlen - offset > FCGI_MAX_LENGTH ? FCGI_MAX_LENGTH : req_cqlen - offset; + + /* we announce toWrite octets + * now take all request_content chunks available + * */ + + fcgi_header(&(header), FCGI_STDIN, request_id, weWant, 0); + chunkqueue_append_mem(hctx->wb, (const char *)&header, sizeof(header)); + hctx->wb_reqlen += sizeof(header); + + if (p->conf.debug > 10) { + log_error_write(srv, __FILE__, __LINE__, "soso", "tosend:", offset, "/", req_cqlen); + } + + chunkqueue_steal(hctx->wb, req_cq, weWant); + /*(hctx->wb_reqlen already includes content_length)*/ + } + + if (hctx->wb->bytes_in == hctx->wb_reqlen) { + /* terminate STDIN */ + fcgi_header(&(header), FCGI_STDIN, request_id, 0, 0); + chunkqueue_append_mem(hctx->wb, (const char *)&header, sizeof(header)); + hctx->wb_reqlen += (int)sizeof(header); + } +} + +static int fcgi_create_env(server *srv, handler_ctx *hctx, int request_id) { FCGI_BeginRequestRecord beginRecord; FCGI_Header header; @@ -2127,38 +2163,13 @@ static int fcgi_create_env(server *srv, handler_ctx *hctx, size_t request_id) { fcgi_header(&(header), FCGI_PARAMS, request_id, 0, 0); buffer_append_string_len(b, (const char *)&header, sizeof(header)); + hctx->wb_reqlen = buffer_string_length(b); chunkqueue_append_buffer(hctx->wb, b); buffer_free(b); } - if (con->request.content_length) { - chunkqueue *req_cq = con->request_content_queue; - off_t offset; - - /* something to send ? */ - for (offset = 0; offset != req_cq->bytes_in; ) { - off_t weWant = req_cq->bytes_in - offset > FCGI_MAX_LENGTH ? FCGI_MAX_LENGTH : req_cq->bytes_in - offset; - - /* we announce toWrite octets - * now take all the request_content chunks that we need to fill this request - * */ - - fcgi_header(&(header), FCGI_STDIN, request_id, weWant, 0); - chunkqueue_append_mem(hctx->wb, (const char *)&header, sizeof(header)); - - if (p->conf.debug > 10) { - log_error_write(srv, __FILE__, __LINE__, "soso", "tosend:", offset, "/", req_cq->bytes_in); - } - - chunkqueue_steal(hctx->wb, req_cq, weWant); - - offset += weWant; - } - } - - /* terminate STDIN */ - fcgi_header(&(header), FCGI_STDIN, request_id, 0, 0); - chunkqueue_append_mem(hctx->wb, (const char *)&header, sizeof(header)); + hctx->wb_reqlen += con->request.content_length;/* (eventual) (minimal) total request size, not necessarily including all fcgi_headers around content length yet */ + fcgi_stdin_append(srv, con, hctx, request_id); return 0; } @@ -2400,10 +2411,10 @@ range_success: ; typedef struct { buffer *b; - size_t len; + unsigned int len; int type; int padding; - size_t request_id; + int request_id; } fastcgi_response_packet; static int fastcgi_get_packet(server *srv, handler_ctx *hctx, fastcgi_response_packet *packet) { @@ -3028,11 +3039,23 @@ static handler_t fcgi_write_request(server *srv, handler_ctx *hctx) { } } - if (hctx->wb->bytes_out == hctx->wb->bytes_in) { + if (hctx->wb->bytes_out == hctx->wb_reqlen) { fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); fcgi_set_state(srv, hctx, FCGI_STATE_READ); } else { - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN|FDEVENT_OUT); + off_t wblen = hctx->wb->bytes_in - hctx->wb->bytes_out; + if (hctx->wb->bytes_in < hctx->wb_reqlen && wblen < 65536 - 16384) { + /*(con->conf.stream_request_body & FDEVENT_STREAM_REQUEST)*/ + if (!(con->conf.stream_request_body & FDEVENT_STREAM_REQUEST_POLLIN)) { + con->conf.stream_request_body |= FDEVENT_STREAM_REQUEST_POLLIN; + con->is_readable = 1; /* trigger optimistic read from client */ + } + } + if (0 == wblen) { + fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + } else { + fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN|FDEVENT_OUT); + } } return HANDLER_WAIT_FOR_EVENT; @@ -3151,12 +3174,29 @@ SUBREQUEST_FUNC(mod_fastcgi_handle_subrequest) { /* not my job */ if (con->mode != p->id) return HANDLER_GO_ON; - if (con->state == CON_STATE_READ_POST) { - handler_t r = connection_handle_read_post_state(srv, con); - if (r != HANDLER_GO_ON) return r; + if (0 == hctx->wb->bytes_in + ? con->state == CON_STATE_READ_POST + : hctx->wb->bytes_in < hctx->wb_reqlen) { + /*(64k - 4k to attempt to avoid temporary files + * in conjunction with FDEVENT_STREAM_REQUEST_BUFMIN)*/ + if (hctx->wb->bytes_in - hctx->wb->bytes_out > 65536 - 4096 + && (con->conf.stream_request_body & FDEVENT_STREAM_REQUEST_BUFMIN)){ + con->conf.stream_request_body &= ~FDEVENT_STREAM_REQUEST_POLLIN; + if (0 != hctx->wb->bytes_in) return HANDLER_WAIT_FOR_EVENT; + } else { + handler_t r = connection_handle_read_post_state(srv, con); + chunkqueue *req_cq = con->request_content_queue; + if (0 != hctx->wb->bytes_in && !chunkqueue_is_empty(req_cq)) { + fcgi_stdin_append(srv, con, hctx, hctx->request_id); + if (fdevent_event_get_interest(srv->ev, hctx->fd) & FDEVENT_OUT) { + return (r == HANDLER_GO_ON) ? HANDLER_WAIT_FOR_EVENT : r; + } + } + if (r != HANDLER_GO_ON) return r; + } } - return (hctx->state != FCGI_STATE_READ) + return (0 == hctx->wb->bytes_in || !chunkqueue_is_empty(hctx->wb)) ? fcgi_send_request(srv, hctx) : HANDLER_WAIT_FOR_EVENT; } @@ -3171,8 +3211,7 @@ static handler_t fcgi_handle_fdevent(server *srv, void *ctx, int revents) { joblist_append(srv, con); - if ((revents & FDEVENT_IN) && - hctx->state == FCGI_STATE_READ) { + if (revents & FDEVENT_IN) { switch (fcgi_demux_response(srv, hctx)) { case 0: break; @@ -3290,19 +3329,7 @@ static handler_t fcgi_handle_fdevent(server *srv, void *ctx, int revents) { } if (revents & FDEVENT_OUT) { - if (hctx->state == FCGI_STATE_CONNECT_DELAYED || - hctx->state == FCGI_STATE_WRITE) { - /* we are allowed to send something out - * - * 1. in an unfinished connect() call - * 2. in an unfinished write() call (long POST request) - */ - return fcgi_send_request(srv, hctx); /*(might invalidate hctx)*/ - } else { - log_error_write(srv, __FILE__, __LINE__, "sd", - "got a FDEVENT_OUT and didn't know why:", - hctx->state); - } + return fcgi_send_request(srv, hctx); /*(might invalidate hctx)*/ } /* perhaps this issue is already handled */ @@ -3318,7 +3345,8 @@ static handler_t fcgi_handle_fdevent(server *srv, void *ctx, int revents) { * */ fcgi_send_request(srv, hctx); - } else if (hctx->state == FCGI_STATE_READ && + } else if (chunkqueue_is_empty(hctx->wb) && + hctx->wb->bytes_in != 0 && hctx->proc->port == 0) { /* FIXME: * diff --git a/src/mod_proxy.c b/src/mod_proxy.c index 8f3403b6..08465b8e 100644 --- a/src/mod_proxy.c +++ b/src/mod_proxy.c @@ -97,6 +97,7 @@ typedef struct { buffer *response_header; chunkqueue *wb; + off_t wb_reqlen; int fd; /* fd to the proxy process */ int fde_ndx; /* index into the fd-event buffer */ @@ -124,6 +125,7 @@ static handler_ctx * handler_ctx_init(void) { hctx->response_header = buffer_init(); hctx->wb = chunkqueue_init(); + hctx->wb_reqlen = 0; hctx->fd = -1; hctx->fde_ndx = -1; @@ -502,6 +504,7 @@ static int proxy_create_env(server *srv, handler_ctx *hctx) { buffer_append_string_len(b, CONST_STR_LEN("Connection: close\r\n\r\n")); + hctx->wb_reqlen = buffer_string_length(b); chunkqueue_append_buffer(hctx->wb, b); buffer_free(b); @@ -510,7 +513,8 @@ static int proxy_create_env(server *srv, handler_ctx *hctx) { if (con->request.content_length) { chunkqueue *req_cq = con->request_content_queue; - chunkqueue_steal(hctx->wb, req_cq, req_cq->bytes_in); + chunkqueue_steal(hctx->wb, req_cq, req_cq->bytes_in); /*(0 == req_cq->bytes_out)*/ + hctx->wb_reqlen += con->request.content_length;/* (eventual) total request size */ } return 0; @@ -816,11 +820,23 @@ static handler_t proxy_write_request(server *srv, handler_ctx *hctx) { return HANDLER_ERROR; } - if (hctx->wb->bytes_out == hctx->wb->bytes_in) { + if (hctx->wb->bytes_out == hctx->wb_reqlen) { proxy_set_state(srv, hctx, PROXY_STATE_READ); fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); } else { - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN|FDEVENT_OUT); + off_t wblen = hctx->wb->bytes_in - hctx->wb->bytes_out; + if (hctx->wb->bytes_in < hctx->wb_reqlen && wblen < 65536 - 16384) { + /*(con->conf.stream_request_body & FDEVENT_STREAM_REQUEST)*/ + if (!(con->conf.stream_request_body & FDEVENT_STREAM_REQUEST_POLLIN)) { + con->conf.stream_request_body |= FDEVENT_STREAM_REQUEST_POLLIN; + con->is_readable = 1; /* trigger optimistic read from client */ + } + } + if (0 == wblen) { + fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + } else { + fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN|FDEVENT_OUT); + } } return HANDLER_WAIT_FOR_EVENT; @@ -905,12 +921,29 @@ SUBREQUEST_FUNC(mod_proxy_handle_subrequest) { /* not my job */ if (con->mode != p->id) return HANDLER_GO_ON; - if (con->state == CON_STATE_READ_POST) { - handler_t r = connection_handle_read_post_state(srv, con); - if (r != HANDLER_GO_ON) return r; + if (0 == hctx->wb->bytes_in + ? con->state == CON_STATE_READ_POST + : hctx->wb->bytes_in < hctx->wb_reqlen) { + /*(64k - 4k to attempt to avoid temporary files + * in conjunction with FDEVENT_STREAM_REQUEST_BUFMIN)*/ + if (hctx->wb->bytes_in - hctx->wb->bytes_out > 65536 - 4096 + && (con->conf.stream_request_body & FDEVENT_STREAM_REQUEST_BUFMIN)){ + con->conf.stream_request_body &= ~FDEVENT_STREAM_REQUEST_POLLIN; + if (0 != hctx->wb->bytes_in) return HANDLER_WAIT_FOR_EVENT; + } else { + handler_t r = connection_handle_read_post_state(srv, con); + chunkqueue *req_cq = con->request_content_queue; + if (0 != hctx->wb->bytes_in && !chunkqueue_is_empty(req_cq)) { + chunkqueue_steal(hctx->wb, req_cq, req_cq->bytes_in - req_cq->bytes_out); + if (fdevent_event_get_interest(srv->ev, hctx->fd) & FDEVENT_OUT) { + return (r == HANDLER_GO_ON) ? HANDLER_WAIT_FOR_EVENT : r; + } + } + if (r != HANDLER_GO_ON) return r; + } } - return (hctx->state != PROXY_STATE_READ) + return (0 == hctx->wb->bytes_in || !chunkqueue_is_empty(hctx->wb)) ? proxy_send_request(srv, hctx) : HANDLER_WAIT_FOR_EVENT; } @@ -922,8 +955,7 @@ static handler_t proxy_handle_fdevent(server *srv, void *ctx, int revents) { joblist_append(srv, con); - if ((revents & FDEVENT_IN) && - hctx->state == PROXY_STATE_READ) { + if (revents & FDEVENT_IN) { if (p->conf.debug) { log_error_write(srv, __FILE__, __LINE__, "sd", @@ -985,18 +1017,7 @@ static handler_t proxy_handle_fdevent(server *srv, void *ctx, int revents) { proxy_set_state(srv, hctx, PROXY_STATE_PREPARE_WRITE); } - if (hctx->state == PROXY_STATE_PREPARE_WRITE || - hctx->state == PROXY_STATE_WRITE) { - /* we are allowed to send something out - * - * 1. after a just finished connect() call - * 2. in a unfinished write() call (long POST request) - */ - return proxy_send_request(srv, hctx); /*(might invalidate hctx)*/ - } else { - log_error_write(srv, __FILE__, __LINE__, "sd", - "proxy: out", hctx->state); - } + return proxy_send_request(srv, hctx); /*(might invalidate hctx)*/ } /* perhaps this issue is already handled */ diff --git a/src/mod_scgi.c b/src/mod_scgi.c index 3e7cd10b..5bd8714d 100644 --- a/src/mod_scgi.c +++ b/src/mod_scgi.c @@ -304,9 +304,6 @@ typedef enum { FCGI_STATE_INIT, FCGI_STATE_CONNECT, FCGI_STATE_PREPARE_WRITE, typedef struct { buffer *response; - size_t response_len; - int response_type; - int response_padding; scgi_proc *proc; scgi_extension_host *host; @@ -314,20 +311,17 @@ typedef struct { scgi_connection_state_t state; time_t state_timestamp; - int reconnects; /* number of reconnect attempts */ - chunkqueue *wb; + off_t wb_reqlen; buffer *response_header; - int delayed; /* flag to mark that the connect() is delayed */ - - size_t request_id; int fd; /* fd to the scgi process */ int fde_ndx; /* index into the fd-event buffer */ pid_t pid; int got_proc; + int reconnects; /* number of reconnect attempts */ plugin_config conf; @@ -367,18 +361,15 @@ static handler_ctx * handler_ctx_init(void) { hctx->response = buffer_init(); hctx->response_header = buffer_init(); - hctx->request_id = 0; hctx->state = FCGI_STATE_INIT; hctx->proc = NULL; - hctx->response_len = 0; - hctx->response_type = 0; - hctx->response_padding = 0; hctx->fd = -1; hctx->reconnects = 0; hctx->wb = chunkqueue_init(); + hctx->wb_reqlen = 0; return hctx; } @@ -1371,7 +1362,6 @@ static int scgi_reconnect(server *srv, handler_ctx *hctx) { scgi_set_state(srv, hctx, FCGI_STATE_INIT); - hctx->request_id = 0; hctx->reconnects++; if (p->conf.debug) { @@ -1736,13 +1726,15 @@ static int scgi_create_env(server *srv, handler_ctx *hctx) { buffer_append_string_buffer(b, p->scgi_env); buffer_append_string_len(b, CONST_STR_LEN(",")); + hctx->wb_reqlen = buffer_string_length(b); chunkqueue_append_buffer(hctx->wb, b); buffer_free(b); if (con->request.content_length) { chunkqueue *req_cq = con->request_content_queue; - chunkqueue_steal(hctx->wb, req_cq, req_cq->bytes_in); + chunkqueue_steal(hctx->wb, req_cq, req_cq->bytes_in); /*(0 == req_cq->bytes_out)*/ + hctx->wb_reqlen += con->request.content_length;/* (eventual) total request size */ } return 0; @@ -2426,11 +2418,23 @@ static handler_t scgi_write_request(server *srv, handler_ctx *hctx) { } } - if (hctx->wb->bytes_out == hctx->wb->bytes_in) { + if (hctx->wb->bytes_out == hctx->wb_reqlen) { fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); scgi_set_state(srv, hctx, FCGI_STATE_READ); } else { - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN|FDEVENT_OUT); + off_t wblen = hctx->wb->bytes_in - hctx->wb->bytes_out; + if (hctx->wb->bytes_in < hctx->wb_reqlen && wblen < 65536 - 16384) { + /*(con->conf.stream_request_body & FDEVENT_STREAM_REQUEST)*/ + if (!(con->conf.stream_request_body & FDEVENT_STREAM_REQUEST_POLLIN)) { + con->conf.stream_request_body |= FDEVENT_STREAM_REQUEST_POLLIN; + con->is_readable = 1; /* trigger optimistic read from client */ + } + } + if (0 == wblen) { + fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + } else { + fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN|FDEVENT_OUT); + } } return HANDLER_WAIT_FOR_EVENT; @@ -2525,12 +2529,29 @@ SUBREQUEST_FUNC(mod_scgi_handle_subrequest) { /* not my job */ if (con->mode != p->id) return HANDLER_GO_ON; - if (con->state == CON_STATE_READ_POST) { - handler_t r = connection_handle_read_post_state(srv, con); - if (r != HANDLER_GO_ON) return r; + if (0 == hctx->wb->bytes_in + ? con->state == CON_STATE_READ_POST + : hctx->wb->bytes_in < hctx->wb_reqlen) { + /*(64k - 4k to attempt to avoid temporary files + * in conjunction with FDEVENT_STREAM_REQUEST_BUFMIN)*/ + if (hctx->wb->bytes_in - hctx->wb->bytes_out > 65536 - 4096 + && (con->conf.stream_request_body & FDEVENT_STREAM_REQUEST_BUFMIN)){ + con->conf.stream_request_body &= ~FDEVENT_STREAM_REQUEST_POLLIN; + if (0 != hctx->wb->bytes_in) return HANDLER_WAIT_FOR_EVENT; + } else { + handler_t r = connection_handle_read_post_state(srv, con); + chunkqueue *req_cq = con->request_content_queue; + if (0 != hctx->wb->bytes_in && !chunkqueue_is_empty(req_cq)) { + chunkqueue_steal(hctx->wb, req_cq, req_cq->bytes_in - req_cq->bytes_out); + if (fdevent_event_get_interest(srv->ev, hctx->fd) & FDEVENT_OUT) { + return (r == HANDLER_GO_ON) ? HANDLER_WAIT_FOR_EVENT : r; + } + } + if (r != HANDLER_GO_ON) return r; + } } - return (hctx->state != FCGI_STATE_READ) + return (0 == hctx->wb->bytes_in || !chunkqueue_is_empty(hctx->wb)) ? scgi_send_request(srv, hctx) : HANDLER_WAIT_FOR_EVENT; } @@ -2546,8 +2567,7 @@ static handler_t scgi_handle_fdevent(server *srv, void *ctx, int revents) { joblist_append(srv, con); - if ((revents & FDEVENT_IN) && - hctx->state == FCGI_STATE_READ) { + if (revents & FDEVENT_IN) { switch (scgi_demux_response(srv, hctx)) { case 0: break; @@ -2643,19 +2663,7 @@ static handler_t scgi_handle_fdevent(server *srv, void *ctx, int revents) { } if (revents & FDEVENT_OUT) { - if (hctx->state == FCGI_STATE_CONNECT || - hctx->state == FCGI_STATE_WRITE) { - /* we are allowed to send something out - * - * 1. in a unfinished connect() call - * 2. in a unfinished write() call (long POST request) - */ - return scgi_send_request(srv, hctx); /*(might invalidate hctx)*/ - } else { - log_error_write(srv, __FILE__, __LINE__, "sd", - "got a FDEVENT_OUT and didn't know why:", - hctx->state); - } + return scgi_send_request(srv, hctx); /*(might invalidate hctx)*/ } /* perhaps this issue is already handled */ @@ -2671,13 +2679,6 @@ static handler_t scgi_handle_fdevent(server *srv, void *ctx, int revents) { * */ scgi_send_request(srv, hctx); - } else if (hctx->state == FCGI_STATE_READ && - hctx->proc->port == 0) { - /* FIXME: - * - * ioctl says 8192 bytes to read from PHP and we receive directly a HUP for the socket - * even if the FCGI_FIN packet is not received yet - */ } else { log_error_write(srv, __FILE__, __LINE__, "sbSBSDSd", "error: unexpected close of scgi connection for", @@ -2827,6 +2828,18 @@ static handler_t scgi_check_extension(server *srv, connection *con, void *p_d, i /* a note about no handler is not sent yet */ extension->note_is_sent = 0; + /* SCGI requires that Content-Length be set. + * Send 411 Length Required if Content-Length missing. + * (Alternatively, collect full request body before proceeding + * in mod_scgi_handle_subrequest()) */ + if (0 == con->request.content_length + && array_get_element(con->request.headers, "Transfer-Encoding")) { + con->keep_alive = 0; + con->http_status = 411; /* Length Required */ + con->mode = DIRECT; + return HANDLER_FINISHED; + } + /* * if check-local is disabled, use the uri.path handler * diff --git a/src/mod_webdav.c b/src/mod_webdav.c index 94bd85f6..1b51a71f 100644 --- a/src/mod_webdav.c +++ b/src/mod_webdav.c @@ -2736,6 +2736,7 @@ PHYSICALPATH_FUNC(mod_webdav_physical_handler) { case HTTP_METHOD_DELETE: case HTTP_METHOD_LOCK: case HTTP_METHOD_UNLOCK: + con->conf.stream_request_body = 0; con->mode = p->id; break; default: From ddfae019cb5ec54636607ac45ac56c7e27a0f16b Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Fri, 10 Jun 2016 23:34:55 -0400 Subject: [PATCH 06/11] separate routines for reading output from backends move code in dynamic handlers (CGI, FastCGI, SCGI, proxy) from *_handle_fdevent() to *_recv_response() for reuse outside the *_handle_fdevent() routine --- src/mod_cgi.c | 22 +++++++++++------ src/mod_fastcgi.c | 25 ++++++++++++++++---- src/mod_proxy.c | 60 ++++++++++++++++++++++++++++++----------------- src/mod_scgi.c | 39 +++++++++++++++++++++--------- 4 files changed, 102 insertions(+), 44 deletions(-) diff --git a/src/mod_cgi.c b/src/mod_cgi.c index 8f630fa5..1764dd61 100644 --- a/src/mod_cgi.c +++ b/src/mod_cgi.c @@ -719,13 +719,7 @@ static handler_t cgi_handle_fdevent_send (server *srv, void *ctx, int revents) { } -static handler_t cgi_handle_fdevent(server *srv, void *ctx, int revents) { - handler_ctx *hctx = ctx; - connection *con = hctx->remote_conn; - - joblist_append(srv, con); - - if (revents & FDEVENT_IN) { +static int cgi_recv_response(server *srv, handler_ctx *hctx) { switch (cgi_demux_response(srv, hctx)) { case FDEVENT_HANDLED_NOT_FINISHED: break; @@ -745,6 +739,20 @@ static handler_t cgi_handle_fdevent(server *srv, void *ctx, int revents) { cgi_connection_close(srv, hctx); return HANDLER_FINISHED; } + + return HANDLER_GO_ON; +} + + +static handler_t cgi_handle_fdevent(server *srv, void *ctx, int revents) { + handler_ctx *hctx = ctx; + connection *con = hctx->remote_conn; + + joblist_append(srv, con); + + if (revents & FDEVENT_IN) { + handler_t rc = cgi_recv_response(srv, hctx);/*(might invalidate hctx)*/ + if (rc != HANDLER_GO_ON) return rc; /*(unless HANDLER_GO_ON)*/ } /* perhaps this issue is already handled */ diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c index 874e357b..5d53ac63 100644 --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -3164,6 +3164,9 @@ static handler_t fcgi_send_request(server *srv, handler_ctx *hctx) { } +static handler_t fcgi_recv_response(server *srv, handler_ctx *hctx); + + SUBREQUEST_FUNC(mod_fastcgi_handle_subrequest) { plugin_data *p = p_d; @@ -3201,17 +3204,14 @@ SUBREQUEST_FUNC(mod_fastcgi_handle_subrequest) { : HANDLER_WAIT_FOR_EVENT; } -static handler_t fcgi_handle_fdevent(server *srv, void *ctx, int revents) { - handler_ctx *hctx = ctx; + +static handler_t fcgi_recv_response(server *srv, handler_ctx *hctx) { connection *con = hctx->remote_conn; plugin_data *p = hctx->plugin_data; fcgi_proc *proc = hctx->proc; fcgi_extension_host *host= hctx->host; - joblist_append(srv, con); - - if (revents & FDEVENT_IN) { switch (fcgi_demux_response(srv, hctx)) { case 0: break; @@ -3326,6 +3326,20 @@ static handler_t fcgi_handle_fdevent(server *srv, void *ctx, int revents) { return HANDLER_FINISHED; } + + return HANDLER_GO_ON; +} + + +static handler_t fcgi_handle_fdevent(server *srv, void *ctx, int revents) { + handler_ctx *hctx = ctx; + connection *con = hctx->remote_conn; + + joblist_append(srv, con); + + if (revents & FDEVENT_IN) { + handler_t rc = fcgi_recv_response(srv, hctx);/*(might invalidate hctx)*/ + if (rc != HANDLER_GO_ON) return rc; /*(unless HANDLER_GO_ON)*/ } if (revents & FDEVENT_OUT) { @@ -3354,6 +3368,7 @@ static handler_t fcgi_handle_fdevent(server *srv, void *ctx, int revents) { * even if the FCGI_FIN packet is not received yet */ } else { + fcgi_proc *proc = hctx->proc; log_error_write(srv, __FILE__, __LINE__, "sBSbsbsd", "error: unexpected close of fastcgi connection for", con->uri.path, "?", con->uri.query, diff --git a/src/mod_proxy.c b/src/mod_proxy.c index 08465b8e..9a4982ec 100644 --- a/src/mod_proxy.c +++ b/src/mod_proxy.c @@ -911,6 +911,10 @@ static handler_t proxy_send_request(server *srv, handler_ctx *hctx) { } } + +static handler_t proxy_recv_response(server *srv, handler_ctx *hctx); + + SUBREQUEST_FUNC(mod_proxy_handle_subrequest) { plugin_data *p = p_d; @@ -948,6 +952,38 @@ SUBREQUEST_FUNC(mod_proxy_handle_subrequest) { : HANDLER_WAIT_FOR_EVENT; } + +static handler_t proxy_recv_response(server *srv, handler_ctx *hctx) { + + switch (proxy_demux_response(srv, hctx)) { + case 0: + break; + case 1: + /* we are done */ + proxy_connection_close(srv, hctx); + + return HANDLER_FINISHED; + case -1: { + connection *con = hctx->remote_conn; + if (con->file_started == 0) { + /* reading response headers failed */ + } else { + /* response might have been already started, kill the connection */ + con->keep_alive = 0; + con->file_finished = 1; + con->mode = DIRECT; /*(avoid sending final chunked block)*/ + } + + proxy_connection_close(srv, hctx); + + return HANDLER_FINISHED; + } + } + + return HANDLER_GO_ON; +} + + static handler_t proxy_handle_fdevent(server *srv, void *ctx, int revents) { handler_ctx *hctx = ctx; connection *con = hctx->remote_conn; @@ -962,27 +998,9 @@ static handler_t proxy_handle_fdevent(server *srv, void *ctx, int revents) { "proxy: fdevent-in", hctx->state); } - switch (proxy_demux_response(srv, hctx)) { - case 0: - break; - case 1: - /* we are done */ - proxy_connection_close(srv, hctx); - - return HANDLER_FINISHED; - case -1: - if (con->file_started == 0) { - /* reading response headers failed */ - } else { - /* response might have been already started, kill the connection */ - con->keep_alive = 0; - con->file_finished = 1; - con->mode = DIRECT; /*(avoid sending final chunked block)*/ - } - - proxy_connection_close(srv, hctx); - - return HANDLER_FINISHED; + { + handler_t rc = proxy_recv_response(srv,hctx);/*(might invalidate hctx)*/ + if (rc != HANDLER_GO_ON) return rc; /*(unless HANDLER_GO_ON)*/ } } diff --git a/src/mod_scgi.c b/src/mod_scgi.c index 5bd8714d..0c674224 100644 --- a/src/mod_scgi.c +++ b/src/mod_scgi.c @@ -2519,6 +2519,10 @@ static handler_t scgi_send_request(server *srv, handler_ctx *hctx) { } } + +static handler_t scgi_recv_response(server *srv, handler_ctx *hctx); + + SUBREQUEST_FUNC(mod_scgi_handle_subrequest) { plugin_data *p = p_d; @@ -2557,17 +2561,8 @@ SUBREQUEST_FUNC(mod_scgi_handle_subrequest) { } -static handler_t scgi_handle_fdevent(server *srv, void *ctx, int revents) { - handler_ctx *hctx = ctx; - connection *con = hctx->remote_conn; - plugin_data *p = hctx->plugin_data; +static handler_t scgi_recv_response(server *srv, handler_ctx *hctx) { - scgi_proc *proc = hctx->proc; - scgi_extension_host *host= hctx->host; - - joblist_append(srv, con); - - if (revents & FDEVENT_IN) { switch (scgi_demux_response(srv, hctx)) { case 0: break; @@ -2576,7 +2571,13 @@ static handler_t scgi_handle_fdevent(server *srv, void *ctx, int revents) { scgi_connection_close(srv, hctx); return HANDLER_FINISHED; - case -1: + case -1: { + connection *con = hctx->remote_conn; + plugin_data *p = hctx->plugin_data; + + scgi_proc *proc = hctx->proc; + scgi_extension_host *host= hctx->host; + if (proc->pid && proc->state != PROC_STATE_DIED) { int status; @@ -2660,6 +2661,21 @@ static handler_t scgi_handle_fdevent(server *srv, void *ctx, int revents) { return HANDLER_FINISHED; } + } + + return HANDLER_GO_ON; +} + + +static handler_t scgi_handle_fdevent(server *srv, void *ctx, int revents) { + handler_ctx *hctx = ctx; + connection *con = hctx->remote_conn; + + joblist_append(srv, con); + + if (revents & FDEVENT_IN) { + handler_t rc = scgi_recv_response(srv, hctx);/*(might invalidate hctx)*/ + if (rc != HANDLER_GO_ON) return rc; /*(unless HANDLER_GO_ON)*/ } if (revents & FDEVENT_OUT) { @@ -2680,6 +2696,7 @@ static handler_t scgi_handle_fdevent(server *srv, void *ctx, int revents) { */ scgi_send_request(srv, hctx); } else { + scgi_extension_host *host= hctx->host; log_error_write(srv, __FILE__, __LINE__, "sbSBSDSd", "error: unexpected close of scgi connection for", con->uri.path, From 18a7b2be37041987c5bde264d03a7ee7440ae788 Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Sat, 11 Jun 2016 11:04:01 -0400 Subject: [PATCH 07/11] [core] option to stream response body to client (fixes #949, #760, #1283, #1387) Set server.stream-response-body = 1 or server.stream-response-body = 2 to have lighttpd stream response body to client as it arrives from the backend (CGI, FastCGI, SCGI, proxy). default: buffer entire response body before sending response to client. (This preserves existing behavior for now, but may in the future be changed to stream response to client, which is the behavior more commonly expected.) x-ref: "fastcgi, cgi, flush, php5 problem." https://redmine.lighttpd.net/issues/949 "Random crashing on FreeBSD 6.1" https://redmine.lighttpd.net/issues/760 "Memory usage increases when proxy+ssl+large file" https://redmine.lighttpd.net/issues/1283 "lighttpd+fastcgi memory problem" https://redmine.lighttpd.net/issues/1387 --- src/fdevent.c | 24 ++++++++++++++++++++ src/fdevent.h | 2 ++ src/http_chunk.c | 7 +++++- src/mod_cgi.c | 32 ++++++++++++++++++++++++++ src/mod_fastcgi.c | 58 +++++++++++++++++++++++++++++++++++++++++++---- src/mod_proxy.c | 52 ++++++++++++++++++++++++++++++++++++++---- src/mod_scgi.c | 39 ++++++++++++++++++++++++++++--- 7 files changed, 201 insertions(+), 13 deletions(-) diff --git a/src/fdevent.c b/src/fdevent.c index 82b71ce6..1e249fbd 100644 --- a/src/fdevent.c +++ b/src/fdevent.c @@ -171,6 +171,30 @@ void fdevent_event_set(fdevents *ev, int *fde_ndx, int fd, int events) { ev->fdarray[fd]->events = events; } +void fdevent_event_add(fdevents *ev, int *fde_ndx, int fd, int event) { + int events; + if (-1 == fd) return; + + events = ev->fdarray[fd]->events; + if ((events & event) || 0 == event) return; /*(no change; nothing to do)*/ + + events |= event; + if (ev->event_set) *fde_ndx = ev->event_set(ev, *fde_ndx, fd, events); + ev->fdarray[fd]->events = events; +} + +void fdevent_event_clr(fdevents *ev, int *fde_ndx, int fd, int event) { + int events; + if (-1 == fd) return; + + events = ev->fdarray[fd]->events; + if (!(events & event)) return; /*(no change; nothing to do)*/ + + events &= ~event; + if (ev->event_set) *fde_ndx = ev->event_set(ev, *fde_ndx, fd, events); + ev->fdarray[fd]->events = events; +} + int fdevent_poll(fdevents *ev, int timeout_ms) { if (ev->poll == NULL) SEGFAULT(); return ev->poll(ev, timeout_ms); diff --git a/src/fdevent.h b/src/fdevent.h index 27a46aaa..361fcb63 100644 --- a/src/fdevent.h +++ b/src/fdevent.h @@ -183,6 +183,8 @@ void fdevent_free(fdevents *ev); #define fdevent_event_get_interest(ev, fd) \ (-1 != (fd) ? (ev)->fdarray[(fd)]->events : 0) void fdevent_event_set(fdevents *ev, int *fde_ndx, int fd, int events); /* events can be FDEVENT_IN, FDEVENT_OUT or FDEVENT_IN | FDEVENT_OUT */ +void fdevent_event_add(fdevents *ev, int *fde_ndx, int fd, int event); /* events can be FDEVENT_IN or FDEVENT_OUT */ +void fdevent_event_clr(fdevents *ev, int *fde_ndx, int fd, int event); /* events can be FDEVENT_IN or FDEVENT_OUT */ void fdevent_event_del(fdevents *ev, int *fde_ndx, int fd); int fdevent_event_get_revent(fdevents *ev, size_t ndx); int fdevent_event_get_fd(fdevents *ev, size_t ndx); diff --git a/src/http_chunk.c b/src/http_chunk.c index d895181f..34d2038f 100644 --- a/src/http_chunk.c +++ b/src/http_chunk.c @@ -133,8 +133,13 @@ static int http_chunk_append_data(server *srv, connection *con, buffer *b, const * file, so not checking if users of this interface have appended large * (references to) files to chunkqueue, which would not be in memory */ + /*(allow slightly larger mem use if FDEVENT_STREAM_RESPONSE_BUFMIN + * to reduce creation of temp files when backend producer will be + * blocked until more data is sent to network to client)*/ + if ((c && c->type == FILE_CHUNK && c->file.is_temp) - || cq->bytes_in - cq->bytes_out + len > 64 * 1024) { + || cq->bytes_in - cq->bytes_out + len + > 1024 * ((con->conf.stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN) ? 128 : 64)) { return http_chunk_append_to_tempfile(srv, con, b ? b->ptr : mem, len); } diff --git a/src/mod_cgi.c b/src/mod_cgi.c index 1764dd61..456e6f87 100644 --- a/src/mod_cgi.c +++ b/src/mod_cgi.c @@ -400,6 +400,7 @@ static int cgi_demux_response(server *srv, handler_ctx *hctx) { if (-1 == (n = read(hctx->fd, hctx->response->ptr, hctx->response->size - 1))) { if (errno == EAGAIN || errno == EINTR) { /* would block, wait for signal */ + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); return FDEVENT_HANDLED_NOT_FINISHED; } /* error */ @@ -550,11 +551,31 @@ static int cgi_demux_response(server *srv, handler_ctx *hctx) { } con->file_started = 1; + } else { + /*(reuse MAX_HTTP_REQUEST_HEADER as max size for response headers from backends)*/ + if (header_len > MAX_HTTP_REQUEST_HEADER) { + log_error_write(srv, __FILE__, __LINE__, "sb", "response headers too large for", con->uri.path); + con->http_status = 502; /* Bad Gateway */ + con->mode = DIRECT; + return FDEVENT_HANDLED_FINISHED; + } } } else { if (0 != http_chunk_append_buffer(srv, con, hctx->response)) { return FDEVENT_HANDLED_ERROR; } + if ((con->conf.stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN) + && chunkqueue_length(con->write_queue) > 65536 - 4096) { + if (!con->is_writable) { + /*(defer removal of FDEVENT_IN interest since + * connection_state_machine() might be able to send data + * immediately, unless !con->is_writable, where + * connection_state_machine() might not loop back to call + * mod_cgi_handle_subrequest())*/ + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + } + break; + } } #if 0 @@ -1510,6 +1531,17 @@ SUBREQUEST_FUNC(mod_cgi_handle_subrequest) { if (con->mode != p->id) return HANDLER_GO_ON; if (NULL == hctx) return HANDLER_GO_ON; + if ((con->conf.stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN) + && con->file_started) { + if (chunkqueue_length(con->write_queue) > 65536 - 4096) { + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + } else if (!(fdevent_event_get_interest(srv->ev, hctx->fd) & FDEVENT_IN)) { + /* optimistic read from backend, which might re-enable FDEVENT_IN */ + handler_t rc = cgi_recv_response(srv, hctx); /*(might invalidate hctx)*/ + if (rc != HANDLER_GO_ON) return rc; /*(unless HANDLER_GO_ON)*/ + } + } + if (cq->bytes_in != (off_t)con->request.content_length) { /*(64k - 4k to attempt to avoid temporary files * in conjunction with FDEVENT_STREAM_REQUEST_BUFMIN)*/ diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c index 5d53ac63..48efd37a 100644 --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -2511,7 +2511,10 @@ static int fcgi_demux_response(server *srv, handler_ctx *hctx) { * check how much we have to read */ if (ioctl(hctx->fd, FIONREAD, &toread)) { - if (errno == EAGAIN) return 0; + if (errno == EAGAIN) { + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + return 0; + } log_error_write(srv, __FILE__, __LINE__, "sd", "unexpected end-of-file (perhaps the fastcgi process died):", fcgi_fd); @@ -2522,12 +2525,26 @@ static int fcgi_demux_response(server *srv, handler_ctx *hctx) { char *mem; size_t mem_len; + if ((con->conf.stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN)) { + off_t cqlen = chunkqueue_length(hctx->rb); + if (cqlen + toread > 65536 + (int)sizeof(FCGI_Header)) { /*(max size of FastCGI packet + 1)*/ + if (cqlen < 65536 + (int)sizeof(FCGI_Header)) { + toread = 65536 + (int)sizeof(FCGI_Header) - cqlen; + } else { /* should not happen */ + toread = toread < 1024 ? toread : 1024; + } + } + } + chunkqueue_get_memory(hctx->rb, &mem, &mem_len, 0, toread); r = read(hctx->fd, mem, mem_len); chunkqueue_use_memory(hctx->rb, r > 0 ? r : 0); if (-1 == r) { - if (errno == EAGAIN) return 0; + if (errno == EAGAIN) { + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + return 0; + } log_error_write(srv, __FILE__, __LINE__, "sds", "unexpected end-of-file (perhaps the fastcgi process died):", fcgi_fd, strerror(errno)); @@ -2586,6 +2603,13 @@ static int fcgi_demux_response(server *srv, handler_ctx *hctx) { buffer_string_set_length(hctx->response_header, hlen); } else { /* no luck, no header found */ + /*(reuse MAX_HTTP_REQUEST_HEADER as max size for response headers from backends)*/ + if (buffer_string_length(hctx->response_header) > MAX_HTTP_REQUEST_HEADER) { + log_error_write(srv, __FILE__, __LINE__, "sb", "response headers too large for", con->uri.path); + con->http_status = 502; /* Bad Gateway */ + con->mode = DIRECT; + fin = 1; + } break; } @@ -2630,6 +2654,18 @@ static int fcgi_demux_response(server *srv, handler_ctx *hctx) { /* error writing to tempfile; * truncate response or send 500 if nothing sent yet */ fin = 1; + break; + } + if ((con->conf.stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN) + && chunkqueue_length(con->write_queue) > 65536 - 4096) { + if (!con->is_writable) { + /*(defer removal of FDEVENT_IN interest since + * connection_state_machine() might be able to send data + * immediately, unless !con->is_writable, where + * connection_state_machine() might not loop back to call + * mod_fastcgi_handle_subrequest())*/ + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + } } } break; @@ -3009,6 +3045,7 @@ static handler_t fcgi_write_request(server *srv, handler_ctx *hctx) { if (-1 == fcgi_create_env(srv, hctx, hctx->request_id)) return HANDLER_ERROR; + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); fcgi_set_state(srv, hctx, FCGI_STATE_WRITE); /* fall through */ case FCGI_STATE_WRITE: @@ -3040,7 +3077,7 @@ static handler_t fcgi_write_request(server *srv, handler_ctx *hctx) { } if (hctx->wb->bytes_out == hctx->wb_reqlen) { - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_OUT); fcgi_set_state(srv, hctx, FCGI_STATE_READ); } else { off_t wblen = hctx->wb->bytes_in - hctx->wb->bytes_out; @@ -3052,9 +3089,9 @@ static handler_t fcgi_write_request(server *srv, handler_ctx *hctx) { } } if (0 == wblen) { - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_OUT); } else { - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN|FDEVENT_OUT); + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_OUT); } } @@ -3177,6 +3214,17 @@ SUBREQUEST_FUNC(mod_fastcgi_handle_subrequest) { /* not my job */ if (con->mode != p->id) return HANDLER_GO_ON; + if ((con->conf.stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN) + && con->file_started) { + if (chunkqueue_length(con->write_queue) > 65536 - 4096) { + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + } else if (!(fdevent_event_get_interest(srv->ev, hctx->fd) & FDEVENT_IN)) { + /* optimistic read from backend, which might re-enable FDEVENT_IN */ + handler_t rc = fcgi_recv_response(srv, hctx); /*(might invalidate hctx)*/ + if (rc != HANDLER_GO_ON) return rc; /*(unless HANDLER_GO_ON)*/ + } + } + if (0 == hctx->wb->bytes_in ? con->state == CON_STATE_READ_POST : hctx->wb->bytes_in < hctx->wb_reqlen) { diff --git a/src/mod_proxy.c b/src/mod_proxy.c index 9a4982ec..823bb5b2 100644 --- a/src/mod_proxy.c +++ b/src/mod_proxy.c @@ -631,6 +631,10 @@ static int proxy_demux_response(server *srv, handler_ctx *hctx) { /* check how much we have to read */ if (ioctl(hctx->fd, FIONREAD, &b)) { + if (errno == EAGAIN) { + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + return 0; + } log_error_write(srv, __FILE__, __LINE__, "sd", "ioctl failed: ", proxy_fd); @@ -644,10 +648,29 @@ static int proxy_demux_response(server *srv, handler_ctx *hctx) { } if (b > 0) { + if ((con->conf.stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN)) { + off_t cqlen = chunkqueue_length(con->write_queue); + if (cqlen + b > 65536 - 4096) { + if (!con->is_writable) { + /*(defer removal of FDEVENT_IN interest since + * connection_state_machine() might be able to send data + * immediately, unless !con->is_writable, where + * connection_state_machine() might not loop back to call + * mod_proxy_handle_subrequest())*/ + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + } + if (cqlen >= 65536-1) return 0; + b = 65536 - 1 - (int)cqlen; + } + } + buffer_string_prepare_append(hctx->response, b); if (-1 == (r = read(hctx->fd, hctx->response->ptr + buffer_string_length(hctx->response), buffer_string_space(hctx->response)))) { - if (errno == EAGAIN) return 0; + if (errno == EAGAIN) { + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + return 0; + } log_error_write(srv, __FILE__, __LINE__, "sds", "unexpected end-of-file (perhaps the proxy process died):", proxy_fd, strerror(errno)); @@ -701,6 +724,15 @@ static int proxy_demux_response(server *srv, handler_ctx *hctx) { } } buffer_reset(hctx->response); + } else { + /* no luck, no header found */ + /*(reuse MAX_HTTP_REQUEST_HEADER as max size for response headers from backends)*/ + if (buffer_string_length(hctx->response) > MAX_HTTP_REQUEST_HEADER) { + log_error_write(srv, __FILE__, __LINE__, "sb", "response headers too large for", con->uri.path); + con->http_status = 502; /* Bad Gateway */ + con->mode = DIRECT; + fin = 1; + } } } else { if (0 != http_chunk_append_buffer(srv, con, hctx->response)) { @@ -802,6 +834,7 @@ static handler_t proxy_write_request(server *srv, handler_ctx *hctx) { case PROXY_STATE_PREPARE_WRITE: proxy_create_env(srv, hctx); + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); proxy_set_state(srv, hctx, PROXY_STATE_WRITE); /* fall through */ @@ -821,8 +854,8 @@ static handler_t proxy_write_request(server *srv, handler_ctx *hctx) { } if (hctx->wb->bytes_out == hctx->wb_reqlen) { + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_OUT); proxy_set_state(srv, hctx, PROXY_STATE_READ); - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); } else { off_t wblen = hctx->wb->bytes_in - hctx->wb->bytes_out; if (hctx->wb->bytes_in < hctx->wb_reqlen && wblen < 65536 - 16384) { @@ -833,9 +866,9 @@ static handler_t proxy_write_request(server *srv, handler_ctx *hctx) { } } if (0 == wblen) { - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_OUT); } else { - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN|FDEVENT_OUT); + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_OUT); } } @@ -925,6 +958,17 @@ SUBREQUEST_FUNC(mod_proxy_handle_subrequest) { /* not my job */ if (con->mode != p->id) return HANDLER_GO_ON; + if ((con->conf.stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN) + && con->file_started) { + if (chunkqueue_length(con->write_queue) > 65536 - 4096) { + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + } else if (!(fdevent_event_get_interest(srv->ev, hctx->fd) & FDEVENT_IN)) { + /* optimistic read from backend, which might re-enable FDEVENT_IN */ + handler_t rc = proxy_recv_response(srv, hctx); /*(might invalidate hctx)*/ + if (rc != HANDLER_GO_ON) return rc; /*(unless HANDLER_GO_ON)*/ + } + } + if (0 == hctx->wb->bytes_in ? con->state == CON_STATE_READ_POST : hctx->wb->bytes_in < hctx->wb_reqlen) { diff --git a/src/mod_scgi.c b/src/mod_scgi.c index 0c674224..72057f8c 100644 --- a/src/mod_scgi.c +++ b/src/mod_scgi.c @@ -1859,6 +1859,7 @@ static int scgi_demux_response(server *srv, handler_ctx *hctx) { if (-1 == (n = read(hctx->fd, hctx->response->ptr, hctx->response->size - 1))) { if (errno == EAGAIN || errno == EINTR) { /* would block, wait for signal */ + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); return 0; } /* error */ @@ -1986,6 +1987,14 @@ static int scgi_demux_response(server *srv, handler_ctx *hctx) { } con->file_started = 1; + } else { + /*(reuse MAX_HTTP_REQUEST_HEADER as max size for response headers from backends)*/ + if (buffer_string_length(hctx->response_header) > MAX_HTTP_REQUEST_HEADER) { + log_error_write(srv, __FILE__, __LINE__, "sb", "response headers too large for", con->uri.path); + con->http_status = 502; /* Bad Gateway */ + con->mode = DIRECT; + return 1; + } } } else { if (0 != http_chunk_append_buffer(srv, con, hctx->response)) { @@ -1993,6 +2002,18 @@ static int scgi_demux_response(server *srv, handler_ctx *hctx) { * truncate response or send 500 if nothing sent yet */ return 1; } + if ((con->conf.stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN) + && chunkqueue_length(con->write_queue) > 65536 - 4096) { + if (!con->is_writable) { + /*(defer removal of FDEVENT_IN interest since + * connection_state_machine() might be able to send data + * immediately, unless !con->is_writable, where + * connection_state_machine() might not loop back to call + * mod_scgi_handle_subrequest())*/ + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + } + break; + } } #if 0 @@ -2370,6 +2391,7 @@ static handler_t scgi_write_request(server *srv, handler_ctx *hctx) { case FCGI_STATE_PREPARE_WRITE: scgi_create_env(srv, hctx); + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); scgi_set_state(srv, hctx, FCGI_STATE_WRITE); /* fall through */ @@ -2419,7 +2441,7 @@ static handler_t scgi_write_request(server *srv, handler_ctx *hctx) { } if (hctx->wb->bytes_out == hctx->wb_reqlen) { - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_OUT); scgi_set_state(srv, hctx, FCGI_STATE_READ); } else { off_t wblen = hctx->wb->bytes_in - hctx->wb->bytes_out; @@ -2431,9 +2453,9 @@ static handler_t scgi_write_request(server *srv, handler_ctx *hctx) { } } if (0 == wblen) { - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_OUT); } else { - fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN|FDEVENT_OUT); + fdevent_event_add(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_OUT); } } @@ -2533,6 +2555,17 @@ SUBREQUEST_FUNC(mod_scgi_handle_subrequest) { /* not my job */ if (con->mode != p->id) return HANDLER_GO_ON; + if ((con->conf.stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN) + && con->file_started) { + if (chunkqueue_length(con->write_queue) > 65536 - 4096) { + fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN); + } else if (!(fdevent_event_get_interest(srv->ev, hctx->fd) & FDEVENT_IN)) { + /* optimistic read from backend, which might re-enable FDEVENT_IN */ + handler_t rc = scgi_recv_response(srv, hctx); /*(might invalidate hctx)*/ + if (rc != HANDLER_GO_ON) return rc; /*(unless HANDLER_GO_ON)*/ + } + } + if (0 == hctx->wb->bytes_in ? con->state == CON_STATE_READ_POST : hctx->wb->bytes_in < hctx->wb_reqlen) { From 923688d2da036f3cefc4fb494dcd770acaab1691 Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Tue, 14 Jun 2016 00:48:28 -0400 Subject: [PATCH 08/11] drain backend socket/pipe bufs upon FDEVENT_HUP (mod_cgi, mod_fastcgi, mod_scgi, mod_proxy) --- src/mod_cgi.c | 23 ++++++++++++++++------- src/mod_fastcgi.c | 19 +++++++++++-------- src/mod_proxy.c | 12 ++++++++++++ src/mod_scgi.c | 12 ++++++++++++ 4 files changed, 51 insertions(+), 15 deletions(-) diff --git a/src/mod_cgi.c b/src/mod_cgi.c index 456e6f87..4b1235d1 100644 --- a/src/mod_cgi.c +++ b/src/mod_cgi.c @@ -778,20 +778,29 @@ static handler_t cgi_handle_fdevent(server *srv, void *ctx, int revents) { /* perhaps this issue is already handled */ if (revents & FDEVENT_HUP) { - /* check if we still have a unfinished header package which is a body in reality */ - if (con->file_started == 0 && !buffer_string_is_empty(hctx->response_header)) { + if (con->file_started) { + /* drain any remaining data from kernel pipe buffers + * even if (con->conf.stream_response_body + * & FDEVENT_STREAM_RESPONSE_BUFMIN) + * since event loop will spin on fd FDEVENT_HUP event + * until unregistered. */ + handler_t rc; + do { + rc = cgi_recv_response(srv,hctx);/*(might invalidate hctx)*/ + } while (rc == HANDLER_GO_ON); /*(unless HANDLER_GO_ON)*/ + return rc; /* HANDLER_FINISHED or HANDLER_ERROR */ + } else if (!buffer_string_is_empty(hctx->response_header)) { + /* unfinished header package which is a body in reality */ con->file_started = 1; if (0 != http_chunk_append_buffer(srv, con, hctx->response_header)) { cgi_connection_close(srv, hctx); return HANDLER_ERROR; } - } - + } else { # if 0 - log_error_write(srv, __FILE__, __LINE__, "sddd", "got HUP from cgi", con->fd, hctx->fd, revents); + log_error_write(srv, __FILE__, __LINE__, "sddd", "got HUP from cgi", con->fd, hctx->fd, revents); # endif - - /* rtsigs didn't liked the close */ + } cgi_connection_close(srv, hctx); } else if (revents & FDEVENT_ERR) { /* kill all connections to the cgi process */ diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c index 48efd37a..8a132587 100644 --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -3407,14 +3407,17 @@ static handler_t fcgi_handle_fdevent(server *srv, void *ctx, int revents) { * */ fcgi_send_request(srv, hctx); - } else if (chunkqueue_is_empty(hctx->wb) && - hctx->wb->bytes_in != 0 && - hctx->proc->port == 0) { - /* FIXME: - * - * ioctl says 8192 bytes to read from PHP and we receive directly a HUP for the socket - * even if the FCGI_FIN packet is not received yet - */ + } else if (con->file_started) { + /* drain any remaining data from kernel pipe buffers + * even if (con->conf.stream_response_body + * & FDEVENT_STREAM_RESPONSE_BUFMIN) + * since event loop will spin on fd FDEVENT_HUP event + * until unregistered. */ + handler_t rc; + do { + rc = fcgi_recv_response(srv,hctx);/*(might invalidate hctx)*/ + } while (rc == HANDLER_GO_ON); /*(unless HANDLER_GO_ON)*/ + return rc; /* HANDLER_FINISHED or HANDLER_ERROR */ } else { fcgi_proc *proc = hctx->proc; log_error_write(srv, __FILE__, __LINE__, "sBSbsbsd", diff --git a/src/mod_proxy.c b/src/mod_proxy.c index 823bb5b2..90003689 100644 --- a/src/mod_proxy.c +++ b/src/mod_proxy.c @@ -855,6 +855,7 @@ static handler_t proxy_write_request(server *srv, handler_ctx *hctx) { if (hctx->wb->bytes_out == hctx->wb_reqlen) { fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_OUT); + shutdown(hctx->fd, SHUT_WR);/* future: remove if HTTP/1.1 request */ proxy_set_state(srv, hctx, PROXY_STATE_READ); } else { off_t wblen = hctx->wb->bytes_in - hctx->wb->bytes_out; @@ -1117,6 +1118,17 @@ static handler_t proxy_handle_fdevent(server *srv, void *ctx, int revents) { proxy_connection_close(srv, hctx); con->http_status = 503; } + } else if (con->file_started) { + /* drain any remaining data from kernel pipe buffers + * even if (con->conf.stream_response_body + * & FDEVENT_STREAM_RESPONSE_BUFMIN) + * since event loop will spin on fd FDEVENT_HUP event + * until unregistered. */ + handler_t rc; + do { + rc = proxy_recv_response(srv,hctx);/*(might invalidate hctx)*/ + } while (rc == HANDLER_GO_ON); /*(unless HANDLER_GO_ON)*/ + return rc; /* HANDLER_FINISHED or HANDLER_ERROR */ } else { proxy_connection_close(srv, hctx); } diff --git a/src/mod_scgi.c b/src/mod_scgi.c index 72057f8c..00653bf4 100644 --- a/src/mod_scgi.c +++ b/src/mod_scgi.c @@ -2442,6 +2442,7 @@ static handler_t scgi_write_request(server *srv, handler_ctx *hctx) { if (hctx->wb->bytes_out == hctx->wb_reqlen) { fdevent_event_clr(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_OUT); + shutdown(hctx->fd, SHUT_WR); scgi_set_state(srv, hctx, FCGI_STATE_READ); } else { off_t wblen = hctx->wb->bytes_in - hctx->wb->bytes_out; @@ -2728,6 +2729,17 @@ static handler_t scgi_handle_fdevent(server *srv, void *ctx, int revents) { * */ scgi_send_request(srv, hctx); + } else if (con->file_started) { + /* drain any remaining data from kernel pipe buffers + * even if (con->conf.stream_response_body + * & FDEVENT_STREAM_RESPONSE_BUFMIN) + * since event loop will spin on fd FDEVENT_HUP event + * until unregistered. */ + handler_t rc; + do { + rc = scgi_recv_response(srv,hctx);/*(might invalidate hctx)*/ + } while (rc == HANDLER_GO_ON); /*(unless HANDLER_GO_ON)*/ + return rc; /* HANDLER_FINISHED or HANDLER_ERROR */ } else { scgi_extension_host *host= hctx->host; log_error_write(srv, __FILE__, __LINE__, "sbSBSDSd", From 4ef4baa59d5459ee5568dc3ca5db4b2df40f426a Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Sat, 18 Jun 2016 20:39:00 -0400 Subject: [PATCH 09/11] http_response_backend_error() consolidate repeated code in dynamic handlers which manipulates con->file_finished. Centralize calls to http_chunk_close(). (mod_cgi, mod_fastcgi, mod_scgi, mod_proxy) --- src/connections.c | 6 ++++-- src/http-header-glue.c | 11 +++++++++++ src/mod_cgi.c | 8 +------- src/mod_fastcgi.c | 26 +++----------------------- src/mod_flv_streaming.c | 1 - src/mod_proxy.c | 29 ++++------------------------- src/mod_scgi.c | 22 +++------------------- src/response.h | 1 + 8 files changed, 27 insertions(+), 77 deletions(-) diff --git a/src/connections.c b/src/connections.c index e1414179..27e6838c 100644 --- a/src/connections.c +++ b/src/connections.c @@ -169,6 +169,7 @@ static void connection_handle_errdoc_init(server *srv, connection *con) { } } + con->response.transfer_encoding = 0; buffer_reset(con->physical.path); array_reset(con->response.headers); chunkqueue_reset(con->write_queue); @@ -291,7 +292,6 @@ static int connection_handle_write_prepare(server *srv, connection *con) { http_chunk_append_buffer(srv, con, b); buffer_free(b); - http_chunk_close(srv, con); response_header_overwrite(srv, con, CONST_STR_LEN("Content-Type"), CONST_STR_LEN("text/html")); } @@ -1031,7 +1031,9 @@ int connection_state_machine(server *srv, connection *con) { switch (r = http_response_prepare(srv, con)) { case HANDLER_WAIT_FOR_EVENT: - if (!con->file_started || 0 == con->conf.stream_response_body) break; /* come back here */ + if (!con->file_finished && (!con->file_started || 0 == con->conf.stream_response_body)) { + break; /* come back here */ + } /* response headers received from backend; fall through to start response */ case HANDLER_FINISHED: if (con->error_handler_saved_status > 0) { diff --git a/src/http-header-glue.c b/src/http-header-glue.c index f72fcf9a..c98c5888 100644 --- a/src/http-header-glue.c +++ b/src/http-header-glue.c @@ -727,6 +727,17 @@ void http_response_xsendfile (server *srv, connection *con, buffer *path, const } } +void http_response_backend_error (server *srv, connection *con) { + UNUSED(srv); + if (con->file_started) { + /*(response might have been already started, kill the connection)*/ + /*(mode == DIRECT to avoid later call to http_response_backend_done())*/ + con->mode = DIRECT; /*(avoid sending final chunked block)*/ + con->keep_alive = 0; /*(no keep-alive; final chunked block not sent)*/ + con->file_finished = 1; + } /*(else error status set later by http_response_backend_done())*/ +} + void http_response_backend_done (server *srv, connection *con) { /* (not CON_STATE_ERROR and not CON_STATE_RESPONSE_END, * i.e. not called from handle_connection_close or connection_reset diff --git a/src/mod_cgi.c b/src/mod_cgi.c index 4b1235d1..dfbc822b 100644 --- a/src/mod_cgi.c +++ b/src/mod_cgi.c @@ -410,12 +410,6 @@ static int cgi_demux_response(server *srv, handler_ctx *hctx) { if (n == 0) { /* read finished */ - - con->file_finished = 1; - - /* send final chunk */ - http_chunk_close(srv, con); - return FDEVENT_HANDLED_FINISHED; } @@ -1589,7 +1583,7 @@ SUBREQUEST_FUNC(mod_cgi_handle_subrequest) { } /* if not done, wait for CGI to close stdout, so we read EOF on pipe */ - return con->file_finished ? HANDLER_FINISHED : HANDLER_WAIT_FOR_EVENT; + return HANDLER_WAIT_FOR_EVENT; } diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c index 8a132587..f61ca98e 100644 --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -1863,7 +1863,6 @@ static connection_result_t fcgi_establish_connection(server *srv, handler_ctx *h #define FCGI_ENV_ADD_CHECK(ret, con) \ if (ret == -1) { \ con->http_status = 400; \ - con->file_finished = 1; \ return -1; \ }; static int fcgi_env_add_request_headers(server *srv, connection *con, plugin_data *p) { @@ -2677,15 +2676,6 @@ static int fcgi_demux_response(server *srv, handler_ctx *hctx) { break; case FCGI_END_REQUEST: - con->file_finished = 1; - - if (host->mode != FCGI_AUTHORIZER || - !(con->http_status == 0 || - con->http_status == 200)) { - /* send chunk-end if necessary */ - http_chunk_close(srv, con); - } - fin = 1; break; default: @@ -3357,21 +3347,15 @@ static handler_t fcgi_recv_response(server *srv, handler_ctx *hctx) { "response not received, request sent:", hctx->wb->bytes_out, "on socket:", proc->connection_name, "for", con->uri.path, "?", con->uri.query, ", closing connection"); - - fcgi_connection_close(srv, hctx); } else { - /* response might have been already started, kill the connection */ log_error_write(srv, __FILE__, __LINE__, "ssbsBSBs", "response already sent out, but backend returned error", "on socket:", proc->connection_name, "for", con->uri.path, "?", con->uri.query, ", terminating connection"); - - con->keep_alive = 0; - con->file_finished = 1; - con->mode = DIRECT; /*(avoid sending final chunked block)*/ - fcgi_connection_close(srv, hctx); } + http_response_backend_error(srv, con); + fcgi_connection_close(srv, hctx); return HANDLER_FINISHED; } @@ -3432,11 +3416,7 @@ static handler_t fcgi_handle_fdevent(server *srv, void *ctx, int revents) { log_error_write(srv, __FILE__, __LINE__, "s", "fcgi: got a FDEVENT_ERR. Don't know why."); - if (con->file_started) { - con->keep_alive = 0; - con->file_finished = 1; - con->mode = DIRECT; /*(avoid sending final chunked block)*/ - } + http_response_backend_error(srv, con); fcgi_connection_close(srv, hctx); } diff --git a/src/mod_flv_streaming.c b/src/mod_flv_streaming.c index 02eae375..c5a6847d 100644 --- a/src/mod_flv_streaming.c +++ b/src/mod_flv_streaming.c @@ -240,7 +240,6 @@ URIHANDLER_FUNC(mod_flv_streaming_path_handler) { chunkqueue_reset(con->write_queue); return HANDLER_GO_ON; } - http_chunk_close(srv, con); response_header_overwrite(srv, con, CONST_STR_LEN("Content-Type"), CONST_STR_LEN("video/x-flv")); diff --git a/src/mod_proxy.c b/src/mod_proxy.c index 90003689..5cb0ac2a 100644 --- a/src/mod_proxy.c +++ b/src/mod_proxy.c @@ -742,13 +742,8 @@ static int proxy_demux_response(server *srv, handler_ctx *hctx) { } buffer_reset(hctx->response); } - } else { /* reading from upstream done */ - con->file_finished = 1; - - http_chunk_close(srv, con); - fin = 1; } @@ -1003,26 +998,14 @@ static handler_t proxy_recv_response(server *srv, handler_ctx *hctx) { switch (proxy_demux_response(srv, hctx)) { case 0: break; + case -1: + http_response_backend_error(srv, hctx->remote_conn); + /* fall through */ case 1: /* we are done */ proxy_connection_close(srv, hctx); return HANDLER_FINISHED; - case -1: { - connection *con = hctx->remote_conn; - if (con->file_started == 0) { - /* reading response headers failed */ - } else { - /* response might have been already started, kill the connection */ - con->keep_alive = 0; - con->file_finished = 1; - con->mode = DIRECT; /*(avoid sending final chunked block)*/ - } - - proxy_connection_close(srv, hctx); - - return HANDLER_FINISHED; - } } return HANDLER_GO_ON; @@ -1135,11 +1118,7 @@ static handler_t proxy_handle_fdevent(server *srv, void *ctx, int revents) { } else if (revents & FDEVENT_ERR) { log_error_write(srv, __FILE__, __LINE__, "sd", "proxy-FDEVENT_ERR, but no HUP", revents); - if (con->file_started) { - con->keep_alive = 0; - con->file_finished = 1; - con->mode = DIRECT; /*(avoid sending final chunked block)*/ - } + http_response_backend_error(srv, con); proxy_connection_close(srv, hctx); } diff --git a/src/mod_scgi.c b/src/mod_scgi.c index 00653bf4..80607f3c 100644 --- a/src/mod_scgi.c +++ b/src/mod_scgi.c @@ -1869,12 +1869,6 @@ static int scgi_demux_response(server *srv, handler_ctx *hctx) { if (n == 0) { /* read finished */ - - con->file_finished = 1; - - /* send final chunk */ - http_chunk_close(srv, con); - return 1; } @@ -2678,21 +2672,15 @@ static handler_t scgi_recv_response(server *srv, handler_ctx *hctx) { "response not sent, request sent:", hctx->wb->bytes_out, "connection-fd:", con->fd, "fcgi-fd:", hctx->fd); - - scgi_connection_close(srv, hctx); } else { - /* response might have been already started, kill the connection */ log_error_write(srv, __FILE__, __LINE__, "ssdsd", "response already sent out, termination connection", "connection-fd:", con->fd, "fcgi-fd:", hctx->fd); - - con->keep_alive = 0; - con->file_finished = 1; - con->mode = DIRECT; /*(avoid sending final chunked block)*/ - scgi_connection_close(srv, hctx); } + http_response_backend_error(srv, con); + scgi_connection_close(srv, hctx); return HANDLER_FINISHED; } } @@ -2758,11 +2746,7 @@ static handler_t scgi_handle_fdevent(server *srv, void *ctx, int revents) { log_error_write(srv, __FILE__, __LINE__, "s", "fcgi: got a FDEVENT_ERR. Don't know why."); - if (con->file_started) { - con->keep_alive = 0; - con->file_finished = 1; - con->mode = DIRECT; /*(avoid sending final chunked block)*/ - } + http_response_backend_error(srv, con); scgi_connection_close(srv, hctx); } diff --git a/src/response.h b/src/response.h index bf88d2bb..3302f3e2 100644 --- a/src/response.h +++ b/src/response.h @@ -19,6 +19,7 @@ int http_response_handle_cachable(server *srv, connection *con, buffer * mtime); void http_response_send_file (server *srv, connection *con, buffer *path); void http_response_xsendfile (server *srv, connection *con, buffer *path, const array *xdocroot); void http_response_backend_done (server *srv, connection *con); +void http_response_backend_error (server *srv, connection *con); buffer * strftime_cache_get(server *srv, time_t last_mod); #endif From bfac0285a7c9ceb658a25b7a9b86d832d146c746 Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Sun, 19 Jun 2016 19:32:46 -0400 Subject: [PATCH 10/11] remove excess calls to joblist_append() (recent commits streamlined dynamic handler flow control) --- src/connections-glue.c | 4 +--- src/connections.c | 3 --- src/mod_fastcgi.c | 12 +++++++----- src/network.c | 4 ---- src/server.c | 1 - 5 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/connections-glue.c b/src/connections-glue.c index 3f4b4738..c242718d 100644 --- a/src/connections-glue.c +++ b/src/connections-glue.c @@ -212,10 +212,8 @@ static int connection_handle_read_ssl(server *srv, connection *con) { return -2; } else { - joblist_append(srv, con); + return 0; } - - return 0; #else UNUSED(srv); UNUSED(con); diff --git a/src/connections.c b/src/connections.c index 27e6838c..c2b5351e 100644 --- a/src/connections.c +++ b/src/connections.c @@ -383,18 +383,15 @@ static int connection_handle_write(server *srv, connection *con) { con->write_request_ts = srv->cur_ts; if (con->file_finished) { connection_set_state(srv, con, CON_STATE_RESPONSE_END); - joblist_append(srv, con); } break; case -1: /* error on our side */ log_error_write(srv, __FILE__, __LINE__, "sd", "connection closed: write failed on fd", con->fd); connection_set_state(srv, con, CON_STATE_ERROR); - joblist_append(srv, con); break; case -2: /* remote close */ connection_set_state(srv, con, CON_STATE_ERROR); - joblist_append(srv, con); break; case 1: con->write_request_ts = srv->cur_ts; diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c index f61ca98e..5a9b983f 100644 --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -2383,9 +2383,6 @@ range_success: ; if (have_sendfile2) { data_string *dcls; - hctx->send_content_body = 0; - joblist_append(srv, con); - /* fix content-length */ if (NULL == (dcls = (data_string *)array_get_unused_element(con->response.headers, TYPE_STRING))) { dcls = data_response_init(); @@ -2397,6 +2394,7 @@ range_success: ; con->parsed_response |= HTTP_CONTENT_LENGTH; con->response.content_length = sendfile2_content_length; + return 200; } /* CGI/1.1 rev 03 - 7.2.1.2 */ @@ -2614,9 +2612,13 @@ static int fcgi_demux_response(server *srv, handler_ctx *hctx) { /* parse the response header */ if ((ret = fcgi_response_parse(srv, con, p, hctx->response_header))) { - con->http_status = ret; - hctx->send_content_body = 0; + if (200 != ret) { /*(200 returned for X-Sendfile2 handled)*/ + con->http_status = ret; + con->mode = DIRECT; + } con->file_started = 1; + hctx->send_content_body = 0; + fin = 1; break; } diff --git a/src/network.c b/src/network.c index 18b8b47e..489c4cae 100644 --- a/src/network.c +++ b/src/network.c @@ -1070,9 +1070,7 @@ int network_write_chunkqueue(server *srv, connection *con, chunkqueue *cq, off_t off_t limit = con->conf.global_kbytes_per_second * 1024 - *(con->conf.global_bytes_per_second_cnt_ptr); if (limit <= 0) { /* we reached the global traffic limit */ - con->traffic_limit_reached = 1; - joblist_append(srv, con); return 1; } else { @@ -1084,9 +1082,7 @@ int network_write_chunkqueue(server *srv, connection *con, chunkqueue *cq, off_t off_t limit = con->conf.kbytes_per_second * 1024 - con->bytes_written_cur_second; if (limit <= 0) { /* we reached the traffic limit */ - con->traffic_limit_reached = 1; - joblist_append(srv, con); return 1; } else { diff --git a/src/server.c b/src/server.c index d0c14f99..22261d56 100644 --- a/src/server.c +++ b/src/server.c @@ -1781,7 +1781,6 @@ int main (int argc, char **argv) { handler = fdevent_get_handler(srv->ev, fd); context = fdevent_get_context(srv->ev, fd); - /* connection_handle_fdevent needs a joblist_append */ #if 0 log_error_write(srv, __FILE__, __LINE__, "sdd", "event for", fd, revents); From 5863cb57522e46cde5a35784f4ce83459a8561f4 Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Sun, 19 Jun 2016 21:05:25 -0400 Subject: [PATCH 11/11] defer choosing "Transfer-Encoding: chunked" defer choosing "Transfer-Encoding: chunked" until response header is about to be written --- src/connections.c | 16 +++++++++++++++- src/mod_cgi.c | 11 ----------- src/mod_fastcgi.c | 6 ------ src/mod_proxy.c | 6 ------ src/mod_scgi.c | 11 ----------- 5 files changed, 15 insertions(+), 35 deletions(-) diff --git a/src/connections.c b/src/connections.c index c2b5351e..b542d19f 100644 --- a/src/connections.c +++ b/src/connections.c @@ -341,7 +341,21 @@ static int connection_handle_write_prepare(server *srv, connection *con) { if (((con->parsed_response & HTTP_CONTENT_LENGTH) == 0) && ((con->response.transfer_encoding & HTTP_TRANSFER_ENCODING_CHUNKED) == 0)) { - con->keep_alive = 0; + if (con->request.http_version == HTTP_VERSION_1_1) { + off_t qlen = chunkqueue_length(con->write_queue); + con->response.transfer_encoding = HTTP_TRANSFER_ENCODING_CHUNKED; + if (qlen) { + /* create initial Transfer-Encoding: chunked segment */ + buffer *b = srv->tmp_chunk_len; + buffer_string_set_length(b, 0); + buffer_append_uint_hex(b, (uintmax_t)qlen); + buffer_append_string_len(b, CONST_STR_LEN("\r\n")); + chunkqueue_prepend_buffer(con->write_queue, b); + chunkqueue_append_mem(con->write_queue, CONST_STR_LEN("\r\n")); + } + } else { + con->keep_alive = 0; + } } /** diff --git a/src/mod_cgi.c b/src/mod_cgi.c index dfbc822b..883444dc 100644 --- a/src/mod_cgi.c +++ b/src/mod_cgi.c @@ -490,11 +490,6 @@ static int cgi_demux_response(server *srv, handler_ctx *hctx) { if (is_header_end) { if (!is_header) { /* no header, but a body */ - - if (con->request.http_version == HTTP_VERSION_1_1) { - con->response.transfer_encoding = HTTP_TRANSFER_ENCODING_CHUNKED; - } - if (0 != http_chunk_append_buffer(srv, con, hctx->response_header)) { return FDEVENT_HANDLED_ERROR; } @@ -531,12 +526,6 @@ static int cgi_demux_response(server *srv, handler_ctx *hctx) { } } - /* enable chunked-transfer-encoding */ - if (con->request.http_version == HTTP_VERSION_1_1 && - !(con->parsed_response & HTTP_CONTENT_LENGTH)) { - con->response.transfer_encoding = HTTP_TRANSFER_ENCODING_CHUNKED; - } - if (blen > 0) { if (0 != http_chunk_append_mem(srv, con, bstart, blen)) { return FDEVENT_HANDLED_ERROR; diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c index 5a9b983f..62cc367e 100644 --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -2642,12 +2642,6 @@ static int fcgi_demux_response(server *srv, handler_ctx *hctx) { hctx->send_content_body = 0; /* ignore the content */ break; } - - /* enable chunked-transfer-encoding */ - if (con->request.http_version == HTTP_VERSION_1_1 && - !(con->parsed_response & HTTP_CONTENT_LENGTH)) { - con->response.transfer_encoding = HTTP_TRANSFER_ENCODING_CHUNKED; - } } if (hctx->send_content_body && !buffer_string_is_empty(packet.b)) { diff --git a/src/mod_proxy.c b/src/mod_proxy.c index 5cb0ac2a..f8717b77 100644 --- a/src/mod_proxy.c +++ b/src/mod_proxy.c @@ -708,12 +708,6 @@ static int proxy_demux_response(server *srv, handler_ctx *hctx) { /* parse the response header */ proxy_response_parse(srv, con, p, hctx->response_header); - /* enable chunked-transfer-encoding */ - if (con->request.http_version == HTTP_VERSION_1_1 && - !(con->parsed_response & HTTP_CONTENT_LENGTH)) { - con->response.transfer_encoding = HTTP_TRANSFER_ENCODING_CHUNKED; - } - con->file_started = 1; if (blen > 0) { if (0 != http_chunk_append_mem(srv, con, c + 4, blen)) { diff --git a/src/mod_scgi.c b/src/mod_scgi.c index 80607f3c..39e5e4a6 100644 --- a/src/mod_scgi.c +++ b/src/mod_scgi.c @@ -1938,11 +1938,6 @@ static int scgi_demux_response(server *srv, handler_ctx *hctx) { if (header_end) { if (c == NULL) { /* no header, but a body */ - - if (con->request.http_version == HTTP_VERSION_1_1) { - con->response.transfer_encoding = HTTP_TRANSFER_ENCODING_CHUNKED; - } - if (0 != http_chunk_append_buffer(srv, con, hctx->response_header)) { /* error writing to tempfile; * truncate response or send 500 if nothing sent yet */ @@ -1965,12 +1960,6 @@ static int scgi_demux_response(server *srv, handler_ctx *hctx) { } } - /* enable chunked-transfer-encoding */ - if (con->request.http_version == HTTP_VERSION_1_1 && - !(con->parsed_response & HTTP_CONTENT_LENGTH)) { - con->response.transfer_encoding = HTTP_TRANSFER_ENCODING_CHUNKED; - } - if (blen > 0) { if (0 != http_chunk_append_mem(srv, con, hctx->response_header->ptr + hlen, blen)) { /* error writing to tempfile;