-
Notifications
You must be signed in to change notification settings - Fork 275
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
Add Java 8 style SNI hostname to OpenSSLEngineImpl #155
Conversation
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.
@kruton can you add a simple test or two for this?
This would be in the integration tests in Android's libcore project. |
095c714
to
50e3210
Compare
Here are the two tests I made for this in the libcore integration tests: Adding this actually found a bug in the existing endpointIdentificationAlgorithm tests. Also the added test found the problem you ran into with |
@kruton you'll want to rebase on the latest master and add any new public getters/setters to the |
@kruton also, it would be good to add one or two similar tests to https://github.com/google/conscrypt/blob/master/openjdk/src/test/java/org/conscrypt/OpenSSLEngineImplTest.java |
Maybe I can use a pared-down Conscrypt Android build manifest in Travis-CI builds to make sure we don't have a regression until #153 is 100% fixed. That will allow us to run the Android tests until we can separate the tests into its own project. |
The SNIHostName, et al., support was lacking from OpenSSLEngineImpl causing endpoint protocol identification to fail in Netty tests.
Sounds reasonable ... You have something in mind? Want to throw together a PR? |
Yes, I will put it together. |
The SNIHostName, et al., support was lacking from OpenSSLEngineImpl causing endpoint protocol identification to fail in Netty tests.
The SNIHostName, et al., support was lacking from OpenSSLEngineImpl
causing endpoint protocol identification to fail in Netty tests.