I would suggest to fix in a separate bug. Thanks, Xuelei
> On Nov 3, 2021, at 2:27 PM, Daniel Jeliński <d...@openjdk.java.net> wrote: > > On Wed, 3 Nov 2021 09:12:29 GMT, Daniel Jeliński <d...@openjdk.java.net> > wrote: > >>> src/java.base/share/classes/sun/security/ssl/SSLEngineOutputRecord.java >>> line 436: >>> >>>> 434: >>>> 435: void queueUpCipherDispose() { >>>> 436: RecordMemo lastMemo = handshakeMemos.getLast(); >>> >>> Sorry, I missed that the getLast could throw exception if it is empty. I >>> may check it before the call to getLast. >>> >>> + if (handshakeMemos.isEmpty()) { >>> + return; >>> + } >> >> my mistake. Replaced with `peekLast`, should be better now. > > Well, looks like this uncovered another, unrelated bug. `handshakeMemos` is > only empty here when we handle TLS1.3 server hello and session ID is empty, > see stack trace: > > Caused by: java.util.NoSuchElementException > at java.base/java.util.LinkedList.getLast(LinkedList.java:261) > at > java.base/sun.security.ssl.SSLEngineOutputRecord$HandshakeFragment.queueUpCipherDispose(SSLEngineOutputRecord.java:436) > at > java.base/sun.security.ssl.SSLEngineOutputRecord.disposeWriteCipher(SSLEngineOutputRecord.java:159) > at > java.base/sun.security.ssl.OutputRecord.changeWriteCiphers(OutputRecord.java:198) > at > java.base/sun.security.ssl.ServerHello$T13ServerHelloConsumer.consume(ServerHello.java:1372) > > This is only supposed to be empty when > [jdk.tls.client.useCompatibilityMode](https://github.com/openjdk/jdk/blob/6765f902505fbdd02f25b599f942437cd805cad1/src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java#L101) > is false, which it never is (also the comment above that line is > copy/pasted, should be fixed). So I did some more digging and found that we > do not set sessionId in clientHello when resuming TLS1.3 session: > - sessionId is set to empty > [here](https://github.com/openjdk/jdk/blob/04a806ec86a388b8de31d42f904c4321beb69e14/src/java.base/share/classes/sun/security/ssl/ClientHello.java#L406) > - not updated > [here](https://github.com/openjdk/jdk/blob/04a806ec86a388b8de31d42f904c4321beb69e14/src/java.base/share/classes/sun/security/ssl/ClientHello.java#L535) > because it's TLS 1.3 > - not updated > [here](https://github.com/openjdk/jdk/blob/04a806ec86a388b8de31d42f904c4321beb69e14/src/java.base/share/classes/sun/security/ssl/ClientHello.java#L577) > because session is not null > > Apparently we have no tests for TLS1.3 session resumption in jdk_security and > no tests for useCompatibilityMode=false (otherwise I would have noticed this > sooner). > > Let me know if I should fix these issues here or in a separate PR. > > ------------- > > PR: https://git.openjdk.java.net/jdk/pull/6084