From 3c0427373381097a9e25ccc2cb46bbc1ccac87a2 Mon Sep 17 00:00:00 2001 From: Vladimir Homutov Date: Wed, 28 Oct 2020 10:56:11 +0300 Subject: Core: added format specifiers to output binary data as hex. Now "s", "V", and "v" format specifiers may be prefixed with "x" (lowercase) or "X" (uppercase) to output corresponding data in hexadecimal format. In collaboration with Maxim Dounin. --- src/http/modules/ngx_http_grpc_module.c | 38 +++++++++------------------------ 1 file changed, 10 insertions(+), 28 deletions(-) (limited to 'src/http') diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c index 7e14af8d9..0b8bb5281 100644 --- a/src/http/modules/ngx_http_grpc_module.c +++ b/src/http/modules/ngx_http_grpc_module.c @@ -1141,20 +1141,11 @@ ngx_http_grpc_create_request(ngx_http_request_t *r) f->flags |= NGX_HTTP_V2_END_HEADERS_FLAG; -#if (NGX_DEBUG) - if (r->connection->log->log_level & NGX_LOG_DEBUG_HTTP) { - u_char buf[512]; - size_t n, m; - - n = ngx_min(b->last - b->pos, 256); - m = ngx_hex_dump(buf, b->pos, n) - buf; - - ngx_log_debug4(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "grpc header: %*s%s, len: %uz", - m, buf, b->last - b->pos > 256 ? "..." : "", - b->last - b->pos); - } -#endif + ngx_log_debug4(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "grpc header: %*xs%s, len: %uz", + (size_t) ngx_min(b->last - b->pos, 256), b->pos, + b->last - b->pos > 256 ? "..." : "", + b->last - b->pos); if (r->request_body_no_buffering) { @@ -1604,20 +1595,11 @@ ngx_http_grpc_process_header(ngx_http_request_t *r) u = r->upstream; b = &u->buffer; -#if (NGX_DEBUG) - if (r->connection->log->log_level & NGX_LOG_DEBUG_HTTP) { - u_char buf[512]; - size_t n, m; - - n = ngx_min(b->last - b->pos, 256); - m = ngx_hex_dump(buf, b->pos, n) - buf; - - ngx_log_debug4(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "grpc response: %*s%s, len: %uz", - m, buf, b->last - b->pos > 256 ? "..." : "", - b->last - b->pos); - } -#endif + ngx_log_debug4(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "grpc response: %*xs%s, len: %uz", + (size_t) ngx_min(b->last - b->pos, 256), + b->pos, b->last - b->pos > 256 ? "..." : "", + b->last - b->pos); ctx = ngx_http_grpc_get_ctx(r); -- cgit From aad0d1bf1cab58a1b1e1499485e63b7a15ab183c Mon Sep 17 00:00:00 2001 From: Ruslan Ermilov Date: Fri, 6 Nov 2020 23:44:47 +0300 Subject: Removed dead code from ngx_http_set_keepalive(). The code removed became dead after 98f03cd8d6cc (0.8.14), circa when the request reference counting was introduced. --- src/http/ngx_http_request.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'src/http') diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index 204a9399d..5ee9dee14 100644 --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -3039,13 +3039,6 @@ ngx_http_set_keepalive(ngx_http_request_t *r) ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "set http keepalive handler"); - if (r->discard_body) { - r->write_event_handler = ngx_http_request_empty_handler; - r->lingering_time = ngx_time() + (time_t) (clcf->lingering_time / 1000); - ngx_add_timer(rev, clcf->lingering_timeout); - return; - } - c->log->action = "closing request"; hc = r->http_connection; -- cgit From ad2b9944b012699c225d75e63aeadf61d9ce3367 Mon Sep 17 00:00:00 2001 From: Ruslan Ermilov Date: Fri, 6 Nov 2020 23:44:54 +0300 Subject: SSL: fixed non-working SSL shutdown on lingering close. When doing lingering close, the socket was first shut down for writing, so SSL shutdown initiated after lingering close was not able to send the close_notify alerts (ticket #2056). The fix is to call ngx_ssl_shutdown() before shutting down the socket. --- src/http/ngx_http_request.c | 39 +++++++++++++++++++++++++++++-------- src/http/ngx_http_request_body.c | 1 + src/http/v2/ngx_http_v2.c | 42 +++++++++++++++++++++++++++++++--------- 3 files changed, 65 insertions(+), 17 deletions(-) (limited to 'src/http') diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index 5ee9dee14..12a68a961 100644 --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -49,7 +49,7 @@ static void ngx_http_request_finalizer(ngx_http_request_t *r); static void ngx_http_set_keepalive(ngx_http_request_t *r); static void ngx_http_keepalive_handler(ngx_event_t *ev); -static void ngx_http_set_lingering_close(ngx_http_request_t *r); +static void ngx_http_set_lingering_close(ngx_connection_t *c); static void ngx_http_lingering_close_handler(ngx_event_t *ev); static ngx_int_t ngx_http_post_action(ngx_http_request_t *r); static void ngx_http_close_request(ngx_http_request_t *r, ngx_int_t error); @@ -2754,7 +2754,7 @@ ngx_http_finalize_connection(ngx_http_request_t *r) || r->header_in->pos < r->header_in->last || r->connection->read->ready))) { - ngx_http_set_lingering_close(r); + ngx_http_set_lingering_close(r->connection); return; } @@ -3368,22 +3368,43 @@ ngx_http_keepalive_handler(ngx_event_t *rev) static void -ngx_http_set_lingering_close(ngx_http_request_t *r) +ngx_http_set_lingering_close(ngx_connection_t *c) { ngx_event_t *rev, *wev; - ngx_connection_t *c; + ngx_http_request_t *r; ngx_http_core_loc_conf_t *clcf; - c = r->connection; + r = c->data; clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); + if (r->lingering_time == 0) { + r->lingering_time = ngx_time() + (time_t) (clcf->lingering_time / 1000); + } + +#if (NGX_HTTP_SSL) + if (c->ssl) { + ngx_int_t rc; + + rc = ngx_ssl_shutdown(c); + + if (rc == NGX_ERROR) { + ngx_http_close_request(r, 0); + return; + } + + if (rc == NGX_AGAIN) { + c->ssl->handler = ngx_http_set_lingering_close; + return; + } + + c->recv = ngx_recv; + } +#endif + rev = c->read; rev->handler = ngx_http_lingering_close_handler; - r->lingering_time = ngx_time() + (time_t) (clcf->lingering_time / 1000); - ngx_add_timer(rev, clcf->lingering_timeout); - if (ngx_handle_read_event(rev, 0) != NGX_OK) { ngx_http_close_request(r, 0); return; @@ -3406,6 +3427,8 @@ ngx_http_set_lingering_close(ngx_http_request_t *r) return; } + ngx_add_timer(rev, clcf->lingering_timeout); + if (rev->ready) { ngx_http_lingering_close_handler(rev); } diff --git a/src/http/ngx_http_request_body.c b/src/http/ngx_http_request_body.c index 71d7e9ab8..f3b938382 100644 --- a/src/http/ngx_http_request_body.c +++ b/src/http/ngx_http_request_body.c @@ -674,6 +674,7 @@ ngx_http_discarded_request_body_handler(ngx_http_request_t *r) if (rc == NGX_OK) { r->discard_body = 0; r->lingering_close = 0; + r->lingering_time = 0; ngx_http_finalize_request(r, NGX_DONE); return; } diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c index 43a4fded5..58916a184 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -60,7 +60,7 @@ typedef struct { static void ngx_http_v2_read_handler(ngx_event_t *rev); static void ngx_http_v2_write_handler(ngx_event_t *wev); static void ngx_http_v2_handle_connection(ngx_http_v2_connection_t *h2c); -static void ngx_http_v2_lingering_close(ngx_http_v2_connection_t *h2c); +static void ngx_http_v2_lingering_close(ngx_connection_t *c); static void ngx_http_v2_lingering_close_handler(ngx_event_t *rev); static u_char *ngx_http_v2_state_proxy_protocol(ngx_http_v2_connection_t *h2c, @@ -664,7 +664,7 @@ ngx_http_v2_handle_connection(ngx_http_v2_connection_t *h2c) } if (h2c->goaway) { - ngx_http_v2_lingering_close(h2c); + ngx_http_v2_lingering_close(c); return; } @@ -703,13 +703,13 @@ ngx_http_v2_handle_connection(ngx_http_v2_connection_t *h2c) static void -ngx_http_v2_lingering_close(ngx_http_v2_connection_t *h2c) +ngx_http_v2_lingering_close(ngx_connection_t *c) { ngx_event_t *rev, *wev; - ngx_connection_t *c; + ngx_http_v2_connection_t *h2c; ngx_http_core_loc_conf_t *clcf; - c = h2c->connection; + h2c = c->data; clcf = ngx_http_get_module_loc_conf(h2c->http_connection->conf_ctx, ngx_http_core_module); @@ -719,12 +719,34 @@ ngx_http_v2_lingering_close(ngx_http_v2_connection_t *h2c) return; } + if (h2c->lingering_time == 0) { + h2c->lingering_time = ngx_time() + + (time_t) (clcf->lingering_time / 1000); + } + +#if (NGX_HTTP_SSL) + if (c->ssl) { + ngx_int_t rc; + + rc = ngx_ssl_shutdown(c); + + if (rc == NGX_ERROR) { + ngx_http_close_connection(c); + return; + } + + if (rc == NGX_AGAIN) { + c->ssl->handler = ngx_http_v2_lingering_close; + return; + } + + c->recv = ngx_recv; + } +#endif + rev = c->read; rev->handler = ngx_http_v2_lingering_close_handler; - h2c->lingering_time = ngx_time() + (time_t) (clcf->lingering_time / 1000); - ngx_add_timer(rev, clcf->lingering_timeout); - if (ngx_handle_read_event(rev, 0) != NGX_OK) { ngx_http_close_connection(c); return; @@ -747,6 +769,8 @@ ngx_http_v2_lingering_close(ngx_http_v2_connection_t *h2c) return; } + ngx_add_timer(rev, clcf->lingering_timeout); + if (rev->ready) { ngx_http_v2_lingering_close_handler(rev); } @@ -4757,7 +4781,7 @@ done: return; } - ngx_http_v2_lingering_close(h2c); + ngx_http_v2_lingering_close(c); } -- cgit From 8ed303c055ee003285231b6d781bfe2dd9c0d3d1 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Mon, 9 Nov 2020 22:40:53 +0300 Subject: Request body: improved logging. Added logging before returning NGX_HTTP_INTERNAL_SERVER_ERROR if there are busy buffers after a request body flush. This should never happen with current code, though bugs can be introduced by 3rd party modules. Make sure debugging will be easy enough. --- src/http/ngx_http_request_body.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/http') diff --git a/src/http/ngx_http_request_body.c b/src/http/ngx_http_request_body.c index f3b938382..07e78d780 100644 --- a/src/http/ngx_http_request_body.c +++ b/src/http/ngx_http_request_body.c @@ -305,6 +305,9 @@ ngx_http_do_read_client_request_body(ngx_http_request_t *r) return NGX_AGAIN; } + ngx_log_error(NGX_LOG_ALERT, c->log, 0, + "busy buffers after request body flush"); + return NGX_HTTP_INTERNAL_SERVER_ERROR; } -- cgit From a3b5ccd0566e618635303309dd8592aa4cfd7eaf Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Mon, 9 Nov 2020 22:41:54 +0300 Subject: Request body: removed error assumption (ticket #2058). Before introduction of request body filter in 42d9beeb22db, the only possible return code from the ngx_http_request_body_filter() call without actual buffers was NGX_HTTP_INTERNAL_SERVER_ERROR, and the code in ngx_http_read_client_request_body() hardcoded the only possible error to simplify the code of initial call to set rb->rest. This is no longer true after introduction of request body filters though, as a request body filter might need to return other errors, such as 403. Fix is to preserve the error code actually returned by the call instead of assuming 500. --- src/http/ngx_http_request_body.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/http') diff --git a/src/http/ngx_http_request_body.c b/src/http/ngx_http_request_body.c index 07e78d780..0cae88f77 100644 --- a/src/http/ngx_http_request_body.c +++ b/src/http/ngx_http_request_body.c @@ -137,8 +137,9 @@ ngx_http_read_client_request_body(ngx_http_request_t *r, } else { /* set rb->rest */ - if (ngx_http_request_body_filter(r, NULL) != NGX_OK) { - rc = NGX_HTTP_INTERNAL_SERVER_ERROR; + rc = ngx_http_request_body_filter(r, NULL); + + if (rc != NGX_OK) { goto done; } } -- cgit From 671cbc18409ec726a540b5cfc0f88bbdc5dba2f1 Mon Sep 17 00:00:00 2001 From: Pavel Pautov Date: Wed, 18 Nov 2020 18:41:16 -0800 Subject: gRPC: RST_STREAM(NO_ERROR) handling after "trailer only" responses. Similarly to the problem fixed in 2096b21fcd10 (ticket #1792), when a "trailer only" gRPC response (that is, a response with the END_STREAM flag in the HEADERS frame) was immediately followed by RST_STREAM(NO_ERROR) in the data preread along with the response header, RST_STREAM wasn't properly skipped and caused "upstream rejected request with error 0" errors. Observed with "unknown service" gRPC errors returned by grpc-go. Fix is to set ctx->done if we are going to parse additional data, so the RST_STREAM(NO_ERROR) is properly skipped. Additionally, now ngx_http_grpc_filter() will complain about frames sent for closed stream if there are any. --- src/http/modules/ngx_http_grpc_module.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src/http') diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c index 0b8bb5281..aa7576561 100644 --- a/src/http/modules/ngx_http_grpc_module.c +++ b/src/http/modules/ngx_http_grpc_module.c @@ -1969,6 +1969,7 @@ ngx_http_grpc_filter_init(void *data) } u->length = 0; + ctx->done = 1; } else { u->length = 1; -- cgit