Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Conversation

BlasiusSecundus
Copy link

Pull Request type

  • Feature

Changes in this PR

Issue #2312 replace Nashorn with GraalVM JS Engine

Nashorn is deprecated in JDK 11 and removed in JDK 15, resulting in runtime errors (and test failures) on JDK 15+

this is required for JDK 15+ compatibility
(Nashorn was removed in JDK 15)
@BlasiusSecundus BlasiusSecundus force-pushed the feature/2312-replace-nashorn-engine-with-graalvm branch from 5532191 to daacd21 Compare September 11, 2022 17:47
@apanicker-nflx apanicker-nflx added the type: important Important changes label Sep 12, 2022
@apanicker-nflx apanicker-nflx changed the title feat(#2312): replace Nashorn with GraalVM JS Engine Replace Nashorn with GraalVM JS Engine Sep 12, 2022
@@ -105,6 +105,7 @@ public Javascript validate() {
LOGGER.error("missing " + ENGINE + " engine. Ensure you are running supported JVM");
return this;
}
scriptEngine.put("polyglot.js.allowHostAccess", true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Author

Choose a reason for hiding this comment

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

This is required to access Java objects bound as input. (Nashorn allows this by default, but GraalVM requires explicit setting for security reasons: https://www.graalvm.org/22.2/reference-manual/js/NashornMigrationGuide/#secure-by-default). You can validate this by removing the line and running the tests: it will result in errors like this: TypeError: Cannot read property 'testKey1' of undefined

This is equivalent to HostAccess.ALL (see https://www.graalvm.org/22.2/reference-manual/js/ScriptEngine/) which should be identical to the Nashorn behavior.

An alternative would be to use HostAccess.EXPLICIT for greater security, but that would result in a breaking change.

To my understanding, workflows are considered part of the source code of the host application and can be trusted at the same level, thus this should not pose a security risk.

@github-actions
Copy link
Contributor

This PR is stale, because it has been open for 45 days with no activity. Remove the stale label or comment, or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Nov 19, 2022
@github-actions
Copy link
Contributor

This PR was closed, because it has been stalled for 7 days with no activity.

@github-actions github-actions bot closed this Nov 27, 2022
@cwensel
Copy link

cwensel commented Aug 2, 2023

Is there any technical reason this cannot be merged?

Java 11 active support ends in two months: https://endoflife.date/java

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Stale type: important Important changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants