RFR: 8378786: PeerConnectionId::cloneBuffer should use absolute bulk get

[Volkan Yazici]

On Fri, 27 Feb 2026 11:47:39 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

> PeerConnectionId has a constructor that takes a ByteBuffer containing the peer connection id bytes. This constructor extracts the connection id bytes from the given byte buffer and makes a private copy for itself. However, while doing so, it uses a relative bulk get instead of an absolute bulk get, which causes the original byte buffer position to move to the limit.
> 
> This is causing trouble on the server side, this is the only place where that constructor is used without making a slice of the original buffer first. In some circumstances (if the connection id is not found) it can cause the first initial packet to get dropped after creating the connection. This went unnoticed because the client will then retransmit that packet, and the connection id will be found the next time around.
> 
> Though the issue is in product source code it only affects the server side used by httpclient tests. 
> 
> The proposed change first adds some assert statements to `QuicConnectionImpl::internalProcessIncoming` and ensure that the connection will get closed if the assert fires. This made it possible to reproduce and surface the issue quite easily using existing httpclient tests: a lot of tests started failing with that change ([this is done by the first commit in this PR](https://github.com/openjdk/jdk/pull/29956/changes/b81ce5d405054b06afdf70738119916bba08ce72)).
> The [second commit](https://github.com/openjdk/jdk/pull/29956/changes/108660362d4fa78e61c784931fe6d45e002381dc) fixes the issue by making `PeerConnectionId::cloneBuffer` use an absolute bulk get instead of a relative bulk get. The absolute bulk get ensures that the position of the input buffer is not moved while copying it.
> 
> I am marking the issue as `noreg-hard` - though it could be very easily verified by undoing the second commit, recompiling the JDK, and running the test with that. Then add the second commit, recompile, and see that all tests are passing.

src/java.net.http/share/classes/jdk/internal/net/http/quic/PeerConnectionId.java line 77:

> 75:         final byte[] idBytes = new byte[src.remaining()];
> 76:         src.get(src.position(), idBytes);
> 77:         return ByteBuffer.wrap(idBytes);

You can alternatively consider replacing these 3 lines with `return src.slice(src.position(), src.limit())`, I guess.

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicConnectionImpl.java line 1949:

> 1947:             if (t instanceof AssertionError) {
> 1948:                 this.terminator.terminate(TerminationCause.forException(t));
> 1949:             }

Would you mind explaining the rationale for this change, please?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/29956#discussion_r2864338979
PR Review Comment: https://git.openjdk.org/jdk/pull/29956#discussion_r2864355187