From 3f4f934485181665720227ce61b0b3508b568f5d Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Mon, 20 Jan 2020 21:05:56 -0500 Subject: [PATCH] [core] http_response_comeback() group HANDLER_COMEBACK logic in http_response_comeback() and call it from places that reset state in order to (sometimes partially) reprocess a request. This includes error handler (server.error-handler), r->handler_module when cgi.local-redir, and looping in http_response_prepare() when modules make changes to the request and return HANDLER_COMEBACK (e.g. mod_rewrite, mod_magnet, mod_cml) Also, set r->conditional_is_valid closer to where elements are set (and become valid for use in condition checks), and parse target in http_request_parse() instead of http_response_prepare() --- src/configfile-glue.c | 1 - src/connections.c | 22 ++++++++++----- src/mod_cgi.c | 20 ++++++++++--- src/request.c | 19 +++++-------- src/response.c | 65 ++++++++++++++++++++++++++++++------------- src/response.h | 4 +++ 6 files changed, 87 insertions(+), 44 deletions(-) diff --git a/src/configfile-glue.c b/src/configfile-glue.c index 88dd3ea7..454fab3f 100644 --- a/src/configfile-glue.c +++ b/src/configfile-glue.c @@ -680,7 +680,6 @@ void config_cond_cache_reset_item(request_st * const r, comp_key_t item) { * reset the config cache to its initial state at connection start */ void config_cond_cache_reset(request_st * const r) { - r->conditional_is_valid = 0; /* resetting all entries; no need to follow children as in config_cond_cache_reset_item */ /* static_assert(0 == COND_RESULT_UNSET); */ const uint32_t used = config_reference.used; diff --git a/src/connections.c b/src/connections.c index 2072d500..16e94aca 100644 --- a/src/connections.c +++ b/src/connections.c @@ -841,7 +841,17 @@ static int connection_handle_read_state(connection * const con) { } r->http_status = http_request_parse(r, hdrs, hoff, con->proto_default_port); - if (0 != r->http_status) { + if (0 == r->http_status) { + r->conditional_is_valid = (1 << COMP_SERVER_SOCKET) + | (1 << COMP_HTTP_SCHEME) + | (1 << COMP_HTTP_HOST) + | (1 << COMP_HTTP_REMOTE_IP) + | (1 << COMP_HTTP_REQUEST_METHOD) + | (1 << COMP_HTTP_URL) + | (1 << COMP_HTTP_QUERY_STRING) + | (1 << COMP_HTTP_REQUEST_HEADER); + } + else { r->keep_alive = 0; r->reqbody_length = 0; @@ -1116,8 +1126,8 @@ connection *connection_accepted(server *srv, server_socket *srv_socket, sock_add con->proto_default_port = 80; /* "http" */ config_cond_cache_reset(r); - r->conditional_is_valid |= (1 << COMP_SERVER_SOCKET) - | (1 << COMP_HTTP_REMOTE_IP); + r->conditional_is_valid = (1 << COMP_SERVER_SOCKET) + | (1 << COMP_HTTP_REMOTE_IP); if (HANDLER_GO_ON != plugins_call_handle_connection_accept(con)) { connection_reset(con); @@ -1214,7 +1224,7 @@ static int connection_handle_request(request_st * const r) { buffer_copy_buffer(&r->target, error_handler); connection_handle_errdoc_init(r); r->http_status = 0; /*(after connection_handle_errdoc_init())*/ - + http_response_comeback(r); return 1; } } @@ -1227,9 +1237,7 @@ static int connection_handle_request(request_st * const r) { connection_fdwaitqueue_append(r->con); break; case HANDLER_COMEBACK: - if (NULL == r->handler_module && buffer_is_empty(&r->physical.path)) { - config_reset_config(r); - } + http_response_comeback(r); return 1; /*case HANDLER_ERROR:*/ default: diff --git a/src/mod_cgi.c b/src/mod_cgi.c index af17f362..5e14df17 100644 --- a/src/mod_cgi.c +++ b/src/mod_cgi.c @@ -439,11 +439,9 @@ static int cgi_recv_response(request_st * const r, handler_ctx * const hctx) { cgi_connection_close(hctx); return HANDLER_FINISHED; case HANDLER_COMEBACK: - /* hctx->conf.local_redir */ + /* flag for mod_cgi_handle_subrequest() */ + hctx->conf.local_redir = 2; buffer_clear(hctx->response); - connection_response_reset(r); /*(includes r->http_status = 0)*/ - plugins_call_connection_reset(r); - /*cgi_connection_close(hctx);*//*(already cleaned up and hctx is now invalid)*/ return HANDLER_COMEBACK; } } @@ -925,6 +923,17 @@ URIHANDLER_FUNC(cgi_is_handled) { return HANDLER_GO_ON; } +__attribute_cold__ +__attribute_noinline__ +static handler_t mod_cgi_local_redir(request_st * const r) { + /* must be called from mod_cgi_handle_subrequest() so that HANDLER_COMEBACK + * return value propagates back through connection_state_machine() */ + connection_response_reset(r); /*(includes r->http_status = 0)*/ + plugins_call_connection_reset(r); + /*cgi_connection_close(hctx);*//*(already cleaned up and hctx is now invalid)*/ + return HANDLER_COMEBACK; +} + /* * - HANDLER_GO_ON : not our job * - HANDLER_FINISHED: got response @@ -935,6 +944,8 @@ SUBREQUEST_FUNC(mod_cgi_handle_subrequest) { handler_ctx * const hctx = r->plugin_ctx[p->id]; if (NULL == hctx) return HANDLER_GO_ON; + if (2 == hctx->conf.local_redir) return mod_cgi_local_redir(r); + if ((r->conf.stream_response_body & FDEVENT_STREAM_RESPONSE_BUFMIN) && r->resp_body_started) { if (chunkqueue_length(r->write_queue) > 65536 - 4096) { @@ -942,6 +953,7 @@ SUBREQUEST_FUNC(mod_cgi_handle_subrequest) { } else if (!(fdevent_fdnode_interest(hctx->fdn) & FDEVENT_IN)) { /* optimistic read from backend */ handler_t rc = cgi_recv_response(r, hctx); /*(might invalidate hctx)*/ + if (rc == HANDLER_COMEBACK) mod_cgi_local_redir(r); if (rc != HANDLER_GO_ON) return rc; /*(unless HANDLER_GO_ON)*/ fdevent_fdnode_event_add(hctx->ev, hctx->fdn, FDEVENT_IN); } diff --git a/src/request.c b/src/request.c index a6e270de..0aa077e4 100644 --- a/src/request.c +++ b/src/request.c @@ -678,14 +678,6 @@ int http_request_parse_target(request_st * const r, int scheme_port) { else buffer_copy_string_len(&r->uri.scheme, CONST_STR_LEN("http")); - if (r->http_host) { /*(might not know until after parsing request headers)*/ - buffer_copy_buffer(&r->uri.authority, r->http_host); - buffer_to_lower(&r->uri.authority); - } - else { - buffer_string_set_length(&r->uri.authority, 0); - } - buffer * const target = &r->target; if (r->http_method == HTTP_METHOD_CONNECT || (r->http_method == HTTP_METHOD_OPTIONS @@ -929,17 +921,20 @@ int http_request_parse(request_st * const restrict r, char * const restrict hdrs status = http_request_parse_reqline(r, hdrs, hoff, http_parseopts); if (0 != status) return status; + status = http_request_parse_target(r, scheme_port); + if (0 != status) return status; + status = http_request_parse_headers(r, hdrs, hoff, http_parseopts); if (0 != status) return status; + /*(r->http_host might not be set until after parsing request headers)*/ + buffer_copy_buffer(&r->uri.authority, r->http_host);/*(copy even if empty)*/ + buffer_to_lower(&r->uri.authority); + /* post-processing */ /* check hostname field if it is set */ if (r->http_host) { - if (buffer_string_is_empty(&r->uri.authority)) { - buffer_copy_buffer(&r->uri.authority, r->http_host); - buffer_to_lower(&r->uri.authority); - } if (0 != http_request_host_policy(r->http_host, http_parseopts, scheme_port)) return http_request_header_line_invalid(r, 400, "Invalid Hostname -> 400"); diff --git a/src/response.c b/src/response.c index 40d7ec1a..b0775d44 100644 --- a/src/response.c +++ b/src/response.c @@ -296,7 +296,9 @@ static handler_t http_status_set_error_close (request_st * const r, int status) } handler_t http_response_prepare(request_st * const r) { - handler_t rc; + handler_t rc; + + do { /* looks like someone has already made a decision */ if (r->http_status != 0 && r->http_status != 200) { @@ -322,23 +324,11 @@ handler_t http_response_prepare(request_st * const r) { if (!r->async_callback) { - config_cond_cache_reset(r); - if (r->conf.log_condition_handling) { log_error(r->conf.errh, __FILE__, __LINE__, "run condition"); } - int status = http_request_parse_target(r, r->con->proto_default_port); - if (0 != status) return http_status_set_error_close(r, status); - - r->conditional_is_valid |= (1 << COMP_SERVER_SOCKET) - | (1 << COMP_HTTP_SCHEME) - | (1 << COMP_HTTP_HOST) - | (1 << COMP_HTTP_REMOTE_IP) - | (1 << COMP_HTTP_REQUEST_METHOD) - | (1 << COMP_HTTP_URL) - | (1 << COMP_HTTP_QUERY_STRING) - | (1 << COMP_HTTP_REQUEST_HEADER); + config_cond_cache_reset(r); config_patch_config(r); /* do we have to downgrade to 1.0 ? */ @@ -385,7 +375,7 @@ handler_t http_response_prepare(request_st * const r) { */ rc = plugins_call_handle_uri_raw(r); - if (HANDLER_GO_ON != rc) return rc; + if (HANDLER_GO_ON != rc) continue; /** * @@ -396,7 +386,7 @@ handler_t http_response_prepare(request_st * const r) { */ rc = plugins_call_handle_uri_clean(r); - if (HANDLER_GO_ON != rc) return rc; + if (HANDLER_GO_ON != rc) continue; if (r->http_method == HTTP_METHOD_OPTIONS && r->uri.path.ptr[0] == '*' && r->uri.path.ptr[1] == '\0') { @@ -492,7 +482,7 @@ handler_t http_response_prepare(request_st * const r) { * for us (all vhost-plugins are supposed to set the doc_root) * */ rc = plugins_call_handle_docroot(r); - if (HANDLER_GO_ON != rc) return rc; + if (HANDLER_GO_ON != rc) continue; /* MacOS X and Windows can't distiguish between upper and lower-case * @@ -536,7 +526,7 @@ handler_t http_response_prepare(request_st * const r) { * http_response_prepare() is called while processing request) */ } else { rc = plugins_call_handle_physical(r); - if (HANDLER_GO_ON != rc) return rc; + if (HANDLER_GO_ON != rc) continue; if (r->conf.log_request_handling) { log_error(r->conf.errh, __FILE__, __LINE__, @@ -569,7 +559,7 @@ handler_t http_response_prepare(request_st * const r) { } rc = http_response_physical_path_check(r); - if (HANDLER_GO_ON != rc) return rc; + if (HANDLER_GO_ON != rc) continue; if (r->conf.log_request_handling) { log_error(r->conf.errh, __FILE__, __LINE__, @@ -589,7 +579,7 @@ handler_t http_response_prepare(request_st * const r) { log_error(r->conf.errh, __FILE__, __LINE__, "-- subrequest finished"); } - return rc; + continue; } /* if we are still here, no one wanted the file, status 403 is ok I think */ @@ -599,4 +589,39 @@ handler_t http_response_prepare(request_st * const r) { } return HANDLER_GO_ON; + + } while (HANDLER_COMEBACK == rc + && HANDLER_GO_ON ==(rc = http_response_comeback(r))); + + return rc; +} + +handler_t http_response_comeback (request_st * const r) +{ + if (NULL != r->handler_module || !buffer_is_empty(&r->physical.path)) + return HANDLER_GO_ON; + + config_reset_config(r); + + buffer_copy_buffer(&r->uri.authority,r->http_host);/*copy even if NULL*/ + buffer_to_lower(&r->uri.authority); + + int status = http_request_parse_target(r, r->con->proto_default_port); + if (0 == status) { + r->conditional_is_valid = (1 << COMP_SERVER_SOCKET) + | (1 << COMP_HTTP_SCHEME) + | (1 << COMP_HTTP_HOST) + | (1 << COMP_HTTP_REMOTE_IP) + | (1 << COMP_HTTP_REQUEST_METHOD) + | (1 << COMP_HTTP_URL) + | (1 << COMP_HTTP_QUERY_STRING) + | (1 << COMP_HTTP_REQUEST_HEADER); + return HANDLER_GO_ON; + } + else { + r->conditional_is_valid = (1 << COMP_SERVER_SOCKET) + | (1 << COMP_HTTP_REMOTE_IP); + config_cond_cache_reset(r); + return http_status_set_error_close(r, status); + } } diff --git a/src/response.h b/src/response.h index 53a5cf95..59a2f7dd 100644 --- a/src/response.h +++ b/src/response.h @@ -44,6 +44,10 @@ int http_cgi_headers(request_st *r, http_cgi_opts *opts, http_cgi_header_append_ handler_t http_response_parse_headers(request_st *r, http_response_opts *opts, buffer *hdrs); handler_t http_response_read(request_st *r, http_response_opts *opts, buffer *b, fdnode *fdn); handler_t http_response_prepare(request_st *r); + +__attribute_cold__ +handler_t http_response_comeback(request_st *r); + int http_response_redirect_to_directory(request_st *r, int status); int http_response_handle_cachable(request_st *r, const buffer *mtime); void http_response_body_clear(request_st *r, int preserve_length);