On Tue, 22 Jun 2021 20:08:03 GMT, Sean Coffey <coff...@openjdk.org> wrote:
>> Sufficient permissions missing if this code was ever to run with
>> SecurityManager.
>>
>> Cleanest approach appears to be use of InnocuousThread to create the
>> cleaner/poller threads.
>> Test case coverage extended to cover the SecurityManager scenario.
>>
>> Reviewer request: @valeriepeng
>
> Sean Coffey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Move TokenPoller to Runnable
test/jdk/sun/security/pkcs11/Provider/MultipleLogins.java line 63:
> 61: Policy.setPolicy(new SimplePolicy());
> 62: System.setSecurityManager(new SecurityManager());
> 63: }
Just curious, why split the loop into 2 and set the SecurityManager in between
the two loops? Can't we just set the policy/security manager before the loop?
test/jdk/sun/security/pkcs11/Provider/MultipleLogins.java line 137:
> 135: perms.add(new SecurityPermission("insertProvider.*"));
> 136: perms.add(new SecurityPermission("removeProvider.*"));
> 137: }
The test still pass without the following permission:
perms.add(new RuntimePermission("accessClassInPackage.sun.*"));
perms.add(new
RuntimePermission("accessClassInPackage.sun.security.pkcs11.*"));
perms.add(new SecurityPermission("clearProviderProperties.*"));
Remove them?
test/jdk/sun/security/pkcs11/Provider/MultipleLogins.sh line 142:
> 140: -Dtest.src=${TESTSRC} \
> 141: -Dtest.classes=${TESTCLASSES} \
> 142: -Djava.security.debug=${DEBUG} \
Save these java options and use it for both invocation? This way it's easier to
tell that there is no difference among these two except for the extra argument.
test/jdk/sun/security/pkcs11/Provider/MultipleLogins.sh line 143:
> 141: -Dtest.classes=${TESTCLASSES} \
> 142: -Djava.security.debug=${DEBUG} \
> 143: MultipleLogins ${TESTSRC}${FS}MultipleLogins.policy || exit 11
There is no MultipleLogins.policy file. The test just uses the internal
SimplePolicy object. Maybe just use a string like "useSimplePolicy".
-------------
PR: https://git.openjdk.java.net/jdk17/pull/117
