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

Do not disable the security manager on Java 17 VMs and newer as it is deprecated for removal. #2123

Merged
merged 4 commits into from Sep 5, 2022

Conversation

raphw
Copy link
Contributor

@raphw raphw commented Jul 27, 2022

This fixes #1579. If needed, disabling the security manager can still be requested on Java 17 and newer by setting the edu.umd.cs.findbugs.securityManagerDisabled property to false. Using this property also allows to disable this functionality on older VMs.

ThrawnCA
ThrawnCA previously approved these changes Jul 31, 2022
@raphw raphw requested a review from KengoTODA August 28, 2022 21:12
@KengoTODA
Copy link
Member

Please merge the latest origin/master a to resolve a conflict in the CHANGELOG.md then I'll review your code. Thanks in advance!

… deprecated for removal.

This fixes spotbugs#1579. If needed, disabling the security manager can still be requested on Java 17
and newer by setting the 'edu.umd.cs.findbugs.securityManagerDisabled' property to 'false'.
Using this property also allows to disable this functionality on older VMs.
@raphw
Copy link
Contributor Author

raphw commented Aug 29, 2022

Indeed, pushed the changes, including the rebase.

KengoTODA
KengoTODA previously approved these changes Aug 29, 2022
Copy link
Member

@KengoTODA KengoTODA left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@KengoTODA KengoTODA mentioned this pull request Sep 1, 2022
1 task
@KengoTODA
Copy link
Member

Not sure why the build is failing. 🤔

It seems that there are some problem like #2146?

ThrawnCA
ThrawnCA previously approved these changes Sep 2, 2022
securityManagerDisabled = false;
LOGGER.debug("failed to detect the ability of security manager feature, so treat it as available", t);
}
SECURITY_MANAGER_DISABLED = securityManagerDisabled;
Copy link
Member

Choose a reason for hiding this comment

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

SpotBugs reports a potential bug here, please consider to make this static field final.

edu.umd.cs.findbugs.util.SecurityManagerHandler.SECURITY_MANAGER_DISABLED isn't final but should be
This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.

@KengoTODA KengoTODA merged commit 4c0c1b9 into spotbugs:master Sep 5, 2022
gtoison added a commit to gtoison/sonar-findbugs that referenced this pull request Feb 12, 2023
Since SpotBugs 4.7.3 and spotbugs/spotbugs#2123
it is no longer necessary to set the security manager6
gtoison added a commit to spotbugs/sonar-findbugs that referenced this pull request Feb 12, 2023
Since SpotBugs 4.7.3 and spotbugs/spotbugs#2123
it is no longer necessary to set the security manager6
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.

System::setSecurityManager is deprecated and will be removed in a future JDK release
3 participants