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/ _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel