Looks good to me.
--Jamil
On 8/19/19 9:02 AM, Xuelei Fan wrote:
Hi,
Could I have the following code cleanup reviewed?
http://cr.openjdk.java.net/~xuelei/8228757/webrev.00/
It is trying to fail fast if unknown handshake type get requested.
Simple fix and hard to capture the fail point, no
Hi,
Could I have the following code cleanup reviewed?
http://cr.openjdk.java.net/~xuelei/8228757/webrev.00/
It is trying to fail fast if unknown handshake type get requested.
Simple fix and hard to capture the fail point, no new regression test.
Thanks,
Xuelei
Hi Jamil,
Thank you for the review.
On 5/10/2019 9:22 AM, Jamil Nimeh wrote:
This looks good to me. One question, more for my curiosity than
anything else: Is the way you loaded the appData array in the test code
done for any specific reason? Or did you just want to make sure you had
printa
This looks good to me. One question, more for my curiosity than
anything else: Is the way you loaded the appData array in the test code
done for any specific reason? Or did you just want to make sure you had
printable ASCII that wasn't all just the same character, so it looked
"random-ish"?
Hi,
Could I get the following update reviewed?
http://cr.openjdk.java.net/~xuelei/8221253/webrev.00/
Because of the padding impact, the TLS 1.3 record in the JDK Reference
implementation could exceed the limit. It is not the expected behavior.
Thanks,
Xuelei
Hi Xuelei,
looks good to me as well.
best regards,
-- daniel
On 05/05/2019 16:18, Xuelei Fan wrote:
All good catches!
I made the update accordingly. Here is the new webrev:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.03/
Thanks,
Xuelei
On 5/3/2019 11:27 PM, Alan Bateman wrote:
O
On 05/05/2019 16:18, Xuelei Fan wrote:
All good catches!
I made the update accordingly. Here is the new webrev:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.03/
This update looks okay to me (an aternative for read is a nested
try/finally but what you have is okay).
-Alan
All good catches!
I made the update accordingly. Here is the new webrev:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.03/
Thanks,
Xuelei
On 5/3/2019 11:27 PM, Alan Bateman wrote:
On 04/05/2019 02:23, Xuelei Fan wrote:
Hi,
There is a surprise in the regression test. I made the update
On 04/05/2019 02:23, Xuelei Fan wrote:
Hi,
There is a surprise in the regression test. I made the update, and
now the testing passed. Here is the new webrev:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.01/
I assume you can save two volatile writes by not initializing isClosing
and
Oops, forgot to update the link of the webrev. It should be:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.02/
Xuelei
On 5/3/2019 6:23 PM, Xuelei Fan wrote:
Hi,
There is a surprise in the regression test. I made the update, and now
the testing passed. Here is the new webrev:
h
Hi,
There is a surprise in the regression test. I made the update, and now
the testing passed. Here is the new webrev:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.01/
Thanks,
Xuelei
On 5/3/2019 6:45 AM, Daniel Fuchs wrote:
Hi Xuelei,
I agree this should fix the issue I was speak
Hi Xuelei,
I agree this should fix the issue I was speaking of.
Looks good to me - but maybe you'll want a second
reviewer from the security-dev team :-)
best regards,
-- daniel
On 03/05/2019 14:03, Xuelei Fan wrote:
Hi Daniel,
Good catch!
Here is a new webrev that's trying to address the p
Hi Daniel,
Good catch!
Here is a new webrev that's trying to address the problem.
http://cr.openjdk.java.net/~xuelei/8219991/webrev.01/
The isClosing field update is moving ahead, and a new filed hasDepleted
was added for threads competition.
The external test described in JDK-8219658 pass
Hi Xuelei,
I believe there is still a small window of opportunity
for which `readLockedDeplete();` will never be called.
The issue is in `deplete()`:
If tryLock() fails to lock at line
1046 if (readLock.tryLock()) {
then there's no guarantee that the reading
thread will not release
Hi,
Could I get the following update reviewed?
http://cr.openjdk.java.net/~xuelei/8219991/webrev.00/
The March5 test looks good, and the external test described in
JDK-8219658 passed. No new regression test.
Thanks,
Xuelei
Looks good, pretty straightforward cleanup.
--Sean
On 2/5/19 1:44 PM, Xuelei Fan wrote:
Hi,
Could I have the update reviewed?
http://cr.openjdk.java.net/~xuelei/8217835/webrev.00/
With this update, the experimental FIPS 140 compliant mode is removed
from the SunJSSE provider. As the SunJ
Looks fine to me.
--Jamil
On 2/8/2019 8:16 AM, Xuelei Fan wrote:
Hi,
Please review this update:
http://cr.openjdk.java.net/~xuelei/8218580/webrev.00/
Objects.equals is case-sensitive, and should not be used to recognize
case-insensitive objects.
Trivial update, no new regression test.
Hi,
Please review this update:
http://cr.openjdk.java.net/~xuelei/8218580/webrev.00/
Objects.equals is case-sensitive, and should not be used to recognize
case-insensitive objects.
Trivial update, no new regression test.
Thanks,
Xuelei
Hi,
Could I have the update reviewed?
http://cr.openjdk.java.net/~xuelei/8217835/webrev.00/
With this update, the experimental FIPS 140 compliant mode is removed
from the SunJSSE provider. As the SunJSSE provider uses the JDK default
cryptography providers, alternatively applications can co
You sure can! Looks good.
--Jamil
On 1/25/2019 12:02 PM, Xuelei Fan wrote:
Hi,
Can I have a code review for a trivial code cleanup?
https://bugs.openjdk.java.net/browse/JDK-8217820
Thanks,
Xuelei
diff -r 1262a93634c2
src/java.base/share/classes/sun/security/util/ECUtil.java
--- a/src/ja
Hi,
Can I have a code review for a trivial code cleanup?
https://bugs.openjdk.java.net/browse/JDK-8217820
Thanks,
Xuelei
diff -r 1262a93634c2
src/java.base/share/classes/sun/security/util/ECUtil.java
--- a/src/java.base/share/classes/sun/security/util/ECUtil.java Thu Jan
24 12:52:37 2019 -
Hi Xuelei, this looks good to me.
--Jamil
On 1/15/2019 7:45 AM, Xue-Lei Fan wrote:
Hi,
Could I have the update reviewed?
http://cr.openjdk.java.net/~xuelei/8216045/webrev.00/
While getting the encoded public key for DH key exchange, the leading
zeros of the key are not trimmed and the ke
Hi,
Could I have the update reviewed?
http://cr.openjdk.java.net/~xuelei/8216045/webrev.00/
While getting the encoded public key for DH key exchange, the leading
zeros of the key are not trimmed and the key bit size is used for byte size.
John helped to verify the fix with the infra testi
Hi Xuelei,
This is not my area of expertise - so I'm going to rephrase
what I understand (which may be wrong):
SSLEngineImpl.java:
This change makes sure that the SSLEngineResult::getStatus()
returns Status.CLOSED when closure has been initiated, even
if the engine is only half-closed at t
On 1/9/2019 10:58 AM, Daniel Fuchs wrote:
Hi Xuelei,
On 22/12/2018 17:20, Xue-Lei Fan wrote:
The issue is caused by the handshake status "NEED_WRAP" while the
connection is half-closed. An application may just call wrap() when
the handshake status is "NEED_WRAP". For compatibility, I changed t
Hi Xuelei,
On 22/12/2018 17:20, Xue-Lei Fan wrote:
The issue is caused by the handshake status "NEED_WRAP" while the
connection is half-closed. An application may just call wrap() when the
handshake status is "NEED_WRAP". For compatibility, I changed the
handshake status from NEED_WRAP back to
Code changes look good to me.
--Jamil
On 1/8/2019 3:00 PM, Xue-Lei Fan wrote:
ping ...
Xuelei
On 12/22/2018 9:20 AM, Xue-Lei Fan wrote:
Hi,
Could I get the update reviewed?
http://cr.openjdk.java.net/~xuelei/8214418/webrev.00/
The reproducing testing case passed with the update.
The i
On 1/9/2019 6:10 AM, Chris Hegarty wrote:
Xuelei,
Is it possible to update the synopsis of this bug to better
reflect the underlying issue ( rather than one particular
symptom )?
Updated.
Also, it is possible to construct a small, non-HTTP related,
targeted test that verifies the fix?
T
Xuelei,
Is it possible to update the synopsis of this bug to better
reflect the underlying issue ( rather than one particular
symptom )?
Also, it is possible to construct a small, non-HTTP related,
targeted test that verifies the fix?
-Chris.
On 08/01/2019 23:00, Xue-Lei Fan wrote:
ping ...
ping ...
Xuelei
On 12/22/2018 9:20 AM, Xue-Lei Fan wrote:
Hi,
Could I get the update reviewed?
http://cr.openjdk.java.net/~xuelei/8214418/webrev.00/
The reproducing testing case passed with the update.
The issue is caused by the handshake status "NEED_WRAP" while the
connection is half-
Hi,
Could I get the update reviewed?
http://cr.openjdk.java.net/~xuelei/8214418/webrev.00/
The reproducing testing case passed with the update.
The issue is caused by the handshake status "NEED_WRAP" while the
connection is half-closed. An application may just call wrap() when the
handshak
On 12/17/18 5:26 PM, Xue-Lei Fan wrote:
On 12/17/2018 1:17 PM, Anthony Scarpino wrote:
It looks like in TransportContext.java:68, you had a mistype that
added "fa" to the end of a comment.
Oops, I will update it.
Also in fatal():267, did you plan to return the exception and have the
calling
On 12/17/2018 1:17 PM, Anthony Scarpino wrote:
It looks like in TransportContext.java:68, you had a mistype that added
"fa" to the end of a comment.
Oops, I will update it.
Also in fatal():267, did you plan to return the exception and have the
calling method throw the exception? As is, the
Hi Xuelei, comments in-line.
On 12/17/2018 12:11 PM, Xue-Lei Fan wrote:
On 12/17/2018 11:28 AM, Jamil Nimeh wrote:
Hi Xuelei, just a couple questions:
* SSLSocketImpl
o 611: Are you sure conContext.inputRecord should go into a
try-with-resources? As far as I can tell, the inhe
It looks like in TransportContext.java:68, you had a mistype that added
"fa" to the end of a comment.
Also in fatal():267, did you plan to return the exception and have the
calling method throw the exception? As is, the exception is never
return and fatal() continues to throw the exceptions.
On 12/17/2018 11:28 AM, Jamil Nimeh wrote:
Hi Xuelei, just a couple questions:
* SSLSocketImpl
o 611: Are you sure conContext.inputRecord should go into a
try-with-resources? As far as I can tell, the inheritence chain
is SSLSocketInputRecord->InputRecord and that direct
Hi Xuelei, just a couple questions:
* SSLSocketImpl
o 611: Are you sure conContext.inputRecord should go into a
try-with-resources? As far as I can tell, the inheritence chain
is SSLSocketInputRecord->InputRecord and that directly or by
extension implements the SSLReco
ping ...
On 12/10/2018 1:14 PM, Xue-Lei Fan wrote:
Hi,
Please review the TLS 1.3 half-close issue in JDK.
http://cr.openjdk.java.net/~xuelei/8209333/webrev.00/
While trying to duplex close a TLS connection upon the half-close
policy, there might be pending receiving data in the closing
Just the complete the thread. Says "fatal() throws exception" is fine.
If this all only trying to make the compiler happy, you could just have
one "return null" at the bottom of the method. It is not necessary to
put a return after each fatal(). I will admit it could be less readable.
Or f
Hi,
Could I have the update reviewed?
http://cr.openjdk.java.net/~xuelei/8215443/webrev.00/
The TransportContext.fatal() methods always throw exception. While the
compiler does not aware of it, and may not happy without following a
return statement. Currently, a lot never executable return
Hi Jamil,
Thanks for the code review.
Okay, I will go ahead with the comment "fatal() always throws, make the
compiler happy" for this update, and file a new bug and see if we could
make an improvement in the future.
Thanks,
Xuelei
On 12/14/2018 4:16 PM, Jamil Nimeh wrote:
Hi Xuelei, thanks
Hi Xuelei, thanks for the clarification. If you want to keep the
structure as-is, then the comment "fatal() always throws, make the
compiler happy" is fine. I think that's a little more helpful to a
future maintainer who might be new to the code.
Thanks,
--Jamil
On 12/14/2018 1:56 PM, Xue-L
On 12/14/2018 1:46 PM, Xue-Lei Fan wrote:
Hi,
The purpose of combination of the two lines together:
shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, "...");
return null; // make the complier happy
is actually for the reading of the code. If one don't know the fatal()
always throw
Hi,
The purpose of combination of the two lines together:
shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, "...");
return null;// make the complier happy
is actually for the reading of the code. If one don't know the fatal()
always throw an exception (compiler is one of them), he ma
On 12/14/2018 12:26 PM, Anthony Scarpino wrote:
Other than my nit about the “make the compiler happy”, this all looks fine.
It makes sense to me. I will remove the comment while pushing.
For KeyUpdate, shouldn’t it never be null given the suite and protocol are
already known good? I have n
Other than my nit about the “make the compiler happy”, this all looks fine.
For KeyUpdate, shouldn’t it never be null given the suite and protocol are
already known good? I have not problem with the check to be cautious even if
it should never happen.
Tony
> On Dec 14, 2018, at 9:00 AM, Xue
My nit is I’d remove the “make compiler happy” comment. If this was something
for Xcomp, I’d understand, but this is standard return value requirement.
Otherwise I’m fine with this
Tony
> On Dec 14, 2018, at 11:49 AM, Jamil Nimeh wrote:
>
> Looks pretty good. I did have one question about a
Looks pretty good. I did have one question about a few of the methods
in KeyShareExtension and PreSharedKeyExtension, specifically where you
return nulls on error branches with the make-compiler-happy comments.
In those cases would it be a bit cleaner to set a byte[] variable to
null at the b
Hi,
Could I have the fix reviewed?
http://cr.openjdk.java.net/~xuelei/8213782/webrev.00/
The SSLCipher.createReadCipher() and createWriteCipher() could return
null if the cipher is not supported or the cipher is not available for a
certain protocol version. The caller should check the null
Hi,
Please review the update:
http://cr.openjdk.java.net/~xuelei/8214339/webrev.00/
In some cases, the SSLProtocolException or SSLHandshakeException may be
thrown if the underlying socket run into problems. An application may
depends on the exception class for further action, for example r
Hi,
Please review the TLS 1.3 half-close issue in JDK.
http://cr.openjdk.java.net/~xuelei/8209333/webrev.00/
While trying to duplex close a TLS connection upon the half-close
policy, there might be pending receiving data in the closing side, and
result in a TCP RST during closing. The TC
Looks good. My only question is whether the apiNote should be an
implNote instead since it refers to what the JDK Implementation does.
But either way seems ok.
--Sean
On 11/26/18 1:14 PM, Xue-Lei Fan wrote:
I made the update accordingly:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.0
On 11/26/18 4:14 PM, Xue-Lei Fan wrote:
Hi,
Please review this code cleanup in SSLCipher.java:
http://cr.openjdk.java.net/~xuelei/8214321/webrev.00/
The code should be fine as readCipherGenerators.length and
writeCipherGenerators.length are the same value in the implementation.
However, i
Hi,
Please review this code cleanup in SSLCipher.java:
http://cr.openjdk.java.net/~xuelei/8214321/webrev.00/
The code should be fine as readCipherGenerators.length and
writeCipherGenerators.length are the same value in the implementation.
However, it is misleading to use readCipherGenerator
I made the update accordingly:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.04/
Thanks,
Xuelei
On 11/19/2018 7:39 AM, Sean Mullan wrote:
On 11/16/18 3:19 PM, Xuelei Fan wrote:
Thanks for the review, Jmail & Sean.
New webrev:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.03/
On 11/16/18 3:19 PM, Xuelei Fan wrote:
Thanks for the review, Jmail & Sean.
New webrev:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.03/
I will update CSR when we come to an agreement.
On 11/16/2018 11:33 AM, Sean Mullan wrote:
122 * @apiNote Both the session timeout and cache
Thanks for the review, Jmail & Sean.
New webrev:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.03/
I will update CSR when we come to an agreement.
On 11/16/2018 11:33 AM, Sean Mullan wrote:
122 * @apiNote Both the session timeout and cache size impact
performance
123 *
122 * @apiNote Both the session timeout and cache size impact
performance
123 * of future connections. It is not recommended to
use too big
124 * or too small timeout or cache size limit.
Applications should
125 * carefully weigh the limits and
Hi Xuelei,
A little wordsmithing, nit picky stuff (sorry for not seeing this earlier):
* @apiNote for setSessionCacheSize: The sentence beginning, "It is not
recommended to use too big..." needs a slight grammatical change
o Suggestion: "It is recommended that applications tune their
It's time to use the systemProperty tag as it is ready.
As we are already there, I also update the setSessionCacheSize() for
more clarification.
Please review both CSR and webrev:
https://bugs.openjdk.java.net/browse/JDK-8213577
http://cr.openjdk.java.net/~xuelei/8210985/webrev.02/
Th
On 11/15/18 3:37 PM, Xuelei Fan wrote:
Hi Sean,
Are you OK if we do it later? I'm waiting for the @systemProperty tag,
proposed within JDK-5076751. I will file a bug to use the tag for more
cleanup.
JDK-5076751 is completed and pushed to JDK 12, so you can use the new
tag now.
I think i
Hi Sean,
Are you OK if we do it later? I'm waiting for the @systemProperty tag,
proposed within JDK-5076751. I will file a bug to use the tag for more
cleanup.
Thanks,
Xuelei
On 11/15/2018 11:55 AM, Sean Mullan wrote:
This is a good opportunity to document the
javax.net.ssl.sessionCacheSi
This is a good opportunity to document the
javax.net.ssl.sessionCacheSize system property in the SSLSessionContext
API (and use the new @systemProperty tag) in an @implNote, for example:
/**
* Returns the size of the cache used for storing
* SSLSession objects grouped under this
On 11/14/2018 9:16 AM, Jamil Nimeh wrote:
Hi Xuelei,
The fix looks fine to me. I think it would be good to have an else
branch off of the check on line 205 so any < 0 value has a warning log
entry stating that an invalid value was detected and the cache is
getting set to DEFAULT_MAX_CACHE_SI
Hi Xuelei,
The fix looks fine to me. I think it would be good to have an else
branch off of the check on line 205 so any < 0 value has a warning log
entry stating that an invalid value was detected and the cache is
getting set to DEFAULT_MAX_CACHE_SIZE.
--Jamil
On 11/14/2018 8:59 AM, Xuele
Hi,
Please review this simple update:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.00/
The default value for the maximum number of entries in the SSL session
cache (SSLSessionContext.getSessionCacheSize()) is infinite now. In the
request, the default value is updated to 20480 for per
Looks fine to me.
On 11/9/2018 8:43 AM, Xuelei Fan wrote:
Hi,
Could I get the following small change reviewed please?
http://cr.openjdk.java.net/~xuelei/8213694/webrev.00/
The test SSLSessionContextImpl/Timeout.java is running in the default
mode. As the test initializes the SSLContext wit
Hi,
Could I get the following small change reviewed please?
http://cr.openjdk.java.net/~xuelei/8213694/webrev.00/
The test SSLSessionContextImpl/Timeout.java is running in the default
mode. As the test initializes the SSLContext with the current System
Properties, while the SunJSSE provider
Looks good to me
Tony
On 10/29/2018 10:41 AM, Xuelei Fan wrote:
Hi,
Please review the update:
http://cr.openjdk.java.net/~xuelei/8212738/webrev.00/
The signature algorithm name should be ""ecdsa_secp521r1_sha512",
instead of "ecdsa_secp512r1_sha512".
No new regression test. Trivial
Hi,
Please review the update:
http://cr.openjdk.java.net/~xuelei/8212738/webrev.00/
The signature algorithm name should be ""ecdsa_secp521r1_sha512",
instead of "ecdsa_secp512r1_sha512".
No new regression test. Trivial update, no impact on the behaviors
except the debug log message.
Ditto.
Brad
On 9/20/2018 1:03 PM, Jamil Nimeh wrote:
Looks good.
On 9/20/2018 1:02 PM, Xuelei Fan wrote:
Hi,
Please review this simple fix for SunJSSE debug log:
http://cr.openjdk.java.net/~xuelei/8210974/webrev.00/
The debug log for ClientHello message does not appear in JDK 12.
Trivia
Looks good.
On 9/20/2018 1:02 PM, Xuelei Fan wrote:
Hi,
Please review this simple fix for SunJSSE debug log:
http://cr.openjdk.java.net/~xuelei/8210974/webrev.00/
The debug log for ClientHello message does not appear in JDK 12.
Trivial update, no new regression test.
Thanks,
Xuelei
Hi,
Please review this simple fix for SunJSSE debug log:
http://cr.openjdk.java.net/~xuelei/8210974/webrev.00/
The debug log for ClientHello message does not appear in JDK 12.
Trivial update, no new regression test.
Thanks,
Xuelei
Looks ok to me too.
Brad
On 9/11/2018 8:43 PM, Jamil Nimeh wrote:
Looks good to me.
Thanks,
--Jamil
On 9/11/2018 7:22 PM, Xuelei Fan wrote:
Hi Jamil,
Would you please review the fix for the NPE issue:
http://cr.openjdk.java.net/~xuelei/8209916/webrev.00/
The issue may happen if the cli
Hi,
does the fix work if there is only one unknown named group ?
Not that the connection fails than with an better error text instead of
skiping the unknown group.
Gruß Thomas
On 12.09.2018 04:22:49, Xuelei Fan wrote:
Hi Jamil,
Would you please review the fix for the NPE issue:
http://cr
Looks good to me.
Thanks,
--Jamil
On 9/11/2018 7:22 PM, Xuelei Fan wrote:
Hi Jamil,
Would you please review the fix for the NPE issue:
http://cr.openjdk.java.net/~xuelei/8209916/webrev.00/
The issue may happen if the client supports a SunJSSE provider known
but not supported named group.
Yes I will take a look at this tonight.
--Jamil
Original message From: Xuelei Fan
Date: 9/11/18 7:22 PM (GMT-08:00) To: security-dev@openjdk.java.net, Jamil
Nimeh Subject: Code Review Request, JDK-8209916 :
NPE in SupportedGroupsExtension
Hi Jamil,
Would you please
Hi Jamil,
Would you please review the fix for the NPE issue:
http://cr.openjdk.java.net/~xuelei/8209916/webrev.00/
The issue may happen if the client supports a SunJSSE provider known but
not supported named group.
Thanks,
Xuelei
Looks good.
Brad
On 9/5/2018 11:01 AM, Xuelei Fan wrote:
Hi,
Please review:
http://cr.openjdk.java.net/~xuelei/8210334/webrev.00/
Simple update, no new regression test.
Thanks,
Xuelei
Looks fine
> On Sep 5, 2018, at 11:01 AM, Xuelei Fan wrote:
>
> Hi,
>
> Please review:
>http://cr.openjdk.java.net/~xuelei/8210334/webrev.00/
>
> Simple update, no new regression test.
>
> Thanks,
> Xuelei
Hi,
Please review:
http://cr.openjdk.java.net/~xuelei/8210334/webrev.00/
Simple update, no new regression test.
Thanks,
Xuelei
Thanks for considering the idea
Tony
> On Aug 27, 2018, at 7:12 AM, Xuelei Fan wrote:
>
> Hi Tony,
>
> I thought about to limit the workaround to TLS 1.2 and prior version.
> However, just as you noticed that the implementation is not effective as it
> is needed to wait and check for the su
Hi Tony,
I thought about to limit the workaround to TLS 1.2 and prior version.
However, just as you noticed that the implementation is not effective as
it is needed to wait and check for the supported_versions extension if
it presents. As may make the workaround a lot complicated. I would
p
The change looks fine but I have a question about restricting it.
This sounds like a problem with servers using 1.2 or before, does it make sense
to throw an error for 1.3? I don’t like allowing buggy implementation to
continue because we will never be able to undo this workaround. It would be
Hi,
Please review a compatibility fix for SunJSSE provider:
http://cr.openjdk.java.net/~xuelei/8209965/webrev.00
There are servers that send the supported_groups extension in the
ServerHello handshake message. It does not comply to the specification.
However, as there are a few deploymen
Hi Brad,
Good points! Here is the updated webrev:
http://cr.openjdk.java.net/~xuelei/8207009/webrev.06/
Please let me know if you have more comments by 11:30AM today.
Thanks,
Xuelei
On 8/13/2018 4:43 PM, Bradford Wetmore wrote:
Hi Xuelei,
> Let's use two to emphasize the behaviors:
> 1
Hi Xuelei,
> Let's use two to emphasize the behaviors:
> 1. both input and output streams should be closed in each side, and
> 2. both client and server should perform #1.
SSLEngine.java
--
159: Both sides (i.e. the peer) may not be a SSLEngine:
both the client and server applicati
Hi Jamil,
Thanks for review. One more step close to the integration.
On 8/13/2018 11:45 AM, Jamil Nimeh wrote:
Hi Xuelei,
* SSLSocket.java
o 134: Nit - You can remove the first "both" in this sentence
since you use it later with the input/output stream closure.
Let's use two
Hi Xuelei,
* SSLSocket.java
o 134: Nit - You can remove the first "both" in this sentence
since you use it later with the input/output stream closure.
Looks good to me otherwise.
--Jamil
On 8/13/2018 11:31 AM, Xue-Lei Fan wrote:
One more update:
http://cr.openjdk.java.net/~xuel
One more update:
http://cr.openjdk.java.net/~xuelei/8207009/webrev.05/
It is desired to make a note in SSLSocket and SSLEngine specification,
so that users have a good sense that an application should close the
input and output stream always.
Updated for SSLEngine.java and SSLSocket.java on
I'm good with the changes.
--Jamil
On 8/7/2018 5:24 PM, Xuelei Fan wrote:
Hi Jamil,
Thanks for comments. Here is the updated webrev:
http://cr.openjdk.java.net/~xuelei/8207009/webrev.04/
Thanks,
Xuelei
On 8/7/2018 3:12 PM, Jamil Nimeh wrote:
Hi Xuelei, mostly small stuff:
* SSLEngine
I'm waiting for the CSR approval for JDK 11:
https://bugs.openjdk.java.net/browse/JDK-8208526
Thanks,
Xuelei
On 8/9/2018 6:50 AM, Norman Maurer wrote:
I there any idea when the patch will be merged and a release will be cut so we
can enable testing with JDK11 again for netty ?
On 31. Jul 20
I there any idea when the patch will be merged and a release will be cut so we
can enable testing with JDK11 again for netty ?
> On 31. Jul 2018, at 07:19, Norman Maurer wrote:
>
> After digging more this morning I noticed the test code did made some wrong
> assumptions which just worked out
Hi Jamil,
Thanks for comments. Here is the updated webrev:
http://cr.openjdk.java.net/~xuelei/8207009/webrev.04/
Thanks,
Xuelei
On 8/7/2018 3:12 PM, Jamil Nimeh wrote:
Hi Xuelei, mostly small stuff:
* SSLEngineImpl.java
o 717: Nit, inbout --> inbound
* SSLEngineOutputRecord.java
Hi Xuelei, mostly small stuff:
* SSLEngineImpl.java
o 717: Nit, inbout --> inbound
* SSLEngineOutputRecord.java
o 162, 169: Nit, applicatoin --> application
o Same section: It looks like the "if" and "else if" clauses take
the same actions with the same message. Maybe jus
New webrev:
http://cr.openjdk.java.net/~xuelei/8207009/webrev.03/
Thanks for a find of Tim Brooks, that the SSLEngine inbound/outbound
status is incorrect if closing during handshake. The above webrev is
trying to fix the problems. See more in the OpenJDK thread:
http://mail.openjdk.java.net
Update: http://cr.openjdk.java.net/~xuelei/8207009/webrev.02/
In webrev.01, the socket close may be blocked by super class close
synchronization. Updated the SSLSocketImpl.java to use handshake only
lock in the startHandshake() implementation.
Thanks,
Xuelei
On 8/1/2018 7:27 PM, Xuelei Fan
Update: http://cr.openjdk.java.net/~xuelei/8207009/webrev.01/
Integrated the fix for JDK-8208642, "Server initiated TLSv1.2
renegotiation fails if Java client allows TLSv1.3". SSLHandshake.java
is updated to use negotiated version so that TLS 1.2 HelloRequest is
acceptable in TLS 1.3 client s
After digging more this morning I noticed the test code did made some wrong
assumptions which just worked out of luck before. After fixing the test
everything passes now.
So +1 from me on the patch :)
Also sorry for the false-alarm.
Niorman
> On 30. Jul 2018, at 22:23, Xuelei Fan wrote:
>
Would you mind send me the debug log (-Djavax.net.debug=all) and the
exception stacks? The "renegotiation" in TLS 1.3 is different from TLS
1.2 and prior specifications. It would be helpful to me to find the
cause of the test failure.
Thanks,
Xuelei
On 7/30/2018 1:11 PM, Norman Maurer wrote
1 - 100 of 2460 matches
Mail list logo
