Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

[Artem Smotrakov]

Hello,

If you don't mind, I moved some common code from SSLSocketTemplate.java 
to SSLTest.java, so that it can be re-used by other tests. It may help 
to avoid duplicate code.


http://cr.openjdk.java.net/~asmotrak/8164591/webrev.01/

Please take a look.

Artem


On 09/15/2016 11:49 AM, Artem Smotrakov wrote:

Hi Xuelei, Chris,

Thank you for looking into it. Please see inline.


On 09/15/2016 12:53 AM, Chris Hegarty wrote:

On 15 Sep 2016, at 02:55, Xuelei Fan  wrote:

On 9/15/2016 9:45 AM, Artem Smotrakov wrote:
Well, in this particular case it's not clear that it has the same 
issue
with free port (at least to me). The exception occurred on client 
side,
so it's not the case where we don't know where the handshake came 
from.


;-) Yeh, you catch the point.  But there is a free-port issue 
although the exception stack in the bug description may be not that 
case.


Let's look at a scenarios:
1. server open a server socket and listen.
2. other test case connect to the server socket.
3. this test case try to connect to server socket.
4. this test case would fail as the server only accept one connections.

I did not check it very carefully, I think for #4, the exception 
stack can be similar to the one in the bug description.

Looks like a rare, but valid case.


Anyway, as a free port is used, there are free-port issues. Please 
consider to make the enhancement in the fix. Otherwise, you cannot 
avoid the intermittent failure for this test case in the current 
testing environment.

+1.   Please remove any use of the free-port anti-pattern.
Just to be clear, this is not an issue with getting a free port with 
ServerSocket.getLocalPort() and them re-using it to create a new 
ServerSocket. It's more tricky (for example, please see the scenario 
above).


Okay, let me update it to follow the approach which is implemented in 
SSLSocketSample.java


Artem


-Chris.


Xuelei

I can make this enhancement, but like I said I don't think it's 
going to

help, so I would like to keep debug output on.

Artem


On 09/14/2016 06:39 PM, Xuelei Fan wrote:

On 9/15/2016 9:23 AM, Artem Smotrakov wrote:

Hi Xuelei,

For this one, I am not sure that it would help here since the test
failed after it already had started handshaking.

It has the same issue as a free-port is used.  We don't actually know
the handshake is coming from the right client.

Xuelei


I would prefer to have it as a separate enhancement.

Artem


On 09/14/2016 06:19 PM, Xuelei Fan wrote:

As you were already there, I would suggest to consider the
SSLSocketSample.java template as the comment in JDK-8163924 review
thread.

Thanks,
Xuelei

On 9/15/2016 9:13 AM, Artem Smotrakov wrote:
Not urgent, but I would appreciate if someone can get a chance 
to look

at this.

Artem


On 09/07/2016 03:17 PM, Artem Smotrakov wrote:

Sending to net-...@openjdk.java.net as well.

Artem


On 09/07/2016 12:28 PM, Artem Smotrakov wrote:

Hello,

Please review the following patch for
sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java

The test has been observed to fail a couple of times, but 
it's still
not clear why it failed because there is not much info in 
logs. The
patch updates the test to enable additional debug output, so 
that we

have more info if it fails next time.

While looking at the test, I notices a couple of issues, but 
they

don't seem to cause these intermittent failures:
- The test sets system properties for JSSE in a loop, but JSSE
provider reads them only once while initialization. As a result,
only
values which were set in the first iteration are actually used.
- The test doesn't close files and sockets sometimes.

The patch also fixed the issues above, and there are a couple
cosmetic changes.

Bug: https://bugs.openjdk.java.net/browse/JDK-8164591
Webrev: http://cr.openjdk.java.net/~asmotrak/8164591/webrev.00/

Artem

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

[Artem Smotrakov]

Hi Xuelei, Chris,

Thank you for looking into it. Please see inline.


On 09/15/2016 12:53 AM, Chris Hegarty wrote:

On 15 Sep 2016, at 02:55, Xuelei Fan  wrote:

On 9/15/2016 9:45 AM, Artem Smotrakov wrote:

Well, in this particular case it's not clear that it has the same issue
with free port (at least to me). The exception occurred on client side,
so it's not the case where we don't know where the handshake came from.


;-) Yeh, you catch the point.  But there is a free-port issue although the 
exception stack in the bug description may be not that case.

Let's look at a scenarios:
1. server open a server socket and listen.
2. other test case connect to the server socket.
3. this test case try to connect to server socket.
4. this test case would fail as the server only accept one connections.

I did not check it very carefully, I think for #4, the exception stack can be 
similar to the one in the bug description.

Looks like a rare, but valid case.


Anyway, as a free port is used, there are free-port issues.  Please consider to 
make the enhancement in the fix.  Otherwise, you cannot avoid the intermittent 
failure for this test case in the current testing environment.

+1.   Please remove any use of the free-port anti-pattern.
Just to be clear, this is not an issue with getting a free port with 
ServerSocket.getLocalPort() and them re-using it to create a new 
ServerSocket. It's more tricky (for example, please see the scenario above).


Okay, let me update it to follow the approach which is implemented in 
SSLSocketSample.java


Artem


-Chris.


Xuelei


I can make this enhancement, but like I said I don't think it's going to
help, so I would like to keep debug output on.

Artem


On 09/14/2016 06:39 PM, Xuelei Fan wrote:

On 9/15/2016 9:23 AM, Artem Smotrakov wrote:

Hi Xuelei,

For this one, I am not sure that it would help here since the test
failed after it already had started handshaking.

It has the same issue as a free-port is used.  We don't actually know
the handshake is coming from the right client.

Xuelei


I would prefer to have it as a separate enhancement.

Artem


On 09/14/2016 06:19 PM, Xuelei Fan wrote:

As you were already there, I would suggest to consider the
SSLSocketSample.java template as the comment in JDK-8163924 review
thread.

Thanks,
Xuelei

On 9/15/2016 9:13 AM, Artem Smotrakov wrote:

Not urgent, but I would appreciate if someone can get a chance to look
at this.

Artem


On 09/07/2016 03:17 PM, Artem Smotrakov wrote:

Sending to net-...@openjdk.java.net as well.

Artem


On 09/07/2016 12:28 PM, Artem Smotrakov wrote:

Hello,

Please review the following patch for
sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java

The test has been observed to fail a couple of times, but it's still
not clear why it failed because there is not much info in logs. The
patch updates the test to enable additional debug output, so that we
have more info if it fails next time.

While looking at the test, I notices a couple of issues, but they
don't seem to cause these intermittent failures:
- The test sets system properties for JSSE in a loop, but JSSE
provider reads them only once while initialization. As a result,
only
values which were set in the first iteration are actually used.
- The test doesn't close files and sockets sometimes.

The patch also fixed the issues above, and there are a couple
cosmetic changes.

Bug: https://bugs.openjdk.java.net/browse/JDK-8164591
Webrev: http://cr.openjdk.java.net/~asmotrak/8164591/webrev.00/

Artem

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

[Daniel Fuchs]

On 15/09/16 08:53, Chris Hegarty wrote:

Anyway, as a free port is used, there are free-port issues.  Please consider to 
make the enhancement in the fix.  Otherwise, you cannot avoid the intermittent 
failure for this test case in the current testing environment.

+1.   Please remove any use of the free-port anti-pattern.



Hi guys,

What is the issue of opening a server socket on port 0?
This is what the test does, I thought that was the recommended
way.

best regards,

-- daniel


Bug: https://bugs.openjdk.java.net/browse/JDK-8164591
Webrev: http://cr.openjdk.java.net/~asmotrak/8164591/webrev.00/

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

[Chris Hegarty]

On 15 Sep 2016, at 02:55, Xuelei Fan  wrote:
> 
> On 9/15/2016 9:45 AM, Artem Smotrakov wrote:
>> Well, in this particular case it's not clear that it has the same issue
>> with free port (at least to me). The exception occurred on client side,
>> so it's not the case where we don't know where the handshake came from.
>> 
> ;-) Yeh, you catch the point.  But there is a free-port issue although the 
> exception stack in the bug description may be not that case.
> 
> Let's look at a scenarios:
> 1. server open a server socket and listen.
> 2. other test case connect to the server socket.
> 3. this test case try to connect to server socket.
> 4. this test case would fail as the server only accept one connections.
> 
> I did not check it very carefully, I think for #4, the exception stack can be 
> similar to the one in the bug description.
> 
> Anyway, as a free port is used, there are free-port issues.  Please consider 
> to make the enhancement in the fix.  Otherwise, you cannot avoid the 
> intermittent failure for this test case in the current testing environment.

+1.   Please remove any use of the free-port anti-pattern.

-Chris.

> Xuelei
> 
>> I can make this enhancement, but like I said I don't think it's going to
>> help, so I would like to keep debug output on.
>> 
>> Artem
>> 
>> 
>> On 09/14/2016 06:39 PM, Xuelei Fan wrote:
>>> On 9/15/2016 9:23 AM, Artem Smotrakov wrote:
 Hi Xuelei,
 
 For this one, I am not sure that it would help here since the test
 failed after it already had started handshaking.
>>> It has the same issue as a free-port is used.  We don't actually know
>>> the handshake is coming from the right client.
>>> 
>>> Xuelei
>>> 
 I would prefer to have it as a separate enhancement.
 
 Artem
 
 
 On 09/14/2016 06:19 PM, Xuelei Fan wrote:
> As you were already there, I would suggest to consider the
> SSLSocketSample.java template as the comment in JDK-8163924 review
> thread.
> 
> Thanks,
> Xuelei
> 
> On 9/15/2016 9:13 AM, Artem Smotrakov wrote:
>> Not urgent, but I would appreciate if someone can get a chance to look
>> at this.
>> 
>> Artem
>> 
>> 
>> On 09/07/2016 03:17 PM, Artem Smotrakov wrote:
>>> Sending to net-...@openjdk.java.net as well.
>>> 
>>> Artem
>>> 
>>> 
>>> On 09/07/2016 12:28 PM, Artem Smotrakov wrote:
 Hello,
 
 Please review the following patch for
 sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java
 
 The test has been observed to fail a couple of times, but it's still
 not clear why it failed because there is not much info in logs. The
 patch updates the test to enable additional debug output, so that we
 have more info if it fails next time.
 
 While looking at the test, I notices a couple of issues, but they
 don't seem to cause these intermittent failures:
 - The test sets system properties for JSSE in a loop, but JSSE
 provider reads them only once while initialization. As a result,
 only
 values which were set in the first iteration are actually used.
 - The test doesn't close files and sockets sometimes.
 
 The patch also fixed the issues above, and there are a couple
 cosmetic changes.
 
 Bug: https://bugs.openjdk.java.net/browse/JDK-8164591
 Webrev: http://cr.openjdk.java.net/~asmotrak/8164591/webrev.00/
 
 Artem
>>> 
>> 
 
>> 

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

[Xuelei Fan]

On 9/15/2016 9:45 AM, Artem Smotrakov wrote:

Well, in this particular case it's not clear that it has the same issue
with free port (at least to me). The exception occurred on client side,
so it's not the case where we don't know where the handshake came from.

;-) Yeh, you catch the point.  But there is a free-port issue although 
the exception stack in the bug description may be not that case.


Let's look at a scenarios:
1. server open a server socket and listen.
2. other test case connect to the server socket.
3. this test case try to connect to server socket.
4. this test case would fail as the server only accept one connections.

I did not check it very carefully, I think for #4, the exception stack 
can be similar to the one in the bug description.


Anyway, as a free port is used, there are free-port issues.  Please 
consider to make the enhancement in the fix.  Otherwise, you cannot 
avoid the intermittent failure for this test case in the current testing 
environment.


Xuelei


I can make this enhancement, but like I said I don't think it's going to
help, so I would like to keep debug output on.

Artem


On 09/14/2016 06:39 PM, Xuelei Fan wrote:

On 9/15/2016 9:23 AM, Artem Smotrakov wrote:

Hi Xuelei,

For this one, I am not sure that it would help here since the test
failed after it already had started handshaking.

It has the same issue as a free-port is used.  We don't actually know
the handshake is coming from the right client.

Xuelei


I would prefer to have it as a separate enhancement.

Artem


On 09/14/2016 06:19 PM, Xuelei Fan wrote:

As you were already there, I would suggest to consider the
SSLSocketSample.java template as the comment in JDK-8163924 review
thread.

Thanks,
Xuelei

On 9/15/2016 9:13 AM, Artem Smotrakov wrote:

Not urgent, but I would appreciate if someone can get a chance to look
at this.

Artem


On 09/07/2016 03:17 PM, Artem Smotrakov wrote:

Sending to net-...@openjdk.java.net as well.

Artem


On 09/07/2016 12:28 PM, Artem Smotrakov wrote:

Hello,

Please review the following patch for
sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java

The test has been observed to fail a couple of times, but it's still
not clear why it failed because there is not much info in logs. The
patch updates the test to enable additional debug output, so that we
have more info if it fails next time.

While looking at the test, I notices a couple of issues, but they
don't seem to cause these intermittent failures:
- The test sets system properties for JSSE in a loop, but JSSE
provider reads them only once while initialization. As a result,
only
values which were set in the first iteration are actually used.
- The test doesn't close files and sockets sometimes.

The patch also fixed the issues above, and there are a couple
cosmetic changes.

Bug: https://bugs.openjdk.java.net/browse/JDK-8164591
Webrev: http://cr.openjdk.java.net/~asmotrak/8164591/webrev.00/

Artem

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

[Artem Smotrakov]

Well, in this particular case it's not clear that it has the same issue 
with free port (at least to me). The exception occurred on client side, 
so it's not the case where we don't know where the handshake came from.


I can make this enhancement, but like I said I don't think it's going to 
help, so I would like to keep debug output on.


Artem


On 09/14/2016 06:39 PM, Xuelei Fan wrote:

On 9/15/2016 9:23 AM, Artem Smotrakov wrote:

Hi Xuelei,

For this one, I am not sure that it would help here since the test
failed after it already had started handshaking.
It has the same issue as a free-port is used.  We don't actually know 
the handshake is coming from the right client.


Xuelei


I would prefer to have it as a separate enhancement.

Artem


On 09/14/2016 06:19 PM, Xuelei Fan wrote:

As you were already there, I would suggest to consider the
SSLSocketSample.java template as the comment in JDK-8163924 review
thread.

Thanks,
Xuelei

On 9/15/2016 9:13 AM, Artem Smotrakov wrote:

Not urgent, but I would appreciate if someone can get a chance to look
at this.

Artem


On 09/07/2016 03:17 PM, Artem Smotrakov wrote:

Sending to net-...@openjdk.java.net as well.

Artem


On 09/07/2016 12:28 PM, Artem Smotrakov wrote:

Hello,

Please review the following patch for
sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java

The test has been observed to fail a couple of times, but it's still
not clear why it failed because there is not much info in logs. The
patch updates the test to enable additional debug output, so that we
have more info if it fails next time.

While looking at the test, I notices a couple of issues, but they
don't seem to cause these intermittent failures:
- The test sets system properties for JSSE in a loop, but JSSE
provider reads them only once while initialization. As a result, 
only

values which were set in the first iteration are actually used.
- The test doesn't close files and sockets sometimes.

The patch also fixed the issues above, and there are a couple
cosmetic changes.

Bug: https://bugs.openjdk.java.net/browse/JDK-8164591
Webrev: http://cr.openjdk.java.net/~asmotrak/8164591/webrev.00/

Artem

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

[Xuelei Fan]

On 9/15/2016 9:23 AM, Artem Smotrakov wrote:

Hi Xuelei,

For this one, I am not sure that it would help here since the test
failed after it already had started handshaking.
It has the same issue as a free-port is used.  We don't actually know 
the handshake is coming from the right client.


Xuelei


I would prefer to have it as a separate enhancement.

Artem


On 09/14/2016 06:19 PM, Xuelei Fan wrote:

As you were already there, I would suggest to consider the
SSLSocketSample.java template as the comment in JDK-8163924 review
thread.

Thanks,
Xuelei

On 9/15/2016 9:13 AM, Artem Smotrakov wrote:

Not urgent, but I would appreciate if someone can get a chance to look
at this.

Artem


On 09/07/2016 03:17 PM, Artem Smotrakov wrote:

Sending to net-...@openjdk.java.net as well.

Artem


On 09/07/2016 12:28 PM, Artem Smotrakov wrote:

Hello,

Please review the following patch for
sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java

The test has been observed to fail a couple of times, but it's still
not clear why it failed because there is not much info in logs. The
patch updates the test to enable additional debug output, so that we
have more info if it fails next time.

While looking at the test, I notices a couple of issues, but they
don't seem to cause these intermittent failures:
- The test sets system properties for JSSE in a loop, but JSSE
provider reads them only once while initialization. As a result, only
values which were set in the first iteration are actually used.
- The test doesn't close files and sockets sometimes.

The patch also fixed the issues above, and there are a couple
cosmetic changes.

Bug: https://bugs.openjdk.java.net/browse/JDK-8164591
Webrev: http://cr.openjdk.java.net/~asmotrak/8164591/webrev.00/

Artem

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

[Artem Smotrakov]

Hi Xuelei,

For this one, I am not sure that it would help here since the test 
failed after it already had started handshaking. I would prefer to have 
it as a separate enhancement.


Artem


On 09/14/2016 06:19 PM, Xuelei Fan wrote:
As you were already there, I would suggest to consider the 
SSLSocketSample.java template as the comment in JDK-8163924 review 
thread.


Thanks,
Xuelei

On 9/15/2016 9:13 AM, Artem Smotrakov wrote:

Not urgent, but I would appreciate if someone can get a chance to look
at this.

Artem


On 09/07/2016 03:17 PM, Artem Smotrakov wrote:

Sending to net-...@openjdk.java.net as well.

Artem


On 09/07/2016 12:28 PM, Artem Smotrakov wrote:

Hello,

Please review the following patch for
sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java

The test has been observed to fail a couple of times, but it's still
not clear why it failed because there is not much info in logs. The
patch updates the test to enable additional debug output, so that we
have more info if it fails next time.

While looking at the test, I notices a couple of issues, but they
don't seem to cause these intermittent failures:
- The test sets system properties for JSSE in a loop, but JSSE
provider reads them only once while initialization. As a result, only
values which were set in the first iteration are actually used.
- The test doesn't close files and sockets sometimes.

The patch also fixed the issues above, and there are a couple
cosmetic changes.

Bug: https://bugs.openjdk.java.net/browse/JDK-8164591
Webrev: http://cr.openjdk.java.net/~asmotrak/8164591/webrev.00/

Artem

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

[Xuelei Fan]

As you were already there, I would suggest to consider the 
SSLSocketSample.java template as the comment in JDK-8163924 review thread.


Thanks,
Xuelei

On 9/15/2016 9:13 AM, Artem Smotrakov wrote:

Not urgent, but I would appreciate if someone can get a chance to look
at this.

Artem


On 09/07/2016 03:17 PM, Artem Smotrakov wrote:

Sending to net-...@openjdk.java.net as well.

Artem


On 09/07/2016 12:28 PM, Artem Smotrakov wrote:

Hello,

Please review the following patch for
sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java

The test has been observed to fail a couple of times, but it's still
not clear why it failed because there is not much info in logs. The
patch updates the test to enable additional debug output, so that we
have more info if it fails next time.

While looking at the test, I notices a couple of issues, but they
don't seem to cause these intermittent failures:
- The test sets system properties for JSSE in a loop, but JSSE
provider reads them only once while initialization. As a result, only
values which were set in the first iteration are actually used.
- The test doesn't close files and sockets sometimes.

The patch also fixed the issues above, and there are a couple
cosmetic changes.

Bug: https://bugs.openjdk.java.net/browse/JDK-8164591
Webrev: http://cr.openjdk.java.net/~asmotrak/8164591/webrev.00/

Artem

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

[Artem Smotrakov]

Not urgent, but I would appreciate if someone can get a chance to look 
at this.


Artem


On 09/07/2016 03:17 PM, Artem Smotrakov wrote:

Sending to net-...@openjdk.java.net as well.

Artem


On 09/07/2016 12:28 PM, Artem Smotrakov wrote:

Hello,

Please review the following patch for 
sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java


The test has been observed to fail a couple of times, but it's still 
not clear why it failed because there is not much info in logs. The 
patch updates the test to enable additional debug output, so that we 
have more info if it fails next time.


While looking at the test, I notices a couple of issues, but they 
don't seem to cause these intermittent failures:
- The test sets system properties for JSSE in a loop, but JSSE 
provider reads them only once while initialization. As a result, only 
values which were set in the first iteration are actually used.

- The test doesn't close files and sockets sometimes.

The patch also fixed the issues above, and there are a couple 
cosmetic changes.


Bug: https://bugs.openjdk.java.net/browse/JDK-8164591
Webrev: http://cr.openjdk.java.net/~asmotrak/8164591/webrev.00/

Artem

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

[Artem Smotrakov]

Sending to net-...@openjdk.java.net as well.

Artem


On 09/07/2016 12:28 PM, Artem Smotrakov wrote:

Hello,

Please review the following patch for 
sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java


The test has been observed to fail a couple of times, but it's still 
not clear why it failed because there is not much info in logs. The 
patch updates the test to enable additional debug output, so that we 
have more info if it fails next time.


While looking at the test, I notices a couple of issues, but they 
don't seem to cause these intermittent failures:
- The test sets system properties for JSSE in a loop, but JSSE 
provider reads them only once while initialization. As a result, only 
values which were set in the first iteration are actually used.

- The test doesn't close files and sockets sometimes.

The patch also fixed the issues above, and there are a couple cosmetic 
changes.


Bug: https://bugs.openjdk.java.net/browse/JDK-8164591
Webrev: http://cr.openjdk.java.net/~asmotrak/8164591/webrev.00/

Artem