On Fri, 7 Apr 2023 02:14:59 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
>> Matthew Donovan has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - added comment referring to relevant RFC
>> - clarified if-statements; fixed exception message wording
>
> src/java.base/share/classes/sun/security/ssl/SSLEngineInputRecord.java line
> 266:
>
>> 264: //
>> 265: if (contentType == ContentType.HANDSHAKE.id) {
>> 266: if (!fragment.hasRemaining()) {
>
> It may be easier to read if using "contentLen != 0", instead of
> "fragment.hasRemaining()". I may add a comment like what is says in RFC
> 8446, "Implementations MUST NOT send zero-length fragments of Handshake
> types, even if those fragments contain padding."
I updated the if-logic and added the comment
> src/java.base/share/classes/sun/security/ssl/SSLEngineInputRecord.java line
> 267:
>
>> 265: if (contentType == ContentType.HANDSHAKE.id) {
>> 266: if (!fragment.hasRemaining()) {
>> 267: throw new SSLProtocolException("Handshake packets may
>> not be zero-length");
>
> "may not" -> "must not"
Fixed
> src/java.base/share/classes/sun/security/ssl/SSLSocketInputRecord.java line
> 287:
>
>> 285: if (contentType == ContentType.HANDSHAKE.id) {
>> 286: ByteBuffer handshakeFrag = fragment;
>> 287: if (!handshakeFrag.hasRemaining()) {
>
> Same comment as
> src/java.base/share/classes/sun/security/ssl/SSLEngineInputRecord.java
Also fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13306#discussion_r1160716144
PR Review Comment: https://git.openjdk.org/jdk/pull/13306#discussion_r1160716277
PR Review Comment: https://git.openjdk.org/jdk/pull/13306#discussion_r1160716381
