On Fri, 1 Nov 2024 23:13:44 GMT, Bradford Wetmore <wetm...@openjdk.org> wrote:
>> Artur Barashev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove logging
>
> src/java.base/share/classes/sun/security/ssl/SSLCipher.java line 1865:
>
>> 1863: if (contentType == ContentType.ALERT.id
>> 1864: && bb.remaining() == 2) {
>> 1865: throw new
>> GeneralSecurityException(String.format(
>
> Ok, I am grudgingly ok with throwing a `GeneralSecurityException` here, but
> only because it is then being rewrapped inside a `SSLProtocolException` in
> `decodeInputRecord`.
>
> `SSLProtocolException` isn't my first choice of exceptions, as it feels to me
> like it should be a `SSLHandshakeException` which is when the client/server
> were unable to negotiate the desired level of security for some reason. But
> I see it's used throughout this class, so ok.
>
> From `SSLProtocolException`
>
>> Normally this indicates a flaw in one of the protocol implementations.
>
> I wouldn't really consider it a flaw in one of the protocol implementations,
> as the implementation is only acting on the data is had available at the time
> it decided to shutdown.
- It would be a flaw of protocol implementation of the other party, so if we
are the server the flaw would be on the client's side which makes sense in this
scenario.
- Yes, I've decided to use `GeneralSecurityException` for all the reasons you
have listed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1827975183
