Support HTTP/2 after proxy CONNECT#2250
Conversation
536b173 to
e14569c
Compare
hyperxpro
left a comment
There was a problem hiding this comment.
Not in this PR's diff, so leaving it here: this change makes HTTP/2 after CONNECT work, which makes the workaround in client/src/test/java/org/asynchttpclient/proxy/HttpsProxyTestcontainersIntegrationTest.java stale and its comment misleading. Could you remove .setHttp2Enabled(false) and the // HTTP/2 ALPN upgrade after proxy CONNECT tunnel is not yet supported comment in both testHttpProxyToHttpsTarget (lines 149-150) and testHttpsProxyToHttpsTarget (lines 172-173)? That lets those external tests exercise the new HTTP/2-through-proxy path instead of forcing HTTP/1.1, and the comment stops claiming something that is no longer true.
|
Done - removed |
|
Hi @hyperxpro, just a gentle reminder when you get a chance. I've addressed the review comments and would appreciate a re-review. Thanks! |
|
Thanks a lot! |
Motivation
When an HTTPS request uses an HTTP proxy, AHC establishes a
CONNECTtunnel and then performs TLS with the target. If ALPN selectsh2, the tunnel path currently leaves the HTTP/1.1 codec in place. The target then receives HTTP/1.1 bytes on a connection that negotiated HTTP/2 and closes it.Modification
h2, upgrade the existing channel pipeline to HTTP/2 and register the physical tunnel connection with its existing pool partition key.CONNECTproxies with a TLS HTTP/2 target.Result
HTTPS requests through HTTP and HTTPS
CONNECTproxies now send HTTP/2 frames when the target negotiatesh2. The regression tests verify successful HTTP/2 responses and reuse of a single physical target connection.Testing
./mvnw -pl client -Dtest=BasicHttp2Test,HttpsProxyTest test— 60 tests passed.verify; the local run then stopped at artifact signing becausegpgis not installed../mvnw -pl client -DskipTests -Dgpg.skip=true verify— packaging and API compatibility checks passed.Fixes #2241