[PATCH 2 of 3] Headers filter: add "add_trailer" directive

[Maxim Dounin]

Hello!

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

> Hey Maxim,
> 
> > This introduces a layering violation between the headers filter
> > and the chunked filter.
> 
> I moved trailer generation to headers filter, let me know if that works for you.
> 
> Unfortunately, because of that change, trailers are evaluated earlier
> in the process, and some variables from body filters that run after
> headers filter (like $gzip_ratio) are not usable anymore.

This looks better.  If there is a need to evaluate trailers later, 
a separate module could be added to a later stage, like it is done 
with ngx_http_range_header_filter vs. ngx_http_range_body_filter.

[...]

> > Predicting conditions which will result in header-only response
> > looks really fragile.  As well as checks for chunked transfer
> > encoding.
> 
> Sending "Trailer" header in responses that cannot have response body
> is known to result in interoperability issues, so I'd rather leave it,
> see:
> https://github.com/nodejs/node/issues/2842

So may be not generating "Trailer" is a way to go?
It doesn't seem to be really needed.

(Just for the record, with the first patch fixed to avoid using 
chunked with HTTP/1.0, the "Trailer" header is expectedly still 
added with HTTP/1.0.  This confirms the idea that the approach 
choosen is somewhat fragile.)

[...]

> > Note that at this point we do not know if the trailer in question
> > is going to be present in the particular response or not, as
> > trailers with empty values are not returned (and this is often
> > used to only return some headers in some responses, at least with
> > add_header).
> 
> But we know that it might be present.
> 
> "Trailer" header is an indicator.

The question is: if we need this indicator to be sent to a 
particular client.

For example, if you are using trailers to pass additional logging 
information to your own frontends, and use something like

    geo $mine {
        127.0.0.1/8 1;
    }

    map $mine $x_request_time {
        1     $request_time;
    }

    add_trailer X-Response-Time $x_request_time;

to send the information to your frontends, but not other clients, 
you probably don't want the X-Response-Time trailer to be 
indicated to other clients.

> > It might be a better idea to avoid generating the
> > "Trailer" header automatically and let users to control this with
> > add_header instead, and/or introduce an option to control this.
> 
> That sounds like a terrible idea, which will result in always missing
> "Trailer" header.

Acutally I don't see how it's a problem, given that "Trailer" is 
not something required.  Moreover, it seems to be not needed or 
even harmful in most of the use cases discussed.

[...]

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