On Fri, 27 Feb 2026 13:26:03 GMT, Volkan Yazici <[email protected]> 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. Not exactly as that would retain the memory from the input buffer which might be large (could be a slice of the whole packet). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29956#discussion_r2864591796
