On Mon, 4 Mar 2024 15:15:54 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fix MBeanServerFileAccessController, more test in SM
>
> test/jdk/javax/management/monitor/ThreadPoolAccTest.java line 69:
>
>> 67: }
>> 68: private void setPrincipal() {
>> 69: Subject subject = Subject.current();
>
> Why change this to `Subject.current()` if you are requiring the SM to be
> allowed?
When SM is allowed, `Subject.current()` is equivalent to
`Subject.getSubject(AccessController.getContext())`. I can revert the change.
> test/jdk/javax/security/auth/Subject/UnsupportedSV.java line 59:
>
>> 57:
>> 58: // TODO: Still has no way to reject the following code.
>> 59: // Here, AccessController::getContext returns a plan ACC without
>
> typo: s/plan/plain/
Yes.
> test/jdk/javax/security/auth/Subject/doAs/NestedActions.java line 283:
>
>> 281: static void readFile(String filename) {
>> 282: System.out.println("ReadFromFileAction: try to read " +
>> filename);
>> 283: Subject subject = Subject.current();
>
> Couldn't we have just left these calling `Subject.getSubject` for now since
> these tests run with an SM enabled?
Yes, we can.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1511755373
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1511756265
PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1511755860
