Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v13]
On Fri, 14 Jun 2024 15:26:54 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:
>
> Unnecessary catches to remove
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
line 1437:
> 1435: } catch (Exception e) {
> 1436: if (e instanceof RuntimeException)
> 1437:throw (RuntimeException) e;
Suggestion:
if (e instanceof RuntimeException rte)
throw rte;
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
line 1450:
> 1448: } catch (Exception e) {
> 1449: if (e instanceof RuntimeException)
> 1450:throw (RuntimeException) e;
Suggestion:
if (e instanceof RuntimeException rte)
throw rte;
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1640033632
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1640034429
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v13]
On Sun, 16 Jun 2024 01:54:34 GMT, Weijun Wang wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Unnecessary catches to remove
>
> src/java.management/share/classes/javax/management/monitor/Monitor.java line
> 1542:
>
>> 1540: if
>> (!SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
>> 1541: // No SecurityManager permitted:
>> 1542: Subject.doAs(s, action); // s is permitted to be null
>
> While `s` is permitted to be null, calling `Subject.doAs(null, action)`
> actually sets the current subject to null while calling `action`. This is not
> same as directly calling `action` where the current subject (could be non
> null) is used.
Yes, good point, got it. 8-)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1642504749
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v13]
On Fri, 14 Jun 2024 15:26:54 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:
>
> Unnecessary catches to remove
src/java.management/share/classes/javax/management/monitor/Monitor.java line
1542:
> 1540: if
> (!SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
> 1541: // No SecurityManager permitted:
> 1542: Subject.doAs(s, action); // s is permitted to be null
While `s` is permitted to be null, calling `Subject.doAs(null, action)`
actually sets the current subject to null while calling `action`. This is not
same as directly calling `action` where the current subject (could be non null)
is used.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1641593366
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v13]
> 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: Unnecessary catches to remove - Changes: - all: https://git.openjdk.org/jdk/pull/19624/files - new: https://git.openjdk.org/jdk/pull/19624/files/d5f68b02..13d9202f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19624&range=12 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19624&range=11-12 Stats: 6 lines in 1 file changed: 0 ins; 6 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
