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