From c263bc6a119faa52a2e2ffd65bb986213fd5280e Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Thu, 7 Apr 2016 19:47:49 -0400 Subject: [PATCH] defer reading request body until handle subrequest (fixes #2541) read request body right before calling subrequest handler, allowing request to be handled prior to reading request body, e.g. to send 401 Unauthorized response when authentication is required (In the future, this might move into each dynamic handler which supports request body (mod_cgi, mod_fastcgi, mod_proxy, mod_scgi, mod_webdav) so that each dynamic handler can choose whether or not to buffer request body or to stream request body to backend as request body is received.) keep-alive is disabled if request body has not been completely read prior to sending response x-ref: "HTTP 401 Unauthorized only sent back after full POST request is read" https://redmine.lighttpd.net/issues/2541 --- src/connections.c | 110 +++++++++++++++++++++++++++++----------------- src/connections.h | 2 +- src/mod_cgi.c | 3 +- src/mod_fastcgi.c | 3 +- src/mod_proxy.c | 3 +- src/mod_scgi.c | 3 +- src/response.c | 5 +++ 7 files changed, 84 insertions(+), 45 deletions(-) diff --git a/src/connections.c b/src/connections.c index fd6cd07a..7d777eec 100644 --- a/src/connections.c +++ b/src/connections.c @@ -118,7 +118,7 @@ static int connection_del(server *srv, connection *con) { return 0; } -int connection_close(server *srv, connection *con) { +static int connection_close(server *srv, connection *con) { #ifdef USE_OPENSSL server_socket *srv_sock = con->srv_socket; #endif @@ -415,6 +415,63 @@ static int connection_handle_read(server *srv, connection *con) { return 0; } +handler_t connection_handle_read_post_state(server *srv, connection *con) { + chunkqueue *cq = con->read_queue; + chunkqueue *dst_cq = con->request_content_queue; + + int is_closed = 0; + + if (con->is_readable) { + con->read_idle_ts = srv->cur_ts; + + switch(connection_handle_read(srv, con)) { + case -1: + return HANDLER_ERROR; + case -2: + is_closed = 1; + break; + default: + break; + } + } + + chunkqueue_remove_finished_chunks(cq); + + if (con->request.content_length <= 64*1024) { + /* don't buffer request bodies <= 64k on disk */ + chunkqueue_steal(dst_cq, cq, (off_t)con->request.content_length - dst_cq->bytes_in); + } + else if (0 != chunkqueue_steal_with_tempfiles(srv, dst_cq, cq, (off_t)con->request.content_length - dst_cq->bytes_in)) { + /* writing to temp file failed */ + con->http_status = 500; /* Internal Server Error */ + con->keep_alive = 0; + con->mode = DIRECT; + chunkqueue_reset(con->write_queue); + + return HANDLER_FINISHED; + } + + chunkqueue_remove_finished_chunks(cq); + + if (dst_cq->bytes_in == (off_t)con->request.content_length) { + /* Content is ready */ + connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST); + return HANDLER_GO_ON; + } else if (is_closed) { + #if 0 + con->http_status = 400; /* Bad Request */ + con->keep_alive = 0; + con->mode = DIRECT; + chunkqueue_reset(con->write_queue); + + return HANDLER_FINISHED; + #endif + return HANDLER_ERROR; + } else { + return HANDLER_WAIT_FOR_EVENT; + } +} + static void connection_handle_errdoc_init(connection *con) { /* reset caching response headers potentially added by mod_expire */ data_string *ds; @@ -609,6 +666,12 @@ 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 @@ -899,11 +962,9 @@ int connection_reset(server *srv, connection *con) { * we get called by the state-engine and by the fdevent-handler */ static int connection_handle_read_state(server *srv, connection *con) { - connection_state_t ostate = con->state; chunk *c, *last_chunk; off_t last_offset; chunkqueue *cq = con->read_queue; - chunkqueue *dst_cq = con->request_content_queue; int is_closed = 0; /* the connection got closed, if we don't have a complete header, -> error */ /* when in CON_STATE_READ: about to receive first byte for a request: */ int is_request_start = chunkqueue_is_empty(cq); @@ -927,8 +988,6 @@ static int connection_handle_read_state(server *srv, connection *con) { /* we might have got several packets at once */ - switch(ostate) { - case CON_STATE_READ: /* update request_start timestamp when first byte of * next request is received on a keep-alive connection */ if (con->request_count > 1 && is_request_start) con->request_start = srv->cur_ts; @@ -1009,33 +1068,11 @@ found_header_end: con->http_status = 414; /* Request-URI too large */ con->keep_alive = 0; connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST); + } else if (is_closed) { + /* the connection got closed and we didn't got enough data to leave CON_STATE_READ; + * the only way is to leave here */ + connection_set_state(srv, con, CON_STATE_ERROR); } - break; - case CON_STATE_READ_POST: - if (con->request.content_length <= 64*1024) { - /* don't buffer request bodies <= 64k on disk */ - chunkqueue_steal(dst_cq, cq, con->request.content_length - dst_cq->bytes_in); - } - else if (0 != chunkqueue_steal_with_tempfiles(srv, dst_cq, cq, con->request.content_length - dst_cq->bytes_in )) { - con->http_status = 413; /* Request-Entity too large */ - con->keep_alive = 0; - connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST); - } - - /* Content is ready */ - if (dst_cq->bytes_in == (off_t)con->request.content_length) { - connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST); - } - - break; - default: break; - } - - /* the connection got closed and we didn't got enough data to leave one of the READ states - * the only way is to leave here */ - if (is_closed && ostate == con->state) { - connection_set_state(srv, con, CON_STATE_ERROR); - } chunkqueue_remove_finished_chunks(cq); @@ -1101,8 +1138,7 @@ static handler_t connection_handle_fdevent(server *srv, void *context, int reven } } - if (con->state == CON_STATE_READ || - con->state == CON_STATE_READ_POST) { + if (con->state == CON_STATE_READ) { connection_handle_read_state(srv, con); } @@ -1285,6 +1321,7 @@ int connection_state_machine(server *srv, connection *con) { connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST); break; + case CON_STATE_READ_POST: case CON_STATE_HANDLE_REQUEST: /* * the request is parsed @@ -1325,8 +1362,6 @@ int connection_state_machine(server *srv, connection *con) { con->in_error_handler = 1; - connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST); - done = -1; break; } else if (con->in_error_handler) { @@ -1349,16 +1384,12 @@ int connection_state_machine(server *srv, connection *con) { fdwaitqueue_append(srv, con); - connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST); - break; case HANDLER_COMEBACK: done = -1; /* fallthrough */ case HANDLER_WAIT_FOR_EVENT: /* come back here */ - connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST); - break; case HANDLER_ERROR: /* something went wrong */ @@ -1494,7 +1525,6 @@ int connection_state_machine(server *srv, connection *con) { } break; - case CON_STATE_READ_POST: case CON_STATE_READ: if (srv->srvconf.log_state_handling) { log_error_write(srv, __FILE__, __LINE__, "sds", diff --git a/src/connections.h b/src/connections.h index f58346d7..daf8eb51 100644 --- a/src/connections.h +++ b/src/connections.h @@ -10,11 +10,11 @@ int connection_reset(server *srv, connection *con); void connections_free(server *srv); connection * connection_accept(server *srv, server_socket *srv_sock); -int connection_close(server *srv, connection *con); int connection_set_state(server *srv, connection *con, connection_state_t state); const char * connection_get_state(connection_state_t state); const char * connection_get_short_state(connection_state_t state); int connection_state_machine(server *srv, connection *con); +handler_t connection_handle_read_post_state(server *srv, connection *con); #endif diff --git a/src/mod_cgi.c b/src/mod_cgi.c index 088de085..3c6b449a 100644 --- a/src/mod_cgi.c +++ b/src/mod_cgi.c @@ -606,7 +606,8 @@ 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) { + 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()) */ diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c index c7cdede0..a87152e9 100644 --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -1542,7 +1542,8 @@ static void fcgi_connection_close(server *srv, handler_ctx *hctx) { con->plugin_ctx[p->id] = NULL; /* finish response (if not already finished) */ - if (con->mode == p->id && con->state == CON_STATE_HANDLE_REQUEST) { + 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()) */ diff --git a/src/mod_proxy.c b/src/mod_proxy.c index ee83d7c8..0bc98eed 100644 --- a/src/mod_proxy.c +++ b/src/mod_proxy.c @@ -352,7 +352,8 @@ static void proxy_connection_close(server *srv, handler_ctx *hctx) { con->plugin_ctx[p->id] = NULL; /* finish response (if not already finished) */ - if (con->mode == p->id && con->state == CON_STATE_HANDLE_REQUEST) { + 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()) */ diff --git a/src/mod_scgi.c b/src/mod_scgi.c index a48c36f9..9c81de2d 100644 --- a/src/mod_scgi.c +++ b/src/mod_scgi.c @@ -1286,7 +1286,8 @@ static void scgi_connection_close(server *srv, handler_ctx *hctx) { con->plugin_ctx[p->id] = NULL; /* finish response (if not already finished) */ - if (con->mode == p->id && con->state == CON_STATE_HANDLE_REQUEST) { + 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()) */ diff --git a/src/response.c b/src/response.c index 3763f26b..bf7b676c 100644 --- a/src/response.c +++ b/src/response.c @@ -750,6 +750,11 @@ handler_t http_response_prepare(server *srv, connection *con) { } + if (con->state == CON_STATE_READ_POST) { + r = connection_handle_read_post_state(srv, con); + if (r != HANDLER_GO_ON) return r; + } + switch(r = plugins_call_handle_subrequest(srv, con)) { case HANDLER_GO_ON: /* request was not handled, looks like we are done */