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 >