On Fri, 1 Nov 2024 23:22:35 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 1868: > >> 1866: "Unexpected plaintext alert received: " + >> 1867: "Level: %s; Alert: %s", >> 1868: Alert.Level.nameOf(bb.get(bb.position())), > > Take or leave this comment: you could dump the raw values as well. > > Unexpected plaintext alert received: Level: 1(warning); Alert: > 90(user_canceled) > > Again, not critical, but might be nice. Well, we'll print raw values (that's how `nameOf` works) if level or alert are not recognized. > test/jdk/sun/security/ssl/SSLCipher/SSLEngineNoServerHelloClientShutdown.java > line 67: > >> 65: "Level: warning; Alert: user_canceled"; >> 66: >> 67: protected SSLEngine clientEngine; // client Engine > > I finally figured out why you had tweaked/removed the `final`s/`private`s > protections in this file: it's because you are having `SSLSocketNoServer...` > reuse some of this code. It was very surprising on first read. > > I probably would have just duplicated in the other file to have the test be > standalone. Yes, I've decided to re-use some code from SSLEngine* test since both tests are closely related. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1828091746 PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1828094040