Aspnetcore: HTTP/1.1 Allow Response Trailer Headers

[benaadams]

Now (shortly?) reading request header trailers #619; ability to add them to chunked encoding responses?

Currently supported by System.Web (Misleading)

Don't think browsers pay attention to them; but some discussion on https://github.com/whatwg/fetch/issues/34 as to their importance for CDNs etc; and whether to include them in the fetchapi

[benaadams]

Would need design

[halter73]

I don't think this is super important.

If you really want this, I think you should be willing to manually chunk your response which you can do if you explicitly set Transfer-Encoding: chunked. So nothing is topping you from sending response trailing headers today. We just don't make it easy.

[madelson]

@halter73 how do you send response trailing headers using existing APIs?

[halter73]

@madelson I'm not aware of any way to do this using HttpClient APIs. You would probably have to build something on raw sockets.

[Tratcher]

Proposal: Request and response trailer headers belong in their own collections. These should get there own features. These features should also have a HasStarted type flag indicating if the trailers have been started or finished yet. The response trailers feature may even have a Flush method to end the body and send the trailers immediately.

Today kestrel puts request trailers into the normal request headers collection, but this may cause conflicts or concurrent modification errors.

[NickCraver]

I hit a desire for these today, in MiniProfiler. There's now a Server-Timing header, which really can't get its content until quite late in the pipe. While I can send the profiler ID (which is saved at the end of a request, but fetched in another request), putting the entirety of the profiling summary in a header send before anything happens has some fundamental issues with respect to our current linear understanding of space-time.

But Trailers! It's ideal here. At the end of the request (the very last step pretty much), we have a complete picture of what happened. In fact in the examples of Server-Timing, it's even recommended. This is definitely a feature I'd use in MiniProfiler, and I totally agree with them being in their own collection (of the same type).

There is a minor question open here, where in the initial headers collection needs knowledge a trailer exists. I'm not sure the best API for that - perhaps there's just a method (called before headers are sent) that takes a header name, which registers them for the Trailers header sent in the initial collection?

[cesarblum]

There is a minor question open here, where in the initial headers collection needs knowledge a trailer exists. I'm not sure the best API for that - perhaps there's just a method (called before headers are sent) that takes a header name, which registers them for the Trailers header sent in the initial collection?

Yeah, you'd have to signal that you'll set a trailer before the response is started. Otherwise the response would be invalid. If you set trailers early enough that could be done automatically, but this wouldn't work for things like Server-Timing where you set the trailer just as you're finishing the response.

[NickCraver]

Just throwing out ideas - perhaps something like this:
c# context.Response.RegisterTrailerHeader("Server-Timing");

[Drawaes]

What if the body ends up not being chunked?

[Drawaes]

How about this?
Sample Code

[Tratcher]

Why not just response.Headers.Append("Trailers", "Server-Timing");?

[Drawaes]

Does it write the trailing header?

[Drawaes]

There are 2 issues you have a header you need to write... that has which headers are in the trailer and write the trailing headers themselves. This middleware takes a call back to write the header content later. When you register it indeed populates the response.headers ["Trailers"] for you but it also takes over chunking for you and then calls your call back after writing the zero chunk but before closing doing the last /r/n

Why they didn't call them footers instead of trailing footers I guess will remain one of life's great mysteries

[NickCraver]

@Drawaes much appreciated - I agree that's a tenable solution (some perf increases to be had there), but not something I'd consider doing in a library...that's quite dramatic on the impact to an application to change out in middleware of a profiling library (IMO).

I think that yes we can do this, but without core support, it'd be ethically questionable for me to do that ;)

[Drawaes]

Ethics, meh. Yeah it's a POC not prod ready code, for instance it doesn't handle the case where no body should be sent. But it's tenable, As for the library question if it's a feature people enable or disable then it's up to them. As for the perf if it was optimised, should be able to be pretty much a single method dispatch the rest of the code is very similar to what the chunker is doing now it's just been replaced with a custom one.

As with all things direct framework support would be better.

[NickCraver]

@Drawaes My point is: if MiniProfiler changed an app's transfer encoding, that'd be really bad and completely unexpected. Framework support is essential here, for several reasons:

1. What if multiple Middlewares wish to add trailers?
2. How would we combine the Trailers header? (it should be comma delimited, I'm not sure if multiple headers are allowed or supported)
3. What if the middleware isn't absolutely last?
4. What if we're not chunked at all?
5. It forces the middleware's position in the pipe (to being last) if in a single package, e.g. in MiniProfiler.

There are just too many things to break here, and I'm very adamant this is outside the purview of a profiler. We'd need framework support for the case where the response isn't chunked, and the header behave as a normal header as well. Since all the pieces need to work together here, I think it being core functionality that contains this logic that we call instead of provide is critical, in this specific case.

[Drawaes]

I agree... of course if 10 apps want to add trailers they can all use the same middleware ;) Ssl for instance isn't baked in nor is connection logging so it could be a connection filter ( or adapter) in the future.

[Drawaes]

What is interesting.... HttpClient works (read ignores) these on windows, but on ubuntu crashes with

System.Net.Http.HttpRequestException : Error while copying content to a stream.
---- System.IO.IOException : The read operation failed, see inner exception.
-------- System.Net.Http.CurlException : Failed writing received data to disk/application

https://github.com/dotnet/corefx/issues/17174

[NickCraver]

Update on motivation for support here - Chrome is landing support for server timings in M65: https://blog.chromium.org/2018/02/chrome-65-beta-css-paint-api-and.html

[serialseb]

Same issue here.

You need to specify which trailers you want before you start sending the response. I'd expect to register the trailers i want when i initialise my code in the request pipeline, specifying i want to send certain headers. This has to be done before the body starts being sent, and forces chunked encoding. After the entity body is sent, i get a OnSendingTrailerHeaders() notification, and the fx puts whatever registered trailers were specified in the trailer part.

This would allow us to use Server-Timings more efficiently than today, where I basically can't measure anything from the moment i start writing stuff when the response is buffered.

Currently openrasta doesn't try and force one mode over the other, and relies on whatever is given to it by the hosting environment (as owin, kestrel, httplistener and systemweb have different behaviours there, and we still support multi hosting).

As it stands, if the response coding part of the pipeline has metrics, we can't put those in the header, unless we magically unable buffering, which has memory implications.

[Tratcher]

Design notes:

This applies to both HTTP/1.1 and HTTP/2, but is more relevant for HTTP/2.

Detecting trailers support?

• Should we do any TE header processing to detect client support for trailers? https://tools.ietf.org/html/rfc7230#section-4.3. Or do we leave leave that as an exercise for the application and just assume they'll work. TE is a strong hint, but we're free to ignore it's absence if the trailers we're sending are optional data.
• What about HTTP/1.0 requests? Should users have to check for that (fragile)? Or (preferred) should we log and silently discard trailers if they aren't supported?

Declaring trailers:

• Primarily sugar for response.Headers.Append("Trailers", "Server-Timing");
• Response extension method response.DeclareTrailer(string name)
• Response.HasStarted should be checked before declaring trailers, like adding any header.
• To what degree should the presence of this header be used as a signal to force chunking? E.g. if the app sets the content-length header should we remove it again and chunk? (http/1.1 only)
• Note the response headers can be cleared by error handling components.

Adding trailers:

• OnSendingHeaders has been a problematic API for mutating response headers and I'd like to avoid repeating that callback pattern for trailers. A callback should not be as appealing for trailers since most components/middleware should have a chance to add trailers directly before they're ultimately sent by the server at the end of the request. Only a manual flush would prevent that.
• Provide an IHeaderDictionary for components to write to.
• There shouldn't be any special concurrency issues here (as opposed to request trailers), only one thread should be controlling the HttpContext at a time and then the headers get sent at the end.

Feature: IHttpResponseTrailersFeature

• IHeaderDictionary Trailers - Normal header collection, throws if modified after trailers are sent. No strong verification that the items you add were listed in the response Trailers header.
• bool? TrailersEnabled - Null if the response as not started. True if the response has started, at least one Trailer was declared, and the response body is chunked or http/2.
• bool TrailersSent - True after the trailers have been sent. Should be checked before modifying the collection.
• Task FlushAsync(CancellationToken) - End the response body and send the headers immediately (even if there aren't any). If the response has not been started then it will also be sent (compare to response.body.FlushAsync()).

Cross server notes:

• Unlike request trailers, we should be able to implement this in HttpSysServer (http/1.1, not http/2?) and maybe even IIS in-proc as we have more control over the response body. IIS (ANCM) as a reverse proxy still won't support it though.

[halter73]

Should we do any TE header processing to detect client support for trailers?

I would let apps do that themselves if they want.

What about HTTP/1.0 requests?

I agree that we should discard and log.

To what degree should the presence of this header be used as a signal to force chunking?

If the DeclareTrailer API is used, I like being strict. If the Trailers header is manually added, I would give maximum leeway to the app. So throw when DeclareTrailer is called if another header like Content-Length that would disable chunking has already been set. Similarly, throw when setting Content-Length if it's done after calling DeclareTrailer.

Some more questions:

• Should we add a IHttpRequestTrailersFeature?
  
  
  
  • If so, could we do this in a backwards compatible way?
  
  
• RFC 7230 Section 4.1.2 states "the sender SHOULD generate a Trailer header field before the message body to indicate which fields will be present in the trailers". Do we want to provide a mechanism for sending trailers without a corresponding Trailer header before the body?
• What if a trailer is declared but not sent? Should we log a warning or something?
• Can you add trailers if the response hasn't started and TrailersEnabled is null?
  
  
  
  • If so, what happens if the Content-Length header is set or later becomes set?
  
  
• Does IHttpResponseTrailersFeature.FlushAsync() get called implicitly if trailers are added and the app doesn't call it?
• Should IHttpResponseFeature.OnCompleted callbacks be called before or after sending trailers?
  
  
  
  • If we don't want to have callbacks mutating the dictionary, I would argue for after.
  
  

I don't have a recommendation yet, but TrailersEnabled isn't very self-explanatory. It being null before the response starts isn't very intuitive. I think I prefer it being true for HTTP/1.1 requests until some header is set that would prevent chunking.

Based on the name alone, I would expect this to communicate client support for trailers instead of verifying the the response was set up correctly, so maybe we can come up with a better name too.

[Tratcher]

Throwing from DeclareTrailer or ContentLength seems like a poor user experience, they'll likely be called by independent components (e.g. tracing middleware and StaticFiles). Fixup or discard would be better, but it might have to happen at the server level when preparing the headers to send.

Should we add a IHttpRequestTrailersFeature?
If so, could we do this in a backwards compatible way?

Tracking request trailers in https://github.com/aspnet/KestrelHttpServer/issues/2051.

RFC 7230 Section 4.1.2 states "the sender SHOULD generate a Trailer header field before the message body to indicate which fields will be present in the trailers". Do we want to provide a mechanism for sending trailers without a corresponding Trailer header before the body?

I think so, it's only a SHOULD. I covered this above as "No strong verification that the items you add were listed in the response Trailers header."

What if a trailer is declared but not sent? Should we log a warning or something?

Hmm. A warning seems a bit strong, debug maybe. Trailers can't be un-declared after the headers are sent if something goes wrong.

Can you add trailers if the response hasn't started and TrailersEnabled is null?

Yes, but you can't guarantee they'll get sent.

If so, what happens if the Content-Length header is set or later becomes set?

I would expect Trailers to commonly be declared before Content-Length, henice the discard or fixup question above.

Does IHttpResponseTrailersFeature.FlushAsync() get called implicitly if trailers are added and the app doesn't call it?

Yes, by the server at the end of the request. It's the new WriteSuffix.

Should IHttpResponseFeature.OnCompleted callbacks be called before or after sending trailers?
If we don't want to have callbacks mutating the dictionary, I would argue for after.

After. That's a final Dispose that is not guaranteed any access to the HttpContext.

TrailersEnabled

Agreed it needs some work. Having it change from true to false seems like it would reduce its usefulness.

[Tratcher]

I have an HTTP/2 prototype over at https://github.com/aspnet/KestrelHttpServer/commit/96b0c85eb9475e9616e7de4d85b8cc92b54c2c10.

I realized that the simplest design is to only provide an IHeadersDictionary and send it at the end of the response stream where we would normally send the terminator. While this does have the added latency of forcing you to wait until the app func unwinds, that's no worse than the current chunked terminator behavior and allows middleware to reliably add trailers.

• TrailersEnabled - Not needed for HTTP/2 since it's always true. Maybe needed for HTTP/1.1.
• TrailersSent - Not needed since trailers aren't sent until the app func exits.
• FlushAsync - Would cut latency but would break intermediaries like response compression.

[Tratcher]

This has been completed for HTTP/2 in 2.2.0-preview3. I'm updating this issue to clarify that it now only tracks HTTP/1.1.

[NickCraver]

Is there any update in the plans for this? I'm just curious if http/1.1 support is planned. I was trying to get them to work in MiniProfiler which is an awesome use case, but the default experience for devs locally is http/1.1 and http/2 is broken in the current preview (see https://github.com/aspnet/AspNetCore/issues/8952).