RFR 8156504/9, java/net/URLPermission/nstest/lookup.sh fails intermittently

[Felix Yang]

Daniel and Chris,

     thanks for the comment, I will push the updated version.

Thanks,
Felix
On 2016/11/2 22:12, Daniel Fuchs wrote:
> On 02/11/16 11:18, Chris Hegarty wrote:
>> Thanks for doing this Felix.
>>
>> I have some comments about the style and the use of 
>> AccessControlContext.
>> Rather than list them I put an alternative version at the following 
>> location, please
>> take a look.
>>
>>   http://cr.openjdk.java.net/~chegar/8156504_alt/
>>
>> Note:  specifically it is good practice to read off the HTTP request 
>> before
>> closing the socket ( on the server socket ). Not doing so can lead to 
>> reset
>> issues.
>
> + 1 for getting rid of the AccessControlContext.
> I was not very comfortable with that either.
>
> cheers,
>
> -- daniel
>
>>
>> -Chris.
>>
>>> On 2 Nov 2016, at 07:10, Felix Yang <felix.yang at oracle.com> wrote:
>>>
>>> Hi Amy,
>>>
>>>    thanks for the comments. Updated webrev:
>>>
>>> http://cr.openjdk.java.net/~xiaofeya/8156504/webrev.01/
>>>
>>> -Felix
>>> On 2016/11/2 14:18, Amy Lu wrote:
>>>> Good to see one more script test be changed to java, thank you Felix.
>>>>
>>>> I'm not official reviewer, but some minor comments.
>>>>
>>>>  30  * @library /lib/testlibrary
>>>>  31  * @build jdk.testlibrary.*
>>>>  32  * @compile LookupTest.java
>>>> I noticed test requires testlibrary, but seems test do not actually 
>>>> depend on that lib, these lines could be removed.
>>>>
>>>>  77                 is.close();
>>>> Is it better to do the close in the finally block?
>>>>
>>>>  128             if (serverSocket == null) {
>>>>  129                 serverSocket.close();
>>>>  130             }
>>>> Typo here?
>>>>
>>>> Even more minor...
>>>> -    private static void addMappingToHostsFile (String host,
>>>> -                                               String addr,
>>>> -                                               String hostsFileName,
>>>> -                                               boolean append)
>>>> -                                             throws Exception {
>>>> +    private static void addMappingToHostsFile(String host, String 
>>>> addr,
>>>> +            String hostsFileName, boolean append) throws Exception {
>>>> This might be reformatted automatically by IDE, but just feel 
>>>> previous one is more easy to read.
>>>>
>>>> Thanks,
>>>> Amy
>>>>
>>>>
>>>> On 11/2/16 10:39 AM, Felix Yang wrote:
>>>>> Hi there,
>>>>>
>>>>>    please review the following patch for an intermittent failing 
>>>>> test. Converted it into plain java test and avoid free port 
>>>>> anti-pattern.
>>>>>
>>>>> Bug:
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8156504
>>>>>
>>>>> Webrev:
>>>>>
>>>>> http://cr.openjdk.java.net/~xiaofeya/8156504/webrev.00/
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Felix
>>>>>
>>>>
>>>
>>
>