[PATCH 1 of 3] HTTP: add support for trailers in HTTP responses

Maxim Dounin mdounin at mdounin.ru
Fri Apr 21 17:42:01 UTC 2017


Hello!

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

> # HG changeset patch
> # User Piotr Sikora <piotrsikora at 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 at 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 at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel

-- 
Maxim Dounin
http://nginx.org/


More information about the nginx-devel mailing list