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

Maxim Dounin mdounin at mdounin.ru
Fri Jun 2 16:20:39 UTC 2017


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 { 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

More information about the nginx-devel mailing list