[PATCH 1 of 3] Added support for trailers in HTTP responses
Piotr Sikora
piotrsikora at google.com
Sat Jun 3 03:32:31 UTC 2017
Hey Maxim,
> Note: the "TE: trailers" requirement is no longer present in the
> code.
Good catch, thanks!
> This code results in using chunked encoding for HTTP/1.0 when
> trailers are expected. Such behaviour is explicitly forbidden by
> the HTTP/1.1 specification, and will very likely result in
> problems (we've seen lots of such problems with broken backends
> when there were no HTTP/1.1 support in the proxy module).
Oops, this regression is a result of removal of r->accept_trailers,
which previously disallowed trailers in HTTP/1.0 requests.
> Something like this should be a better solution:
>
> if (r->headers_out.content_length_n == -1
> || r->expect_trailers)
> {
> clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
>
> if (r->http_version >= NGX_HTTP_VERSION_11
> && clcf->chunked_transfer_encoding)
> {
> if (r->expect_trailers) {
> ngx_http_clear_content_length(r);
> }
>
> r->chunked = 1;
>
> 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 if (r->headers_out.content_length_n == -1) {
> r->keepalive = 0;
> }
> }
Applied with small style changes.
> Instead of providing two separate code paths for "with trailer
> headers" and "without trailer headers", it might be better and
> more readable to generate last-chunk in one function regardless of
> whether trailer headers are present or not.
>
> It will also make error handling better: as of now, an allocation
> error in ngx_http_chunked_create_trailers() will result in "no
> trailers" code path being tried instead of returning an
> unconditional error.
Done.
> There is no need to write sizeof() so many times, just
>
> len += sizeof(CRLF "0" CRLF CRLF) - 1;
>
> would be enough.
Done.
Best regards,
Piotr Sikora
More information about the nginx-devel
mailing list