-
Notifications
You must be signed in to change notification settings - Fork 358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support TLSv1.3 #4528
Support TLSv1.3 #4528
Conversation
Signed-off-by: Jorge Bescos Gascon <[email protected]>
stepFinished = true; | ||
handshakeFinished = true; | ||
break; | ||
throw new IllegalStateException("Trying to handshake, but SSL engine not in HANDSHAKING state." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would advice to move direct text into localization.properties and use LocalizableMessageFactory instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@senivam although that is unreachable (in theory), I decided to keep it as it was. It will not throw exception.
I added the localization property for the case of NOT_HANDSHAKING, because it was throwing the same exception.
I also modified the test to force to test all supported SSL protocols. For the case of JDK11 the TLSv13 is included.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just verified with Wireshark that we need to execute the tests in different JVMs. So I had to set <reuseForks>false</reuseForks>
and move each protocol to a different test class. It seems when you use @Parametrized
tests are executed in the same JVM.
Signed-off-by: Jorge Bescos Gascon <[email protected]>
Signed-off-by: Jorge Bescos Gascon <[email protected]>
...ector/src/main/resources/org/glassfish/jersey/jdk/connector/internal/localization.properties
Show resolved
Hide resolved
Signed-off-by: Jorge Bescos Gascon <[email protected]>
I fixed recently the same issue in Tyrus.
eclipse-ee4j/tyrus#707
Find in that PR the information.