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

SLING-11610 - Sling XSS API 2.3.0 does not work on Java 17 #32

Merged
merged 9 commits into from
Jun 30, 2023

Conversation

rombert
Copy link
Contributor

@rombert rombert commented Jun 14, 2023

Switch from reflective access to using sun.misc.Unsafe. Even though 'unsafe' looks scary, it's part of the jdk.unsupported module which both 'exports' and 'opens' its packages.

Reference: https://blogs.oracle.com/javamagazine/post/a-peek-into-java-17-continuing-the-drive-to-encapsulate-the-java-runtime-internals

Switch from reflective access to using sun.misc.Unsafe. Even though 'unsafe' looks scary,
it's part of the jdk.unsupported module which both 'exports' and 'opens' its packages.

Reference: https://blogs.oracle.com/javamagazine/post/a-peek-into-java-17-continuing-the-drive-to-encapsulate-the-java-runtime-internals
Ensure that the jdk.unsupported module is available when compiling.
- update to the latest version of Mockito
- drop Powermock
@rombert rombert marked this pull request as draft June 15, 2023 11:28
Exclude Java 8, for now, since we have a baseline Java 11 version.
Restore Java 8 compatibility, but with Java 9+ builds broken.
@rombert
Copy link
Contributor Author

rombert commented Jun 15, 2023

I've tried to set this up in a way that works with Java 8 through 21. I think the code is working fine, the problem is the build.

In 29c1733 I see Java 11, 17 and 21 builds passing. I've reconfigured the project for Java 8 in 674c7cb, but at that point Java 11+ builds start failing with

option --add-modules not allowed with target 1.8

That error makes sense, since the JPMS has been introduced with Java 9.

I think supporting Java 11, 17, and 21 is preferred to only supporting Java 8 and 11, but I don't have an idea about how to fix the build.

@kwin - maybe you have an idea? I know you've worked on the parent POM plumbing for the compiler plugin.

@kwin
Copy link
Member

kwin commented Jun 15, 2023

Not sure here TBH.

Configure the maven-compiler-plugin to use source/target instead of release, even for newer JDK
versions. This has the effect of running javac with fewer checks, and allows compilation on
Java 8 to complete succesfully.
@sonarcloud
Copy link

sonarcloud bot commented Jun 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@rombert rombert marked this pull request as ready for review June 28, 2023 22:10
@rombert
Copy link
Contributor Author

rombert commented Jun 28, 2023

It seems that I figured out the right incantations to allow compilation and test runs on Java 8. The CI tests make me pretty confident that this change works. Also ran the Starter tests with this change on Java 11 and 17.

@rombert rombert requested review from raducotescu and kwin June 28, 2023 22:11
@rombert rombert merged commit bc0a76f into master Jun 30, 2023
3 checks passed
@rombert rombert deleted the issue/SLING-11610 branch June 30, 2023 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants