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

[Piotr Sikora]

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.

> There should be a space between "NULL" and "}".  It might worth
> fixing the style in previously exiting directives in a separate
> commit.

Done.

> 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

> Note: this can result in an access to uninitialized memory if
> ngx_pnalloc() fails and $sent_http_trailer used in logs.

Fixed.

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

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

> Note well that with the approach taken it will be very
> inconvenient for other modules to add their own trailers, due to
> both a) layering violation introduced, which makes headers filter
> special, and b) no easy way to add anything to the Trailer header
> generated.

This is well-formatted header, so other modules can emit their own
"Trailer" headers, or append to it.

Also, I'm don't see how this is different from virtually any other
header in NGINX.

> There is no need to set hash for output headers, just t->hash = 1
> is good enough.

Actually, it's not "good enough". Using "hash = 1" has absolutely no
performance benefit and it results in full search instead of simple
hash comparison during header lookup, because some headers won't have
hash set to proper value.

Anyway, I'm going set it to "1" to avoid side-tracking this discussion.

Best regards,
Piotr Sikora