On Fri, 16 Dec 2022 11:48:54 GMT, Sibabrata Sahoo <[email protected]> wrote:
>> Bill Huang has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains five additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into JDK-8295087
>> - Added an extra line to the end of the policy file.
>> - AssertThrows an exception in InconsistentEntries test.
>> - Refactored to use testng framework for test enviroment setup.
>> - Converted security manual tests to automated tests.
>
> test/jdk/javax/crypto/CryptoPermissions/InconsistentEntries.java line 52:
>
>> 50: private static final String JDK_HOME =
>> System.getProperty("test.jdk");
>> 51: private static final String TEST_SRC =
>> System.getProperty("test.src");
>> 52: private static final Path POLICY_DIR = Paths.get(JDK_HOME, "conf",
>> "security",
>
> This doesn't looks like a safe Test to be automated. Can it create conflict
> with any other existing Test requiring "testlimited" with
> default_local.policy? This need to be verified. Also changing anything inside
> an installed JDK probably not a good choice. It's just a thought from my side
> and it could be different for others.
Good points. I searched the entire repo and this is the only instance that uses
the "testlimited" with default_local.policy. Looking over the logic, the test
sets the crypto.policy property to "testlimited". So I am wondering if the
"testlimited" is created for test purposes. If so, are we allowed to rename
"testlimited" to be more specific, eg. "testcryptoperms"?
`Security.setProperty("crypto.policy", "testlimited");`
-------------
PR: https://git.openjdk.org/jdk/pull/10637