8349910: Implement JEP 517: HTTP/3 for the HTTP Client API #24751

dfuch · 2025-04-18T13:05:24Z

Hi,

Please find here a PR for the implementation of JEP 517: HTTP/3 for the HTTP Client API.

The CSR can be viewed at JDK-8350588: Implement JEP 517: HTTP/3 for the HTTP Client API

This JEP proposes to enhance the HttpClient implementation to support HTTP/3.
It adds a non-exposed / non-exported internal implementation of the QUIC protocol based on DatagramChannel and the SunJSSE SSLContext provider.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires CSR request JDK-8350588 to be approved

Issues

JDK-8349910: Implement JEP 517: HTTP/3 for the HTTP Client API (Enhancement - P3)
JDK-8350588: Implement JEP 517: HTTP/3 for the HTTP Client API (CSR)

Contributors

Aleksei Efimov <[email protected]>
Bradford Wetmore <[email protected]>
Daniel Jeliński <[email protected]>
Darragh Clarke <[email protected]>
Jaikiran Pai <[email protected]>
Michael McMahon <[email protected]>
Volkan Yazici <[email protected]>
Conor Cleary <[email protected]>
Patrick Concannon <[email protected]>
Rahul Yadav <[email protected]>
Daniel Fuchs <[email protected]>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24751/head:pull/24751
$ git checkout pull/24751

Update a local copy of the PR:
$ git checkout pull/24751
$ git pull https://git.openjdk.org/jdk.git pull/24751/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24751

View PR using the GUI difftool:
$ git pr show -t 24751

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24751.diff

Using Webrev

Link to Webrev Comment

…oesn't support HTTP/3, the request version is HTTP_3, and HTTP_ONLY is not specified

…roposed to mainline)

…ending a new request

… intermittent failures in H3ErrorHandlingTest.java

…ending the packet

…d logic

…maxStreamLimitTimeout=0 too

src/java.base/share/classes/jdk/internal/net/quic/QuicTLSContext.java

dfuch · 2025-06-26T17:01:18Z

src/java.base/share/classes/jdk/internal/net/quic/QuicTLSContext.java

+     * @param peerHost The peer hostname or IP address. Can be null.
+     * @param peerPort The peer port, can be -1 if the port is unknown


Would that be the hostname in the URI, or in the AltService?
Maybe we could add an @apiNote here to clarify it.

Well the javadoc here was written to match the one on SSLContext#createSSLEngine. The peer information is used for caching, but it's also used in the SNI extension, so ideally users should use the URI address, not the alt service one.

OK. Maybe that would deserve a note - since with HTTP/3 we have potentially two addresses and two ports.

dfuch · 2025-06-26T17:21:20Z

src/java.base/share/classes/jdk/internal/net/quic/QuicVersion.java

+import java.util.Optional;
+
+/**
+ * Represents the Quic versions defined in their corresponding RFCs


Maybe add a note here to warn that new enum constants may be added to this enum in the future.

dfuch · 2025-06-26T17:28:11Z

src/java.base/share/classes/jdk/internal/net/quic/QuicVersion.java

+    QUIC_V1(1), // RFC-9000
+    QUIC_V2(0x6b3343cf); // RFC 9369
+
+    private final int versionNumber;


Could add a comment here to say that the version is defined as a 32bits unsigned number, and we chose a simple int to represent it.

djelinski · 2025-06-26T18:06:03Z

src/java.base/share/classes/sun/security/ssl/Finished.java

+                if (shc.sslConfig.isQuic) {
+                    QuicTLSEngineImpl engine =
+                            (QuicTLSEngineImpl) shc.conContext.transport;
+                    engine.deriveOneRTTKeys();


We should not derive the server's 1RTT read keys before processing the client's Finished message.

Also, we could skip calculating the SSL WriteCipher when QUIC is in use. Also, we're calculating the baseWriteSecret twice (deriveOneRTTKeys calculates the same secret)

dfuch · 2025-06-26T17:34:45Z

src/java.base/share/classes/sun/security/ssl/KeyShareExtension.java

@@ -46,7 +46,7 @@ final class KeyShareExtension {
            new CHKeyShareProducer();
    static final ExtensionConsumer chOnLoadConsumer =
            new CHKeyShareConsumer();
-    static final HandshakeAbsence chOnTradAbsence =
+    static final HandshakeAbsence chOnTradeAbsence =


Could we fix that one in mainline to remove this file from the PR?

JDK-8361125

dfuch · 2025-06-26T17:43:21Z

src/java.base/share/classes/sun/security/ssl/OutputRecord.java

+    // apply to QuicEngine only
+    byte[] getHandshakeMessage() {
+        throw new UnsupportedOperationException();
+    }
+
+    // apply to QuicEngine only
+    QuicTLSEngine.KeySpace getHandshakeMessageKeySpace() {
+        throw new UnsupportedOperationException();
+    }
+


Do we need those two methods here? Or could we just have them on QuicEngineOutputRecord and do the appropriate cast at the point they are called?
Or do we have that at this level for performance reasons?

This was done to match the surrounding code. There's a bunch of methods in OutputRecord that throw UOE and are overridden by one class only. I suppose it would be cleaner to just do the cast, but "when in Rome, do as the Romans do"

dfuch · 2025-06-26T17:44:43Z

src/java.base/share/classes/sun/security/ssl/PreSharedKeyExtension.java

@@ -58,7 +58,7 @@ final class PreSharedKeyExtension {
            new CHPreSharedKeyOnLoadAbsence();
    static final HandshakeConsumer chOnTradeConsumer =
            new CHPreSharedKeyUpdate();
-    static final HandshakeAbsence chOnTradAbsence =
+    static final HandshakeAbsence chOnTradeAbsence =


Could we bring this change to mainline to remove this file from this PR?

JDK-8361125

dfuch · 2025-06-27T08:06:03Z

src/java.base/share/classes/sun/security/ssl/QuicCipher.java

+                throws GeneralSecurityException {
+            super(cipherSuite, secret, new T13AESHPCipher(hp), keyPhase);
+            this.key = key;
+            this.iv = iv;


Suggested change

this.iv = iv;

this.iv = iv.clone();

dfuch · 2025-06-27T08:14:45Z

src/java.base/share/classes/sun/security/ssl/QuicCipher.java

+                final int keyPhase) throws GeneralSecurityException {
+            super(cipherSuite, secret, new T13AESHPCipher(hp), keyPhase);
+            this.key = key;
+            this.iv = iv;


Suggested change

this.iv = iv;

this.iv = iv.clone();

dfuch · 2025-06-27T08:15:27Z

src/java.base/share/classes/sun/security/ssl/QuicCipher.java

+                throws GeneralSecurityException {
+            super(cipherSuite, secret, new T13CC20HPCipher(hp), keyPhase);
+            this.key = key;
+            this.iv = iv;


Suggested change

this.iv = iv;

this.iv = iv.clone();

dfuch · 2025-06-27T09:49:44Z

src/java.base/share/classes/sun/security/ssl/QuicTransportParametersExtension.java

+ * QuicTransportParametersExtension is an implementation of RFC 9000.
+ * It is only used by QUIC connections.


I believe referencing RFC 9001 Section 8.2 would be more appropriate here.
The QUIC transport parameters themselves are defined in RFC 9000, but the use of the quic_transport_parameters extension in the ClientHello and the EncryptedExtensions messages is described in RFC 9001.
Or else the purpose and description of this class could be clarified.

Most of the extensions have a generic JavaDoc, I will update this one to match the others.

dfuch · 2025-06-27T09:56:45Z

src/java.base/share/classes/sun/security/ssl/SSLExtension.java

@@ -458,6 +458,28 @@ enum SSLExtension implements SSLStringizer {
                                null, null, null, null,
                                KeyShareExtension.hrrStringizer),

+    // Extension defined in RFC 9000


Shouldn't this be RFC 9001 here too?

Will be updated in the next refresh.

djelinski · 2025-06-27T11:22:51Z

src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java

                }
            }
        }
    }

+    // true if previous attempt resulted in streamLimitReached
+    public boolean hasReachedStreamLimit(Version version) { return streamLimitReached == version; }


it seems that streamLimitReached will either be equal to version or null. If that's the case, let's use a boolean instead.

That's a good point.

djelinski · 2025-06-27T11:37:00Z

src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java

@@ -111,8 +120,12 @@ HttpClientImpl client() {

    // Keeps track of the underlying connection when establishing an HTTP/2


since this is now used with HTTP/3, should we update this comment?

I have verified the other comments in this class and updated to mention or HTTP/3 in a few places in this class, in my local repo. Will appear the next time I sync this PR.

dfuch · 2025-06-27T13:19:01Z

src/java.base/share/classes/sun/security/ssl/SupportedGroupsExtension.java

@@ -47,7 +47,7 @@ final class SupportedGroupsExtension {
            new CHSupportedGroupsProducer();
    static final ExtensionConsumer chOnLoadConsumer =
            new CHSupportedGroupsConsumer();
-    static final HandshakeAbsence chOnTradAbsence =
+    static final HandshakeAbsence chOnTradeAbsence =


Another one that could disappear if we fixed that typo in mainline first.
@djelinski would you be willing to do that?

JDK-8361125

dfuch · 2025-06-27T13:21:12Z

src/java.base/share/classes/sun/security/ssl/X509Authentication.java

+            // TODO in future, when QuicTLSEngine might become a public exported interface
+            // we should have a method on javax.net.ssl.X509ExtendedKeyManager that
+            // takes QuicTLSEngine.


Suggested change

// TODO in future, when QuicTLSEngine might become a public exported interface

// we should have a method on javax.net.ssl.X509ExtendedKeyManager that

// takes QuicTLSEngine.

// TODO in future, if QuicTLSEngine ever becomes a public exported interface

// we might have a method on javax.net.ssl.X509ExtendedKeyManager that

// takes QuicTLSEngine.

Maybe we could stop guessing the future, and use this instead:

Suggested change

// TODO in future, when QuicTLSEngine might become a public exported interface

// we should have a method on javax.net.ssl.X509ExtendedKeyManager that

// takes QuicTLSEngine.

// TODO add a method on javax.net.ssl.X509ExtendedKeyManager that

// takes QuicTLSEngine.

Removed speculations about the future

dfuch · 2025-06-27T13:22:09Z

src/java.base/share/classes/sun/security/ssl/X509Authentication.java

+                // TODO in future, when QuicTLSEngine might become a public exported interface
+                // we should have a method on javax.net.ssl.X509ExtendedKeyManager that
+                // takes QuicTLSEngine.


Suggested change

// TODO in future, when QuicTLSEngine might become a public exported interface

// we should have a method on javax.net.ssl.X509ExtendedKeyManager that

// takes QuicTLSEngine.

// TODO in future, if QuicTLSEngine ever becomes a public exported interface

// we might have a method on javax.net.ssl.X509ExtendedKeyManager that

// takes QuicTLSEngine.

dfuch · 2025-06-27T14:02:17Z

src/java.net.http/share/classes/java/net/http/package-info.java

+ * <a href="https://tools.ietf.org/html/rfc9114">Hypertext Transfer Protocol
+ * Version 3 (HTTP/3)</a>, the <a href="https://tools.ietf.org/html/rfc7540">
+ * Hypertext Transfer Protocol Version 2 (HTTP/2)</a>, the
+ * <a href="https://tools.ietf.org/html/rfc2616">


It is strange to link to a different place here than in the @spec tags below. I will fix that.

dfuch · 2025-06-27T14:55:10Z

src/java.net.http/share/classes/jdk/internal/net/http/AltServicesRegistry.java

+        static Origin of(final String scheme, final InetSocketAddress addr)
+                throws IllegalArgumentException {
+            // we use getHostString(), since the address could be unresolved in the case where
+            // proxy is configured
+            final String originHost = addr.getHostString();
+            return new Origin(scheme, originHost, addr.getPort());
+        }
+    }


I don't think it's a good idea to rely on InetSocketAddress::getHostString() as this can change depending on how the InetSocketAddress was constructed and whether getHost() has been called.
I believe it would be more reliable to add a new URI origin parameter to the HttpConnection constructor. We could build that origin from the request URI for which the connection was created. Probably origin should be null for PlainProxyConnection.

I've created https://bugs.openjdk.org/browse/JDK-8361060 to track this.

djelinski · 2025-06-30T09:34:48Z

src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java

@@ -111,8 +120,12 @@ HttpClientImpl client() {

    // Keeps track of the underlying connection when establishing an HTTP/2
    // exchange so that it can be aborted/timed out mid setup.
-    static final class ConnectionAborter {
+    final class ConnectionAborter {
+        // In case of HTTP/3 direct connection we may have


"direct" is usually used to refer to connections that don't use a proxy; since H3 connections never use a proxy, can we drop the "direct" here?

Good point. I will rephrase. We should be speaking of HTTP/3 requests (and not connection) here anyway.

dfuch · 2025-06-27T15:23:37Z

src/java.net.http/share/classes/jdk/internal/net/http/AltSvcProcessor.java

+            // there's nothing more left to parse
+            return new ParsedHeaderValue(altValue, alpnName, hostPort.host(), hostPort.port(), Map.of());
+        }
+        // parse the semi-colon delimited parameters out of the rest of the remaining string


Suggested change

// parse the semi-colon delimited parameters out of the rest of the remaining string

// parse the semicolon delimited parameters out of the rest of the remaining string

dfuch · 2025-06-27T15:27:10Z

src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java

@@ -79,6 +81,13 @@ final class Exchange<T> {

    final AtomicInteger nonFinalResponses = new AtomicInteger();

+    // This will be set to true only when it is guaranteed that the server hasn't processed
+    // the request. Typically this happens when the server explicitly states (through a GOAWAY frame


Suggested change

// the request. Typically this happens when the server explicitly states (through a GOAWAY frame

// the request. Typically, this happens when the server explicitly states (through a GOAWAY frame

dfuch · 2025-06-30T09:45:12Z

src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java

@@ -111,8 +120,12 @@ HttpClientImpl client() {

    // Keeps track of the underlying connection when establishing an HTTP/2
    // exchange so that it can be aborted/timed out mid setup.
-    static final class ConnectionAborter {
+    final class ConnectionAborter {
+        // In case of HTTP/3 direct connection we may have


Good point. I will rephrase. We should be speaking of HTTP/3 requests (and not connection) here anyway.

djelinski · 2025-07-01T11:13:44Z

src/java.net.http/share/classes/jdk/internal/net/http/http3/frames/FramesDecoder.java

+     * {@return the current partial frame, if any}
+     * This method doesn't try to do any decoding.
+     */
+    public Http3Frame peek() {


This method is unused. I'll remove it.

djelinski · 2025-07-01T19:51:06Z

src/java.net.http/share/classes/jdk/internal/net/http/http3/streams/UniStreamPair.java

+    private volatile QuicStreamWriter localWriter;
+    private final CompletableFuture<QuicStreamWriter> streamWriterCF;
+    // a queue of ByteBuffers submitted for writing.
+    // might be null if not used. Only used in QueuingStreamPair.


It seems to be true. I'll see if I can move it to QueuingStreamPair.

djelinski · 2025-07-01T20:05:23Z

src/java.net.http/share/classes/jdk/internal/net/http/http3/streams/UniStreamPair.java

+            SequentialScheduler.lockingScheduler(this::peerReaderLoop);
+    // The QuicStreamReader for the peer control stream
+    volatile QuicStreamReader peerReader;
+    private final CompletableFuture<QuicStreamReader> streamReaderCF;


djelinski · 2025-07-01T20:05:59Z

src/java.net.http/share/classes/jdk/internal/net/http/http3/streams/UniStreamPair.java

+     *  to the receiver part of this stream pair after the remote HTTP/3 stream
+     *  type has been read off the remote initiated stream}
+     */
+    public final CompletableFuture<QuicStreamReader> futureReceiverStreamReader() {


djelinski and others added 30 commits March 21, 2025 12:07

Revert unrelated change

3759c87

Revert unrelated change

26cc681

Simplify processAltSvcHeader

63c7bbe

Clean up connectionEstablished

7c19fc8

http3: upgrade more tests

4da8dbd

http3: retry a request only once on ConnectException

4ca33f5

Remove pointless while loop

8793e84

Reduce repetitive logging (see refreshDeadline)

816012c

http3: downgrade to HTTP/2 rather than failing if the client config d…

68a5558

…oesn't support HTTP/3, the request version is HTTP_3, and HTTP_ONLY is not specified

http3: upgrade UnauthorizedTest to use HTTP/3 and also fix 8352623 (p…

1492416

…roposed to mainline)

http3: more tests upgraded

540c5d9

http3: improved java/net/httpclient/RequestBuilderTest.java

40cb71c

http3: missing @SInCE

6e9226b

http3: make it clearer that StreamLimitException may be raised when s…

fd6ca77

…ending a new request

http3: update StreamFrame::slice

bcd2073

Remove redundant AltSvcFrame.originLength

892c089

Fix FramesDecoder snippet

fab8c39

Fix FramesDecoder snippet again

84e0b6e

http3: remove unused method

b224b9b

http3: no need for an additional field

25d2614

http3: remove caching of toString()

e89450e

http3: remove redundant check

697b407

http3: make fields private

5c51a75

http3: fix typo and remove unused method

4dc27f2

http3: use typeFieldHasOFF in StreamFrame

bfa3c3c

http3: minor change to javadoc

386fbf2

http3: make field private

2ff12fe

http3: fix typo in comment

86e50bd

http3: remove unused method

f2cef20

http3: minor code comment cleanup in HttpResponse and StreamFrame

c8502d1

djelinski and others added 11 commits June 25, 2025 10:40

Remove unused else clauses

fefa748

Implement X509TrustManagerImpl#checkClientTrusted for QUIC

70142b1

http3: review feedback - revert HPACK.java

249858a

http3: fix exception handling in CancelRequestTest.java

33de679

http3: improve exceptions in Http3ServerExchange.java

0843d3e

http3: fix race with ping requests in PacketSpaceManager.java causing…

a33ea31

… intermittent failures in H3ErrorHandlingTest.java

http3: pending acknowledgement should be registered before actually s…

ea6cced

…ending the packet

http3: fix pending connection and reconnection on stream limit reache…

ca830ec

…d logic

retry the ResetControlStream test as needed

800b565

http3: run H3StreamLimitReachedTest.java with -Djdk.httpclient.http3.…

8f89ef4

…maxStreamLimitTimeout=0 too

merge latest changes from master branch

0229c21

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Jun 26, 2025

dfuch commented Jun 26, 2025

View reviewed changes

djelinski reviewed Jun 26, 2025

View reviewed changes

dfuch commented Jun 26, 2025

View reviewed changes

dfuch commented Jun 27, 2025

View reviewed changes

djelinski reviewed Jun 27, 2025

View reviewed changes

dfuch commented Jun 27, 2025

View reviewed changes

djelinski reviewed Jun 30, 2025

View reviewed changes

dfuch commented Jun 30, 2025

View reviewed changes

djelinski reviewed Jul 1, 2025

View reviewed changes

		* @param peerHost The peer hostname or IP address. Can be null.
		* @param peerPort The peer port, can be -1 if the port is unknown

		* QuicTransportParametersExtension is an implementation of RFC 9000.
		* It is only used by QUIC connections.

		@@ -111,8 +120,12 @@ HttpClientImpl client() {

		// Keeps track of the underlying connection when establishing an HTTP/2

	// parse the semi-colon delimited parameters out of the rest of the remaining string
	// parse the semicolon delimited parameters out of the rest of the remaining string

	// the request. Typically this happens when the server explicitly states (through a GOAWAY frame
	// the request. Typically, this happens when the server explicitly states (through a GOAWAY frame

8349910: Implement JEP 517: HTTP/3 for the HTTP Client API #24751

Are you sure you want to change the base?

8349910: Implement JEP 517: HTTP/3 for the HTTP Client API #24751

Conversation

dfuch commented Apr 18, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Contributors

Reviewing

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfuch Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfuch Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfuch commented Apr 18, 2025 •

edited by openjdk bot

Loading

dfuch Jun 27, 2025 •

edited

Loading

dfuch Jun 27, 2025 •

edited

Loading