Re: RFR 8114860: Behavior of java.net.URLPermission.getActions() contradicts spec

[Pavel Rappo]

Looks good to me. Thanks.

> On 21 Jun 2016, at 08:10, Vyom Tewari  wrote:
> 
> Hi Pavel,
> 
> Please find the updated 
> webrev(http://cr.openjdk.java.net/~vtewari/8114860/webrev0.2/index.html 
> ). I 
> reverted the commented "hashtest" test.
> 
> Thanks,
> Vyom
> 
> 
> On Monday 20 June 2016 10:53 PM, Pavel Rappo wrote:
>> Same here. I got this only after I had a look at the history of changes.
>> 
>>> On 20 Jun 2016, at 17:53, Vyom Tewari  wrote:
>>> 
>>> I was not expecting that hashcodetest is testing NPE by comparing the 
>>> "hardcoded" hash values.
> 

Re: RFR 8114860: Behavior of java.net.URLPermission.getActions() contradicts spec

[Vyom Tewari]

Hi Pavel,

Please find the updated 
webrev(http://cr.openjdk.java.net/~vtewari/8114860/webrev0.2/index.html 
). I 
reverted the commented "hashtest" test.


Thanks,
Vyom


On Monday 20 June 2016 10:53 PM, Pavel Rappo wrote:

Same here. I got this only after I had a look at the history of changes.


On 20 Jun 2016, at 17:53, Vyom Tewari  wrote:

I was not expecting that hashcodetest is testing NPE by comparing the 
"hardcoded" hash values.

Re: RFR 8114860: Behavior of java.net.URLPermission.getActions() contradicts spec

[Pavel Rappo]

Same here. I got this only after I had a look at the history of changes.

> On 20 Jun 2016, at 17:53, Vyom Tewari  wrote:
> 
> I was not expecting that hashcodetest is testing NPE by comparing the 
> "hardcoded" hash values.

Re: RFR 8114860: Behavior of java.net.URLPermission.getActions() contradicts spec

[Vyom Tewari]

Hi,

Please find the updated 
webrev(http://cr.openjdk.java.net/~vtewari/8114860/webrev0.1/index.html 
). I 
incorporated the review comments and remove the "HashCodeTest" where it 
compares the hashcode with hard coded hashcode.


Thanks,
Vyom


On Monday 20 June 2016 03:57 PM, Pavel Rappo wrote:

Hi Vyom, thanks for looking into this!

0.  * Copyright (c) 2013,2016 Oracle and/or its affiliates. All rights reserved.

If I understand correctly, we're not required to update years in the header
manually. But if you decided to do so, please follow the established pattern (in
both files you've changed):

 * Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights 
reserved.
  ^^

1. I wonder if we could use String convenience methods instead of bringing
dependency on java.util.stream.Collector. Could something like this be a bit
better both in terms of dependencies and readability?

 private String actions() {
 String b = String.join(",", methods);
 if (!requestHeaders.isEmpty()) {
 b = ":" + String.join(",", requestHeaders);
 }
 return b;
 }

2. Is there a particular reason for these parentheses?

 150 return (expectedActions.equals(urlp.getActions()));
^ ^

3. Why are these lines commented out?

 259 static Test[] hashTests = {
 260 //hashtest("http://www.foo.com/path;, "GET:X-Foo", 388644203),
 261 //hashtest("http:*", "*:*", 3255810)
 262 };
 
IMO they either have to be updates or removed altogether.


Thanks,
-Pavel


On 20 Jun 2016, at 08:44, Vyom Tewari  wrote:

ping!

On Saturday 11 June 2016 10:34 AM, vyom wrote:

Hi All,

Please review the below fix.

Bug   : JDK-8114860 Behavior of java.net.URLPermission.getActions() 
contradicts spec
Webrev : http://cr.openjdk.java.net/~vtewari/8114860/webrev0.0/index.html 


Thanks,
Vyom

Re: RFR 8114860: Behavior of java.net.URLPermission.getActions() contradicts spec

[Pavel Rappo]

Hi Vyom, thanks for looking into this!

0.  * Copyright (c) 2013,2016 Oracle and/or its affiliates. All rights reserved.

If I understand correctly, we're not required to update years in the header
manually. But if you decided to do so, please follow the established pattern (in
both files you've changed):

* Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights 
reserved.
 ^^

1. I wonder if we could use String convenience methods instead of bringing
dependency on java.util.stream.Collector. Could something like this be a bit
better both in terms of dependencies and readability?

private String actions() {
String b = String.join(",", methods);
if (!requestHeaders.isEmpty()) {
b = ":" + String.join(",", requestHeaders);
}
return b;
}

2. Is there a particular reason for these parentheses?

150 return (expectedActions.equals(urlp.getActions()));
   ^ ^

3. Why are these lines commented out?

259 static Test[] hashTests = {
260 //hashtest("http://www.foo.com/path;, "GET:X-Foo", 388644203),
261 //hashtest("http:*", "*:*", 3255810)
262 };

IMO they either have to be updates or removed altogether.

Thanks,
-Pavel

> On 20 Jun 2016, at 08:44, Vyom Tewari  wrote:
> 
> ping!
> 
> On Saturday 11 June 2016 10:34 AM, vyom wrote:
>> Hi All,
>> 
>> Please review the below fix.
>> 
>> Bug   : JDK-8114860 Behavior of 
>> java.net.URLPermission.getActions() contradicts spec
>> Webrev : 
>> http://cr.openjdk.java.net/~vtewari/8114860/webrev0.0/index.html 
>> 
>> 
>> Thanks,
>> Vyom
> 

RFR 8114860: Behavior of java.net.URLPermission.getActions() contradicts spec

[vyom]

Hi All,

Please review the below fix.

Bug   : JDK-8114860 Behavior of 
java.net.URLPermission.getActions() contradicts spec
Webrev : 
http://cr.openjdk.java.net/~vtewari/8114860/webrev0.0/index.html 



Thanks,
Vyom