On Mon, 28 Oct 2024 12:29:07 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> This is the implementation of JEP 486: Permanently Disable the Security >> Manager. See [JEP 486](https://openjdk.org/jeps/486) for more details. The >> [CSR](https://bugs.openjdk.org/browse/JDK-8338412) describes in detail the >> main changes in the JEP and also includes an apidiff of the specification >> changes. >> >> NOTE: the majority (~95%) of the changes in this PR are test updates >> (removal/modifications) and API specification changes, the latter mostly to >> remove `@throws SecurityException`. The remaining changes are primarily the >> removal of the `SecurityManager`, `Policy`, `AccessController` and other >> Security Manager API implementations. There is very little new code. >> >> The code changes can be broken down into roughly the following categories: >> >> 1. Degrading the behavior of Security Manager APIs to either throw >> Exceptions by default or provide an execution environment that disallows >> access to all resources by default. >> 2. Changing hundreds of methods and constructors to no longer throw a >> `SecurityException` if a Security Manager was enabled. They will operate as >> they did in JDK 23 with no Security Manager enabled. >> 3. Changing the `java` command to exit with a fatal error if a Security >> Manager is enabled. >> 4. Removing the hotspot native code for the privileged stack walk and the >> inherited access control context. The remaining hotspot code and tests >> related to the Security Manager will be removed immediately after >> integration - see [JDK-8341916](https://bugs.openjdk.org/browse/JDK-8341916). >> 5. Removing or modifying hundreds of tests. Many tests that tested Security >> Manager behavior are no longer relevant and thus have been removed or >> modified. >> >> There are a handful of Security Manager related tests that are failing and >> are at the end of the `test/jdk/ProblemList.txt`, >> `test/langtools/ProblemList.txt` and `test/hotspot/jtreg/ProblemList.txt` >> files - these will be removed or separate bugs will be filed before >> integrating this PR. >> >> Inside the JDK, we have retained calls to >> `SecurityManager::getSecurityManager` and `AccessController::doPrivileged` >> for now, as these methods have been degraded to behave the same as they did >> in JDK 23 with no Security Manager enabled. After we integrate this JEP, >> those calls will be removed in each area (client-libs, core-libs, security, >> etc). >> >> I don't expect each reviewer to review all the code changes in this JEP. >> Rather, I advise that you only focus on the changes for the area >> (client-libs, core-libs, net, ... > > Sean Mullan has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 175 commits: > > - Merge remote-tracking branch 'jdk-sandbox/jep486' into JDK-8338411 > - Specify that params passed to getPermissions and implies are ignored and > implies always returns false. > - Change deprecated annotations to api notes on getPolicy and setPolicy. > - clientlibs: Updated Problemlist > Deleted javax/swing/JPopupMenu/6694823/bug6694823.java entry since it was > determined that it is not a JDK bug but expected behavior on windows and > linux platform. > - clientlibs: Deleted JPopupMenu tests > The following tests are deleted as they don't hold value without SM > test/jdk/javax/swing/JPopupMenu/6691503/bug6691503.java - It tests that > the popup are > always-on-top for apps which doesn't hold value after SM removal. > > test/jdk/javax/swing/JPopupMenu/6694823/bug6694823.java - Tests whether > popup can overlap taskbar. > Works differently on macOS and windows & linux but this is the expected > behavior. > Details in JDK-8342012. Not a functional issue. > - clientlibs: GetSoundBankSecurityException.java renamed to > EmptySoundBankTest.java > - clientlibs: GetSoundBankSecurityException.java renamed to > EmptySoundBankTest.java > test renamed, test summary updated > - Restore note for implementers in > src/java.prefs/share/classes/java/util/prefs/AbstractPreferences.java > - Change "SecurityManager" to "Security Manager". Add some missing code and > linkplain tags. > - Add api note to class description that permission checking is not > supported and remove text about getting permissions from system policy. In > getPermissions(), change "granted to the given codesource" to "for the > codesource". > - ... and 165 more: https://git.openjdk.org/jdk/compare/1476f6c4...e490f698 Reviewed all tests under test/jaxp/javax/xml/jaxp. A few imports moved around unnecessarily but otherwise looks fine. test/jaxp/javax/xml/jaxp/functional/org/w3c/dom/ptests/NodeTest.java line 53: > 51: import org.w3c.dom.Node; > 52: import org.w3c.dom.NodeList; > 53: import static jaxp.library.JAXPTestUtilities.USER_DIR; The import change was unnecessary. test/jaxp/javax/xml/jaxp/unittest/sax/Bug7057778Test.java line 48: > 46: import org.xml.sax.XMLReader; > 47: import org.xml.sax.ext.DefaultHandler2; > 48: import static jaxp.library.JAXPTestUtilities.USER_DIR; Keep imports JAXPTestUtilities imports together. test/jaxp/javax/xml/jaxp/unittest/stream/XMLEventReaderTest/EventReaderTest.java line 32: > 30: import javax.xml.stream.XMLStreamException; > 31: import javax.xml.stream.events.StartDocument; > 32: import static org.testng.Assert.assertEquals; Import change is unnecessary. test/jaxp/javax/xml/jaxp/unittest/validation/Bug6925531Test.java line 52: > 50: + " targetNamespace='jaxp13_test'\n" > 51: + " elementFormDefault='qualified'>\n" > 52: + " <element name='test' type='string'/>\n" Would be a good use for multi-line strings. """ """ But not worth changing. test/jaxp/javax/xml/jaxp/unittest/xpath/XPathPrecedingTest.java line 2: > 1: /* > 2: * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights > reserved. no change necessary ------------- PR Review: https://git.openjdk.org/jdk/pull/21498#pullrequestreview-2399985101 PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1819611468 PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1819667734 PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1819672433 PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1819646448 PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1819624631
