RFR: 8378003: JarURLConnection.getCertificates() and getCodeSigners() incorrectly return null for signed JAR files after JDK-8377338 [v3]

Sean Mullan mullan at openjdk.org
Wed Feb 18 16:16:40 UTC 2026


On Wed, 18 Feb 2026 12:38:34 GMT, Jaikiran Pai <jpai at openjdk.org> 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 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 six additional commits since the last revision:
> 
>  - Revert "Daniel's review - completely undo the changes done in JDK-8377338"
>    
>    This reverts commit 697a251ed6488e553a204f9aa16e3eb8a2a9d2f0.
>  - verify getCertificates() and getCodeSigners() from JarURLConnection return new arrays on each invocation
>  - merge latest from master branch
>  - Daniel's review - completely undo the changes done in JDK-8377338
>  - 8378003: JarURLConnection.getCertificates() incorrectly returns null for signed JAR files after JDK-8377338
>  - Add a test

src/java.base/share/classes/sun/net/www/protocol/jar/URLJarFile.java line 202:

> 200:         @Override
> 201:         public CodeSigner[] getCodeSigners() {
> 202:             // super.getCodeSigners() returns CodeSigner that were

s/CodeSigner/CodeSigners/

test/jdk/sun/net/www/protocol/jar/JarURLConnectionCertsAndCodeSigners.java line 183:

> 181:     }
> 182: 
> 183:     private static void checkSignedBy(JarEntry e, String expectedDn) throws Exception {

Not called by any code?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/29748#discussion_r2823124097
PR Review Comment: https://git.openjdk.org/jdk/pull/29748#discussion_r2823161413


More information about the net-dev mailing list