Re: RFR: 8282723: Add constructors taking a cause to JSSE exceptions [v5]
> Please review this small API enhancement to add the usual constructors taking > a cause to javax.net.ssl exceptions. The use of initCause in the JSSE > implementation code is updated to use the new constructors accordingly. > > Please review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282724 Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: more test case udpate - Changes: - all: https://git.openjdk.java.net/jdk/pull/7722/files - new: https://git.openjdk.java.net/jdk/pull/7722/files/37953310..61cc7934 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7722&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7722&range=03-04 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7722.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7722/head:pull/7722 PR: https://git.openjdk.java.net/jdk/pull/7722
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket [v3]
> Fixed API Note in javadoc for javax.net.ssl.SSLSocket class. API Note was > introduced by JDK-8208526 [1]. At that point both Socket.shutdownInput() / > Socket.shutdownOutput() and InputStream.close() / OutputStream.close() > performed half-close of TLS-1.3 connection. However this behaviour has > changed as result of JDK-8216326 [2]. InputStream.close() / > OutputStream.close() no longer perform half-close but full socket close, but > API Note was never updated. > > [1] https://bugs.openjdk.java.net/browse/JDK-8208526 > [2] https://bugs.openjdk.java.net/browse/JDK-8216326 zzambers has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit: javax/net/ssl/SSLSocket: Fixed API Note in javadoc - Changes: https://git.openjdk.java.net/jdk/pull/7648/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7648&range=02 Stats: 13 lines in 1 file changed: 2 ins; 0 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/7648.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7648/head:pull/7648 PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket [v2]
On Mon, 7 Mar 2022 21:01:12 GMT, Bradford Wetmore wrote: >> @bradfordwetmore Sure if more changes are desired I can pull your changes. >> When It comes to CSR I am not fully familiar with the process. Is action >> expected from my side? > >> Sure if more changes are desired I can pull your changes. When It comes to >> CSR I am not fully familiar with the > process. Is action expected from my side? > > One of us needs to get the CSR approved. Why don't you pull the changes > described in: > > https://mail.openjdk.java.net/pipermail/security-dev/2022-March/029209.html > > Assuming you are ok with the updated wording, I can create the CSR and > submit. Once that it approved, then we can get this PR approved and you can > integrate. > > Are you at contributor status? @bradfordwetmore I have updated this PR with your patch. I am covered by OCA. - PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: 8282723: Add constructors taking a cause to JSSE exceptions [v2]
On Tue, 8 Mar 2022 05:51:21 GMT, Xue-Lei Andrew Fan wrote:
>> src/java.base/share/classes/sun/security/ssl/ECDHKeyExchange.java line 204:
>>
>>> 202: } catch (GeneralSecurityException | java.io.IOException e)
>>> {
>>> 203: throw new SSLHandshakeException(
>>> 204: "Could not generate ECPublicKey", e);
>>
>> Nit: I think combining these lines would be < 80 chars
>
> The exception name is too long to have in one line. There are 83 characters
> in total if combining line 203 and 204. 3 characters exceeding 80 chars per
> line limit is not easy to tell with eyes.
The 80 character recommendation isn't a hard limit, I favor readability in the
80-100 char range.
-
PR: https://git.openjdk.java.net/jdk/pull/7722
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket [v3]
On Tue, 8 Mar 2022 14:21:19 GMT, zzambers wrote: >> Fixed API Note in javadoc for javax.net.ssl.SSLSocket class. API Note was >> introduced by JDK-8208526 [1]. At that point both Socket.shutdownInput() / >> Socket.shutdownOutput() and InputStream.close() / OutputStream.close() >> performed half-close of TLS-1.3 connection. However this behaviour has >> changed as result of JDK-8216326 [2]. InputStream.close() / >> OutputStream.close() no longer perform half-close but full socket close, but >> API Note was never updated. >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8208526 >> [2] https://bugs.openjdk.java.net/browse/JDK-8216326 > > zzambers has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains one commit: > > javax/net/ssl/SSLSocket: Fixed API Note in javadoc But I am not a Commiter (I need sponsor). - PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket [v4]
> Fixed API Note in javadoc for javax.net.ssl.SSLSocket class. API Note was > introduced by JDK-8208526 [1]. At that point both Socket.shutdownInput() / > Socket.shutdownOutput() and InputStream.close() / OutputStream.close() > performed half-close of TLS-1.3 connection. However this behaviour has > changed as result of JDK-8216326 [2]. InputStream.close() / > OutputStream.close() no longer perform half-close but full socket close, but > API Note was never updated. > > [1] https://bugs.openjdk.java.net/browse/JDK-8208526 > [2] https://bugs.openjdk.java.net/browse/JDK-8216326 zzambers has updated the pull request incrementally with one additional commit since the last revision: small fixes - Changes: - all: https://git.openjdk.java.net/jdk/pull/7648/files - new: https://git.openjdk.java.net/jdk/pull/7648/files/ddd59a9e..38d649e9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7648&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7648&range=02-03 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7648.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7648/head:pull/7648 PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket [v2]
On Mon, 7 Mar 2022 21:01:12 GMT, Bradford Wetmore wrote: >> @bradfordwetmore Sure if more changes are desired I can pull your changes. >> When It comes to CSR I am not fully familiar with the process. Is action >> expected from my side? > >> Sure if more changes are desired I can pull your changes. When It comes to >> CSR I am not fully familiar with the > process. Is action expected from my side? > > One of us needs to get the CSR approved. Why don't you pull the changes > described in: > > https://mail.openjdk.java.net/pipermail/security-dev/2022-March/029209.html > > Assuming you are ok with the updated wording, I can create the CSR and > submit. Once that it approved, then we can get this PR approved and you can > integrate. > > Are you at contributor status? @bradfordwetmore Your changes look good to me. When it comes to wording, I'll let that to native english speaker(s) to judge :) (As I am not native english speaker myself). I built docs locally and result looks good (also links work), there was just one problem with brace, but I have fixed that. - PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: 8275640 (win) java.net.NetworkInterface issues with IPv6-only environments [v7]
On Mon, 7 Feb 2022 09:14:51 GMT, Daniel Jeliński wrote: >> Clean up of various issues related to error handling and memory management > > Daniel Jeliński has updated the pull request incrementally with one > additional commit since the last revision: > > Initialize return value in all cases Marked as reviewed by msheppar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6090
Re: RFR: 8275640 (win) java.net.NetworkInterface issues with IPv6-only environments [v7]
On Mon, 7 Feb 2022 09:14:51 GMT, Daniel Jeliński wrote: >> Clean up of various issues related to error handling and memory management > > Daniel Jeliński has updated the pull request incrementally with one > additional commit since the last revision: > > Initialize return value in all cases Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6090
Integrated: 8275640 (win) java.net.NetworkInterface issues with IPv6-only environments
On Sat, 23 Oct 2021 10:26:34 GMT, Daniel Jeliński wrote: > Clean up of various issues related to error handling and memory management This pull request has now been integrated. Changeset: 2549e550 Author:Daniel Jeliński Committer: Daniel Fuchs URL: https://git.openjdk.java.net/jdk/commit/2549e5503806447733572643603af9a2bf4b52e5 Stats: 68 lines in 2 files changed: 35 ins; 20 del; 13 mod 8275640: (win) java.net.NetworkInterface issues with IPv6-only environments Reviewed-by: msheppar, dfuchs - PR: https://git.openjdk.java.net/jdk/pull/6090
Re: RFR: 8280494: (D)TLS signature schemes [v19]
On Sun, 6 Mar 2022 05:40:59 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
>> Release-note: https://bugs.openjdk.java.net/browse/JDK-8281290
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> add test for DTLS
Changes requested by mullan (Reviewer).
src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java line 501:
> 499:
> 500: // Note that if the System Property value is not defined (JDK
> 501: // default value) or empty, the provider-specific default is
> used.
I think you can remove this comment as it is repeated on lines 507-508 (and
makes more sense there).
src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 564:
> 562: String[] signatureSchemes) {
> 563:
> 564: if (signatureSchemes == null || signatureSchemes.length == 0) {
Nit: remove extra space after `||`.
src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 576:
> 574: SSLLogger.finest(
> 575: "Ignore the signature algorithm (" + ss
> 576: + "), unsupported or unavailable");
Should this message be more consistent with the message in
`getCustomizedSignatureScheme`?: "The current installed providers do not
support signature scheme: " + schemeName
test/jdk/javax/net/ssl/DTLS/DTLSSignatureSchemes.java line 125:
> 123: testCase.runTest(testCase);
> 124: if (exceptionExpected) {
> 125: throw new RuntimeException("Unexpected success!");
The catch block on line 127 will end up catching this exception and swallowing
it, and the test will incorrectly pass.
test/jdk/javax/net/ssl/SSLParameters/SignatureSchemes.java line 81:
> 79: super.runClientApplication(sslSocket);
> 80: if (exceptionExpected) {
> 81: throw new RuntimeException("Unexpected success!");
The catch block on line 83 will end up catching this exception and swallowing
it, and the test will incorrectly pass.
-
PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket [v4]
On Tue, 8 Mar 2022 15:03:57 GMT, zzambers wrote: >> Fixed API Note in javadoc for javax.net.ssl.SSLSocket class. API Note was >> introduced by JDK-8208526 [1]. At that point both Socket.shutdownInput() / >> Socket.shutdownOutput() and InputStream.close() / OutputStream.close() >> performed half-close of TLS-1.3 connection. However this behaviour has >> changed as result of JDK-8216326 [2]. InputStream.close() / >> OutputStream.close() no longer perform half-close but full socket close, but >> API Note was never updated. >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8208526 >> [2] https://bugs.openjdk.java.net/browse/JDK-8216326 > > zzambers has updated the pull request incrementally with one additional > commit since the last revision: > > small fixes LGTM. I can sponsor after CSR approvals granted. Please hold off on integrating until then. - Marked as reviewed by wetmore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket [v2]
On Tue, 8 Mar 2022 15:23:13 GMT, zzambers wrote: >>> Sure if more changes are desired I can pull your changes. When It comes to >>> CSR I am not fully familiar with the >> process. Is action expected from my side? >> >> One of us needs to get the CSR approved. Why don't you pull the changes >> described in: >> >> https://mail.openjdk.java.net/pipermail/security-dev/2022-March/029209.html >> >> Assuming you are ok with the updated wording, I can create the CSR and >> submit. Once that it approved, then we can get this PR approved and you can >> integrate. >> >> Are you at contributor status? > > @bradfordwetmore Your changes look good to me. When it comes to wording, I'll > let that to native english speaker(s) to judge :) (As I am not native english > speaker myself). I built docs locally and result looks good (also links > work), there was just one problem with brace, but I have fixed that. @zzambers Thanks for catching the two little issues with the closing brackets/missing space. CSR has been finalized, just need to wait for approval. - PR: https://git.openjdk.java.net/jdk/pull/7648
