RFR 8071660 :URLPermission not handling empty method lists correctly

[Roger Riggs]

+1

Thanks, Roger


On 6/17/2016 10:27 AM, Chris Hegarty wrote:
> 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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>