On Tue, 30 Jan 2024 14:19:02 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> src/java.management/share/classes/com/sun/jmx/remote/internal/ServerNotifForwarder.java
>> line 349:
>>
>>> 347: @SuppressWarnings("removal")
>>> 348: private Subject getSubject() {
>>> 349: return Subject.current();
>>
>> Since `Subject::current` is not deprecated the annotation at line 347 above
>> should be removed.
>
> OK - things seem to be a bit convoluted here and some pieces might be
> missing. I suspect that what needs to be done is more complicated:
>
> `RMIConnectionImpl` sets up an ACC and calls doPrivileged with that ACC, on
> the assumption that the subject is tied to the ACC and it can be retrieved
> down the road from the ACC. `RMIConnectionImpl` has the subject, and the
> delegation subject too.
>
> So for `Subject::current` to work here, shouldn't
> `RMIConnectionImpl::doPrivilegedOperation` use `Subject::callAs` when the
> security manager is disallowed?
>
> It seems that when `Subject::current` is used, some analysis should be done
> to verify where the Subject is supposed to come from - that is - how the
> caller is expecting the subject to reach the callee.
>
> Typically, JMX doesn't use `Subject::doAs` but ties a `Subject` to an
> `AccessControlContext` and uses `doPrivileged` instead.
This is complicated.
The benefit of `current()` is that it is always equivalent to
`getSubject(AccessController.getContext())` *as long as the latter works*.
However, in this case, when a security manager is not allowed, the latter DOES
NOT work but `current()` still works.
I'll revert the change. Before finding an alternative solution for
`JMXSubjectDomainCombiner`, I assume this code only works when a security
manager is allowed. It's better to throw an UOE instead of returning null.
I'm not sure of the other `getSubject` call (below), but I'll revert the change
as all.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1471591997
