Bypass parsing HTTP request on HttpSM when request is HTTP/2

[masaori335]

When Http2Stream passes HTTP request to HttpSM, it dump HTTP/1.1 request header in text from HTTPHdr. HttpSM needs to parse the text and build HTTPHdr again. I'm not sure how much this has performance overhead. But, obviously, this is not ideal. Currently, HTTP/3 impl is following same logic.

trafficserver/proxy/http2/Http2Stream.cc

Line 166 in 089316d

done = _req_header.print(block->start(), block->write_avail(), &bufindex, &tmp);

[maskit]

I think it's a blocker of H2 (and H3) to origin. Because headers sent by a client with Literal Header Field Never Indexed representation have to be transferred with the same representation. We can't retain the information on HTTP/1.1 header representation (plain text).

[masaori335]

A similar overhead exists on sending responses and it's significant. ( HttpSM dump response header in a buffer and HTTP/2 parse it again ).
30% of Http2Stream::update_write_request is HTTPHdr::parse_resp.

[shinrich]

@maskit I am not following your comment about the "Literal Field Never Indexed". Can you say a few more words on this?

[maskit]

It's a header field representation defined on HPACK.

https://tools.ietf.org/html/rfc7541#section-6.2.3

A literal header field never-indexed representation results in
appending a header field to the decoded header list without altering
the dynamic table. Intermediaries MUST use the same representation
for encoding this header field.

https://tools.ietf.org/html/rfc7541#section-7.1.3

An intermediary MUST NOT re-encode a value that uses the never-
indexed literal representation with another representation that would
index it. If HPACK is used for re-encoding, the never-indexed
literal representation MUST be used.

[shinrich]

Hmm. That is an unfortunate tie between the inbound and outbound sides. It seems that some implementations like Cloud Flare must be ignoring this constraint since they are mapping to HTTP/1.1 in their environment (multi-process) before mapping back to HTTP/2 before sending the packets on. It seems unlikely they can be accurately tracking the never-indexed aspects of the headers in that environment. https://blog.cloudflare.com/road-to-grpc/

[shinrich]

My thoughts on this just thinking about the HttpSM and the ProxyTransactions, is that the we simply push the parsing/serialization into the ProxyTransaction classes. In that case the HttpSM only deals with a possibly canonicalized form of HttpHdr.

The tricky bit with that is the buffer based do_io interface between the ProxyTransaction and the HttpSM. We would need to augment that interface to allow for HttpHdr's to be passed along. Perhaps ProxyTransaction classes can have another flavor of virtual do_io_write/do_io_read added that take a HttpHdr explicitly. Or @SolidWallOfCode suggested adding the ability to add a HttpHdr to the VIO object directly.

For non-transform plugins, this should require no code change. However @SolidWallOfCode also pointed out that transform plugins act as ProxyTransaction classes (via the PluginVC inheritance), so they currently are also written to parse out headers. Presumably just the Response headers. So any solution would need to identify the presence of Transforms in the chain and serialize in that case anyway (losing a degree of code clean up) or the transform plugins would need to be rewritten to adapt to the updated do_io functions for interacting with headers.

We need to look at some of the transform plugins and make some judges on how onerous the do_io interface change would be for them.

[maskit]

The limitation for intermediary is also on QPACK spec.
https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#section-4.5.4
https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#section-7.4

I think keeping the representation info is not so difficult if we use HttpHdr. I'd add "sensitive" flag to MIMEField. I proposed it once at the very beginning of HTTP/2 implementation, but it was omitted because it was considered as not worth increasing the size of MIMEField.

Because of that, we currently don't have a way to protect header fields on HTTP layer, and we do special handling inside HPACK for two specific headers.

trafficserver/proxy/http2/HPACK.cc

Lines 958 to 960 in b71249c

	if ((ptr_len_casecmp(name, name_len, MIME_FIELD_COOKIE, MIME_LEN_COOKIE) == 0 && value_len < 20) ||
	(ptr_len_casecmp(name, name_len, MIME_FIELD_AUTHORIZATION, MIME_LEN_AUTHORIZATION) == 0)) {
	field_type = HpackField::NEVERINDEX_LITERAL;

This code could be generalized if we added the flag. And then just setting the flag on decoding should retain the representation.

For the interface between ProxyTransaction and HttpSM, I have no idea. I understand we have to deal with transform plugins, but I'm not so sure about extending VIO. It's already hard to use correctly. I saw many bugs that were related to nbytes and ndone.