Skip to content

http2: add compat trailers, adjust multi-headers

Rodrigo Muino Tomonari requested to merge github/fork/apapirovski/patch-http2-compat-additions into master

Some changes to compatibility layer to bring it closer to the h1 implementation.

  • Adds Http2ServerRequest trailers & rawTrailers functionality. Previously didn't work at all and the request would also never end. Which actually makes me wonder, is this even correct or am I compensating for a deeper issue?

  • Fixes behaviour of multi-headers to conform with the spec (all values but set-cookie and cookie should be comma delimited, cookie should be semi-colon delimited and only set-cookie should be an array). I think it's important we conform to the spec and also be backwards compatible, even if arrays are easier to work with for multi-values. (H2 doesn't specify this directly but it links to https://tools.ietf.org/html/rfc7230#section-3.2.2)

  • Adds setter for statusMessage that warns (same as the getter), for backwards compatibility. See https://github.com/expressjs/express/pull/3390#discussion_r136718729

@jasnell, @mcollina I would also like to start a discussion re: rawHeaders if possible. The current behaviour doesn't result in true rawHeaders as we have no way of knowing in what format something like cookie was sent to us based on the headers object (could've already been joined or could've been separate). Should we just emit the raw headers object alongside headers & flags from onSessionHeaders (and then pass to onServerStream too)? If we were to add it as the last argument then it wouldn't really interfere with anything but I'm open to alternate suggestions. (We could also hide it on the headers object as non-enumerable property but that seems really dirty.)

Thanks in advance for any feedback!

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

http2, test

Merge request reports