RFR 8071660 :URLPermission not handling empty method lists correctly

[Vyom Tewari]

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


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