On Thu, 25 Jun 2026 18:18:12 GMT, Hai-May Chao <[email protected]> wrote:
>> This change adds the `jdk.crypto.legacyAlgorithms` security property to >> `java.security`. At the JCE layer, the JDK checks this property and emits a >> runtime warning when a configured legacy algorithm is requested. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Hai-May Chao has updated the pull request incrementally with one additional > commit since the last revision: > > Update java.security and Javadoc for getInstance() test/jdk/java/security/KeyStore/TestLegacyAlgorithms.java line 1: > 1: /* Suggest adding an additional test where you also disable the algorithm and check that a warning is not emitted test/jdk/java/security/KeyStore/TestLegacyAlgorithms.java line 56: > 54: List.of("JKS", "jkS"); > 55: > 56: private static String saveWarn(ThrowingRunnable action) throws > Exception { Did you consider checking that each specific `getInstance` case produces the expected warning? I think it is harder to check all the warnings at the end and be sure all of them were produced as expected for all `getInstance` calls. Maybe if you flushed the stream, then checked the output for each test case, and then reset the `ByteArrayOutputStream` before the next test, and repeat for each test. test/jdk/java/security/KeyStore/TestLegacyAlgorithms.java line 77: > 75: > 76: KeyStore k; > 77: if (p == null) { This case is never tested AFAICT. test/jdk/java/security/KeyStore/TestLegacyAlgorithms.java line 135: > 133: "Expected legacy warning for KeyStore but not > found"); > 134: for (String a : ALG_LIST) { > 135: Asserts.assertTrue(warnS.contains("WARNING: " + a + > warn2), This only checks if at least one of the `getInstance` calls emitted a warning. Can you check for all of them? test/jdk/java/security/KeyStore/TestLegacyAlgorithms.java line 142: > 140: "Expected warning not preserve caller: " > 141: + warnS); > 142: } else { It would also be useful to call the methods twice, and make sure you only get one warning per caller. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31472#discussion_r3477126779 PR Review Comment: https://git.openjdk.org/jdk/pull/31472#discussion_r3477306535 PR Review Comment: https://git.openjdk.org/jdk/pull/31472#discussion_r3477089744 PR Review Comment: https://git.openjdk.org/jdk/pull/31472#discussion_r3477124171 PR Review Comment: https://git.openjdk.org/jdk/pull/31472#discussion_r3477258247
