Support RFC 8441 WebSocket upgrade with HTTP/2 CONNECT

[jazdw]

f477c16 which closed #34044 was an incomplete fix. Further changes are necessary to support HTTP/2 CONNECT WebSocket upgrades (RFC 8441).

I have manually tested this PR and the CONNECT WebSocket upgrade works as expected using Jetty 12 EE10.

Link to RFC:
https://datatracker.ietf.org/doc/html/rfc8441

The RFC explicitly says that the Sec-WebSocket-Key header is not required:

Implementations using this extended CONNECT to bootstrap WebSockets
do not do the processing of the Sec-WebSocket-Key and Sec-WebSocket-
Accept header fields of [RFC6455] as that functionality has been
superseded by the :protocol pseudo-header field.

The RFC explicitly says that the Connection and Upgrade headers are not required:

[RFC6455] requires the use of Connection and Upgrade header fields
that are not part of HTTP/2. They MUST NOT be included in the
CONNECT request defined here.

TODO

• Look into if we need to verify that we are using HTTP/2 and verify the :protocol pseudo-header
• Fix WebFlux implementation also

[rstoyanchev]

@jazdw thanks for testing and sending this. Could you check the DCO instructions on how to sign off your commit and update the PR branch? This is required for us to process this.

About checking for HTTP/2, ServletRequest has a getProtocolVersion() method that we could use, but would have to expose it first through ServerHttpRequest. I don't think that's truly necessary.

[jazdw]

About checking for HTTP/2, ServletRequest has a getProtocolVersion() method that we could use, but would have to expose it first through ServerHttpRequest. I don't think that's truly necessary.

I dug into this a little more, Jetty verifies the :protocol pseudo header internally, see org.eclipse.jetty.websocket.core.server.internal.RFC8441Negotiation#validateHeaders.

https://github.com/jetty/jetty.project/blob/a171ad3a4d46bde2dfcc7cc5803f12f6919088fb/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC8441Negotiation.java#L31-L37

It also verifies the method is CONNECT and the HTTP version is HTTP/2.0 in org.eclipse.jetty.websocket.core.server.internal.RFC8441Handshaker#isWebSocketUpgradeRequest

https://github.com/jetty/jetty.project/blob/a171ad3a4d46bde2dfcc7cc5803f12f6919088fb/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC8441Handshaker.java#L35-L47

I am wondering if we should add a supportsRfc8441() method to the RequestUpgradeStrategy? That way we wouldn't attempt the upgrade if we know the implementation doesn't support CONNECT.

[rstoyanchev]

I don't mind a dedicated supports check for RFC 8441 as long as it is possible through the Servlet API. Jetty is at a lower level and can perform more checks I suspect.

[jazdw]

@rstoyanchev I've pushed another commit, I don't know that it is necessary though. Happy for you to drop the second commit if you are happy with just the first. I've tested it on Jetty 12 EE10 with both GET setConnectProtocolEnabled(false) and CONNECT setConnectProtocolEnabled(true)