RFR 8071660 :URLPermission not handling empty method lists correctly

Daniel Fuchs daniel.fuchs at oracle.com
Fri Jun 17 12:42:30 UTC 2016


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



More information about the net-dev mailing list