Re: [PATCH] HTTP/2: add debug logging of control frames
Hey Valentin, > I actually agree with that, but let's try to reduce the size of printing. > > http2 send SETTINGS frame MAX_CONCURRENT_STREAMS > http2 send SETTINGS frame INITIAL_WINDOW_SIZE > http2 send SETTINGS frame MAX_FRAME_SIZE > > This looks like too verbose for just one SETTINGS frame. What do you suggest instead? All 3 params in the same line? http2 send SETTINGS frame MAX_CONCURRENT_STREAMS:%ui INITIAL_WINDOW_SIZE:%uz MAX_FRAME_SIZE:%ud What about receiving part, then? Do you want to put all 6 params in the same line? http2 recv SETTINGS frame HEADER_TABLE_SIZE:%ui (ignored) ENABLE_PUSH:%ui (ignored) MAX_CONCURRENT_STREAMS:%ui (ignored) INITIAL_WINDOW_SIZE:%ui MAX_FRAME_SIZE:%ui MAX_HEADER_LIST_SIZE:%ui (ignored) It makes this way less readable, IMHO. > Also, literally > reading these lines can be misinterpreted as sending multiple SETTINGS > frames. That's because you skipped the first line, i.e.: http2 send SETTINGS frame params:3 http2 send SETTINGS frame MAX_CONCURRENT_STREAMS http2 send SETTINGS frame INITIAL_WINDOW_SIZE http2 send SETTINGS frame MAX_FRAME_SIZE Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: add debug logging of pseudo-headers
Hey Valentin, > I think that pseudo-headers have different processing and shouldn't be > a part of the same block as normal http headers. That's why I prefer > the second variant: > > http2 pseudo-header: ":method: GET" Fair enough, patch updated. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: add debug logging of pseudo-headers and cookies
# HG changeset patch
# User Piotr Sikora
# Date 1490516711 25200
# Sun Mar 26 01:25:11 2017 -0700
# Node ID 164b95f24f414359c5b8045415da3de82653c4db
# Parent 2c4dbcd6f2e4c9c2a1eb8dc1f0d39c99975ae208
HTTP/2: add debug logging of pseudo-headers and cookies.
Signed-off-by: Piotr Sikora
diff -r 2c4dbcd6f2e4 -r 164b95f24f41 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -1589,6 +1589,10 @@ ngx_http_v2_state_process_header(ngx_htt
rc = ngx_http_v2_pseudo_header(r, header);
if (rc == NGX_OK) {
+ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+ "http2 pseudo-header: \":%V: %V\"",
+ &header->name, &header->value);
+
return ngx_http_v2_state_header_complete(h2c, pos, end);
}
@@ -1630,6 +1634,10 @@ ngx_http_v2_state_process_header(ngx_htt
NGX_HTTP_V2_INTERNAL_ERROR);
}
+ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+ "http2 http header: \"%V: %V\"",
+ &header->name, &header->value);
+
return ngx_http_v2_state_header_complete(h2c, pos, end);
}
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 4 of 4] HTTP/2: don't send SETTINGS ACK before already queued DATA frames
# HG changeset patch
# User Piotr Sikora
# Date 1493073310 25200
# Mon Apr 24 15:35:10 2017 -0700
# Node ID 3624fa075acac110a08c0f1c928c545a58c5801f
# Parent b8d7f4a4d5abb4a27a772910358e263d49c618ef
HTTP/2: don't send SETTINGS ACK before already queued DATA frames.
Previously, SETTINGS ACK was sent immediately upon receipt of SETTINGS
frame, before already queued DATA frames created using old SETTINGS.
This incorrect behavior was source of interoperability issues, because
peers rely on the fact that new SETTINGS are in effect after receiving
SETTINGS ACK.
Reported by Feng Li.
Signed-off-by: Piotr Sikora
diff -r b8d7f4a4d5ab -r 3624fa075aca src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -2043,7 +2043,7 @@ ngx_http_v2_state_settings_params(ngx_ht
return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_INTERNAL_ERROR);
}
-ngx_http_v2_queue_blocked_frame(h2c, frame);
+ngx_http_v2_queue_ordered_frame(h2c, frame);
if (adjustment) {
if (ngx_http_v2_adjust_windows(h2c, adjustment) != NGX_OK) {
diff -r b8d7f4a4d5ab -r 3624fa075aca src/http/v2/ngx_http_v2.h
--- a/src/http/v2/ngx_http_v2.h
+++ b/src/http/v2/ngx_http_v2.h
@@ -261,6 +261,15 @@ ngx_http_v2_queue_blocked_frame(ngx_http
}
+static ngx_inline void
+ngx_http_v2_queue_ordered_frame(ngx_http_v2_connection_t *h2c,
+ngx_http_v2_out_frame_t *frame)
+{
+frame->next = h2c->last_out;
+h2c->last_out = frame;
+}
+
+
void ngx_http_v2_init(ngx_event_t *rev);
void ngx_http_v2_request_headers_init(void);
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 3 of 4] HTTP/2: make SETTINGS ACK frame reusable
# HG changeset patch
# User Piotr Sikora
# Date 1493073310 25200
# Mon Apr 24 15:35:10 2017 -0700
# Node ID b8d7f4a4d5abb4a27a772910358e263d49c618ef
# Parent a8cfd4c454ff5433629bfd16444c6c71ee932fa1
HTTP/2: make SETTINGS ACK frame reusable.
Signed-off-by: Piotr Sikora
diff -r a8cfd4c454ff -r b8d7f4a4d5ab src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -28,6 +28,7 @@
#define NGX_HTTP_V2_HTTP_1_1_REQUIRED0xd
/* frame sizes */
+#define NGX_HTTP_V2_SETTINGS_ACK_SIZE0
#define NGX_HTTP_V2_RST_STREAM_SIZE 4
#define NGX_HTTP_V2_PRIORITY_SIZE5
#define NGX_HTTP_V2_PING_SIZE8
@@ -128,8 +129,7 @@ static ngx_http_v2_node_t *ngx_http_v2_g
#define ngx_http_v2_index_size(h2scf) (h2scf->streams_index_mask + 1)
#define ngx_http_v2_index(h2scf, sid) ((sid >> 1) & h2scf->streams_index_mask)
-static ngx_int_t ngx_http_v2_send_settings(ngx_http_v2_connection_t *h2c,
-ngx_uint_t ack);
+static ngx_int_t ngx_http_v2_send_settings(ngx_http_v2_connection_t *h2c);
static ngx_int_t ngx_http_v2_settings_frame_handler(
ngx_http_v2_connection_t *h2c, ngx_http_v2_out_frame_t *frame);
static ngx_int_t ngx_http_v2_send_window_update(ngx_http_v2_connection_t *h2c,
@@ -269,7 +269,7 @@ ngx_http_v2_init(ngx_event_t *rev)
return;
}
-if (ngx_http_v2_send_settings(h2c, 0) == NGX_ERROR) {
+if (ngx_http_v2_send_settings(h2c) == NGX_ERROR) {
ngx_http_close_connection(c);
return;
}
@@ -1980,7 +1980,8 @@ static u_char *
ngx_http_v2_state_settings_params(ngx_http_v2_connection_t *h2c, u_char *pos,
u_char *end)
{
-ngx_uint_t id, value, adjustment;
+ngx_uint_tid, value, adjustment;
+ngx_http_v2_out_frame_t *frame;
adjustment = 0;
@@ -2035,7 +2036,14 @@ ngx_http_v2_state_settings_params(ngx_ht
pos += NGX_HTTP_V2_SETTINGS_PARAM_SIZE;
}
-ngx_http_v2_send_settings(h2c, 1);
+frame = ngx_http_v2_get_frame(h2c, NGX_HTTP_V2_SETTINGS_ACK_SIZE,
+ NGX_HTTP_V2_SETTINGS_FRAME,
+ NGX_HTTP_V2_ACK_FLAG, 0);
+if (frame == NULL) {
+return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_INTERNAL_ERROR);
+}
+
+ngx_http_v2_queue_blocked_frame(h2c, frame);
if (adjustment) {
if (ngx_http_v2_adjust_windows(h2c, adjustment) != NGX_OK) {
@@ -2487,7 +2495,7 @@ ngx_http_v2_parse_int(ngx_http_v2_connec
static ngx_int_t
-ngx_http_v2_send_settings(ngx_http_v2_connection_t *h2c, ngx_uint_t ack)
+ngx_http_v2_send_settings(ngx_http_v2_connection_t *h2c)
{
size_tlen;
ngx_buf_t*buf;
@@ -2495,8 +2503,8 @@ ngx_http_v2_send_settings(ngx_http_v2_co
ngx_http_v2_srv_conf_t *h2scf;
ngx_http_v2_out_frame_t *frame;
-ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
- "http2 send SETTINGS frame ack:%ui", ack);
+ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+ "http2 send SETTINGS frame params:3");
frame = ngx_palloc(h2c->pool, sizeof(ngx_http_v2_out_frame_t));
if (frame == NULL) {
@@ -2508,7 +2516,7 @@ ngx_http_v2_send_settings(ngx_http_v2_co
return NGX_ERROR;
}
-len = ack ? 0 : (sizeof(uint16_t) + sizeof(uint32_t)) * 3;
+len = NGX_HTTP_V2_SETTINGS_PARAM_SIZE * 3;
buf = ngx_create_temp_buf(h2c->pool, NGX_HTTP_V2_FRAME_HEADER_SIZE + len);
if (buf == NULL) {
@@ -2532,28 +2540,26 @@ ngx_http_v2_send_settings(ngx_http_v2_co
buf->last = ngx_http_v2_write_len_and_type(buf->last, len,
NGX_HTTP_V2_SETTINGS_FRAME);
-*buf->last++ = ack ? NGX_HTTP_V2_ACK_FLAG : NGX_HTTP_V2_NO_FLAG;
+*buf->last++ = NGX_HTTP_V2_NO_FLAG;
buf->last = ngx_http_v2_write_sid(buf->last, 0);
-if (!ack) {
-h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx,
- ngx_http_v2_module);
-
-buf->last = ngx_http_v2_write_uint16(buf->last,
- NGX_HTTP_V2_MAX_STREAMS_SETTING);
-buf->last = ngx_http_v2_write_uint32(buf->last,
- h2scf->concurrent_streams);
-
-buf->last = ngx_http_v2_write_uint16(buf->last,
+h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx,
+ ngx_http_v2_module);
+
+buf->last = ngx_http_v2_write_uint16(buf->last,
+ NGX_HTTP_V2_MAX_STREAMS_SETTING);
+buf->last = ngx_http_v2_write_uint32(buf->last,
+ h2scf->concurrent_streams);
+
+buf->last = ngx_http_v2_write_uint16(buf->last,
NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING);
-buf->last = ngx_http_v2_wri
[PATCH 2 of 4] HTTP/2: send SETTINGS ACK after applying all SETTINGS params
# HG changeset patch
# User Piotr Sikora
# Date 1493073310 25200
# Mon Apr 24 15:35:10 2017 -0700
# Node ID a8cfd4c454ff5433629bfd16444c6c71ee932fa1
# Parent 07adf0a7009c3244de4b795c0c06927f4316a87f
HTTP/2: send SETTINGS ACK after applying all SETTINGS params.
This avoids sending unnecessary SETTINGS ACK in case of PROTOCOL_ERROR.
Signed-off-by: Piotr Sikora
diff -r 07adf0a7009c -r a8cfd4c454ff src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -1972,8 +1972,6 @@ ngx_http_v2_state_settings(ngx_http_v2_c
return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR);
}
-ngx_http_v2_send_settings(h2c, 1);
-
return ngx_http_v2_state_settings_params(h2c, pos, end);
}
@@ -2037,6 +2035,8 @@ ngx_http_v2_state_settings_params(ngx_ht
pos += NGX_HTTP_V2_SETTINGS_PARAM_SIZE;
}
+ngx_http_v2_send_settings(h2c, 1);
+
if (adjustment) {
if (ngx_http_v2_adjust_windows(h2c, adjustment) != NGX_OK) {
return ngx_http_v2_connection_error(h2c,
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 1 of 4] HTTP/2: emit new frames only after applying all SETTINGS params
# HG changeset patch
# User Piotr Sikora
# Date 1493073310 25200
# Mon Apr 24 15:35:10 2017 -0700
# Node ID 07adf0a7009c3244de4b795c0c06927f4316a87f
# Parent 2c4dbcd6f2e4c9c2a1eb8dc1f0d39c99975ae208
HTTP/2: emit new frames only after applying all SETTINGS params.
Previously, new frames could be emitted in the middle of applying
new (and already acknowledged) SETTINGS params, which is illegal.
Signed-off-by: Piotr Sikora
diff -r 2c4dbcd6f2e4 -r 07adf0a7009c src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -1982,7 +1982,9 @@ static u_char *
ngx_http_v2_state_settings_params(ngx_http_v2_connection_t *h2c, u_char *pos,
u_char *end)
{
-ngx_uint_t id, value;
+ngx_uint_t id, value, adjustment;
+
+adjustment = 0;
while (h2c->state.length) {
if (end - pos < NGX_HTTP_V2_SETTINGS_PARAM_SIZE) {
@@ -2008,13 +2010,7 @@ ngx_http_v2_state_settings_params(ngx_ht
NGX_HTTP_V2_FLOW_CTRL_ERROR);
}
-if (ngx_http_v2_adjust_windows(h2c, value - h2c->init_window)
-!= NGX_OK)
-{
-return ngx_http_v2_connection_error(h2c,
-
NGX_HTTP_V2_INTERNAL_ERROR);
-}
-
+adjustment = value - h2c->init_window;
h2c->init_window = value;
break;
@@ -2041,6 +2037,13 @@ ngx_http_v2_state_settings_params(ngx_ht
pos += NGX_HTTP_V2_SETTINGS_PARAM_SIZE;
}
+if (adjustment) {
+if (ngx_http_v2_adjust_windows(h2c, adjustment) != NGX_OK) {
+return ngx_http_v2_connection_error(h2c,
+NGX_HTTP_V2_INTERNAL_ERROR);
+}
+}
+
return ngx_http_v2_state_complete(h2c, pos, end);
}
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] Cleaned up r->headers_out.headers allocation error handling.
Hello! On Mon, Apr 24, 2017 at 07:23:13PM +0300, Sorin Manole wrote: > >> For the Cache-Control header, the fix is to postpone pushing > >> r->headers_out.cache_control until its value is completed. > > Why not do the same thing for a bunch of other places like: > ngx_http_auth_basic_set_realm > ngx_http_range_not_satisfiable > > That is, for the latter, first initialize content_range, and then add it to > headers. > Looks like a safer strategy then rolling back a change in case of failure. This was considered in earlier versions of the patch. The result seems harder to read though, and also requires much more code changes. -- Maxim Dounin http://nginx.org/ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] Cleaned up r->headers_out.headers allocation error handling.
Hello!
>> For the Cache-Control header, the fix is to postpone pushing
>> r->headers_out.cache_control until its value is completed.
Why not do the same thing for a bunch of other places like:
ngx_http_auth_basic_set_realm
ngx_http_range_not_satisfiable
That is, for the latter, first initialize content_range, and then add it to
headers.
Looks like a safer strategy then rolling back a change in case of failure.
Thank you!
2017-04-20 19:33 GMT+03:00 Sergey Kandaurov :
> details: http://hg.nginx.org/nginx/rev/0cdee26605f3
> branches:
> changeset: 6986:0cdee26605f3
> user: Sergey Kandaurov
> date: Thu Apr 20 18:26:37 2017 +0300
> description:
> Cleaned up r->headers_out.headers allocation error handling.
>
> If initialization of a header failed for some reason after ngx_list_push(),
> leaving the header as is can result in uninitialized memory access by
> the header filter or the log module. The fix is to clear partially
> initialized headers in case of errors.
>
> For the Cache-Control header, the fix is to postpone pushing
> r->headers_out.cache_control until its value is completed.
>
> diffstat:
>
> src/http/modules/ngx_http_auth_basic_module.c | 2 ++
> src/http/modules/ngx_http_dav_module.c| 1 +
> src/http/modules/ngx_http_headers_filter_module.c | 21
> +++--
> src/http/modules/ngx_http_range_filter_module.c | 4
> src/http/modules/ngx_http_static_module.c | 1 +
> src/http/modules/perl/nginx.xs| 2 ++
> src/http/ngx_http_core_module.c | 1 +
> src/http/ngx_http_upstream.c | 13 +++--
> 8 files changed, 29 insertions(+), 16 deletions(-)
>
> diffs (162 lines):
>
> diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_
> auth_basic_module.c
> --- a/src/http/modules/ngx_http_auth_basic_module.c Thu Apr 20
> 13:58:16 2017 +0300
> +++ b/src/http/modules/ngx_http_auth_basic_module.c Thu Apr 20
> 18:26:37 2017 +0300
> @@ -361,6 +361,8 @@ ngx_http_auth_basic_set_realm(ngx_http_r
>
> basic = ngx_pnalloc(r->pool, len);
> if (basic == NULL) {
> +r->headers_out.www_authenticate->hash = 0;
> +r->headers_out.www_authenticate = NULL;
> return NGX_HTTP_INTERNAL_SERVER_ERROR;
> }
>
> diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_dav_
> module.c
> --- a/src/http/modules/ngx_http_dav_module.cThu Apr 20 13:58:16 2017
> +0300
> +++ b/src/http/modules/ngx_http_dav_module.cThu Apr 20 18:26:37 2017
> +0300
> @@ -1080,6 +1080,7 @@ ngx_http_dav_location(ngx_http_request_t
> } else {
> location = ngx_pnalloc(r->pool, r->uri.len);
> if (location == NULL) {
> +ngx_http_clear_location(r);
> return NGX_ERROR;
> }
>
> diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_
> headers_filter_module.c
> --- a/src/http/modules/ngx_http_headers_filter_module.c Thu Apr 20
> 13:58:16 2017 +0300
> +++ b/src/http/modules/ngx_http_headers_filter_module.c Thu Apr 20
> 18:26:37 2017 +0300
> @@ -271,11 +271,6 @@ ngx_http_set_expires(ngx_http_request_t
> return NGX_ERROR;
> }
>
> -ccp = ngx_array_push(&r->headers_out.cache_control);
> -if (ccp == NULL) {
> -return NGX_ERROR;
> -}
> -
> cc = ngx_list_push(&r->headers_out.headers);
> if (cc == NULL) {
> return NGX_ERROR;
> @@ -283,6 +278,12 @@ ngx_http_set_expires(ngx_http_request_t
>
> cc->hash = 1;
> ngx_str_set(&cc->key, "Cache-Control");
> +
> +ccp = ngx_array_push(&r->headers_out.cache_control);
> +if (ccp == NULL) {
> +return NGX_ERROR;
> +}
> +
> *ccp = cc;
>
> } else {
> @@ -470,11 +471,6 @@ ngx_http_add_cache_control(ngx_http_requ
> }
> }
>
> -ccp = ngx_array_push(&r->headers_out.cache_control);
> -if (ccp == NULL) {
> -return NGX_ERROR;
> -}
> -
> cc = ngx_list_push(&r->headers_out.headers);
> if (cc == NULL) {
> return NGX_ERROR;
> @@ -484,6 +480,11 @@ ngx_http_add_cache_control(ngx_http_requ
> ngx_str_set(&cc->key, "Cache-Control");
> cc->value = *value;
>
> +ccp = ngx_array_push(&r->headers_out.cache_control);
> +if (ccp == NULL) {
> +return NGX_ERROR;
> +}
> +
> *ccp = cc;
>
> return NGX_OK;
> diff -r 23ecffd5bcfe -r 0cdee26605f3 src/http/modules/ngx_http_
> range_filter_module.c
> --- a/src/http/modules/ngx_http_range_filter_module.c Thu Apr 20
> 13:58:16 2017 +0300
> +++ b/src/http/modules/ngx_http_range_filter_module.c Thu Apr 20
> 18:26:37 2017 +0300
> @@ -425,6 +425,8 @@ ngx_http_range_singlepart_header(ngx_htt
> content_range->value.data = ngx_pnalloc(r->pool,
> sizeof("bytes -/") - 1 + 3 *
> NGX_OFF_T_LEN);
> if (content_range->value.data == NULL) {
> +content_range->hash = 0;
> +
Re: [PATCH] FYI: Fix FTBFS on Debian GNU/kFreeBSD
Hello! On Mon, Apr 24, 2017 at 10:58:26AM +0900, HAYASHI Kentaro wrote: > Hi. > > I'm happy if the following patch is merged into upstream. > > nginx-kfreebsd.diff > > https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=826061;filename=nginx-kfreebsd.diff;msg=10 > > Here is more detail about this patch: > > Above patch is aimed to fix FTBFS on Debian GNU/kFreeBSD. > This FTBFS bug is already fixed on Debian [1], and patch is maintained > as > debian/patches/0003-define_gnu_source-on-other-glibc-based-platforms.patch. > > [1] nginx: FTBFS on kfreebsd: incomplete type 'struct in6_pktinfo' > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=826061 > > I think this patch should be forwarded to upstream and merged because third > party software > which bundles nginx on kFreeBSD also shoots in the legs without this patch. https://trac.nginx.org/nginx/ticket/1028 -- Maxim Dounin http://nginx.org/ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[nginx] HTTP/2: reduced difference to HTTP/1.x in reading request body.
details: http://hg.nginx.org/nginx/rev/2c4dbcd6f2e4
branches:
changeset: 6989:2c4dbcd6f2e4
user: Valentin Bartenev
date: Mon Apr 24 14:17:13 2017 +0300
description:
HTTP/2: reduced difference to HTTP/1.x in reading request body.
Particularly, this eliminates difference in behavior for requests without body
and deduplicates code.
Prodded by Piotr Sikora.
diffstat:
src/http/ngx_http_request_body.c | 20
src/http/v2/ngx_http_v2.c| 28 +---
src/http/v2/ngx_http_v2.h| 3 +--
3 files changed, 18 insertions(+), 33 deletions(-)
diffs (115 lines):
diff -r cc823122d50d -r 2c4dbcd6f2e4 src/http/ngx_http_request_body.c
--- a/src/http/ngx_http_request_body.c Mon Apr 24 14:16:57 2017 +0300
+++ b/src/http/ngx_http_request_body.c Mon Apr 24 14:17:13 2017 +0300
@@ -46,13 +46,6 @@ ngx_http_read_client_request_body(ngx_ht
return NGX_OK;
}
-#if (NGX_HTTP_V2)
-if (r->stream) {
-rc = ngx_http_v2_read_request_body(r, post_handler);
-goto done;
-}
-#endif
-
if (ngx_http_test_expect(r) != NGX_OK) {
rc = NGX_HTTP_INTERNAL_SERVER_ERROR;
goto done;
@@ -85,6 +78,13 @@ ngx_http_read_client_request_body(ngx_ht
return NGX_OK;
}
+#if (NGX_HTTP_V2)
+if (r->stream) {
+rc = ngx_http_v2_read_request_body(r);
+goto done;
+}
+#endif
+
preread = r->header_in->last - r->header_in->pos;
if (preread) {
@@ -805,7 +805,11 @@ ngx_http_test_expect(ngx_http_request_t
if (r->expect_tested
|| r->headers_in.expect == NULL
-|| r->http_version < NGX_HTTP_VERSION_11)
+|| r->http_version < NGX_HTTP_VERSION_11
+#if (NGX_HTTP_V2)
+|| r->stream != NULL
+#endif
+ )
{
return NGX_OK;
}
diff -r cc823122d50d -r 2c4dbcd6f2e4 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c Mon Apr 24 14:16:57 2017 +0300
+++ b/src/http/v2/ngx_http_v2.c Mon Apr 24 14:17:13 2017 +0300
@@ -3522,8 +3522,7 @@ ngx_http_v2_run_request(ngx_http_request
ngx_int_t
-ngx_http_v2_read_request_body(ngx_http_request_t *r,
-ngx_http_client_body_handler_pt post_handler)
+ngx_http_v2_read_request_body(ngx_http_request_t *r)
{
off_t len;
size_t size;
@@ -3536,33 +3535,14 @@ ngx_http_v2_read_request_body(ngx_http_r
ngx_http_v2_connection_t *h2c;
stream = r->stream;
+rb = r->request_body;
if (stream->skip_data) {
r->request_body_no_buffering = 0;
-post_handler(r);
+rb->post_handler(r);
return NGX_OK;
}
-rb = ngx_pcalloc(r->pool, sizeof(ngx_http_request_body_t));
-if (rb == NULL) {
-return NGX_HTTP_INTERNAL_SERVER_ERROR;
-}
-
-/*
- * set by ngx_pcalloc():
- *
- * rb->bufs = NULL;
- * rb->buf = NULL;
- * rb->received = 0;
- * rb->free = NULL;
- * rb->busy = NULL;
- */
-
-rb->rest = 1;
-rb->post_handler = post_handler;
-
-r->request_body = rb;
-
h2scf = ngx_http_get_module_srv_conf(r, ngx_http_v2_module);
clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
@@ -3612,6 +3592,8 @@ ngx_http_v2_read_request_body(ngx_http_r
return NGX_HTTP_INTERNAL_SERVER_ERROR;
}
+rb->rest = 1;
+
buf = stream->preread;
if (stream->in_closed) {
diff -r cc823122d50d -r 2c4dbcd6f2e4 src/http/v2/ngx_http_v2.h
--- a/src/http/v2/ngx_http_v2.h Mon Apr 24 14:16:57 2017 +0300
+++ b/src/http/v2/ngx_http_v2.h Mon Apr 24 14:17:13 2017 +0300
@@ -264,8 +264,7 @@ ngx_http_v2_queue_blocked_frame(ngx_http
void ngx_http_v2_init(ngx_event_t *rev);
void ngx_http_v2_request_headers_init(void);
-ngx_int_t ngx_http_v2_read_request_body(ngx_http_request_t *r,
-ngx_http_client_body_handler_pt post_handler);
+ngx_int_t ngx_http_v2_read_request_body(ngx_http_request_t *r);
ngx_int_t ngx_http_v2_read_unbuffered_request_body(ngx_http_request_t *r);
void ngx_http_v2_close_stream(ngx_http_v2_stream_t *stream, ngx_int_t rc);
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
[nginx] HTTP/2: rejecting zero WINDOW_UPDATE with PROTOCOL_ERROR.
details: http://hg.nginx.org/nginx/rev/cc823122d50d
branches:
changeset: 6988:cc823122d50d
user: Valentin Bartenev
date: Mon Apr 24 14:16:57 2017 +0300
description:
HTTP/2: rejecting zero WINDOW_UPDATE with PROTOCOL_ERROR.
It's required by RFC 7540. While there is no real harm from such frames,
that should help to detect broken clients.
Based on a patch by Piotr Sikora.
diffstat:
src/http/v2/ngx_http_v2.c | 38 ++
1 files changed, 38 insertions(+), 0 deletions(-)
diffs (48 lines):
diff -r 5116cfea1e9a -r cc823122d50d src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c Thu Apr 20 18:26:38 2017 +0300
+++ b/src/http/v2/ngx_http_v2.c Mon Apr 24 14:16:57 2017 +0300
@@ -2166,6 +2166,44 @@ ngx_http_v2_state_window_update(ngx_http
"http2 WINDOW_UPDATE frame sid:%ui window:%uz",
h2c->state.sid, window);
+if (window == 0) {
+if (h2c->state.sid == 0) {
+ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
+ "client sent WINDOW_UPDATE frame "
+ "with incorrect window increment 0");
+
+return ngx_http_v2_connection_error(h2c,
+NGX_HTTP_V2_PROTOCOL_ERROR);
+}
+
+ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
+ "client sent WINDOW_UPDATE frame for stream %ui "
+ "with incorrect window increment 0", h2c->state.sid);
+
+node = ngx_http_v2_get_node_by_id(h2c, h2c->state.sid, 0);
+
+if (node && node->stream) {
+if (ngx_http_v2_terminate_stream(h2c, node->stream,
+ NGX_HTTP_V2_PROTOCOL_ERROR)
+== NGX_ERROR)
+{
+return ngx_http_v2_connection_error(h2c,
+
NGX_HTTP_V2_INTERNAL_ERROR);
+}
+
+} else {
+if (ngx_http_v2_send_rst_stream(h2c, h2c->state.sid,
+NGX_HTTP_V2_PROTOCOL_ERROR)
+== NGX_ERROR)
+{
+return ngx_http_v2_connection_error(h2c,
+
NGX_HTTP_V2_INTERNAL_ERROR);
+}
+}
+
+return ngx_http_v2_state_complete(h2c, pos, end);
+}
+
if (h2c->state.sid) {
node = ngx_http_v2_get_node_by_id(h2c, h2c->state.sid, 0);
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
