jar signing and verification
Hi In attempting to validate jar signing I am seeing warnings as such displaying Invalid certificate chain --- >jarsigner -verify -certs bcprov-jdk15on-1.66.jar s 606774 Sat Jul 04 15:48:42 EDT 2020 META-INF/MANIFEST.MF >>> Signer X.509, CN=Legion of the Bouncy Castle Inc., OU=Java Software Code Signing, O=Sun Microsystems Inc [certificate expired on 4/25/20 3:00 AM] X.509, CN=JCE Code Signing CA, OU=Java Software Code Signing, O=Sun Microsystems Inc, L=Palo Alto, ST=CA, C=US [certificate expired on 4/25/20 3:00 AM] [Invalid certificate chain: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target] >>> Signer X.509, CN=Legion of the Bouncy Castle Inc., OU=Java Software Code Signing, O=Oracle Corporation [certificate is valid from 3/10/17 8:07 PM to 3/10/22 8:07 PM] X.509, CN=JCE Code Signing CA, OU=Java Software Code Signing, O=Oracle Corporation [certificate is valid from 7/6/16 7:48 PM to 12/30/30 7:00 PM] [Invalid certificate chain: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target] --- jarsigner -keystore ...\jre\lib\security\cacerts -verify -verbose -certs bcprov-jdk15on-1.66.jar >out.txt s 606774 Sat Jul 04 15:48:42 EDT 2020 META-INF/MANIFEST.MF X.509, CN=Legion of the Bouncy Castle Inc., OU=Java Software Code Signing, O=Sun Microsystems Inc [certificate expired on 4/25/20 3:00 AM] X.509, CN=JCE Code Signing CA, OU=Java Software Code Signing, O=Sun Microsystems Inc, L=Palo Alto, ST=CA, C=US [certificate expired on 4/25/20 3:00 AM] [CertPath not validated: Path does not chain with any of the trust anchors] [entry was signed on 7/4/20 1:48 AM] X.509, CN=Legion of the Bouncy Castle Inc., OU=Java Software Code Signing, O=Oracle Corporation [certificate is valid from 3/10/17 8:07 PM to 3/10/22 8:07 PM] X.509, CN=JCE Code Signing CA, OU=Java Software Code Signing, O=Oracle Corporation [certificate is valid from 7/6/16 7:48 PM to 12/30/30 7:00 PM] [CertPath not validated: Path does not chain with any of the trust anchors] --- Why do we get warnings of "Invalid certificate chain"? (I do not believe it's related to the expired warning as I see newer jars exhibiting the same outcome without an expired notice) Is the jar incorrectly signed or is the required chain simply not found in cacerts and if so why is it not there. I do see that at the end of the listing it says "jar verified" The samples in https://docs.oracle.com/en/java/javase/15/docs/specs/man/jarsigner.html#errors-and-warnings seem to show a fully validated chain. thanks Raj
Integrated: JDK-8166596: TLS support for the EdDSA signature algorithm
On Fri, 13 Nov 2020 04:57:12 GMT, Jamil Nimeh wrote: > Hello all, > This change brings in support for certificates with EdDSA keys (both Ed25519 > and Ed448) allowing those signature algorithms to be used both on the > certificates themselves and used during the handshaking process for messages > like CertificateVerify, ServerKeyExchange and so forth. This pull request has now been integrated. Changeset: d80ae05f Author:Jamil Nimeh URL: https://git.openjdk.java.net/jdk/commit/d80ae05f Stats: 843 lines in 9 files changed: 783 ins; 18 del; 42 mod 8166596: TLS support for the EdDSA signature algorithm Reviewed-by: xuelei - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: 8026976: ECParameters, Point does not match field size
On Wed, 2 Dec 2020 18:06:33 GMT, Xue-Lei Andrew Fan wrote: >> I only used "!" for consistency with existing usage in P11Key.java:1080. >> Is there a reason to avoid "!" other than maybe readability? > > Save a operation could get a little bit performance. Comparing to "if (!a)", > "if (a)" is easier to read to me, and save me a cycle to compute the "!". > Anyway, not a big concern of mine, you can leave it as is if you prefer the > "if (!a)" style. Just to finish up the thought. The assembly instructions are the same using if(!a) or if(a). The difference would be how the code is setup during compile time and I believe that would be the same speed as well.. - PR: https://git.openjdk.java.net/jdk/pull/1568
Integrated: 8253821: Improve ByteBuffer performance with GCM
On Tue, 29 Sep 2020 20:22:55 GMT, Anthony Scarpino wrote: > 8253821: Improve ByteBuffer performance with GCM This pull request has now been integrated. Changeset: cc1915b3 Author:Anthony Scarpino URL: https://git.openjdk.java.net/jdk/commit/cc1915b3 Stats: 2187 lines in 15 files changed: 2027 ins; 50 del; 110 mod 8253821: Improve ByteBuffer performance with GCM Reviewed-by: xuelei, valeriep - PR: https://git.openjdk.java.net/jdk/pull/411
Re: RFR: 8253821: Improve ByteBuffer performance with GCM [v6]
On Wed, 2 Dec 2020 04:58:13 GMT, Anthony Scarpino wrote: >> The biggest part of this change is the addition of overlap protection and >> the tests to verify it for GCM, as there were none in the open repo. >> Additionally, GCMBufferTest had some significant changes to clean it up and >> handle offsets better. All tests pass. With RDP1 coming, I want to get >> this into the repo soon, so please limit comments to bugs. Any "nice to >> have" changes, they can be added onto follow-on changes I plan. > > Webrev updated with latest comments.. rev 5. Ok, I have no more comments. - PR: https://git.openjdk.java.net/jdk/pull/411
Re: RFR: 8253821: Improve ByteBuffer performance with GCM [v6]
On Wed, 2 Dec 2020 05:01:28 GMT, Anthony Scarpino wrote: >> 8253821: Improve ByteBuffer performance with GCM > > Anthony Scarpino has updated the pull request incrementally with one > additional commit since the last revision: > > comments v4 Marked as reviewed by valeriep (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/411
Integrated: 8256818: SSLSocket that is never bound or connected leaks socket resources
On Sat, 21 Nov 2020 08:32:17 GMT, Christoph Langer wrote: > There is a flaw in sun.security.ssl.SSLSocketImpl::close() which leads to > leaking socket resources after JDK-8224829. > > The close method calls duplexCloseOutput() and duplexCloseInput(). In case of > an exception in any of these methods, the call to closeSocket() is bypassed, > and the underlying Socket may not be closed. > > This manifests in a real life leak after JDK-8224829 has introduced a call to > getSoLinger() on the path of duplexCloseOutput -> closeNotify. If socket impl > / OS socket hadn't been created yet it is done at that place. But then after > duplexCloseOutput eventually fails with a SocketException since the socket > wasn't connected, closing fails to call Socket::close(). > > This problem can be reproduced by this code: > SSLSocket sslSocket = > (SSLSocket)SSLSocketFactory.getDefault().createSocket(); > sslSocket.getSSLParameters(); > sslSocket.close(); > > This is what happens when SSLContext.getDefault().getDefaultSSLParameters() > is called, with close() being eventually called by the finalizer. > > I'll open this PR as draft for now to start discussion. I'll create a > testcase to reproduce the issue and add it soon. > > I propose to modify the close method such that duplexClose is only done on a > connected/bound socket. Maybe it even suffices to only do it when connected. > > Secondly, I'm proposing to improve exception handling a bit. So in case > there's an IOException on the path of duplexClose, it is caught and logged. > But the real close moves to the finally block since it should be done > unconditionally. This pull request has now been integrated. Changeset: 93b6ab56 Author:Christoph Langer URL: https://git.openjdk.java.net/jdk/commit/93b6ab56 Stats: 129 lines in 5 files changed: 97 ins; 15 del; 17 mod 8256818: SSLSocket that is never bound or connected leaks socket resources Reviewed-by: xuelei - PR: https://git.openjdk.java.net/jdk/pull/1363
Re: RFR: 8256818: SSLSocket that is never bound or connected leaks socket resources [v3]
On Wed, 2 Dec 2020 18:42:36 GMT, Christoph Langer wrote: >> test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketLeak.java line 37: >> >>> 35: * will not leave leaking socket file descriptors >>> 36: * @library /test/lib >>> 37: * @run main/othervm SSLSocketLeak >> >> See bellow comment, I may suggest to have it as a manual test case if you >> agree the test case could be impacted. >> @run main/manual SSLSocketLeak > > Hm, I think it's fine as it is. Running it in othervm will make sure the test > runs in its own vm (see http://openjdk.java.net/jtreg/command-help.html). So > within the VM process there should not be any interference by other workload. > And we check open files before testing and afterwards, and allow for some > margin. > > The test has been running in our test setup for several days now, so I think > it should be ok. And if worst comes to worse, and we see test noise, we might > change the test to manual later on. Sounds good to me. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/1363
Re: RFR: 8256818: SSLSocket that is never bound or connected leaks socket resources [v3]
On Sun, 22 Nov 2020 18:27:56 GMT, Christoph Langer wrote: >> There is a flaw in sun.security.ssl.SSLSocketImpl::close() which leads to >> leaking socket resources after JDK-8224829. >> >> The close method calls duplexCloseOutput() and duplexCloseInput(). In case >> of an exception in any of these methods, the call to closeSocket() is >> bypassed, and the underlying Socket may not be closed. >> >> This manifests in a real life leak after JDK-8224829 has introduced a call >> to getSoLinger() on the path of duplexCloseOutput -> closeNotify. If socket >> impl / OS socket hadn't been created yet it is done at that place. But then >> after duplexCloseOutput eventually fails with a SocketException since the >> socket wasn't connected, closing fails to call Socket::close(). >> >> This problem can be reproduced by this code: >> SSLSocket sslSocket = >> (SSLSocket)SSLSocketFactory.getDefault().createSocket(); >> sslSocket.getSSLParameters(); >> sslSocket.close(); >> >> This is what happens when SSLContext.getDefault().getDefaultSSLParameters() >> is called, with close() being eventually called by the finalizer. >> >> I'll open this PR as draft for now to start discussion. I'll create a >> testcase to reproduce the issue and add it soon. >> >> I propose to modify the close method such that duplexClose is only done on a >> connected/bound socket. Maybe it even suffices to only do it when connected. >> >> Secondly, I'm proposing to improve exception handling a bit. So in case >> there's an IOException on the path of duplexClose, it is caught and logged. >> But the real close moves to the finally block since it should be done >> unconditionally. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Small test improvement Marked as reviewed by xuelei (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1363
Re: RFR: 8256818: SSLSocket that is never bound or connected leaks socket resources [v3]
On Wed, 2 Dec 2020 18:01:04 GMT, Xue-Lei Andrew Fan wrote: >> Christoph Langer has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Small test improvement > > test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketLeak.java line 37: > >> 35: * will not leave leaking socket file descriptors >> 36: * @library /test/lib >> 37: * @run main/othervm SSLSocketLeak > > See bellow comment, I may suggest to have it as a manual test case if you > agree the test case could be impacted. > @run main/manual SSLSocketLeak Hm, I think it's fine as it is. Running it in othervm will make sure the test runs in its own vm (see http://openjdk.java.net/jtreg/command-help.html). So within the VM process there should not be any interference by other workload. And we check open files before testing and afterwards, and allow for some margin. The test has been running in our test setup for several days now, so I think it should be ok. And if worst comes to worse, and we see test noise, we might change the test to manual later on. - PR: https://git.openjdk.java.net/jdk/pull/1363
Re: RFR: 8026976: ECParameters, Point does not match field size
On Wed, 2 Dec 2020 17:49:10 GMT, Anthony Scarpino wrote:
>> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11ECKeyFactory.java
>> line 303:
>>
>>> 301: } else {
>>> 302: point = decodePoint(attributes[0].getByteArray(),
>>> params.getCurve());
>>> 303: }
>>
>> You could save a "!" operation in 299 if switch line 302 and line 300.
>
> I only used "!" for consistency with existing usage in P11Key.java:1080.
> Is there a reason to avoid "!" other than maybe readability?
Save a operation could get a little bit performance. Comparing to "if (!a)",
"if (a)" is easier to read to me, and save me a cycle to compute the "!".
Anyway, not a big concern of mine, you can leave it as is if you prefer the "if
(!a)" style.
-
PR: https://git.openjdk.java.net/jdk/pull/1568
Re: RFR: 8256818: SSLSocket that is never bound or connected leaks socket resources [v3]
On Sun, 22 Nov 2020 18:27:56 GMT, Christoph Langer wrote:
>> There is a flaw in sun.security.ssl.SSLSocketImpl::close() which leads to
>> leaking socket resources after JDK-8224829.
>>
>> The close method calls duplexCloseOutput() and duplexCloseInput(). In case
>> of an exception in any of these methods, the call to closeSocket() is
>> bypassed, and the underlying Socket may not be closed.
>>
>> This manifests in a real life leak after JDK-8224829 has introduced a call
>> to getSoLinger() on the path of duplexCloseOutput -> closeNotify. If socket
>> impl / OS socket hadn't been created yet it is done at that place. But then
>> after duplexCloseOutput eventually fails with a SocketException since the
>> socket wasn't connected, closing fails to call Socket::close().
>>
>> This problem can be reproduced by this code:
>> SSLSocket sslSocket =
>> (SSLSocket)SSLSocketFactory.getDefault().createSocket();
>> sslSocket.getSSLParameters();
>> sslSocket.close();
>>
>> This is what happens when SSLContext.getDefault().getDefaultSSLParameters()
>> is called, with close() being eventually called by the finalizer.
>>
>> I'll open this PR as draft for now to start discussion. I'll create a
>> testcase to reproduce the issue and add it soon.
>>
>> I propose to modify the close method such that duplexClose is only done on a
>> connected/bound socket. Maybe it even suffices to only do it when connected.
>>
>> Secondly, I'm proposing to improve exception handling a bit. So in case
>> there's an IOException on the path of duplexClose, it is caught and logged.
>> But the real close moves to the finally block since it should be done
>> unconditionally.
>
> Christoph Langer has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Small test improvement
test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketLeak.java line 57:
> 55: if ((fds_end - fds_start) > (NUM_TEST_SOCK / 10)) {
> 56: throw new RuntimeException("Too many open file descriptors.
> Looks leaky.");
> 57: }
This test case may be not reliable if there are some other test cases or
applications running at the same time. It's a good manual test, but might be
not suitable for OpenJDK automation regression test if it could be impacted.
test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketLeak.java line 37:
> 35: * will not leave leaking socket file descriptors
> 36: * @library /test/lib
> 37: * @run main/othervm SSLSocketLeak
See bellow comment, I may suggest to have it as a manual test case if you agree
the test case could be impacted.
@run main/manual SSLSocketLeak
-
PR: https://git.openjdk.java.net/jdk/pull/1363
Re: RFR: 8026976: ECParameters, Point does not match field size
On Wed, 2 Dec 2020 17:34:11 GMT, Xue-Lei Andrew Fan wrote:
>> I need a code review for this small code change. The code did not run the
>> data through the DER decoding class before setting it to the point when the
>> SunPKCS11 configuration had UseEcX963Encoding set to false.
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11ECKeyFactory.java
> line 303:
>
>> 301: } else {
>> 302: point = decodePoint(attributes[0].getByteArray(),
>> params.getCurve());
>> 303: }
>
> You could save a "!" operation in 299 if switch line 302 and line 300.
I only used "!" for consistency with existing usage in PK11Key.java:1080.Is
there a reason to avoid "!" other than maybe readability?
-
PR: https://git.openjdk.java.net/jdk/pull/1568
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v5]
On Wed, 2 Dec 2020 17:14:11 GMT, Jamil Nimeh wrote: >> Hello all, >> This change brings in support for certificates with EdDSA keys (both Ed25519 >> and Ed448) allowing those signature algorithms to be used both on the >> certificates themselves and used during the handshaking process for messages >> like CertificateVerify, ServerKeyExchange and so forth. > > Jamil Nimeh has updated the pull request incrementally with one additional > commit since the last revision: > > Applied code review comments There are still a few minor concerns, but nothing significant. We could have them addressed in separate bugs. - Marked as reviewed by xuelei (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: 8026976: ECParameters, Point does not match field size
On Wed, 2 Dec 2020 17:08:11 GMT, Anthony Scarpino wrote:
> I need a code review for this small code change. The code did not run the
> data through the DER decoding class before setting it to the point when the
> SunPKCS11 configuration had UseEcX963Encoding set to false.
Marked as reviewed by xuelei (Reviewer).
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11ECKeyFactory.java
line 303:
> 301: } else {
> 302: point = decodePoint(attributes[0].getByteArray(),
> params.getCurve());
> 303: }
You could save a "!" operation in 299 if switch line 302 and line 300.
-
PR: https://git.openjdk.java.net/jdk/pull/1568
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v5]
> Hello all, > This change brings in support for certificates with EdDSA keys (both Ed25519 > and Ed448) allowing those signature algorithms to be used both on the > certificates themselves and used during the handshaking process for messages > like CertificateVerify, ServerKeyExchange and so forth. Jamil Nimeh has updated the pull request incrementally with one additional commit since the last revision: Applied code review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/1197/files - new: https://git.openjdk.java.net/jdk/pull/1197/files/58651fc5..9f9164a5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1197=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1197=03-04 Stats: 31 lines in 2 files changed: 10 ins; 12 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/1197.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1197/head:pull/1197 PR: https://git.openjdk.java.net/jdk/pull/1197
RFR: 8026976: ECParameters, Point does not match field size
I need a code review for this small code change. The code did not run the data through the DER decoding class before setting it to the point when the SunPKCS11 configuration had UseEcX963Encoding set to false. - Commit messages: - Remove from problemlist - Allow use of ASN.1 for publickeyspec Changes: https://git.openjdk.java.net/jdk/pull/1568/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1568=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8026976 Stats: 8 lines in 2 files changed: 6 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1568.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1568/head:pull/1568 PR: https://git.openjdk.java.net/jdk/pull/1568
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Wed, 2 Dec 2020 15:33:20 GMT, Jamil Nimeh wrote: >> SunEC's algorithm name for keys are always "EdDSA", but I know BC returns >> "Ed25519" or "Ed448". > > Filed and took ownership of JDK-8257607 to address BC JCE provider issues for > both XDH and EdDSA when used with SunJSSE. Also, specific to this particular algorithm the signature type can be EdDSA for both BC and SunJCE, regardless of if the key type is EdDSA (SunJCE) or Ed25519 or Ed448 (BC). Creating a signature with algorithm EdDSA and the use of the key from either provider will perform a signature of the proper kind. - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v4]
On Wed, 2 Dec 2020 16:28:44 GMT, Kartik Ohri
wrote:
>> src/java.net.http/share/classes/jdk/internal/net/http/Http1HeaderParser.java
>> line 119:
>>
>>> 117: while (canContinueParsing(input)) {
>>> 118: switch (state) {
>>> 119: case INITIAL ->
>>> state = State.STATUS_LINE;
>>
>> Looks good. Although, I think you can improve it further if you align the
>> lambda operators as well
>
> Yes right. Just noticed I had missed that. Fixed in latest commit :)
Looks much better, Kartik. Thanks
-
PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v5]
On Wed, 2 Dec 2020 16:34:17 GMT, Kartik Ohri wrote: >> Hi! >> Kindly review this patch to replace switch statements with switch >> expressions (where it makes sense) in the http client modules. The rationale >> is to improve readability of the code. >> Regards, >> Kartik > > Kartik Ohri has updated the pull request incrementally with one additional > commit since the last revision: > > Align -> and remove trailing whitespace Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v4]
On Wed, 2 Dec 2020 16:23:11 GMT, Daniel Fuchs wrote: >> Kartik Ohri has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. > > src/java.net.http/share/classes/jdk/internal/net/http/frame/SettingsFrame.java > line 82: > >> 80: case MAX_HEADER_LIST_SIZE ->"MAX_HEADER_LIST_SIZE"; >> 81: >> 82: default -> "unknown parameter"; > >> Check failure on line 81 in >> src/java.net.http/share/classes/jdk/internal/net/http/frame/SettingsFrame.java >> openjdk / jcheck >> >> Whitespace error >> >> Column 0: trailing whitespace >> ... > > WRT to whitespace errors detected by `jcheck`, note that you can fix them by > running the script: > > make/scripts/normalizer.pl > > on this file. That's handy. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v4]
On Wed, 2 Dec 2020 16:26:43 GMT, Patrick Concannon
wrote:
>> Kartik Ohri has refreshed the contents of this pull request, and previous
>> commits have been removed. The incremental views will show differences
>> compared to the previous content of the PR.
>
> src/java.net.http/share/classes/jdk/internal/net/http/Http1HeaderParser.java
> line 119:
>
>> 117: while (canContinueParsing(input)) {
>> 118: switch (state) {
>> 119: case INITIAL ->
>> state = State.STATUS_LINE;
>
> Looks good. Although, I think you can improve it further if you align the
> lambda operators as well
Yes right. Just noticed I had missed that. Fixed in latest commit :)
-
PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v4]
On Wed, 2 Dec 2020 09:42:09 GMT, Kartik Ohri
wrote:
>> Hi!
>> Kindly review this patch to replace switch statements with switch
>> expressions (where it makes sense) in the http client modules. The rationale
>> is to improve readability of the code.
>> Regards,
>> Kartik
>
> Kartik Ohri has refreshed the contents of this pull request, and previous
> commits have been removed. The incremental views will show differences
> compared to the previous content of the PR.
src/java.net.http/share/classes/jdk/internal/net/http/Http1HeaderParser.java
line 119:
> 117: while (canContinueParsing(input)) {
> 118: switch (state) {
> 119: case INITIAL ->
> state = State.STATUS_LINE;
Looks good. Although, I think you can improve it further if you align the
lambda operators as well
-
PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v4]
On Wed, 2 Dec 2020 16:15:13 GMT, Chris Hegarty wrote: >> Kartik Ohri has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. > > I think that the actual source changes look good. > > A few notes: > 1. there are whitespace issues. jcheck is failing. > 2. Please do not force push - just push. Force push messes up prior comments > in the thread. @ChrisHegarty Thanks for the review. I'll keep in mind not to use force push again. - PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v5]
> Hi! > Kindly review this patch to replace switch statements with switch expressions > (where it makes sense) in the http client modules. The rationale is to > improve readability of the code. > Regards, > Kartik Kartik Ohri has updated the pull request incrementally with one additional commit since the last revision: Align -> and remove trailing whitespace - Changes: - all: https://git.openjdk.java.net/jdk/pull/1364/files - new: https://git.openjdk.java.net/jdk/pull/1364/files/3e667427..e73ab02b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1364=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1364=03-04 Stats: 50 lines in 13 files changed: 0 ins; 0 del; 50 mod Patch: https://git.openjdk.java.net/jdk/pull/1364.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1364/head:pull/1364 PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v4]
On Wed, 2 Dec 2020 09:42:09 GMT, Kartik Ohri wrote: >> Hi! >> Kindly review this patch to replace switch statements with switch >> expressions (where it makes sense) in the http client modules. The rationale >> is to improve readability of the code. >> Regards, >> Kartik > > Kartik Ohri has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. src/java.net.http/share/classes/jdk/internal/net/http/frame/SettingsFrame.java line 82: > 80: case MAX_HEADER_LIST_SIZE ->"MAX_HEADER_LIST_SIZE"; > 81: > 82: default -> "unknown parameter"; > Check failure on line 81 in > src/java.net.http/share/classes/jdk/internal/net/http/frame/SettingsFrame.java > openjdk / jcheck > > Whitespace error > > Column 0: trailing whitespace > ... WRT to whitespace errors detected by `jcheck`, note that you can fix them by running the script: make/scripts/normalizer.pl on this file. - PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v4]
On Wed, 2 Dec 2020 09:42:09 GMT, Kartik Ohri wrote: >> Hi! >> Kindly review this patch to replace switch statements with switch >> expressions (where it makes sense) in the http client modules. The rationale >> is to improve readability of the code. >> Regards, >> Kartik > > Kartik Ohri has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. I think that the actual source changes look good. A few notes: 1. there are whitespace issues. jcheck is failing. 2. Please so not force push - just push. Force push messes up prior comments in the thread. - Marked as reviewed by chegar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Fri, 20 Nov 2020 20:05:09 GMT, Weijun Wang wrote: >> src/java.base/share/classes/sun/security/ssl/JsseJce.java line 97: >> >>> 95: */ >>> 96: static final String SIGNATURE_EDDSA = "EdDSA"; >>> 97: >> >> Please update the copyright year. >> >> Is it possible that "ed25519" or "ed448" is used as the signature algorithm, >> especially in the X.509 certificate implementation? > > SunEC's algorithm name for keys are always "EdDSA", but I know BC returns > "Ed25519" or "Ed448". Filed and took ownership of JDK-8257607 to address BC JCE provider issues for both XDH and EdDSA when used with SunJSSE. - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v4]
> Hi! > Kindly review this patch to replace switch statements with switch expressions > (where it makes sense) in the http client modules. The rationale is to > improve readability of the code. > Regards, > Kartik Kartik Ohri has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: Vertically align statements in switch expressions - Changes: - all: https://git.openjdk.java.net/jdk/pull/1364/files - new: https://git.openjdk.java.net/jdk/pull/1364/files/7fa11daf..3e667427 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1364=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1364=02-03 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/1364.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1364/head:pull/1364 PR: https://git.openjdk.java.net/jdk/pull/1364
Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v3]
> Hi! > Kindly review this patch to replace switch statements with switch expressions > (where it makes sense) in the http client modules. The rationale is to > improve readability of the code. > Regards, > Kartik Kartik Ohri has updated the pull request incrementally with one additional commit since the last revision: Vertically align statements in switch expressions - Changes: - all: https://git.openjdk.java.net/jdk/pull/1364/files - new: https://git.openjdk.java.net/jdk/pull/1364/files/542298e0..7fa11daf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1364=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1364=01-02 Stats: 74 lines in 14 files changed: 14 ins; 0 del; 60 mod Patch: https://git.openjdk.java.net/jdk/pull/1364.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1364/head:pull/1364 PR: https://git.openjdk.java.net/jdk/pull/1364
