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

Maxim Dounin mdounin at mdounin.ru
Fri Jun 2 12:30:35 UTC 2017


Hello!

On Fri, Jun 02, 2017 at 02:05:47AM -0700, Piotr Sikora via nginx-devel wrote:

> Hey Maxim,
> 
> > In your patch, you test r->expect_trailers in two places in
> > chunked filter:
> >
> > 1. when you decide whether to use chunked encoding or not, in
> >    ngx_http_chunked_header_filter();
> >
> > 2. when you generate trailer, in ngx_http_chunked_body_filter().
> >
> > I mostly agree with (1) (I would like see it configurable too, but
> > it's probably up to a module which adds trailers to decide, so
> > don't belong to this particular patch), but don't see any reasons
> > for (2).
> 
> This is done to avoid unexpected behavior when r->expect_trailers
> isn't set, i.e. if module added trailers, but didn't set
> r->expect_trailers, then without (2) trailers would be emitted only if
> Content-Encoding was already chunked, but not otherwise.
> 
> Testing r->expect_trailers in both (1) and (2) ensures that Trailers
> are emitted in consistent manner, without depending on unrelated
> factors, like gzip support by the client, etc.

I see two problems here:

a. There may be use cases when forcing chunked encoding is not 
desired, but emitting trailers if it is used still makes sense.

b. Nothing stops modules from changing r->expect_trailers when the 
response header was already sent and it is already too late to 
switch to chunked transfer encoding.  Moreover, this will 
naturally happen with any module which is simply following the 
requirement to set r->expect_trailers to 1 as in your commit log.

So (a) makes (2) excessively limiting, and (b) makes it useless.

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


More information about the nginx-devel mailing list