RFR 8071660 :URLPermission not handling empty method lists correctly

[Roger Riggs]

Hi Vyom,

URLPermissionTest.java:

line 125:  it looks odd to assign the same variable twice with the same 
value.
    Perhaps it should have been assigning this.url2 = url2.

line 330-332: please add a space after "," in argument lists.

Roger




On 6/17/2016 8:42 AM, Daniel Fuchs wrote:
> On 17/06/16 13:25, Vyom Tewari wrote:
>> Hi Daniel,
>>
>> thanks for review please find the latest
>> webrev(http://cr.openjdk.java.net/~vtewari/8071660/webrev0.2/index.html
>> <http://cr.openjdk.java.net/%7Evtewari/8071660/webrev0.2/index.html>) .
>> I added some more tests where base url is different.
>
> Thanks Vyom,
> Looks good to me.
>
> See if you can get someone more experienced in the area
> to give a thumb up too :-)
>
> best regards,
>
> -- daniel
>
>
>>
>> Thanks,
>> Vyom
>>
>> On Friday 17 June 2016 04:57 PM, Daniel Fuchs wrote:
>>> On 17/06/16 09:21, Vyom Tewari wrote:
>>>> Hi All,
>>>>
>>>> Please find the new
>>>> webrev(http://cr.openjdk.java.net/~vtewari/8071660/webrev0.1/index.html 
>>>>
>>>> <http://cr.openjdk.java.net/%7Evtewari/8071660/webrev0.1/index.html>).
>>>> I  addressed the review comments given by Daniel.
>>>
>>> Hi Vyom,
>>>
>>> Looks good to me - but I'm a bit concerned that the previous
>>> mistake was not caught that by any test.
>>> Could you add a test that fails with you previous fix
>>> but passes with the new one?
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>>>
>>>> Thanks,
>>>> Vyom
>>>>
>>>>
>>>> On Sunday 12 June 2016 02:04 PM, Daniel Fuchs wrote:
>>>>> Hi Vyom,
>>>>>
>>>>> This looks strange to me:
>>>>>
>>>>>  268         if(!this.methods.isEmpty() && that.methods.isEmpty())
>>>>>  269             return true;
>>>>>  270         if(this.methods.isEmpty() && !that.methods.isEmpty())
>>>>>  271             return false;
>>>>>  272         if(this.methods.isEmpty() && that.methods.isEmpty())
>>>>>  273             return true;
>>>>>
>>>>> Namely, lines 269 & 273 will return true before the URL part
>>>>> of the permission has been checked.
>>>>> Is that really the expected behavior?
>>>>>
>>>>> best regards,
>>>>>
>>>>> -- daniel
>>>>>
>>>>> On 11/06/16 05:50, vyom wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> Please review the below fix.
>>>>>>
>>>>>> Bug       :         JDK-8071660 URLPermission not handling empty
>>>>>> method
>>>>>> lists correctly
>>>>>> Webrev :
>>>>>> http://cr.openjdk.java.net/~vtewari/8071660/webrev0.0/index.html
>>>>>> <http://cr.openjdk.java.net/%7Evtewari/8071660/webrev0.0/index.html>
>>>>>>
>>>>>> Thanks,
>>>>>> Vyom
>>>>>
>>>>
>>>
>>
>