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

Maxim Dounin mdounin at mdounin.ru
Thu Apr 27 18:46:23 UTC 2017


Hello!

On Wed, Apr 26, 2017 at 11:35:02AM -0700, Piotr Sikora via nginx-devel wrote:

> > Note that we don't use the "HTTP:" prefix.
> 
> Maybe it's time to start using it, then? Otherwise, commit messages
> are inconsistent.

We are happy enough with the current practice.

> > Overral, I see at least the following problems with the approach
> > taken:
> >
> > 1. The behaviour depends on the "TE: trailers" header - trailers
> > are not sent if the client did not used it.
> >
> > This is not what HTTP specification says though.  Trailers can be
> > sent regardless of "TE: trailers" if they are optional.  Moreover,
> > "TE: trailers" does not guarantee anything either, see
> > https://github.com/httpwg/http11bis/issues/18.
> 
> From RFC7230, sec. 4.3:
> 
>    The presence of the keyword "trailers" indicates that the client is
>    willing to accept trailer fields in a chunked transfer coding, as
>    defined in Section 4.1.2, on behalf of itself and any downstream
>    clients.
> 
> and sec. 4.1.2:
> 
>    Unless the request includes a TE header field indicating "trailers"
>    is acceptable, as described in Section 4.3, a server SHOULD NOT
>    generate trailer fields that it believes are necessary for the user
>    agent to receive.  Without a TE containing "trailers", the server
>    ought to assume that the trailer fields might be silently discarded
>    along the path to the user agent.

As you can see from the quote, it talks about not generating 
"trailer fields that it believes are necessary for the user agent 
to receive".  RFC 2616 is even more clear on this, specifically 
lists two cases when trailers can be generated, section 3.6.1:

   A server using chunked transfer-coding in a response MUST NOT use the
   trailer for any header fields unless at least one of the following is
   true:

   a)the request included a TE header field that indicates "trailers" is
     acceptable in the transfer-coding of the  response, as described in
     section 14.39; or,

   b)the server is the origin server for the response, the trailer
     fields consist entirely of optional metadata, and the recipient
     could use the message (in a manner acceptable to the origin server)
     without receiving this metadata.  In other words, the origin server
     is willing to accept the possibility that the trailer fields might
     be silently discarded along the path to the client.

That is, there clearly two cases when a server can / should send 
trailers:

   - when there is "TE: trailers";

   - when trailers are optional.

Moreover, given that "TE: trailers" does _not_ guarantee anything 
either (see link above), the only remaining case seems to be "when 
trailers are optional".

> Also, in practice, most (all?) clients that consume trailers always
> send "TE: trailers" header, so I'm not sure why do you think that's
> invalid behavior.

As per previous discussion, main cases, as listed by you, are:

1. Checksums, like Content-SHA256.
2. Logging, like X-Log-Something for centralized logging on a 
   frontend.
3. Trailing status, like X-Status, to provide additional error 
   information to a frontend.

All these uses cases hardly require "TE: trailers", as in all 
cases information seems optional.  Moreover, in cases (2) and (3), 
backend cannot have "TE: trailers" unless it was already present 
in the original request from the client.  Adding such a header 
would contradict RFC 7230, which says:

   The presence of the keyword "trailers" indicates that the client is
   willing to accept trailer fields in a chunked transfer coding, as
   defined in Section 4.1.2, on behalf of itself and any downstream
   clients.  For requests from an intermediary, this implies that
   either: (a) all downstream clients are willing to accept trailer
   fields in the forwarded response; or, (b) the intermediary will
   attempt to buffer the response on behalf of downstream recipients.

So both cases (2) and (3) generally require that "TE: trailers" 
should be ignored, or it won't be possible to implement them.

> I'm not particularly interested in fighting over this for another X
> months, so I'm going to remove this requirement, but for the record, I
> think that's a mistake. Especially, since enabling trailers will spam
> browsers (which at this point don't care about trailers) with
> unnecessary traffic.
> 
> Also, repeating myself from last year:
> 
> I can drop this requirement if you insist, but that's much less
> conservative approach than NGINX usually takes and I expect that some
> obscure HTTP clients will break because of lack of proper support for
> trailer-part in chunked encoding.

Current nginx behaviour is to don't emit trailers by default, and 
I don't think we anyhow change this beaviour unless explicitly 
requested via appropriate configuration directives.  This looks 
conservative enough for me, actually.

> > 2. The code doesn't try to send trailers if r->expect_trailers is
> > not set even if we can do so in a particular connection.  This
> > seems to be completely unneed limitation.
> 
> This was added in response to your comments about original patch
> forcing chunked encoding, even when trailers were not being emitted.
> 
> Using r->expect_trailers allows trailer producers (headers filter,
> proxy module, etc.) to indicate that trailers might be produced and
> therefore force chunked encoding in HTTP/1.1 requests when needed.
> 
> Also, I'm not sure why do you think that's unnecessary limitation,
> since r->expect_trailers will be always set if there are trailers.

I can see that r->expect_trailers can be useful to indicate that 
the code wants to use trailers, and would like to force chunked 
encoding if possible.

I don't see why it should be required when encoding is already 
choosen though.  If we know that we can use trailers or don't 
care, we can just add trailers to the response, and assume they 
will be sent if it is possible to do so.

> > 3. The approach with headers and trailers clearly separated and
> > never merged has obvious benefits, but it doesn't allow to use trailers in
> > header-only responses.  This is likely to result in multiple
> > problems if we'll try to support anything more than just adding
> > trailers for some responses: e.g., caching will certainly loose
> > trailers in some cases.  The particular patch also creates an
> > inconsistency between HTTP/1.1 and HTTP/2 by trying to send
> > trailers in HTTP/2 header-only responses.  This is likely to
> > result in additional problems as well.
> 
> What problems, exactly?

If two identical requests over HTTP/1.1 and HTTP/2 return 
different results - this looks like a problem for me.  Sooner or 
later this difference will become ciritical in some workload, 
leading to hard-to-diagnose bugs.

> > This creates a serious inconsistency between HTTP/1.1 and HTTP/2 by
> > sending trailers in header-only responses with HTTP/2, but
> > not HTTP/1.1.
> 
> There are no means for sending trailers in HTTP/1.1 responses without
> a body, and I don't see a reason why we should cripple HTTP/2, simply
> because HTTP/1.1 didn't support this.

As per HTTP specification, trailers are expected to be sent as 
normal headers when there is no body, for both HTTP/1.1 and 
HTTP/2.  And I suspect that the fact that HTTP/2 allows two 
HEADERS frames without body inbetween is more or less 
unintentional, too.

> Let me know if this is a blocker for you or not.

I certainly against intruduction of such a difference between 
HTTP/1.1 and HTTP/2 behaviour.

> > Logically, trailer headers make no sense in a response without a
> > body: if there is no body, there should be no trailer headers, and
> > all headers should be normal headers.
> 
> I disagree. Trailers are separate from headers and there are valid use
> cases (for example, calculating checksum of uploaded object), where
> delaying headers until trailer value is calculated could result in
> timeouts and/or invalid retries.

Quoting RFC 7320, section 4.1.2:

   The trailer fields are identical to header fields, except
   they are sent in a chunked trailer instead of the message's header
   section.

> > This also brings back the old question of merging trailer headers
> > and normal headers.  It doesn't seem to be possible to properly
> > return trailers via HTTP/1.1 if there are 304 reponses and/or HEAD
> > requests without merging them with normal headers.  Yet we already
> > agreed that merging is bad idea from security point of view.
> 
> Just don't send trailers in 304 and/or HEAD requests, when using HTTP/1.1.

Sure, but this breaks the original idea of trailers, and 
introduces various cases when they simply don't work.

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


More information about the nginx-devel mailing list