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

Support of Scala.js 1.10.0 #2116

Closed
catap opened this issue Apr 5, 2022 · 6 comments
Closed

Support of Scala.js 1.10.0 #2116

catap opened this issue Apr 5, 2022 · 6 comments

Comments

@catap
Copy link
Contributor

catap commented Apr 5, 2022

Scala.js released 1.10.0 which contains broking changes inside java.lang.UUID which forces to use java.security.SecureRandom: https://www.scala-js.org/news/2022/04/04/announcing-scalajs-1.10.0/

TestSortingReporter is using UUID.randomUUID which can't be used in sjs since this release, it fails on compiling like:

[error] Referring to non-existent class java.security.SecureRandom
[error]   called from private java.util.UUID$.csprng$lzycompute()java.util.Random
[error]   called from private java.util.UUID$.csprng()java.util.Random
[error]   called from java.util.UUID$.randomUUID()java.util.UUID
[error]   called from static java.util.UUID.randomUUID()java.util.UUID
[error]   called from org.scalatest.tools.TestSortingReporter.distributingTest(java.lang.String)void
[error]   called from org.scalatest.AsyncSuperEngine.runTestImpl(org.scalatest.Suite,java.lang.String,org.scalatest.Args,boolean,boolean,scala.Function2,scala.concurrent.ExecutionContext)org.scalatest.Status
[error]   called from static org.scalatest.wordspec.AsyncWordSpecLike$class.runTest(org.scalatest.wordspec.AsyncWordSpecLike,java.lang.String,org.scalatest.Args)org.scalatest.Status
[error]   called from
...

I not sure how this should be fixed on the right way:

  • by adding dependency from fake SecureRandom;
  • moving away from UUID.

Impact? Huge. AsyncWordSpec and similar things are affected.

@cheeseng

@gzm0
Copy link

gzm0 commented Apr 6, 2022

Oh dear. /cc @sjrd for visibility.

by adding dependency from fake SecureRandom;

Please do not do that. It'll spread the fake SecureRandom everywhere (even if it is just in the test config).

moving away from UUID.

You can write your own implementation of UUID.randomUUID (using a java.util.Random). The rest of UUID is unaffected. (please make sure you can actually live with predictable UUIDs, but at least superficially, it seems in a testing framework to identify test runs, that should be fine).

@gzm0
Copy link

gzm0 commented Apr 6, 2022

Implementation of randomUUID in Scala.js:

https://github.com/scala-js/scala-js/blob/e41949e96b91fb4d48e65e3cd9a15b57e518d673/javalib/src/main/scala/java/util/UUID.scala#L143-L160

As a stop gap measure, you could just copy this and use val rng: Random = new java.util.Random() instead of csprng.

@catap
Copy link
Contributor Author

catap commented Apr 6, 2022

@gzm0 to be honest I really think that this changes in SJS a kind of disaster.

@catap catap changed the title Support os Scala.js 1.10.0 Support of Scala.js 1.10.0 Apr 6, 2022
@sjrd
Copy link
Contributor

sjrd commented Apr 6, 2022

by adding dependency from fake SecureRandom;

Please do not do that. It'll spread the fake SecureRandom everywhere (even if it is just in the test config).

I cannot emphasize this enough. Don't depend on scalajs-fake-insecure-java-securerandom in a library, ever.

You can write your own implementation of UUID.randomUUID (using a java.util.Random). The rest of UUID is unaffected. (please make sure you can actually live with predictable UUIDs, but at least superficially, it seems in a testing framework to identify test runs, that should be fine).

This is probably the way to go.

Another possibility is to indeed move away from UUID in the specific case of TestSortingReporter. The UUID only seems to serve as an in-memory identifier, privately within TestSortingReporter. An object identity would achieve the same result more efficiently and more reliably.

@sjrd
Copy link
Contributor

sjrd commented Apr 6, 2022

@gzm0 to be honest I really think that this changes in SJS a kind of disaster.

Yes, it is. So is leaving a gaping security hole in the middle of a codebase as widely depended on as Scala.js.

sjrd added a commit to sjrd/scalatest that referenced this issue Apr 6, 2022
…tingReporter.

The UUIDs are only used as an in-memory identifier for `Slot`s.
It is more efficient and more reliable to use an object identity
instead, for that purpose.

This removes the dependency on `UUID.randomUUID()`, which unblocks
using ScalaTest with Scala.js >= 1.10.0. See
GHSA-j2f9-w8wh-9ww4
and
scala-js/scala-js#4657
for the reason why `randomUUID()` fails to link by default in
recent Scala.js.
@sjrd
Copy link
Contributor

sjrd commented Apr 6, 2022

Here is a proposed fix: #2117

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

No branches or pull requests

3 participants