On Mon, 16 Feb 2026 17:30:02 GMT, Jaikiran Pai <[email protected]> wrote:
>> Can I please get a review of this change which addresses a regression that >> was introduced after the integration of >> https://bugs.openjdk.org/browse/JDK-8377338? >> >> As noted in https://bugs.openjdk.org/browse/JDK-8378003, after the change in >> JDK-8377338, `JarURLConnection.getCertificates()` and `getCodeSigners()` >> started incorrectly returning null for JAR entries in signed JAR files. >> >> The original change in JDK-8377338 removed the overridden >> `getCertificates()` and `getCodeSigners()` methods from >> `URLJarFile$URLJarFileEntry`. When doing that I had verified that the >> `getCertificates()` and `getCodeSigners()` that would get now invoked on >> `java.util.jar.JarEntry` (due to the removal of these overridden methods) >> were indeed returning the certificates and codesigners that belong to the >> underlying `JarEntry` `je`. I was assured of this because the `certs` and >> `signers` fields were captured in the constructor of `JarEntry` constructor: >> >> >> public JarEntry(JarEntry je) { >> this((ZipEntry)je); >> this.attr = je.attr; >> this.certs = je.certs; >> this.signers = je.signers; >> } >> >> and then JarEntry.getCertificates() and getCodeSigners() did this: >> >> >> public Certificate[] getCertificates() { >> return certs == null ? null : certs.clone(); >> } >> >> public CodeSigner[] getCodeSigners() { >> return signers == null ? null : signers.clone(); >> } >> >> So removal of the overrides appeared harmless. I however missed the fact >> that some JarEntry implementations like `java.util.jar.JarFile$JarFileEntry` >> compute the `certs` and `signers` field lazily. So when such `JarEntry` is >> passed to the ("copy") constructor of `JarEntry`, `null` values are captured >> for those fields. The removal of the overridden methods thus meant that the >> explicit calls to `JarEntry.getCertificates/getCodeSigners()` to compute the >> certs and signers no longer happened. This is what caused the regression. >> >> This also exposed the lack of tests in this area. I have now introduced a >> jtreg test to reproduce the issue and verify the fix. The fix reintroduces >> the overrides in `URLJarFile$URLJarFileEntry` and explicitly calls the >> `getCertificates()` and `getCodeSigners()` on the underlying `je` JarEntry. >> At the same time, it retains the original goal of JDK-8377338 and doesn't >> clone the returned arrays to prevent the duplicated array cloning. >> >> P.S: I am willing to completely backout the change done in JDK-8377338 and >> retain just this new test, if that's preferred. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Daniel's review - completely undo the changes done in JDK-8377338 Marked as reviewed by dfuchs (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/29748#pullrequestreview-3813572729
