On Fri, 20 Sep 2024 15:47:14 GMT, Jamil Nimeh <jni...@openjdk.org> wrote:
>> Artur Barashev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add assertions. Add the final server wrap > > src/java.base/share/classes/sun/security/ssl/SSLTransport.java line 133: > >> 131: byte contentType = packet.get(); // >> pos: 0 >> 132: byte majorVersion = packet.get(); // >> pos: 1 >> 133: byte minorVersion = packet.get(); // >> pos: 2 > > A bit of a nit: You might consider using `Record.getInt8()` here for these > 1-byte calls. It might require you to do a casts to byte since those getInt8 > calls return int, but the good thing is that if there's an attempted read off > the end of the buffer you get SSLException which is consistent with the > exception type you get from the `Record.getInt16()` on line 134. The > `Buffer.get()` calls throw BufferUnderflowException which is a > RuntimeException and might be handled differently or in a different part of > the code further up the stack than SSLException would be. Great suggestion, I'll update the code, thanks! ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1768871812
