On 6 Jun 2013, at 21:58, Kurchi Hazra <kurchi.subhra.ha...@oracle.com> wrote:

> How does this look:
> http://cr.openjdk.java.net/~khazra/7051862/webrev.01/

Looks fine to me.

-Chris.

> 
> - Kurchi
> 
> 
> On 6/6/2013 11:15 AM, Kurchi Hazra wrote:
>> 
>> On 6/6/2013 2:07 AM, Chris Hegarty wrote:
>>> On 06/06/2013 01:06 AM, Kurchi Hazra wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> Please review this change to fix 7051862. For ACCEPT_ORIGINAL_SERVER,
>>>> shouldAccept() throws a NullPointerException for null arguments.
>>>> Out of the many options to fix this, I think the best way is to return
>>>> false if either of the arguments is null - based on the fact that
>>>> HttpCookie.domainMathes() returns false for null arguments.
>>> 
>>> I agree with this change. It is a change in behavior ( returns false where 
>>> use to throw NPE ) but unlikely to cause surprise. It is also worth noting 
>>> that we cannot, easily, specify that shouldAccept throw NPE since there are 
>>> two other concrete CookiePolicy instances that would need to change ( they 
>>> would then throw NPE where previously returned true/false ), and that would 
>>> arguably be more surprising.
>> - Thanks Chris, those indded are the other fixes I had considered.
>> 
>>> 
>>> Many a simple test? Or amend an existing one?
>> 
>> - Working on it, I'll get back today.
>> 
>> 
>>> 
>>> -Chris.
>>> 
>>>> 
>>>>  Bug: http://bugs.sun.com/view_bug.do?bug_id=7051862
>>>>  Webrev: http://cr.openjdk.java.net/~khazra/7051862/webrev.00/
>>>> 
>>>> Thanks,
>>>> Kurchi
> 

Reply via email to