Hello!

On Mon, Apr 03, 2017 at 02:32:11AM -0700, Piotr Sikora via nginx-devel wrote:

> # HG changeset patch
> # User Piotr Sikora <piotrsikora@google.com>
> # Date 1490351854 25200
> # Fri Mar 24 03:37:34 2017 -0700
> # Node ID 8af81a0d66c0f69bcf501edcf10deed4c8f7fbd4
> # Parent 39ff6939266e913e8bfd400e60f9520e70725a4d
> HTTP: add support for trailers in HTTP responses.

Note that we don't use the "HTTP:" prefix.

>
> Example:
>
> ngx_table_elt_t *h;
>
> h = ngx_list_push(&r->headers_out.trailers);
> if (h == NULL) {
> return NGX_ERROR;
> }
>
> ngx_str_set(&h->key, "Fun");
> ngx_str_set(&h->value, "with trailers");
> h->hash = ngx_hash_key_lc(h->key.data, h->key.len);
>
> The code above adds "Fun: with trailers" trailer to the response to
> the request with "TE: trailers" header (which indicates support for
> trailers).
>
> Modules that want to emit trailers must set r->expect_trailers = 1,
> otherwise they are going to be ignored.
>
> This change also adds $sent_trailer_* variables.
>
> Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Overral, I see at least the following problems with the approach
taken:

1. The behaviour depends on the "TE: trailers" header - trailers
are not sent if the client did not used it.

This is not what HTTP specification says though. Trailers can be
sent regardless of "TE: trailers" if they are optional. Moreover,
"TE: trailers" does not guarantee anything either, see
https://github.com/httpwg/http11bis/issues/18.

2. The code doesn't try to send trailers if r->expect_trailers is
not set even if we can do so in a particular connection. This
seems to be completely unneed limitation.

3. The approach with headers and trailers clearly separated and
never merged has obvious benefits, but it doesn't allow to use trailers in
header-only responses. This is likely to result in multiple
problems if we'll try to support anything more than just adding
trailers for some responses: e.g., caching will certainly loose
trailers in some cases. The particular patch also creates an
inconsistency between HTTP/1.1 and HTTP/2 by trying to send
trailers in HTTP/2 header-only responses. This is likely to
result in additional problems as well.

See also additional comments below.

Some links to consider:

https://github.com/whatwg/fetch/issues/34
(Fetch API about trailes)

https://github.com/whatwg/fetch/issues/473
(Fetch API, links to issues created in major browsers about
trailers support)

https://bugs.chromium.org/p/chromium/issues/detail?id=691599
(the only issue about trailers support with a discussion; no
trailers support now, security concerns, waiting for other
browsers)

https://github.com/httpwg/http11bis/issues/16
(trailers need to be separated from headers)

https://github.com/httpwg/http11bis/issues/18
("TE: trailers" doesn't seem to be useful)

>
> diff -r 39ff6939266e -r 8af81a0d66c0 src/http/modules/ngx_http_chunked_filter_module.c
> --- a/src/http/modules/ngx_http_chunked_filter_module.c
> +++ b/src/http/modules/ngx_http_chunked_filter_module.c
> @@ -17,6 +17,7 @@ typedef struct {
>
>
> static ngx_int_t ngx_http_chunked_filter_init(ngx_conf_t *cf);
> +static ngx_chain_t *ngx_http_chunked_create_trailers(ngx_http_request_t *r);
>
>
> static ngx_http_module_t ngx_http_chunked_filter_module_ctx = {
> @@ -69,28 +70,33 @@ ngx_http_chunked_header_filter(ngx_http_
> return ngx_http_next_header_filter(r);
> }
>
> - if (r->headers_out.content_length_n == -1) {
> + clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> +
> + if (clcf->chunked_transfer_encoding
> + && r->allow_trailers && r->expect_trailers)
> + {
> + ngx_http_clear_content_length(r);
> + r->chunked = 1;

See above, the logic here looks wrong. There should be no
difference if client used "TE: trailers" or not, as there is no
real difference.

> +
> + } else if (r->headers_out.content_length_n == -1) {
> if (r->http_version < NGX_HTTP_VERSION_11) {
> r->keepalive = 0;
>
> + } else if (clcf->chunked_transfer_encoding) {
> + r->chunked = 1;
> +
> } else {
> - clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> + r->keepalive = 0;
> + }
> + }
>
> - if (clcf->chunked_transfer_encoding) {
> - r->chunked = 1;
> + if (r->chunked) {
> + ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_chunked_filter_ctx_t));
> + if (ctx == NULL) {
> + return NGX_ERROR;
> + }
>
> - ctx = ngx_pcalloc(r->pool,
> - sizeof(ngx_http_chunked_filter_ctx_t));
> - if (ctx == NULL) {
> - return NGX_ERROR;
> - }
> -
> - ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module);
> -
> - } else {
> - r->keepalive = 0;
> - }
> - }
> + ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module);
> }
>
> return ngx_http_next_header_filter(r);
> @@ -201,6 +207,15 @@ ngx_http_chunked_body_filter(ngx_http_re
> b->pos += 2;
> }
>
> + if (r->allow_trailers && r->expect_trailers) {
> + tl->next = ngx_http_chunked_create_trailers(r);
> +
> + if (tl->next != NULL) {
> + b->last -= 2;
> + b->last_buf = 0;
> + }
> + }
> +

Here you only add trailers if

a) client sent "TE: trailers", and
b) it was previously advertized that there will be trailers.

I can't say I agree with this approach. HTTP/1.1 perfectly allows
to send trailers even if there are no "TE: trailers" indicated.
It is also not clear how r->expect_trailers check adds any value
here, as it can be set at any time.

Additionally, the last chunk is created in a suboptimal way -
certainly there is no need to create two buffers, just one with
trailers included should be enough.

> } else if (size > 0) {
> tl = ngx_chain_get_free_buf(r->pool, &ctx->free);
> if (tl == NULL) {
> @@ -230,6 +245,108 @@ ngx_http_chunked_body_filter(ngx_http_re
> }
>
>
> +static ngx_chain_t *
> +ngx_http_chunked_create_trailers(ngx_http_request_t *r)
> +{
> + size_t len;
> + ngx_buf_t *b;
> + ngx_uint_t i;
> + ngx_chain_t *cl;
> + ngx_list_part_t *part;
> + ngx_table_elt_t *header;
> + ngx_http_chunked_filter_ctx_t *ctx;
> +
> + len = 0;
> +
> + part = &r->headers_out.trailers.part;
> + header = part->elts;
> +
> + for (i = 0; /* void */; i++) {
> +
> + if (i >= part->nelts) {
> + if (part->next == NULL) {
> + break;
> + }
> +
> + part = part->next;
> + header = part->elts;
> + i = 0;
> + }
> +
> + if (header[i].hash == 0) {
> + continue;
> + }
> +
> + len += header[i].key.len + sizeof(": ") - 1
> + + header[i].value.len + sizeof(CRLF) - 1;
> + }
> +
> + if (len == 0) {
> + return NULL;
> + }
> +
> + len += sizeof(CRLF) - 1;
> +
> + ctx = ngx_http_get_module_ctx(r, ngx_http_chunked_filter_module);
> +
> + cl = ngx_chain_get_free_buf(r->pool, &ctx->free);
> + if (cl == NULL) {
> + return NULL;
> + }
> +
> + b = cl->buf;
> +
> + b->tag = (ngx_buf_tag_t) &ngx_http_chunked_filter_module;
> + b->temporary = 0;
> + b->memory = 1;
> + b->last_buf = 1;
> +
> + b->start = ngx_palloc(r->pool, len);
> + if (b->start == NULL) {
> + return NULL;
> + }
> +
> + b->end = b->last + len;
> + b->pos = b->start;
> + b->last = b->start;
> +
> + part = &r->headers_out.trailers.part;
> + header = part->elts;
> +
> + for (i = 0; /* void */; i++) {
> +
> + if (i >= part->nelts) {
> + if (part->next == NULL) {
> + break;
> + }
> +
> + part = part->next;
> + header = part->elts;
> + i = 0;
> + }
> +
> + if (header[i].hash == 0) {
> + continue;
> + }
> +
> + ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
> + "http trailer: \"%V: %V\"",
> + &header[i].key, &header[i].value);
> +
> + b->last = ngx_copy(b->last, header[i].key.data, header[i].key.len);
> + *b->last++ = ':'; *b->last++ = ' ';
> +
> + b->last = ngx_copy(b->last, header[i].value.data, header[i].value.len);
> + *b->last++ = CR; *b->last++ = LF;
> + }
> +
> + /* the end of HTTP trailer */
> + *b->last++ = CR; *b->last++ = LF;
> +
> + return cl;
> +}
> +
> +
> static ngx_int_t
> ngx_http_chunked_filter_init(ngx_conf_t *cf)
> {
> diff -r 39ff6939266e -r 8af81a0d66c0 src/http/ngx_http_core_module.c
> --- a/src/http/ngx_http_core_module.c
> +++ b/src/http/ngx_http_core_module.c
> @@ -2477,6 +2477,13 @@ ngx_http_subrequest(ngx_http_request_t *
> return NGX_ERROR;
> }
>
> + if (ngx_list_init(&sr->headers_out.trailers, r->pool, 4,
> + sizeof(ngx_table_elt_t))
> + != NGX_OK)
> + {
> + return NGX_ERROR;
> + }
> +
> cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
> sr->main_conf = cscf->ctx->main_conf;
> sr->srv_conf = cscf->ctx->srv_conf;
> diff -r 39ff6939266e -r 8af81a0d66c0 src/http/ngx_http_request.c
> --- a/src/http/ngx_http_request.c
> +++ b/src/http/ngx_http_request.c
> @@ -27,6 +27,8 @@ static ngx_int_t ngx_http_process_host(n
> ngx_table_elt_t *h, ngx_uint_t offset);
> static ngx_int_t ngx_http_process_connection(ngx_http_request_t *r,
> ngx_table_elt_t *h, ngx_uint_t offset);
> +static ngx_int_t ngx_http_process_te(ngx_http_request_t *r,
> + ngx_table_elt_t *h, ngx_uint_t offset);
> static ngx_int_t ngx_http_process_user_agent(ngx_http_request_t *r,
> ngx_table_elt_t *h, ngx_uint_t offset);
>
> @@ -125,6 +127,10 @@ ngx_http_header_t ngx_http_headers_in[]
> offsetof(ngx_http_headers_in_t, if_range),
> ngx_http_process_unique_header_line },
>
> + { ngx_string("TE"),
> + offsetof(ngx_http_headers_in_t, te),
> + ngx_http_process_te },
> +
> { ngx_string("Transfer-Encoding"),
> offsetof(ngx_http_headers_in_t, transfer_encoding),
> ngx_http_process_header_line },
> @@ -559,6 +565,14 @@ ngx_http_create_request(ngx_connection_t
> return NULL;
> }
>
> + if (ngx_list_init(&r->headers_out.trailers, r->pool, 4,
> + sizeof(ngx_table_elt_t))
> + != NGX_OK)
> + {
> + ngx_destroy_pool(r->pool);
> + return NULL;
> + }
> +
> r->ctx = ngx_pcalloc(r->pool, sizeof(void *) * ngx_http_max_module);
> if (r->ctx == NULL) {
> ngx_destroy_pool(r->pool);
> @@ -1671,6 +1685,63 @@ ngx_http_process_connection(ngx_http_req
>
>
> static ngx_int_t
> +ngx_http_process_te(ngx_http_request_t *r, ngx_table_elt_t *h,
> + ngx_uint_t offset)
> +{
> + u_char *p;
> +
> + if (r->headers_in.te == NULL) {
> + r->headers_in.te = h;
> + }
> +
> + if (r->http_version < NGX_HTTP_VERSION_11) {
> + return NGX_OK;
> + }
> +
> + if (h->value.len == sizeof("trailers") - 1
> + && ngx_memcmp(h->value.data, "trailers", sizeof("trailers") - 1) == 0)
> + {
> + r->allow_trailers = 1;
> + return NGX_OK;
> + }
> +
> +#if (NGX_HTTP_V2)
> +
> + if (r->http_version >= NGX_HTTP_VERSION_20) {
> + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
> + "client sent HTTP/2 request with invalid header value: "
> + "\"TE: %V\"", &h->value);
> +
> + ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
> + return NGX_ERROR;
> + }
> +
> +#endif
> +
> + if (h->value.len < sizeof("trailers") - 1) {
> + return NGX_OK;
> + }
> +
> + p = ngx_strcasestrn(h->value.data, "trailers", sizeof("trailers") - 2);
> + if (p == NULL) {
> + return NGX_OK;
> + }
> +
> + if (p == h->value.data || *(p - 1) == ',' || *(p - 1) == ' ') {
> +
> + p += sizeof("trailers") - 1;
> +
> + if (p == h->value.data + h->value.len || *p == ',' || *p == ' ') {
> + r->allow_trailers = 1;
> + return NGX_OK;
> + }
> + }
> +
> + return NGX_OK;
> +}
> +
> +
> +static ngx_int_t
> ngx_http_process_user_agent(ngx_http_request_t *r, ngx_table_elt_t *h,
> ngx_uint_t offset)
> {
> diff -r 39ff6939266e -r 8af81a0d66c0 src/http/ngx_http_request.h
> --- a/src/http/ngx_http_request.h
> +++ b/src/http/ngx_http_request.h
> @@ -191,6 +191,7 @@ typedef struct {
> ngx_table_elt_t *range;
> ngx_table_elt_t *if_range;
>
> + ngx_table_elt_t *te;
> ngx_table_elt_t *transfer_encoding;
> ngx_table_elt_t *expect;
> ngx_table_elt_t *upgrade;
> @@ -247,6 +248,7 @@ typedef struct {
>
> typedef struct {
> ngx_list_t headers;
> + ngx_list_t trailers;
>
> ngx_uint_t status;
> ngx_str_t status_line;
> @@ -510,6 +512,8 @@ struct ngx_http_request_s {
> unsigned pipeline:1;
> unsigned chunked:1;
> unsigned header_only:1;
> + unsigned allow_trailers:1;
> + unsigned expect_trailers:1;
> unsigned keepalive:1;
> unsigned lingering_close:1;
> unsigned discard_body:1;
> diff -r 39ff6939266e -r 8af81a0d66c0 src/http/ngx_http_variables.c
> --- a/src/http/ngx_http_variables.c
> +++ b/src/http/ngx_http_variables.c
> @@ -38,6 +38,8 @@ static ngx_int_t ngx_http_variable_unkno
> ngx_http_variable_value_t *v, uintptr_t data);
> static ngx_int_t ngx_http_variable_unknown_header_out(ngx_http_request_t *r,
> ngx_http_variable_value_t *v, uintptr_t data);
> +static ngx_int_t ngx_http_variable_unknown_trailer_out(ngx_http_request_t *r,
> + ngx_http_variable_value_t *v, uintptr_t data);
> static ngx_int_t ngx_http_variable_request_line(ngx_http_request_t *r,
> ngx_http_variable_value_t *v, uintptr_t data);
> static ngx_int_t ngx_http_variable_cookie(ngx_http_request_t *r,
> @@ -365,6 +367,9 @@ static ngx_http_variable_t ngx_http_cor
> { ngx_string("sent_http_"), NULL, ngx_http_variable_unknown_header_out,
> 0, NGX_HTTP_VAR_PREFIX, 0 },
>
> + { ngx_string("sent_trailer_"), NULL, ngx_http_variable_unknown_trailer_out,
> + 0, NGX_HTTP_VAR_PREFIX, 0 },
> +
> { ngx_string("cookie_"), NULL, ngx_http_variable_cookie,
> 0, NGX_HTTP_VAR_PREFIX, 0 },
>
> @@ -934,6 +939,16 @@ ngx_http_variable_unknown_header_out(ngx
> }
>
>
> +static ngx_int_t
> +ngx_http_variable_unknown_trailer_out(ngx_http_request_t *r,
> + ngx_http_variable_value_t *v, uintptr_t data)
> +{
> + return ngx_http_variable_unknown_header(v, (ngx_str_t *) data,
> + &r->headers_out.trailers.part,
> + sizeof("sent_trailer_") - 1);
> +}
> +
> +
> ngx_int_t
> ngx_http_variable_unknown_header(ngx_http_variable_value_t *v, ngx_str_t *var,
> ngx_list_part_t *part, size_t prefix)
> diff -r 39ff6939266e -r 8af81a0d66c0 src/http/v2/ngx_http_v2_filter_module.c
> --- a/src/http/v2/ngx_http_v2_filter_module.c
> +++ b/src/http/v2/ngx_http_v2_filter_module.c
> @@ -50,13 +50,17 @@
> #define NGX_HTTP_V2_SERVER_INDEX 54
> #define NGX_HTTP_V2_VARY_INDEX 59
>
> +#define NGX_HTTP_V2_FRAME_ERROR (ngx_http_v2_out_frame_t *) -1
> +
>
> static u_char *ngx_http_v2_string_encode(u_char *dst, u_char *src, size_t len,
> u_char *tmp, ngx_uint_t lower);
> static u_char *ngx_http_v2_write_int(u_char *pos, ngx_uint_t prefix,
> ngx_uint_t value);
> static ngx_http_v2_out_frame_t *ngx_http_v2_create_headers_frame(
> - ngx_http_request_t *r, u_char *pos, u_char *end);
> + ngx_http_request_t *r, u_char *pos, u_char *end, ngx_uint_t fin);
> +static ngx_http_v2_out_frame_t *ngx_http_v2_create_trailers_frame(
> + ngx_http_request_t *r);
>
> static ngx_chain_t *ngx_http_v2_send_chain(ngx_connection_t *fc,
> ngx_chain_t *in, off_t limit);
> @@ -129,12 +133,12 @@ ngx_http_v2_header_filter(ngx_http_reque
> u_char status, *pos, *start, *p, *tmp;
> size_t len, tmp_len;
> ngx_str_t host, location;
> - ngx_uint_t i, port;
> + ngx_uint_t i, port, fin;
> ngx_list_part_t *part;
> ngx_table_elt_t *header;
> ngx_connection_t *fc;
> ngx_http_cleanup_t *cln;
> - ngx_http_v2_out_frame_t *frame;
> + ngx_http_v2_out_frame_t *headers, *trailers;
> ngx_http_core_loc_conf_t *clcf;
> ngx_http_core_srv_conf_t *cscf;
> u_char addr[NGX_SOCKADDR_STRLEN];
> @@ -612,13 +616,6 @@ ngx_http_v2_header_filter(ngx_http_reque
> header[i].value.len, tmp);
> }
>
> - frame = ngx_http_v2_create_headers_frame(r, start, pos);
> - if (frame == NULL) {
> - return NGX_ERROR;
> - }
> -
> - ngx_http_v2_queue_blocked_frame(r->stream->connection, frame);
> -
> cln = ngx_http_cleanup_add(r, 0);
> if (cln == NULL) {
> return NGX_ERROR;
> @@ -627,8 +624,32 @@ ngx_http_v2_header_filter(ngx_http_reque
> cln->handler = ngx_http_v2_filter_cleanup;
> cln->data = r->stream;
>
> + if (r->header_only && r->allow_trailers && r->expect_trailers) {
> + trailers = ngx_http_v2_create_trailers_frame(r);
> + if (trailers == NGX_HTTP_V2_FRAME_ERROR) {
> + return NGX_ERROR;
> + }
> +
> + fin = trailers ? 0 : 1;

This creates a serious inconsistency between HTTP/1.1 and HTTP/2 by
sending trailers in header-only responses with HTTP/2, but
not HTTP/1.1.

Logically, trailer headers make no sense in a response without a
body: if there is no body, there should be no trailer headers, and
all headers should be normal headers.

This also brings back the old question of merging trailer headers
and normal headers. It doesn't seem to be possible to properly
return trailers via HTTP/1.1 if there are 304 reponses and/or HEAD
requests without merging them with normal headers. Yet we already
agreed that merging is bad idea from security point of view.

I also not sure how HTTP/2 clients would interpret such "two
HEADERS frames". While it looks formally permitted by RFC 7540
(for no aparent reason), I'm pretty sure it will cause various
problems.

(It might also be a good idea to keep HTTP/2 changes in a separate
patch.)

> +
> + } else {
> + trailers = NULL;
> + fin = r->header_only;
> + }
> +
> + headers = ngx_http_v2_create_headers_frame(r, start, pos, fin);
> + if (headers == NULL) {
> + return NGX_ERROR;
> + }
> +
> + ngx_http_v2_queue_blocked_frame(r->stream->connection, headers);
> r->stream->queued = 1;
>
> + if (trailers) {
> + ngx_http_v2_queue_blocked_frame(r->stream->connection, trailers);
> + r->stream->queued++;
> + }
> +
> fc->send_chain = ngx_http_v2_send_chain;
> fc->need_last_buf = 1;
>
> @@ -636,6 +657,129 @@ ngx_http_v2_header_filter(ngx_http_reque
> }
>
>
> +static ngx_http_v2_out_frame_t *
> +ngx_http_v2_create_trailers_frame(ngx_http_request_t *r)
> +{
> + u_char *pos, *start, *tmp;
> + size_t len, tmp_len;
> + ngx_uint_t i;
> + ngx_list_part_t *part;
> + ngx_table_elt_t *header;
> + ngx_http_v2_out_frame_t *frame;
> +
> + len = 0;
> + tmp_len = 0;
> +
> + part = &r->headers_out.trailers.part;
> + header = part->elts;
> +
> + for (i = 0; /* void */; i++) {
> +
> + if (i >= part->nelts) {
> + if (part->next == NULL) {
> + break;
> + }
> +
> + part = part->next;
> + header = part->elts;
> + i = 0;
> + }
> +
> + if (header[i].hash == 0) {
> + continue;
> + }
> +
> + if (header[i].key.len > NGX_HTTP_V2_MAX_FIELD) {
> + ngx_log_error(NGX_LOG_WARN, r->connection->log, 0,
> + "too long response trailer name: \"%V\"",
> + &header[i].key);
> +
> + return NGX_HTTP_V2_FRAME_ERROR;
> + }
> +
> + if (header[i].value.len > NGX_HTTP_V2_MAX_FIELD) {
> + ngx_log_error(NGX_LOG_WARN, r->connection->log, 0,
> + "too long response trailer value: \"%V: %V\"",
> + &header[i].key, &header[i].value);
> +
> + return NGX_HTTP_V2_FRAME_ERROR;
> + }
> +
> + len += 1 + NGX_HTTP_V2_INT_OCTETS + header[i].key.len
> + + NGX_HTTP_V2_INT_OCTETS + header[i].value.len;
> +
> + if (header[i].key.len > tmp_len) {
> + tmp_len = header[i].key.len;
> + }
> +
> + if (header[i].value.len > tmp_len) {
> + tmp_len = header[i].value.len;
> + }
> + }
> +
> + if (len == 0) {
> + return NULL;
> + }
> +
> + tmp = ngx_palloc(r->pool, tmp_len);
> + pos = ngx_pnalloc(r->pool, len);
> +
> + if (pos == NULL || tmp == NULL) {
> + return NGX_HTTP_V2_FRAME_ERROR;
> + }
> +
> + start = pos;
> +
> + part = &r->headers_out.trailers.part;
> + header = part->elts;
> +
> + for (i = 0; /* void */; i++) {
> +
> + if (i >= part->nelts) {
> + if (part->next == NULL) {
> + break;
> + }
> +
> + part = part->next;
> + header = part->elts;
> + i = 0;
> + }
> +
> + if (header[i].hash == 0
> + || header[i].key.len > NGX_HTTP_V2_MAX_FIELD
> + || header[i].value.len > NGX_HTTP_V2_MAX_FIELD)
> + {
> + continue;
> + }
> +
> +#if (NGX_DEBUG)
> + if (r->connection->log->log_level & NGX_LOG_DEBUG_HTTP) {
> + ngx_strlow(tmp, header[i].key.data, header[i].key.len);
> +
> + ngx_log_debug3(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
> + "http2 output trailer: \"%*s: %V\"",
> + header[i].key.len, tmp, &header[i].value);
> + }
> +#endif
> +
> + *pos++ = 0;
> +
> + pos = ngx_http_v2_write_name(pos, header[i].key.data,
> + header[i].key.len, tmp);
> +
> + pos = ngx_http_v2_write_value(pos, header[i].value.data,
> + header[i].value.len, tmp);
> + }
> +
> + frame = ngx_http_v2_create_headers_frame(r, start, pos, 1);
> + if (frame == NULL) {
> + return NGX_HTTP_V2_FRAME_ERROR;
> + }
> +
> + return frame;
> +}
> +
> +
> static u_char *
> ngx_http_v2_string_encode(u_char *dst, u_char *src, size_t len, u_char *tmp,
> ngx_uint_t lower)
> @@ -686,7 +830,7 @@ ngx_http_v2_write_int(u_char *pos, ngx_u
>
> static ngx_http_v2_out_frame_t *
> ngx_http_v2_create_headers_frame(ngx_http_request_t *r, u_char *pos,
> - u_char *end)
> + u_char *end, ngx_uint_t fin)
> {
> u_char type, flags;
> size_t rest, frame_size;
> @@ -707,12 +851,12 @@ ngx_http_v2_create_headers_frame(ngx_htt
> frame->stream = stream;
> frame->length = rest;
> frame->blocked = 1;
> - frame->fin = r->header_only;
> + frame->fin = fin;
>
> ll = &frame->first;
>
> type = NGX_HTTP_V2_HEADERS_FRAME;
> - flags = r->header_only ? NGX_HTTP_V2_END_STREAM_FLAG : NGX_HTTP_V2_NO_FLAG;
> + flags = fin ? NGX_HTTP_V2_END_STREAM_FLAG : NGX_HTTP_V2_NO_FLAG;
> frame_size = stream->connection->frame_size;
>
> for ( ;; ) {
> @@ -774,7 +918,7 @@ ngx_http_v2_create_headers_frame(ngx_htt
> continue;
> }
>
> - b->last_buf = r->header_only;
> + b->last_buf = fin;
> cl->next = NULL;
> frame->last = cl;
>
> @@ -796,7 +940,7 @@ ngx_http_v2_send_chain(ngx_connection_t
> ngx_http_request_t *r;
> ngx_http_v2_stream_t *stream;
> ngx_http_v2_loc_conf_t *h2lcf;
> - ngx_http_v2_out_frame_t *frame;
> + ngx_http_v2_out_frame_t *frame, *trailers;
> ngx_http_v2_connection_t *h2c;
>
> r = fc->data;
> @@ -870,6 +1014,8 @@ ngx_http_v2_send_chain(ngx_connection_t
> frame_size = (h2lcf->chunk_size < h2c->frame_size)
> ? h2lcf->chunk_size : h2c->frame_size;
>
> + trailers = NULL;
> +
> #if (NGX_SUPPRESS_WARN)
> cl = NULL;
> #endif
> @@ -932,17 +1078,36 @@ ngx_http_v2_send_chain(ngx_connection_t
> size -= rest;
> }
>
> - frame = ngx_http_v2_filter_get_data_frame(stream, frame_size, out, cl);
> - if (frame == NULL) {
> - return NGX_CHAIN_ERROR;
> + if (cl->buf->last_buf && r->allow_trailers && r->expect_trailers) {
> + trailers = ngx_http_v2_create_trailers_frame(r);
> + if (trailers == NGX_HTTP_V2_FRAME_ERROR) {
> + return NGX_CHAIN_ERROR;
> + }
> +
> + if (trailers) {
> + cl->buf->last_buf = 0;
> + }
> }
>
> - ngx_http_v2_queue_frame(h2c, frame);
> + if (frame_size || cl->buf->last_buf) {
> + frame = ngx_http_v2_filter_get_data_frame(stream, frame_size, out,
> + cl);
> + if (frame == NULL) {
> + return NGX_CHAIN_ERROR;
> + }
>
> - h2c->send_window -= frame_size;
> + ngx_http_v2_queue_frame(h2c, frame);
>
> - stream->send_window -= frame_size;
> - stream->queued++;
> + h2c->send_window -= frame_size;
> +
> + stream->send_window -= frame_size;
> + stream->queued++;
> + }
> +
> + if (trailers) {
> + ngx_http_v2_queue_frame(h2c, trailers);
> + stream->queued++;
> + }
>
> if (in == NULL) {
> break;
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel

--
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel