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.
Looks good!
-- daniel
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
