RFR 8071660 :URLPermission not handling empty method lists correctly

[Chris Hegarty]

On 17/06/16 15:09, Vyom Tewari wrote:
> Hi,
>
> Please find the latest
> webrev(http://cr.openjdk.java.net/~vtewari/8071660/webrev0.3/index.html
> <http://cr.openjdk.java.net/%7Evtewari/8071660/webrev0.3/index.html>). I
> fixed the typo along with spaces issue.

Thanks Vyom.  Looks good.

-Chris.

> Thanks,
> Vyom
>
>
> On Friday 17 June 2016 06:34 PM, Roger Riggs wrote:
>> 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
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>