[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 <xuelei.fan at oracle.com> 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-dev at 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