Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo This PR changes a comment in javax/swing/RepaintManager.java. This commented out code should be removed altogether in another PR? Because its an System.out.println and because its commented out code. - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo Nice, good work matteo Should I change the JBS issue title to match the PR title, or is it preferred for the PR title to change? - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282632: Cleanup unnecessary calls to Throwable.initCause() in java.security.jgss [v2]
On Fri, 4 Mar 2022 15:39:56 GMT, Sean Mullan wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8282632: Cleanup unnecessary calls to Throwable.initCause() in
>> java.security.jgss
>> typo
>
> src/java.security.jgss/share/classes/sun/security/krb5/internal/crypto/dk/ArcFourCrypto.java
> line 161:
>
>> 159:Ksign = getHmac(baseKey, new_ss);
>> 160: } catch (Exception e) {
>> 161: throw new GeneralSecurityException("Calculate Checkum
>> Failed!", e);
>
> Can you also fix the typo, "Checkum" should be "Checksum". Same comment
> applies to line 172.
Sure. Fixed
-
PR: https://git.openjdk.java.net/jdk/pull/7682
Re: RFR: 8282632: Cleanup unnecessary calls to Throwable.initCause() in java.security.jgss [v2]
> Pass cause exception as constructor parameter is shorter and easier to read. Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8282632: Cleanup unnecessary calls to Throwable.initCause() in java.security.jgss typo - Changes: - all: https://git.openjdk.java.net/jdk/pull/7682/files - new: https://git.openjdk.java.net/jdk/pull/7682/files/524ba92d..490f7ecb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7682=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7682=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7682.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7682/head:pull/7682 PR: https://git.openjdk.java.net/jdk/pull/7682
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo I eyeballed the diff file and all seems okay. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7268
Re: [External] : Re: [Internet]Recent SSLSocket close() @apiNote Changes.
On 3/2/2022 11:46 PM, xueleifan(XueleiFan) wrote:
I think you are right that this design is actually for TLSv1.3 half-close mode.
For TLS 1.3, there is no duplex closure design. The close() implementation
in JDK is actually a workaround for compatibility. Application can use either
the half-close mode
socket.shutdownOutput();
socket.shutdownInput();
or duplex close mode for compatibilty:
socket.close();
Unfortunately, in practice the half-close and duplex close are mixed with three
lines:
socket.shutdownOutput();
socket.shutdownInput();
socket.close();
Yeah, I've seen that in things like Apache HttpClient Core, which has
subsequently been removed.
How about something like this:
http://cr.openjdk.java.net/~wetmore/8282529/webrev.00/
Thanks,
Brad
It was not something I expected when I designed the spec for half-close mode.
It should be helpful if having another iteration for the @apiNote.
Thanks,
Xuelei
On Mar 2, 2022, at 5:14 PM, Bradford Wetmore
wrote:
Hi Xuelei,
I am working on some close code including the recent PR[1] for:
8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket
and ran into a change I hadn't noticed before.
* @apiNote
* When the connection is no longer needed, the client and server
* applications should each close both sides of their respective connection.
* For {@code SSLSocket} objects, for example, an application can call
* {@link Socket#shutdownOutput()} for output stream close and call
* {@link Socket#shutdownInput()} for input stream close.
It used to be that just a single SSLSocket.close() was sufficient to completely
shutdown the SSLSocket, and under the hood it closed the output/input in the
right order.
I believe this code still closes everything as before, but the updated @apiNote
encourages the user to do a three-part shutdown instead.
socket.shutdownOutput();
socket.shutdownInput();
socket.close();// mostly repeats of above.
This approach seems unnecessary unless the user is interested in the TLSv1.3
half-close mode.
What is the rationale for recommending this way of doing closes in general? Or
does this @apiNote need another iteration?
Thanks,
Brad
[1]
https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/7648__;!!ACWV5N9M2RV99hQ!cHw7i1wGs-eyCPUcrFXtAdFiUZL6aCPUGpGEQ9u56HHSuwew1j6YHapR8sSefEwr7TRXKQ$
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket [v2]
On Thu, 3 Mar 2022 12:36:33 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: > > Updated copyright for SSLSocket.java There are more changes needed, and should probably be done as part of this issue since we're already in this code anyway. The current code only talks about shutdownInput/shutdownOutput, but makes no mention of the duplex close(), which is the other way to shutdown the SSLSocket. It only needs a few words. https://mail.openjdk.java.net/pipermail/security-dev/2022-March/029167.html These two changes will require a CSR to update the @apiNote. - PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7268
RFR: JDK-8282686: Add constructors taking a cause to SocketException
Please review this small API enhancement to add the usual constructors taking a cause to SocketException and then update uses of initiCause on creating SocketException to instead pass the cause via the constructor. Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688 - Commit messages: - JDK-8282686: Add constructors take a cause to SocketException Changes: https://git.openjdk.java.net/jdk/pull/7705/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7705=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282686 Stats: 48 lines in 6 files changed: 23 ins; 12 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/7705.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7705/head:pull/7705 PR: https://git.openjdk.java.net/jdk/pull/7705
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo LGTM also. Similar suggestion for updating copyrights. - Marked as reviewed by wetmore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo Nice tidy of the code. Is there anything that can be done to prevent re-introduction of this trivial problem? Perhaps a new Skara bot jcheck option similar to what is already in place for trailing whitespace? - Marked as reviewed by iris (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo Marked as reviewed by prr (Reviewer). Looks like there's only one client source code file touched Most of the client changes are only in jtreg tests - and this is very trivial, so I'm OK with them being included here. - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 7192189: Support endpoint identification algorithm in RFC 6125
On Fri, 4 Mar 2022 14:59:54 GMT, Sean Mullan wrote: > Please review this change to fully support RFC 6125 in the TLS > implementation. This change forbids wildcard domains in TLS certificates > unless the wildcard is in the left-most component. Certificates of this > nature should be rare and are not allowed per the CABForum baseline > requirements. However there may be a small compatibility risk associated with > this change, so a CSR has also been filed. Marked as reviewed by xuelei (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7697
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 4 Mar 2022 17:17:12 GMT, Roger Riggs wrote: >> Hi >> >> I have reviewed the code for removing double semicolons at the end of lines >> >> all the best >> matteo > > We usually request that these be be broken up by area to attract the > appropriate reviewers and avoid eye-strain. The client modules are usually > separated out, as are hotspot. > And corresponding tests. > This kind of change is pretty low value for the code base and requires the > involvement of quite a few reviewers. > If you had ask ahead of time, I would have suggested finding something with > more value. @RogerRiggs Otoh, this change is really trivial. I have verified that all changes are replacing trailing ";;" in Java code. These are all clearly typos. I think it's nice that we try to strive for a high quality, and while you are correct this is maybe not the most pressing issue, I think it's nice to get a cleanup like this done. I'd argue that this is a trivial change. If you are worried, let's get a couple of more reviewers. I can't see the need to split this up per area. - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 7192189: Support endpoint identification algorithm in RFC 6125
On Fri, 4 Mar 2022 16:48:47 GMT, Sean Mullan wrote: > > About the CSR, did you have a plan to update the "Endpoint Identification > > Algorithms" in the [Java Security Standard Algorithm > > Names](https://docs.oracle.com/en/java/javase/17/docs/specs/security/standard-names.html#endpoint-identification-algorithms) > > documentation? Currently, the "HTTPS" name is defined for RFC 2818. With > > this update is may be worth to mention the compliant to RFC 6125, like > > ``` > > HTTPS | RFC 2818, compliant with RFC 6125 > > ``` > > I thought about that but I was hesitant to do that, because technically RFC > 6125 does not obsolete RFC 2818 and there has been no successor to RFC 2818. > So I would rather treat RFC 6125 as an implementation-specific feature of the > JDK TLS implementation; in other words we chose to make our implementation > compliant with RFC 6125 but other implementations may choose not to and still > be compliant with RFC 2818. Since RFC 2818 is somewhat ambiguous/vague with > respect to what components can use wildcards, I believe the JDK > implementation is still compliant with 2818. I realize this is not a perfect > situation, but if we do this via the API, then I think we need new APIs such > that older implementations that may be less strict about wildcards are still > compatible with 2818 if they choose. It makes sense to me. - PR: https://git.openjdk.java.net/jdk/pull/7697
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo We usually request that these be be broken up by area to attract the appropriate reviewers and avoid eye-strain. The client modules are usually separated out, as are hotspot. And corresponding tests. This kind of change is pretty low value for the code base and requires the involvement of quite a few reviewers. If you had ask ahead of time, I would have suggested finding something with more value. - Changes requested by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo Hi Lance I can make a second commit updating the copyright year Tell me if this is necessary ciao matteo - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo The changes look OK. The copyright year probably should be updated as part of this PR - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 25 Feb 2022 15:40:09 GMT, Matteo Baccan wrote: >> Hi >> >> I have reviewed the code for removing double semicolons at the end of lines >> >> all the best >> matteo > > Hi > > I have pushed this PR about 1 month ago. Only 3 days ago OCA was accepted. > Now: what is the next step? > > This is a cleanup PR that removes some double semicolons at the end of some > lines inside the JDK code. > > ciao > matteo Hi @matteobaccan The next step would be to create a relevant issue on the tracker and set it to track this PR. Since you don't have the ability to create new issues on the JBS yet I'll help you create one, please rename your PR title to 8282657 and the system should take care of the rest and automatically mark your PR as ready for review after setting it to track the corresponding JBS entry. Cheers, Julian - PR: https://git.openjdk.java.net/jdk/pull/7268
RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
Hi I have reviewed the code for removing double semicolons at the end of lines all the best matteo - Commit messages: - Removed double semicolon at the end of lines Changes: https://git.openjdk.java.net/jdk/pull/7268/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7268=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282657 Stats: 93 lines in 82 files changed: 0 ins; 0 del; 93 mod Patch: https://git.openjdk.java.net/jdk/pull/7268.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7268/head:pull/7268 PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo Hi I have pushed this PR about 1 month ago. Only 3 days ago OCA was accepted. Now: what is the next step? This is a cleanup PR that removes some double semicolons at the end of some lines inside the JDK code. ciao matteo - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 7192189: Support endpoint identification algorithm in RFC 6125
On Fri, 4 Mar 2022 16:33:45 GMT, Xue-Lei Andrew Fan wrote: > About the CSR, did you have a plan to update the "Endpoint Identification > Algorithms" in the [Java Security Standard Algorithm > Names](https://docs.oracle.com/en/java/javase/17/docs/specs/security/standard-names.html#endpoint-identification-algorithms) > documentation? Currently, the "HTTPS" name is defined for RFC 2818. With > this update is may be worth to mention the compliant to RFC 6125, like > > ``` > HTTPS | RFC 2818, compliant with RFC 6125 > ``` I thought about that but I was hesitant to do that, because technically RFC 6125 does not obsolete RFC 2818 and there has been no successor to RFC 2818. So I would rather treat RFC 6125 as an implementation-specific feature of the JDK TLS implementation; in other words we chose to make our implementation compliant with RFC 6125 but other implementations may choose not to and still be compliant with RFC 2818. Since RFC 2818 is somewhat ambiguous/vague with respect to what components can use wildcards, I believe the JDK implementation is still compliant with 2818. I realize this is not a perfect situation, but if we do this via the API, then I think we need new APIs such that older implementations that may be less strict about wildcards are still compatible with 2818 if they choose. - PR: https://git.openjdk.java.net/jdk/pull/7697
Re: RFR: 7192189: Support endpoint identification algorithm in RFC 6125
On Fri, 4 Mar 2022 14:59:54 GMT, Sean Mullan wrote: > Please review this change to fully support RFC 6125 in the TLS > implementation. This change forbids wildcard domains in TLS certificates > unless the wildcard is in the left-most component. Certificates of this > nature should be rare and are not allowed per the CABForum baseline > requirements. However there may be a small compatibility risk associated with > this change, so a CSR has also been filed. About the CSR, did you have a plan to update the "Endpoint Identification Algorithms" in the [Java Security Standard Algorithm Names](https://docs.oracle.com/en/java/javase/17/docs/specs/security/standard-names.html#endpoint-identification-algorithms) documentation? Currently, the "HTTPS" name is defined for RFC 2818. With this update is may be worth to mention the compliant to RFC 6125, like HTTPS | RFC 2818, compliant with RFC 6125 - PR: https://git.openjdk.java.net/jdk/pull/7697
Re: RFR: 8280494: (D)TLS signature schemes [v18]
On Thu, 17 Feb 2022 18:57:02 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: > > more spec update per CSR feedback Since this new API is also intended to be supported for DTLS, have you added implementation support for that, and if so have you added a test for it? - PR: https://git.openjdk.java.net/jdk/pull/7252
Re: RFR: 8282632: Cleanup unnecessary calls to Throwable.initCause() in java.security.jgss
On Thu, 3 Mar 2022 19:33:21 GMT, Andrey Turbanov wrote:
> Pass cause exception as constructor parameter is shorter and easier to read.
src/java.security.jgss/share/classes/sun/security/krb5/internal/crypto/dk/ArcFourCrypto.java
line 161:
> 159:Ksign = getHmac(baseKey, new_ss);
> 160: } catch (Exception e) {
> 161: throw new GeneralSecurityException("Calculate Checkum
> Failed!", e);
Can you also fix the typo, "Checkum" should be "Checksum". Same comment applies
to line 172.
-
PR: https://git.openjdk.java.net/jdk/pull/7682
RFR: 7192189: Support endpoint identification algorithm in RFC 6125
Please review this change to fully support RFC 6125 in the TLS implementation. This change forbids wildcard domains in TLS certificates unless the wildcard is in the left-most component. Certificates of this nature should be rare and are not allowed per the CABForum baseline requirements. However there may be a small compatibility risk associated with this change, so a CSR has also been filed. - Commit messages: - Initial revision. Changes: https://git.openjdk.java.net/jdk/pull/7697/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7697=00 Issue: https://bugs.openjdk.java.net/browse/JDK-7192189 Stats: 121 lines in 3 files changed: 80 ins; 34 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/7697.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7697/head:pull/7697 PR: https://git.openjdk.java.net/jdk/pull/7697
Re: RFR: 8282632: Cleanup unnecessary calls to Throwable.initCause() in java.security.jgss
On Thu, 3 Mar 2022 19:33:21 GMT, Andrey Turbanov wrote: > Pass cause exception as constructor parameter is shorter and easier to read. LGTM - Marked as reviewed by stsypa...@github.com (no known OpenJDK username). PR: https://git.openjdk.java.net/jdk/pull/7682
