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

[#286] disable SecurityManager starting with Java 21 #287

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bmarwell
Copy link
Contributor

@bmarwell bmarwell commented Oct 19, 2023

fixes #286

@bmarwell
Copy link
Contributor Author

You can ignore the Maven warning for now, it will go away with the next maven-compiler-plugin release: apache/maven-compiler-plugin#202

@bmarwell
Copy link
Contributor Author

@keeganwitt please review (cannot tag you as a reviewer)

@keeganwitt
Copy link
Member

Do you think we should include something about -Djava.security.manager=allow? for Java 18, 19, 20 users? I thought it might be useful, but then started second-guessing it since those aren't LTS releases.

pom.xml Show resolved Hide resolved
@keeganwitt
Copy link
Member

keeganwitt commented Oct 23, 2023

Do you think we should include something about -Djava.security.manager=allow? for Java 18, 19, 20 users? I thought it might be useful, but then started second-guessing it since those aren't LTS releases.

Actually this applies to Java 21 users as well. I don't think it's been removed (yet), just turned off by default. This being the case, should we really be turning it off entirely?

@bmarwell
Copy link
Contributor Author

Do you think we should include something about -Djava.security.manager=allow? for Java 18, 19, 20 users? I thought it might be useful, but then started second-guessing it since those aren't LTS releases.

Actually this applies to Java 21 users as well. I don't think it's been removed (yet), just turned off by default. This being the case, should we really be turning it off entirely?

That's true.
Other things we could do:

  • guess any future version (Java 24, Java 30, whatever) and change all java21 occurrences to any future version
  • For now, just print a warning that the feature will be removed (regardless of the -XX options of the user)
  • and/or add the META-INF/versions/javaXX code path later

@bmarwell
Copy link
Contributor Author

@keeganwitt this pr is flawed, it should be META-INF/versions/21, not ../java21. Will update the PR.

@keeganwitt
Copy link
Member

For now, I'm gonna proceed with the warning. But this PR will likely prove useful down the road.

…ty_manager_java21

# Conflicts:
#	pom.xml
#	src/main/java/org/codehaus/gmavenplus/mojo/ShellMojo.java
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.

Remove Security Manager (for Java 21+)
2 participants