[teststabilization] RFR: 8225430: Replace wildcard address with loopback or local host in tests - part 14

[Aleks Efimov]

Hi Daniel,
HandleContentTypeWithAttrs correction - removed bind and used three-args 
constructor instead - looks much nicer.
The webrev location is same: http://cr.openjdk.java.net/~aefimov/8225430/01

Best Regards,
Aleksei

On 07/08/2019 17:09, Aleks Efimov wrote:
> Hi Daniel,
> Thank you for the review. The replies are in-lined below. The new 
> version of the webrev can be found here:
> http://cr.openjdk.java.net/~aefimov/8225430/01
>
> With Best Regards,
> Aleksei
>
>
> On 06/08/2019 18:09, Daniel Fuchs wrote:
>> Hi Aleksei,
>>
>> Thanks for taking care of these tests!
>>
>> http://cr.openjdk.java.net/~aefimov/8225430/00/test/jdk/java/net/Socket/NullHost.java 
>>
>>
>>   39             svr.bind(new 
>> InetSocketAddress(InetAddress.getLoopbackAddress(), 0));
>>
>>   maybe it would be worth adding a comment just before this line:
>>
>>   // The client side calls Socket((String) null, ...) which
>>   // resolves to InetAddress.getByName((String)null) which in
>>   // turns will resolve to the loopback address.
> Thanks - comment added.
>
>>
>> HandleContentTypeWithAttrs.java:
>>
>>  Since the original test had a backlog of 50:
>>  138         serverSocket = new ServerSocket(port, 50);
>>  maybe we should preserve it, and uses the ServerSocket constructor
>>  that takes a backlog number in the new code.
> I've removed 50 value because the default value in ServerSocket is 
> also 50. I do not see this value documented in javadoc [1] and it 
> could be changed in future, so let me return it back to the test code.
> But I couldn't see how two-integers ServerSocket constructor can be 
> used here since it will bind the socket address to 
> InetAddress.anyLocalAddress():
>     var ss = new ServerSocket(12345,50);
>     ss.isBound() returns true
>     ss.getLocalSocketAddress() returns 0.0.0.0/0.0.0.0:12345
>
> Therefore I've added 50 backlog value to the ServerSocket::bind call
>
> [1] 
> https://docs.oracle.com/en/java/javase/12/docs/api/java.base/java/net/ServerSocket.html
>>
>> best regards,
>>
>> -- daniel
>>
>> PS: RedirectLimit.java
>>
>> One trick that's been used before - and that might be of interest
>> here if this test is observed failing again, is to make the server
>> side ignore any request that obviously don't come from the
>> test itself. One way of doing that could be e.g to ignore
>> (closing the accepted socket and doing as if nothing had been
>> received) a request whose URI path doesn't match what the
>> client is supposed to send, and possibly make that URI path
>> sufficiently unique so that the test is the only one to use it.
>>
>> I'm not suggesting we should do this here, but something to keep
>> in mind if tests are observed failing due to rogue connections
>> from other processes on the machine.
>
> Thanks I will keep that trick in mind and apply it if test fails again.
>
>>
>>
>> On 06/08/2019 15:35, Aleks Efimov wrote:
>>> Hi,
>>>
>>> Please help to review few test fixes which address intermittent 
>>> networking failures:
>>> http://cr.openjdk.java.net/~aefimov/8225430/00
>>>
>>> JBS:
>>> https://bugs.openjdk.java.net/browse/JDK-8225430
>>>
>>> The following tests have been marked with @intermittent keyword:
>>> java/net/DatagramSocket/ReuseAddressTest.java
>>> java/net/ServerSocket/AcceptCauseFileDescriptorLeak.java
>>>
>>> With Best Regards,
>>> Aleksei
>>
>