PUT request over plain text connection with chunk encoded body fails when upgrading to HTTP/2 about jetty.project HOT 1 OPEN

Below some thoughts about supporting this.

First of all, RFC 9113, section 3.1 states:

The "h2c" string was previously used as a token for use in the HTTP Upgrade mechanism's Upgrade header field (Section 7.8 of [HTTP]). This usage was never widely deployed and is deprecated by this document. The same applies to the HTTP2-Settings header field, which was used with the upgrade to "h2c".

So the upgrade is deprecated, and only really specified, at this point, by the older RFC 7540.

We have a test case that reproduces this issue:
HttpClientTransportDynamicTest.testHTTP11UpgradeToH2CWithRequestContentDoesNotUpgrade().

It expects requests with content to not upgrade.
The reason for this is that the HTTP/1.1 to HTTP/2 upgrade is a "mixed" thing: the request is supposed to be in HTTP/1.1 format, but the response is in HTTP/2 format.

We would need to call the application, which must be able to read the content in HTTP/1.1, but also write the response content in HTTP/2.
An "echo" would therefore require the read half to still be HTTP/1.1, and the write half to be HTTP/2.

Jetty would need to read the full HTTP/1.1 content before upgrading with a 101.
After the 101, the client is supposed to send the HTTP/2 client preface, which the JDK client does.

In the wiremock test linked above, JDK's HttpClient sends an HTTP/1.1 Transfer-Encoding: chunked request, so it definitely needs an HTTP/1.1 parser to be understood.
Jetty performs an early upgrade, and in HTTP2CServerConnectionFactory.upgradeConnection() there is a specific check for the request length: if contentLength > 0 the upgrade is denied.
If it is Transfer-Encoding: chunked the content length check fails, and we actually do the upgrade, but we expect the next bytes to be the HTTP/2 client preface, but they are not (it's still the HTTP/1.1 content), and we fail the connection.

We should delay the upgrade and setup things in this "mixed" mode until the HTTP/1.1 request content is fully read.

Note that an application may not read the request content, so Jetty will try its best to do it, but still needs to write the response in HTTP/2 format.
Which begs the question: where do we draw the line "we have upgraded to HTTP/2"?
Or, in other words, when do we decide whether the response is in HTTP/1.1 format or HTTP/2 format?

We need to take care of various failure scenarios:

• Bad request URI => response in HTTP/1.1 or HTTP/2?
• Bad request chunk => response in HTTP/1.1 or HTTP/2?
• Application does not read request content; either it all arrived (should upgrade), or not all of it arrived (stay in HTTP/1.1?)
• Idle timeout waiting for request content.

Say, for example, an application reads a first good request content chunk and echoes it back. The response content must be HTTP/2, so the server should send a 101, the HTTP/2 server preface, and then the first response content.
But if the second request content chunk is bad, now the upgrade has been done, so there is no other solution than closing the connection, as we cannot read more request content.

Consider also this case: a PUT with content for a URI that requires authentication. The client should receive a 401, but obviously the server application has not been called, but now the upgrade is done (or not?) and the initial request content lost.

In summary, it is a deprecated feature, gives out feelings of "this is a bad idea" similar to pipelined requests, and I think it would be best for clients to first perform a GET to upgrade and then go HTTP/2.

We'll try to see if we can delay the upgrade in Jetty, but seems a lot of work for a deprecated mechanism, and for a few uncommon cases (upgrades with content) within that mechanism.

@gregw thoughts?

from jetty.project.