On Fri, 27 Feb 2026 16:03:00 GMT, Daniel Fuchs <[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. > > Daniel Fuchs has updated the pull request incrementally with one additional > commit since the last revision: > > Review feedback: add comment Marked as reviewed by vyazici (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/29956#pullrequestreview-3867751498
