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

Re-introduce deprecated methods #342

Merged
merged 4 commits into from
Mar 16, 2022

Conversation

raboof
Copy link
Contributor

@raboof raboof commented Mar 14, 2022

Without implementation: only for improved binary compatibility
with 0.4.0

@raboof raboof force-pushed the reintroduce-deprecated-methods branch from e0221be to 769774b Compare March 14, 2022 16:16
For improved binary compatibility with 0.4.0
@raboof raboof force-pushed the reintroduce-deprecated-methods branch from 769774b to 6a85d84 Compare March 14, 2022 16:21
@raboof raboof marked this pull request as draft March 14, 2022 16:22
@mkurz
Copy link
Contributor

mkurz commented Mar 15, 2022

@raboof Anything missing, can I help?

@raboof
Copy link
Contributor Author

raboof commented Mar 16, 2022

Hmm, looks like we need at least part of Ciphers as well...

Error injecting constructor, java.lang.NoClassDefFoundError: com/typesafe/sslconfig/ssl/Ciphers$
[info]   at play.api.libs.ws.ahc.AsyncHttpClientProvider.<init>(AhcWSModule.scala:51)
[info]   at play.api.libs.ws.ahc.AsyncHttpClientProvider.class(AhcWSModule.scala:50)

@raboof
Copy link
Contributor Author

raboof commented Mar 16, 2022

Hmm, looks like we need at least part of Ciphers as well...

Error injecting constructor, java.lang.NoClassDefFoundError: com/typesafe/sslconfig/ssl/Ciphers$
[info]   at play.api.libs.ws.ahc.AsyncHttpClientProvider.<init>(AhcWSModule.scala:51)
[info]   at play.api.libs.ws.ahc.AsyncHttpClientProvider.class(AhcWSModule.scala:50)

Ah, that's only in Play 2.7, that might no longer be relevant

@raboof raboof marked this pull request as ready for review March 16, 2022 13:35
raboof added a commit to raboof/akka that referenced this pull request Mar 16, 2022
raboof added a commit to raboof/akka that referenced this pull request Mar 16, 2022
Copy link
Contributor

@mkurz mkurz left a comment

Choose a reason for hiding this comment

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

If akka is happy, I am too 👍

@raboof raboof merged commit 8c29624 into lightbend:main Mar 16, 2022
@mkurz
Copy link
Contributor

mkurz commented Mar 16, 2022

@raboof Did you also run sbt mimaReportBinaryIssues? If I set mimaPreviousArtifacts here

ssl-config/build.sbt

Lines 22 to 24 in 8c29624

mimaPreviousArtifacts := (CrossVersion.partialVersion(scalaVersion.value) match {
case Some((2, _)) =>
Set("0.5.0")

to 0.4.2, MiMa finds 79 potential problems... Or is the MiMa output not relevant here?
(0.4.2 is what akka 2.6.18 is using).

@raboof
Copy link
Contributor Author

raboof commented Mar 16, 2022

@raboof Did you also run sbt mimaReportBinaryIssues? If I set mimaPreviousArtifacts here

ssl-config/build.sbt

Lines 22 to 24 in 8c29624

mimaPreviousArtifacts := (CrossVersion.partialVersion(scalaVersion.value) match {
case Some((2, _)) =>
Set("0.5.0")

to 0.4.2, MiMa finds 79 potential problems... Or is the MiMa output not relevant here?
(0.4.2 is what akka 2.6.18 is using).

Well, yes and no: I only ran MiMa against 0.5.0.

The differences between 0.4.2 and 0.5.0 were already identified and reviewed, for example in https://github.com/lightbend/ssl-config/pull/302/files#diff-5634c415cd8c8504fdb973a3ed092300b43c4b8fc1e184f7249eb29a55511f91R40 . If we were going for complete safety then we shouldn't have updated ssl-config in Akka at all. On the other hand it is perhaps unlikely that downstream projects other than play-ws were using these API's, so given that it's perhaps OK to break the parts that play-ws 2.8 and later don't touch.

@mkurz
Copy link
Contributor

mkurz commented Mar 16, 2022

👍 So let's release 0.6.1 😉

raboof added a commit to raboof/akka that referenced this pull request Mar 17, 2022
raboof added a commit to raboof/akka that referenced this pull request Mar 17, 2022
raboof added a commit to akka/akka that referenced this pull request Mar 17, 2022
@OlegYch
Copy link

OlegYch commented Apr 21, 2022

Hmm, looks like we need at least part of Ciphers as well...

Error injecting constructor, java.lang.NoClassDefFoundError: com/typesafe/sslconfig/ssl/Ciphers$
[info]   at play.api.libs.ws.ahc.AsyncHttpClientProvider.<init>(AhcWSModule.scala:51)
[info]   at play.api.libs.ws.ahc.AsyncHttpClientProvider.class(AhcWSModule.scala:50)

Ah, that's only in Play 2.7, that might no longer be relevant

it is used in play 2.8 and akka-stream 2.6 as well:

[error] Category: Method being called not found
[error]   In artifact: play-ahc-ws-standalone_2.13-2.1.7.jar
[error]     In class: play.api.libs.ws.ahc.AhcConfigBuilder
[error]       In method:  $anonfun$validateDefaultTrustManager$2(play.api.libs.ws.ahc.AhcConfigBuilder, com.typesafe.sslconfig.ssl.AlgorithmChecker, java.security.cert.X509Certificate):334
[error]       Call to: com.typesafe.sslconfig.ssl.AlgorithmChecker.checkKeyAlgorithms(java.security.cert.X509Certificate)
[error]       Problem: Method not found: com.typesafe.sslconfig.ssl.AlgorithmChecker.checkKeyAlgorithms(java.security.cert.X509Certificate)
[error]       Found in: ssl-config-core_2.13-0.6.1.jar
[error]       --------
[error] Category: Class being called not found
[error]   In artifact: play-ahc-ws-standalone_2.13-2.1.7.jar
[error]     In class: play.api.libs.ws.ahc.AhcConfigBuilder
[error]       In method:  $anonfun$validateDefaultTrustManager$1(java.lang.String):330
[error]       Call to: com.typesafe.sslconfig.ssl.AlgorithmConstraintsParser$.parseAll(scala.util.parsing.combinator.Parsers$Parser, java.lang.CharSequence)
[error]       Problem: Class not found: com.typesafe.sslconfig.ssl.AlgorithmConstraintsParser$
[error]       --------
[error]       In method:  $anonfun$validateDefaultTrustManager$1(java.lang.String):330
[error]       Call to: com.typesafe.sslconfig.ssl.AlgorithmConstraintsParser$.expression()
[error]       Problem: Class not found: com.typesafe.sslconfig.ssl.AlgorithmConstraintsParser$
[error]       --------
[error]       In method:  $anonfun$validateDefaultTrustManager$1(java.lang.String):330
[error]       Access to: com.typesafe.sslconfig.ssl.AlgorithmConstraintsParser$.MODULE$
[error]       Problem: Class not found: com.typesafe.sslconfig.ssl.AlgorithmConstraintsParser$
[error]       --------
[error]       In method:  configureCipherSuites(java.lang.String[], com.typesafe.sslconfig.ssl.SSLConfigSettings):250
[error]       Call to: com.typesafe.sslconfig.ssl.Ciphers$.recommendedCiphers()
[error]       Problem: Class not found: com.typesafe.sslconfig.ssl.Ciphers$
[error]       --------
[error]       In method:  configureCipherSuites(java.lang.String[], com.typesafe.sslconfig.ssl.SSLConfigSettings):254
[error]       Call to: com.typesafe.sslconfig.ssl.Ciphers$.deprecatedCiphers()
[error]       Problem: Class not found: com.typesafe.sslconfig.ssl.Ciphers$
[error]       --------
[error]       In method:  configureCipherSuites(java.lang.String[], com.typesafe.sslconfig.ssl.SSLConfigSettings):250
[error]       Access to: com.typesafe.sslconfig.ssl.Ciphers$.MODULE$
[error]       Problem: Class not found: com.typesafe.sslconfig.ssl.Ciphers$
[error]       --------
[error]       In method:  configureCipherSuites(java.lang.String[], com.typesafe.sslconfig.ssl.SSLConfigSettings):254
[error]       Access to: com.typesafe.sslconfig.ssl.Ciphers$.MODULE$
[error]       Problem: Class not found: com.typesafe.sslconfig.ssl.Ciphers$
[error]       --------

@OlegYch
Copy link

OlegYch commented Apr 21, 2022

@raboof note com.typesafe.sslconfig.ssl.AlgorithmChecker.checkKeyAlgorithms and AlgorithmConstraintsParser missing as well

@raboof
Copy link
Contributor Author

raboof commented Apr 21, 2022

@mkurz I guess those are encountered in code paths that are not necessarily always used in play? will a play-ws release fix those?

@mkurz
Copy link
Contributor

mkurz commented May 19, 2022

@OlegYch @raboof Sorry for my later answer, I just released play-ws 2.1.10 which should fix the reported problem: https://github.com/playframework/play-ws/releases/tag/2.1.10

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.

3 participants