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

Daniel Fuchs daniel.fuchs at oracle.com
Wed Nov 2 14:12:26 UTC 2016


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
>>>>
>>>
>>
>



More information about the net-dev mailing list