Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays [v4]
> The implementation of JarEntry.getCodeSigners() and getCertificates() both > return a copy of the original array. However, the documentation of these 2 > methods currently doesn't specify this. Koushik Muthukrishnan Thirupattur has updated the pull request incrementally with one additional commit since the last revision: 8370688: Addressed review comments-removed this method in the return label - Changes: - all: https://git.openjdk.org/jdk/pull/28615/files - new: https://git.openjdk.org/jdk/pull/28615/files/d53fe8ff..cc77e339 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=28615&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=28615&range=02-03 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/28615.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/28615/head:pull/28615 PR: https://git.openjdk.org/jdk/pull/28615
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays [v3]
On Thu, 4 Dec 2025 21:28:30 GMT, Koushik Muthukrishnan Thirupattur
wrote:
>> The implementation of JarEntry.getCodeSigners() and getCertificates() both
>> return a copy of the original array. However, the documentation of these 2
>> methods currently doesn't specify this.
>
> Koushik Muthukrishnan Thirupattur has updated the pull request incrementally
> with one additional commit since the last revision:
>
> 8370688: Addressed review comments-moved the sentence to return label
src/java.base/share/classes/java/util/jar/JarEntry.java line 118:
> 116: *
> 117: * @return the {@code Certificate} objects for this entry, or
> 118: * {@code null} if none. If non-null, this method returns a new
I think you can remove the "this method" words as it is implied by being in the
"@return".
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28615#discussion_r2592754088
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays [v3]
> The implementation of JarEntry.getCodeSigners() and getCertificates() both > return a copy of the original array. However, the documentation of these 2 > methods currently doesn't specify this. Koushik Muthukrishnan Thirupattur has updated the pull request incrementally with one additional commit since the last revision: 8370688: Addressed review comments-moved the sentence to return label - Changes: - all: https://git.openjdk.org/jdk/pull/28615/files - new: https://git.openjdk.org/jdk/pull/28615/files/59abe38b..d53fe8ff Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=28615&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=28615&range=01-02 Stats: 8 lines in 1 file changed: 2 ins; 4 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/28615.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/28615/head:pull/28615 PR: https://git.openjdk.org/jdk/pull/28615
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays [v2]
On Wed, 3 Dec 2025 18:46:40 GMT, Koushik Muthukrishnan Thirupattur wrote: >> The implementation of JarEntry.getCodeSigners() and getCertificates() both >> return a copy of the original array. However, the documentation of these 2 >> methods currently doesn't specify this. > > Koushik Muthukrishnan Thirupattur has updated the pull request incrementally > with one additional commit since the last revision: > > 8370688: Addressed review comments - add explicit note similar to > SSLParameters src/java.base/share/classes/java/util/jar/JarEntry.java line 117: > 115: * to trust the entry signed by the signers. > 116: * > 117: * This method will return a new array each time it is invoked. This sentence is not completely true, because the method may also return `null`. I suggest moving this sentence to the @return label (as the second sentence), and rephrasing it as "If non-null, this method returns a new array each time it is invoked". I removed "will" as I think present tense sounds better. - PR Review Comment: https://git.openjdk.org/jdk/pull/28615#discussion_r2589119626
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays [v2]
On Wed, 3 Dec 2025 18:46:40 GMT, Koushik Muthukrishnan Thirupattur wrote: >> The implementation of JarEntry.getCodeSigners() and getCertificates() both >> return a copy of the original array. However, the documentation of these 2 >> methods currently doesn't specify this. > > Koushik Muthukrishnan Thirupattur has updated the pull request incrementally > with one additional commit since the last revision: > > 8370688: Addressed review comments - add explicit note similar to > SSLParameters Looks good to me. - Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/28615#pullrequestreview-3538474729
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays [v2]
> The implementation of JarEntry.getCodeSigners() and getCertificates() both > return a copy of the original array. However, the documentation of these 2 > methods currently doesn't specify this. Koushik Muthukrishnan Thirupattur has updated the pull request incrementally with one additional commit since the last revision: 8370688: Addressed review comments - add explicit note similar to SSLParameters - Changes: - all: https://git.openjdk.org/jdk/pull/28615/files - new: https://git.openjdk.org/jdk/pull/28615/files/c9c87bfa..59abe38b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=28615&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=28615&range=00-01 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/28615.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/28615/head:pull/28615 PR: https://git.openjdk.org/jdk/pull/28615
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays
On Wed, 3 Dec 2025 08:41:55 GMT, Alan Bateman wrote: > There are a lot of APIs that return an array. Some of them use an array > internally and so need to make a defensive copy/clone to return. Jai may be > able to say more on the motivation for JDK-8370688. Maybe a concern with code > uses identity to check equality, or maybe the concern was that the caller > could modify the JarEntry's certs/signers by modifying the array? > > I don't think the proposed change addresses either concern. We could > potentially change the `@return` description to say that it returns a new > array, which makes it a testable assertion. There are many other methods that > return arrays, including other methods that return arrays of certs and code > signers so we might want to change these at the same time. I agree. The original commit was more aligned with what should be done. For security related information like arrays of certificates, I think the caller often needs to know one way or the other whether mutating the returned value affects the original object's contents. And I think in general, `JarEntry` should be returning a new array each time these methods are called. We can at least ensure that the JDK implementation follows the specification. > @seanjmullan @wangweij Do you know if there has been any discussion about > deprecating getCertificates? Developers have been re-directed to use > getCodeSigners since JDK 5. Not specifically, although I agree that it probably should be deprecated. - PR Comment: https://git.openjdk.org/jdk/pull/28615#issuecomment-3606987622
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays
On Wed, 3 Dec 2025 10:35:40 GMT, Alan Bateman wrote: > > > As far as I remember, we have similar text in some API specification for > > > methods that return a copy of the array, reusing that text might be > > > useful (I'll try and find such an instance). > > > > > > `javax.net.ssl.SSLParameters` has a few APIs which word this in a couple of > > different ways: > > https://docs.oracle.com/en/java/javase/25/docs/api/java.base/javax/net/ssl/SSLParameters.html#getCipherSuites() > > ``` > > Returns: > > a copy of the array of ciphersuites or null if none have been set. > > ``` > > A SSLParameters is optionally created with array of cipher suites so it works > there. A JarEntry is not created with an array so I don't think this wording > make sense. > > > https://docs.oracle.com/en/java/javase/25/docs/api/java.base/javax/net/ssl/SSLParameters.html#getApplicationProtocols() > > ``` > > This method will return a new array each time it is invoked. > > ``` > > That could work for JarEntry although debatable if we really need this. The second one looks good to me. This will need a CSR. - PR Comment: https://git.openjdk.org/jdk/pull/28615#issuecomment-3606998031
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays
On Wed, 3 Dec 2025 09:43:27 GMT, Jaikiran Pai wrote: > > As far as I remember, we have similar text in some API specification for > > methods that return a copy of the array, reusing that text might be useful > > (I'll try and find such an instance). > > `javax.net.ssl.SSLParameters` has a few APIs which word this in a couple of > different ways: > > https://docs.oracle.com/en/java/javase/25/docs/api/java.base/javax/net/ssl/SSLParameters.html#getCipherSuites() > > ``` > Returns: > a copy of the array of ciphersuites or null if none have been set. > ``` A SSLParameters is optionally created with array of cipher suites so it works there. A JarEntry is not created with an array so I don't think this wording make sense. > https://docs.oracle.com/en/java/javase/25/docs/api/java.base/javax/net/ssl/SSLParameters.html#getApplicationProtocols() > > ``` > This method will return a new array each time it is invoked. > ``` That could work for JarEntry although debatable if we really need this. - PR Comment: https://git.openjdk.org/jdk/pull/28615#issuecomment-3606176594
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays
On Wed, 3 Dec 2025 09:28:39 GMT, Jaikiran Pai wrote: > As far as I remember, we have similar text in some API specification for > methods that return a copy of the array, reusing that text might be useful > (I'll try and find such an instance). `javax.net.ssl.SSLParameters` has a few APIs which word this in a couple of different ways: https://docs.oracle.com/en/java/javase/25/docs/api/java.base/javax/net/ssl/SSLParameters.html#getCipherSuites() Returns: a copy of the array of ciphersuites or null if none have been set. https://docs.oracle.com/en/java/javase/25/docs/api/java.base/javax/net/ssl/SSLParameters.html#getApplicationProtocols() This method will return a new array each time it is invoked. I prefer the "a copy of the array of ..." style, but I'll let Sean and others decide how to word this. - PR Comment: https://git.openjdk.org/jdk/pull/28615#issuecomment-3605950544
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays
On Wed, 3 Dec 2025 08:41:55 GMT, Alan Bateman wrote: > Jai may be able to say more on the motivation for JDK-8370688. Maybe a > concern with code uses identity to check equality, or maybe the concern was > that the caller could modify the JarEntry's certs/signers by modifying the > array? What prompted me to file JDK-8370688 is that there's an internal class in the JDK (URLJarFileEntry) which extends `JarEntry` and overrides `getCodeSigners()` and `getCertificates()` to merely clone the returned array. With the way these 2 methods are implemented in `JarEntry`, the returned array is already cloned. So `URLJarFileEntry.getCodeSigners()/getCertificates()` ends up cloning that array twice. The intention of the JBS issue was to have JarEntry specify this current implementation, so that these sub classes of JarEntry can then rely on that specification and don't have to do the clone() themselves. The current proposed text in this PR, I think, needs to be more precise. The way it's worded currently doesn't allow for applications (or JDK internal classes) to rely on the returned array being a copy of the underlying one. As far as I remember, we have similar text in some API specification for methods that return a copy of the array, reusing that text might be useful (I'll try and find such an instance). - PR Comment: https://git.openjdk.org/jdk/pull/28615#issuecomment-3605890862
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays
On Tue, 2 Dec 2025 20:28:50 GMT, Koushik Muthukrishnan Thirupattur wrote: > The implementation of JarEntry.getCodeSigners() and getCertificates() both > return a copy of the original array. However, the documentation of these 2 > methods currently doesn't specify this. There are a lot of APIs that return an array. Some of them use an array internally and so need to make a defensive copy/clone to return. Jai may be able to say more on the motivation for JDK-8370688. Maybe a concern with code uses identity to check equality, or maybe the concern was that the caller could modify the JarEntry's certs/signers by modifying the array? I don't think the proposed change addresses either concern. We could potentially change the `@return` description to say that it returns a new array, which makes it a testable assertion. There are many other methods that return arrays, including other methods that return arrays of certs and code signers so we might want to change these at the same time. @seanjmullan @wangweij Do you know if there has been any discussion about deprecating getCertificates? Developers have been re-directed to use getCodeSigners since JDK 5. - PR Comment: https://git.openjdk.org/jdk/pull/28615#issuecomment-3605690364
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays
On Tue, 2 Dec 2025 20:28:50 GMT, Koushik Muthukrishnan Thirupattur wrote: > The implementation of JarEntry.getCodeSigners() and getCertificates() both > return a copy of the original array. However, the documentation of these 2 > methods currently doesn't specify this. Looks good in principle to indicate non-uniqueness. Please find security area reviewers to double check. You should also create a CSR for this specification change. - Marked as reviewed by liach (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/28615#pullrequestreview-3532560572
Re: RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays
On Tue, 2 Dec 2025 20:28:50 GMT, Koushik Muthukrishnan Thirupattur wrote: > The implementation of JarEntry.getCodeSigners() and getCertificates() both > return a copy of the original array. However, the documentation of these 2 > methods currently doesn't specify this. I don't think we need to go this far as to add a new paragraph. Looking at examples like `Class::getMethods`, I think we can just change the first sentences of "Returns the ... objects" to "Returns an array containing the ... objects", which means the returned array is not the same as the underlying storage format. - PR Comment: https://git.openjdk.org/jdk/pull/28615#issuecomment-3604141759
RFR: 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays
The implementation of JarEntry.getCodeSigners() and getCertificates() both return a copy of the original array. However, the documentation of these 2 methods currently doesn't specify this. - Commit messages: - 8370688: Addressed review comments - 8370688: java.util.jar.JarEntry.getCodeSigners() and getCertificates() should specify that they return a copy of the arrays Changes: https://git.openjdk.org/jdk/pull/28615/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=28615&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8370688 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/28615.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/28615/head:pull/28615 PR: https://git.openjdk.org/jdk/pull/28615
