Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v17]

2024-06-17 Thread Weijun Wang
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]

2024-06-17 Thread Sean Mullan
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]

2024-06-17 Thread Sean Mullan
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]

2024-06-17 Thread Kevin Walls
> 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