Closed Bug 1436610 Opened 2 years ago Closed 2 years ago

Send "TE: Trailers" on requests


(Core :: Networking: HTTP, enhancement, P2)




Tracking Status
firefox62 --- fixed


(Reporter: u408661, Assigned: u408661)



(Whiteboard: [necko-triaged])


(1 file)

Really, we should've been doing this all along (since we do, in fact, decode trailers... even if we've ignored them until now). But, given we're now explicitly supporting trailers (in the form of server-timing) we should be more explicit about that on the wire. Also good so that servers can (in theory) rely on clients appropriately advertising their support for something when they're supposed to.

Even if FF appends TE: trailers, it won't be clear to servers what it means; just because FF is willing to process *some* HTTP headers in trailers, doesn't mean any trailer sent will be honoured. 

E.g., ETag, Cache-Control.

Personally, I think there needs to be a spec change here.
Patrick - this bug came out of a conversation you and I were having via email. :mnot makes a (at the least) not-horrible point - most trailers we're still ignoring. How reasonable do you think it would be to send TE: trailers when we only interpret one trailer.
Flags: needinfo?(mcmanus)
we're basically using it as a signal that we won't break (as some clients do in the presence of trailers). There's nothing that obligates us to process (beyond parsing) any particular trailer based header at any time but there is an obligation to send this request header.

We could pursue some kind of Accept-Trailers: Server-Timing approach but I don't think that removes our obligation to send TE: trailers under the existing specs.

we should only do this on h2 btw due to header pressure. We'll just give up the feature for h1 environments.
Flags: needinfo?(mcmanus)
Hrm... coming back to this (now that it's been forcibly paged back in for me), do we want to disable this for all h1, or only for plaintext h1? I ask because doing this for all tls connections (h1 included) will make it possible to do this in a non-hacky way that doesn't require significantly reworking how/when the transaction creates its request stream (the transaction currently doesn't know it's h2 until well after its created the request stream including flattened headers).
Flags: needinfo?(mcmanus)
OK, we're going to do the hacky h2 version. h1 will just take what it gets :)
Flags: needinfo?(mcmanus)
As discussed in Berlin, will also add the header to the request head in the transaction so it can be shown in devtools.
Comment on attachment 8974367 [details]
Bug 1436610 - Send "TE: Trailers" header on h2 requests.
Attachment #8974367 - Flags: review?(mcmanus) → review+
Pushed by
Send "TE: Trailers" header on h2 requests. r=mcmanus
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Depends on: 1523253
Depends on: 1526415
No longer depends on: 1523253
Regressions: 1523253
You need to log in before you can comment on or make changes to this bug.