RFR: 8273553: sun.security.ssl.SSLEngineImpl.closeInbound also has similar error of JDK-8253368 [v2]

Xue-Lei Andrew Fan xuelei at openjdk.java.net
Wed Mar 23 18:17:39 UTC 2022


On Wed, 23 Mar 2022 17:01:12 GMT, Bradford Wetmore <wetmore at openjdk.org> wrote:

>> JDK-8253368 changed the behavior of SSLSocket to no longer throw a fatal internal_error (80) and invalidate existing sessions (either completed or under construction) as described in (RFC 4346/TLSv1.1+) if a connection was closed without receiving a close_notify alert from the peer.  
>> 
>> This change introduces similar behavior to SSLEngine.
>> 
>> The unit test checks that closing the read(input) sides of the SSLSocket/SSLEngine throws an SSLException, but doesn't invalidate their respective sessions.
>> 
>> Tier1/2 mach5 tests have been successfully run.
>
> Bradford Wetmore has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision:
> 
>  - Code review comment: enclose conContext.closeInbound() in a try/finally block.
>  - Merge branch 'master' into JDK-8273553
>  - Merge branch 'master' into JDK-8273553
>  - Added SSLSocket bugid since we're actually checking both sides now.
>  - I/O Issues, rewrite the I/O section so that early Socket closes don't kill our server-side reads.
>  - Merge branch 'master' into JDK-8273553
>  - Merge branch 'master' into JDK-8273553
>  - Merge
>  - Minor test tweaks.
>  - Remove inadvertent whitespace
>  - ... and 2 more: https://git.openjdk.java.net/jdk/compare/1662472a...b2f64d92

src/java.base/share/classes/sun/security/ssl/SSLEngineImpl.java line 802:

> 800:             } finally {
> 801:                 engineLock.unlock();
> 802:             }

I looked the update further.  It would be nice if the try-statement could support more than one finally blocks in the future so that the code could be more clear.

try {
    ...
} finally {
   // the 1st final block
} finally {
   // the 2nd final block
}


For the block from 781 to 787, it may be more effective if moving the block out of the synchronization lock (that's, moving 781-787 to line 779.)

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

PR: https://git.openjdk.java.net/jdk/pull/7796



More information about the security-dev mailing list