RFR: Merge jdk18
Forwardport JDK 18 -> JDK 19 - Commit messages: - Merge remote-tracking branch 'jdk18/master' into Merge_jdk18 - 8279385: [test] Adjust sun/security/pkcs12/KeytoolOpensslInteropTest.java after 8278344 The merge commit only contains trivial merges, so no merge-specific webrevs have been generated. Changes: https://git.openjdk.java.net/jdk/pull/7351/files Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7351.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7351/head:pull/7351 PR: https://git.openjdk.java.net/jdk/pull/7351
RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()
Hi all, Please review the attached patch to address - That JarFile::getInputStream did not check for a null ZipEntry passed as a parameter - Have Zip/JarFile::getInputStream throw a ZipException in the event that an unexpected exception occurs Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar test runs Best Lance - Commit messages: - Update copyright year - Address Zip/JarFile::getInputStream Exception handling Changes: https://git.openjdk.java.net/jdk/pull/7348/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7348&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280409 Stats: 1081 lines in 3 files changed: 1026 ins; 26 del; 29 mod Patch: https://git.openjdk.java.net/jdk/pull/7348.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7348/head:pull/7348 PR: https://git.openjdk.java.net/jdk/pull/7348
Integrated: Merge jdk18
On Fri, 4 Feb 2022 13:38:52 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 18 -> JDK 19 This pull request has now been integrated. Changeset: 7207f2a3 Author:Jesper Wilhelmsson URL: https://git.openjdk.java.net/jdk/commit/7207f2a3b59c684d9d51d378257629729fa7041d Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Merge - PR: https://git.openjdk.java.net/jdk/pull/7351
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()
On Fri, 4 Feb 2022 12:42:39 GMT, Lance Andersen wrote:
> Hi all,
>
> Please review the attached patch to address
>
> - That JarFile::getInputStream did not check for a null ZipEntry passed as a
> parameter
> - Have Zip/JarFile::getInputStream throw a ZipException in the event that an
> unexpected exception occurs
>
> Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar
> test runs
>
> Best
> Lance
src/java.base/share/classes/java/util/jar/JarFile.java line 840:
> 838: throws IOException
> 839: {
> 840: Objects.requireNonNull(ze, "ze");
Is the NPE specified?
src/java.base/share/classes/java/util/jar/JarFile.java line 866:
> 864: } catch (Exception e2) {
> 865: // Any other Exception should be a ZipException
> 866: throw (ZipException) new ZipException("Zip file format
> error").initCause(e2);
If there is ZIP format error then I would expect ZipException or the more
general IOException is already thrown. So I think this is catching other cases,
maybe broken manifests or signed JAR files, in which case a JarException may be
better.
-
PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()
On Fri, 4 Feb 2022 15:06:59 GMT, Alan Bateman wrote:
>> Hi all,
>>
>> Please review the attached patch to address
>>
>> - That JarFile::getInputStream did not check for a null ZipEntry passed as a
>> parameter
>> - Have Zip/JarFile::getInputStream throw a ZipException in the event that an
>> unexpected exception occurs
>>
>> Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar
>> test runs
>>
>> Best
>> Lance
>
> src/java.base/share/classes/java/util/jar/JarFile.java line 840:
>
>> 838: throws IOException
>> 839: {
>> 840: Objects.requireNonNull(ze, "ze");
>
> Is the NPE specified?
Yes it is specified in the JarFile main paragraph
`Unless otherwise noted, passing a null argument to a constructor or method in
this class will cause a NullPointerException to be thrown.`
ZipFile::getInputStream validates the argument as soon as it is passed to
getInputStream.
> src/java.base/share/classes/java/util/jar/JarFile.java line 866:
>
>> 864: } catch (Exception e2) {
>> 865: // Any other Exception should be a ZipException
>> 866: throw (ZipException) new ZipException("Zip file format
>> error").initCause(e2);
>
> If there is ZIP format error then I would expect ZipException or the more
> general IOException is already thrown. So I think this is catching other
> cases, maybe broken manifests or signed JAR files, in which case a
> JarException may be better.
JarFile::getInputStream. mentions ZipException but not JarException which is
why I chose this. If we change this to JarException, I would need to update
the javadoc and create a CSR.
Please let me know your preference
-
PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()
On Fri, 4 Feb 2022 12:42:39 GMT, Lance Andersen wrote: > Hi all, > > Please review the attached patch to address > > - That JarFile::getInputStream did not check for a null ZipEntry passed as a > parameter > - Have Zip/JarFile::getInputStream throw a ZipException in the event that an > unexpected exception occurs > > Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar > test runs > > Best > Lance Could these unexpected exceptions also occur when using the `JarInputStream` API? - PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()
On Fri, 4 Feb 2022 15:55:33 GMT, Sean Mullan wrote: > Could these unexpected exceptions also occur when using the `JarInputStream` > API? It's a different code path as Zip/JarFile leverage the CEN where Zip/JarInputStream leverage the LOC. I can give it a go and if there is an issue will create a separate issue - PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280494: (D)TLS signature schemes [v9]
On Wed, 2 Feb 2022 19:12:56 GMT, Xue-Lei Andrew Fan wrote:
>> This update is to support signature schemes customization for individual
>> (D)TLS connection. Please review the CSR as well:
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8280495
>> RFE: https://bugs.openjdk.java.net/browse/JDK-8280494
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> More update for the sec and impl
Changes requested by mullan (Reviewer).
src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 713:
> 711: * in this list or may not use the recommended name for a certain
> 712: * signature scheme.
> 713: *
Regarding my previous comment about providers not supporting these new methods,
I think something like the following might suffice:
@apiNote Note that a provider may not have been updated to support this method
and in that case may return null instead of the default signature schemes for
pre-populated objects.
@implNote The SunJSSE provider supports this method.
src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 735:
> 733: * {@link #setSignatureSchemes} method has not been called, this
> method
> 734: * should return the default signature schemes for connection
> populated
> 735: * objects, or {@code null} for pre-populated objects.
I think this sentence should be part of the specification and not the
implementation note since ideally you would want all provider implementations
to behave this way (assuming they supported this method):
"If the {@link #setSignatureSchemes} method has not been called, this method
should return the default signature schemes for connection populated objects,
or {@code null} for pre-populated objects."
src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 752:
> 750:
> 751: /**
> 752: * Sets the prioritized array of signature scheme names that
If the system properties are set, does it override this API? I think it is
important to mention that here in an @implNote. I know you discuss them in
`getSignatureSchemes` but I think a developer is more likely to want to know
the behavior of the properties for this method especially if they will override
this setting.
src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 761:
> 759: * Names Specification. Providers may support signature schemes not
> defined
> 760: * in this list or may not use the recommended name for a certain
> 761: * signature scheme.
Did you want to say something about what the behavior should be if a provider
does not support one of these schemes? Is it ignored, or is an exception thrown?
src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 762:
> 760: * in this list or may not use the recommended name for a certain
> 761: * signature scheme.
> 762: *
Regarding my previous comment about providers not supporting these new methods,
I think something like the following might suffice:
@apiNote Note that a provider may not have been updated to support this method
and in that case may ignore the schemes that are set.
@implNote The SunJSSE provider supports this method.
-
PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()
On Fri, 4 Feb 2022 16:06:34 GMT, Lance Andersen wrote: > > Could these unexpected exceptions also occur when using the > > `JarInputStream` API? > > It's a different code path as Zip/JarFile leverage the CEN where > Zip/JarInputStream leverage the LOC. I can give it a go and if there is an > issue will create a separate issue Not an issue given how Zip/JarInputStream work - PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280494: (D)TLS signature schemes [v10]
> This update is to support signature schemes customization for individual > (D)TLS connection. Please review the CSR as well: > CSR: https://bugs.openjdk.java.net/browse/JDK-8280495 > RFE: https://bugs.openjdk.java.net/browse/JDK-8280494 Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: More spec udpate - Changes: - all: https://git.openjdk.java.net/jdk/pull/7252/files - new: https://git.openjdk.java.net/jdk/pull/7252/files/6b74767e..9670efbd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=08-09 Stats: 39 lines in 2 files changed: 34 ins; 3 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7252.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7252/head:pull/7252 PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280494: (D)TLS signature schemes [v9]
On Fri, 4 Feb 2022 16:34:14 GMT, Sean Mullan wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> More update for the sec and impl
>
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 735:
>
>> 733: * {@link #setSignatureSchemes} method has not been called, this
>> method
>> 734: * should return the default signature schemes for connection
>> populated
>> 735: * objects, or {@code null} for pre-populated objects.
>
> I think this sentence should be part of the specification and not the
> implementation note since ideally you would want all provider implementations
> to behave this way (assuming they supported this method):
>
> "If the {@link #setSignatureSchemes} method has not been called, this method
> should return the default signature schemes for connection populated objects,
> or {@code null} for pre-populated objects."
I agreed.
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 752:
>
>> 750:
>> 751: /**
>> 752: * Sets the prioritized array of signature scheme names that
>
> If the system properties are set, does it override this API? I think it is
> important to mention that here in an @implNote. I know you discuss them in
> `getSignatureSchemes` but I think a developer is more likely to want to know
> the behavior of the properties for this method especially if they will
> override this setting.
I though it is a spec of the get method. But I agree with you that it could be
helpful if we have it in the set method as well. Updated.
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 761:
>
>> 759: * Names Specification. Providers may support signature schemes
>> not defined
>> 760: * in this list or may not use the recommended name for a certain
>> 761: * signature scheme.
>
> Did you want to say something about what the behavior should be if a provider
> does not support one of these schemes? Is it ignored, or is an exception
> thrown?
Just as your suggested in a later comment, an apiNote was added.
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 762:
>
>> 760: * in this list or may not use the recommended name for a certain
>> 761: * signature scheme.
>> 762: *
>
> Regarding my previous comment about providers not supporting these new
> methods, I think something like the following might suffice:
>
>
> @apiNote Note that a provider may not have been updated to support this
> method and in that case may ignore the schemes that are set.
> @implNote The SunJSSE provider supports this method.
Thank you for the suggestion. Updated.
-
PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280494: (D)TLS signature schemes [v11]
> This update is to support signature schemes customization for individual > (D)TLS connection. Please review the CSR as well: > CSR: https://bugs.openjdk.java.net/browse/JDK-8280495 > RFE: https://bugs.openjdk.java.net/browse/JDK-8280494 Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: Add missed spec, and delete swp file - Changes: - all: https://git.openjdk.java.net/jdk/pull/7252/files - new: https://git.openjdk.java.net/jdk/pull/7252/files/9670efbd..70382768 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=09-10 Stats: 7 lines in 2 files changed: 7 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7252.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7252/head:pull/7252 PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280494: (D)TLS signature schemes [v12]
> This update is to support signature schemes customization for individual > (D)TLS connection. Please review the CSR as well: > CSR: https://bugs.openjdk.java.net/browse/JDK-8280495 > RFE: https://bugs.openjdk.java.net/browse/JDK-8280494 Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: Correct the set spec - Changes: - all: https://git.openjdk.java.net/jdk/pull/7252/files - new: https://git.openjdk.java.net/jdk/pull/7252/files/70382768..70c3755b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=10-11 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7252.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7252/head:pull/7252 PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8280494: (D)TLS signature schemes [v9]
On Fri, 4 Feb 2022 16:35:22 GMT, Sean Mullan wrote: >> Xue-Lei Andrew Fan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> More update for the sec and impl > > src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 713: > >> 711: * in this list or may not use the recommended name for a certain >> 712: * signature scheme. >> 713: * > > Regarding my previous comment about providers not supporting these new > methods, I think something like the following might suffice: > > > @apiNote Note that a provider may not have been updated to support this > method and in that case may return null instead of the default signature > schemes for pre-populated objects. > @implNote The SunJSSE provider supports this method. It looks like a typo. Replaced "pre-populated objects" with "connection populated objects", and added to the spec. - PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8281175: Add a -providerPath option to jarsigner [v2]
On Thu, 3 Feb 2022 18:32:42 GMT, Weijun Wang wrote: >> Add the `-providerPath` option to jarsigner to be consistent with keytool. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > no need to append to null Marked as reviewed by hchao (Committer). Updated CSR looks good. - PR: https://git.openjdk.java.net/jdk/pull/7338
Re: RFR: 8280890: Cannot use '-Djava.system.class.loader' with class loader in signed JAR
On Tue, 1 Feb 2022 21:54:29 GMT, Sean Mullan wrote: > This fixes a bootstrapping issue if a custom system class loader is set with > the `-Djava.system.class.loader` option and the custom class loader is inside > a signed JAR. In order to load the custom class loader, the runtime must > verify the signed JAR first, and the algorithm constraint code tries to load > a `Locale` provider using a `ServiceLoader` before the class loader is set, > and this causes a `ServiceConfigurationError`. > > The fix removes a dependency from the security algorithm "denyAfter" > constraint parsing code on the `Calendar` API which uses a `ServiceLoader` > for gathering default locale information. Instead the `ZonedDateTime` API is > now used, which simplifies the code and removes some unnecessary code from > `keytool` as well. LGTM. - Marked as reviewed by weijun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7316
Re: RFR: 8280890: Cannot use '-Djava.system.class.loader' with class loader in signed JAR
On Tue, 1 Feb 2022 21:54:29 GMT, Sean Mullan wrote: > This fixes a bootstrapping issue if a custom system class loader is set with > the `-Djava.system.class.loader` option and the custom class loader is inside > a signed JAR. In order to load the custom class loader, the runtime must > verify the signed JAR first, and the algorithm constraint code tries to load > a `Locale` provider using a `ServiceLoader` before the class loader is set, > and this causes a `ServiceConfigurationError`. > > The fix removes a dependency from the security algorithm "denyAfter" > constraint parsing code on the `Calendar` API which uses a `ServiceLoader` > for gathering default locale information. Instead the `ZonedDateTime` API is > now used, which simplifies the code and removes some unnecessary code from > `keytool` as well. Marked as reviewed by hchao (Committer). Looks good to me. - PR: https://git.openjdk.java.net/jdk/pull/7316
Re: RFR: 8280494: (D)TLS signature schemes [v13]
> This update is to support signature schemes customization for individual > (D)TLS connection. Please review the CSR as well: > CSR: https://bugs.openjdk.java.net/browse/JDK-8280495 > RFE: https://bugs.openjdk.java.net/browse/JDK-8280494 Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: correct null tags - Changes: - all: https://git.openjdk.java.net/jdk/pull/7252/files - new: https://git.openjdk.java.net/jdk/pull/7252/files/70c3755b..be947693 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=11-12 Stats: 8 lines in 1 file changed: 1 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/7252.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7252/head:pull/7252 PR: https://git.openjdk.java.net/jdk/pull/7252
RFR: 8281289: Improve with List.copyOf
Please review this trivial code clean up, for a little bit better performance. - Commit messages: - 8281289: Improve with List.copyOf Changes: https://git.openjdk.java.net/jdk/pull/7356/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7356&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8281289 Stats: 5 lines in 1 file changed: 0 ins; 3 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7356.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7356/head:pull/7356 PR: https://git.openjdk.java.net/jdk/pull/7356
Re: RFR: 8281289: Improve with List.copyOf
On Fri, 4 Feb 2022 23:02:21 GMT, Xue-Lei Andrew Fan wrote: > Please review this trivial code clean up, for a little bit better performance. Looks good to me. - Marked as reviewed by jnimeh (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7356
Re: RFR: 8281289: Improve with List.copyOf
On Fri, 4 Feb 2022 23:02:21 GMT, Xue-Lei Andrew Fan wrote: > Please review this trivial code clean up, for a little bit better performance. Marked as reviewed by hchao (Committer). Looks good to me. - PR: https://git.openjdk.java.net/jdk/pull/7356
Integrated: 8281289: Improve with List.copyOf
On Fri, 4 Feb 2022 23:02:21 GMT, Xue-Lei Andrew Fan wrote: > Please review this trivial code clean up, for a little bit better performance. This pull request has now been integrated. Changeset: 42e272e1 Author:Xue-Lei Andrew Fan URL: https://git.openjdk.java.net/jdk/commit/42e272e181f188c89fa88f144715f19235943fca Stats: 5 lines in 1 file changed: 0 ins; 3 del; 2 mod 8281289: Improve with List.copyOf Reviewed-by: jnimeh, hchao - PR: https://git.openjdk.java.net/jdk/pull/7356
