On Fri, 30 Jan 2026 23:49:08 GMT, Bradford Wetmore <[email protected]> wrote:

>> Sean Coffey has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review comments from Brad
>
> src/java.base/share/classes/sun/security/ssl/DTLSInputRecord.java line 1648:
> 
>> 1646:             for (RecordFragment fragment : bufferedFragments) {
>> 1647:                 if (fragment.contentType == 
>> ContentType.CHANGE_CIPHER_SPEC.id) {
>> 1648:                     if (hasFin) {
> 
> This appears to be another merge issue from a recent changeset (October).  
> 8367059 removed the `if (hasFin)` check.  
> 
> Please check this one closely.  Here's the changeset ID.
> 436dc687ba2ead1662a4e0125cea0966fac825e5

Thanks for highlighting this. I've gone through this src again and fixed up. 
Only delta in these files (outside of SSLLogger) should only include 
introduction of SSLLogger Opt enum use and line width corrections where 
possible.

> test/jdk/sun/security/ssl/SSLEngineImpl/SSLEngineKeyLimit.java line 114:
> 
>> 112:                     " -Dtest.src=" + System.getProperty("test.src") +
>> 113:                             " -Dtest.jdk=" + 
>> System.getProperty("test.jdk") +
>> 114:                             " -Djavax.net.debug=ssl" +
> 
> I'm assuming that changing from `ssl,handshake` to just `ssl` is ok?  I 
> didn't actually check.  I notice this in several tests.

yes - so, these tests depend on output that comes from the "SSL" level. 

e.g. sun/security/ssl/SSLSocketImpl.java

            if (SSLLogger.isOn() && SSLLogger.isOn(SSLLogger.Opt.SSL)) {
                SSLLogger.finest("trigger key update");
            }

While "ssl, handshake" would print any ssl message in the past, this fix means 
that "ssl, handshake" now filters on handshake logs

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2769408284
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2769403110

Reply via email to