Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v17]
On Mon, 17 Jun 2024 19:56:38 GMT, Sean Mullan wrote: > AFAICT, the only test modified in this PR that actually enables a Security > Manager is test/jdk/javax/management/security/AuthorizationTest.java. Is that > test sufficient to exercise the code changes in this PR when an SM is enabled? While this code change has done its best to avoid any behavior change when SM is allowed, I agree with Sean to run tests in both modes. When these tests were originally introduced they were meant to test the old implementation. Keep testing eith both implementations help us to avoid any future (might not be long) regression. - PR Comment: https://git.openjdk.org/jdk/pull/19624#issuecomment-2174568617
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v17]
On Mon, 17 Jun 2024 15:59:37 GMT, Kevin Walls wrote: >> JMX uses APIs related to the Security Mananger which are deprecated. Use of >> AccessControlContext will be removed when Security Manager is removed. >> >> Until then, updates are needed to not require setting >> -Djava.security.manager=allow to use JMX authentication. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > Remove import src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java line 1306: > 1304: return action.run(); > 1305: else > 1306: return Subject.doAs(subject, action); Code style comment: put braces around the if/else to be consistent with lines below. - PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1643390138
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v17]
On Mon, 17 Jun 2024 15:59:37 GMT, Kevin Walls wrote: >> JMX uses APIs related to the Security Mananger which are deprecated. Use of >> AccessControlContext will be removed when Security Manager is removed. >> >> Until then, updates are needed to not require setting >> -Djava.security.manager=allow to use JMX authentication. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > Remove import AFAICT, the only test modified in this PR that actually enables a Security Manager is test/jdk/javax/management/security/AuthorizationTest.java. Is that test sufficient to exercise the code changes in this PR when an SM is enabled? - PR Comment: https://git.openjdk.org/jdk/pull/19624#issuecomment-2174313063
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v17]
> JMX uses APIs related to the Security Mananger which are deprecated. Use of > AccessControlContext will be removed when Security Manager is removed. > > Until then, updates are needed to not require setting > -Djava.security.manager=allow to use JMX authentication. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: Remove import - Changes: - all: https://git.openjdk.org/jdk/pull/19624/files - new: https://git.openjdk.org/jdk/pull/19624/files/597264b9..67aca736 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19624&range=16 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19624&range=15-16 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19624.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19624/head:pull/19624 PR: https://git.openjdk.org/jdk/pull/19624