Skip to content
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

Decomission DefaultHostnameVerifier #138

Merged
merged 10 commits into from
Oct 27, 2018
Merged

Decomission DefaultHostnameVerifier #138

merged 10 commits into from
Oct 27, 2018

Conversation

dwijnand
Copy link
Member

Upstream playframework/playframework@f35dd09

Fixes #98 (verified test:compile succeeds locally)
Fixes #100

@SethTisue
Copy link
Member

MiMa is squawking

@dwijnand
Copy link
Member Author

It's not wrong ;-)

@SethTisue
Copy link
Member

MiMa is annoying that way

@SethTisue
Copy link
Member

SethTisue commented Oct 25, 2018

in the Scala community build this causes

[akka-stream] [error] /home/jenkins/workspace/scala-2.12.x-integrate-community-build/target-0.9.16/project-builds/akka-stream-fe1d9d022b8160be9527102bfb64d1e77c200315/akka-stream/src/main/scala/com/typesafe/sslconfig/akka/AkkaSSLConfig.scala:113:61: not found: type DisabledComplainingHostnameVerifier
[akka-stream] [error]       if (config.loose.disableHostnameVerification) classOf[DisabledComplainingHostnameVerifier].asInstanceOf[Class[HostnameVerifier]]
[akka-stream] [error]                                                             ^
[akka-stream] [error] /home/jenkins/workspace/scala-2.12.x-integrate-community-build/target-0.9.16/project-builds/akka-stream-fe1d9d022b8160be9527102bfb64d1e77c200315/akka-stream/src/main/scala/com/typesafe/sslconfig/akka/AkkaSSLConfig.scala:114:19: value hostnameVerifierClass is not a member of com.typesafe.sslconfig.ssl.SSLConfigSettings
[akka-stream] [error]       else config.hostnameVerifierClass.asInstanceOf[Class[HostnameVerifier]]
[akka-stream] [error]                   ^

@marcospereira
Copy link
Contributor

Should we add oraclejdk11 to Travis configuration?

@SethTisue
Copy link
Member

trying it: scala/community-build@2606f80

@wsargent
Copy link
Contributor

You'll want to remove the debug section from the docs (or add a note) to reflect the package removal

@dwijnand
Copy link
Member Author

Good idea, @wsargent. Do you mind reviewing if I dropped too much?

@wsargent
Copy link
Contributor

@dwijnand I don't have review rights :-)

Maybe add a warning if ssl-config.debug is defined in config (since it no longer does anything) and refer to https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/ReadDebug.html as the main debugging reference?

@dwijnand
Copy link
Member Author

Thanks, Will. Done.

(Btw, anyone in GitHub can review pull requests 🙂)

@wsargent
Copy link
Contributor

wsargent commented Oct 27, 2018

@dwijnand https://github.com/tersesystems/debugjsse and https://github.com/tersesystems/ocapjsse#logging are better debugging solutions anyway -- I can open up a PR to have them map to the debug config and preserve functionality without the hacks...

@SethTisue
Copy link
Member

trying it: scala/community-build@2606f80

green at https://scala-ci.typesafe.com/view/scala-2.12.x/job/scala-2.12.x-jdk11-integrate-community-build/138/

@dwijnand
Copy link
Member Author

Sounds good to me, @wsargent.

Excellent. Thanks, @SethTisue.

Copy link
Contributor

@marcospereira marcospereira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@dwijnand dwijnand merged commit e8baac6 into lightbend:master Oct 27, 2018
@dwijnand dwijnand deleted the HostnameChecker branch October 27, 2018 17:03
@SethTisue
Copy link
Member

@dwijnand in the Scala 2.12 community build at e.g. https://scala-ci.typesafe.com/view/scala-2.12.x/job/scala-2.12.x-integrate-community-build/3722/ , play-ws is now failing because DebugConfiguration vanished. could I convince you to submit a small play-ws PR that fixes it...?

(I don't know the code, but perhaps an alternative would be to reintroduce DebugConfiguration here in this repo but as a deprecated no-op?)

@dwijnand
Copy link
Member Author

Damn. Just for context, the reason I dropped it is because it doesn't run on Java 11, as it's using privileged reflection actions, that are now enforced by the JVM (IIUC).

So I wonder how much play-ws actually depends on the behaviour of DebugConfiguration...

I'll get back to this when I'm done travelling.

@SethTisue
Copy link
Member

thanks. it was easy to get the community build green again for now with SethTisue/play-ws@a99b2b6 and scala/community-build@bb8b9d2

@SethTisue
Copy link
Member

er, well, except for:

[playframework] [error] /home/jenkins/workspace/scala-2.12.x-integrate-community-build/target-0.9.16/project-builds/playframework-336d962b6fcd9f1f9964169addd9c496e6290d3f/framework/src/play-ahc-ws/src/main/scala/play/api/libs/ws/ahc/AhcWSModule.scala:15: object debug is not a member of package com.typesafe.sslconfig.ssl
[playframework] [error] import com.typesafe.sslconfig.ssl.debug.DebugConfiguration
[playframework] [error]                                   ^
[playframework] [error] /home/jenkins/workspace/scala-2.12.x-integrate-community-build/target-0.9.16/project-builds/playframework-336d962b6fcd9f1f9964169addd9c496e6290d3f/framework/src/play-ahc-ws/src/main/scala/play/api/libs/ws/ahc/AhcWSModule.scala:71: not found: type DebugConfiguration
[playframework] [error]       new DebugConfiguration(loggerFactory).configure(wsClientConfig.ssl.debug)
[playframework] [error]           ^
[playframework] [error] two errors found
[playframework] [error] (Play-AHC-WS/compile:compileIncremental) Compilation failed

SethTisue/playframework@747409c
scala/community-build@b4c8d93

@dwijnand
Copy link
Member Author

dwijnand commented Nov 8, 2018

Opened #142 to restore the debug packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HostnameChecker can be retired Support Java 11
5 participants