On Tue, 7 Apr 2026 14:06:23 GMT, Jaikiran Pai <[email protected]> wrote:
> Can I please get a review of this test-only change which updates the > `test/jdk/java/util/jar/JarEntry/GetMethodsReturnClones.java` test to use a > dynamically generated signed JAR file to run its tests? This addresses > https://bugs.openjdk.org/browse/JDK-8378291. > > This test exercises the `JarEntry.getCertificates()` and > `JarEntry.getCodeSigners()` method to verify that those methods return a copy > of the original array thus preventing any updates to the returned array from > being propagated to the original array. This test was using a pre-generated > `test.jar` file which was signed using a (now) weak key and the JAR was being > treated as unsigned: > > > jarsigner -certs -verbose -verify test/jdk/java/util/jar/JarEntry/test.jar > > 140 Sun Feb 08 17:22:36 IST 2004 META-INF/MANIFEST.MF > 202 Sun Feb 08 17:22:36 IST 2004 META-INF/TEST.SF > 1595 Sun Feb 08 17:22:36 IST 2004 META-INF/TEST.RSA > 0 Tue Sep 17 13:07:14 IST 2002 META-INF/ > m ? 1770 Tue Sep 17 13:04:40 IST 2002 WriteFileTest.class > > s = signature was verified > m = entry is listed in manifest > k = at least one certificate was found in keystore > ? = unsigned entry > > ... > WARNING: The jar will be treated as unsigned, because it is signed with a > weak algorithm that is now disabled by the security property: > > jdk.jar.disabledAlgorithms=MD2, MD5, RSA keySize < 1024, DSA keySize < > 1024, SHA1 denyAfter 2019-01-01 > > > The test has now been updated to dynamically generate the JAR file and verify > these methods. Existing tests in `test/jdk/java/util/jar` continue to pass > and tier1 testing is currently in progress. test/jdk/java/util/jar/JarEntry/GetMethodsReturnClones.java line 76: > 74: static void beforeAll() throws Exception { > 75: Path unsigned = createJar(); > 76: Path signed = signJar(unsigned); Should these files be deleted again (maybe in a `@AfterAll`?) after the test finished? test/jdk/java/util/jar/JarEntry/GetMethodsReturnClones.java line 115: > 113: certs = je.getCertificates(); // now get the certs again > 114: assertNotNull(certs, "JarEntry.getCertificates() returned > null for " + je.getName()); > 115: // verify that the newly returned array doesn't have the > overwrittent value Typo? Suggestion: // verify that the newly returned array doesn't have the overwritten value ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30602#discussion_r3046464973 PR Review Comment: https://git.openjdk.org/jdk/pull/30602#discussion_r3046439959
