RFR JDK8015799

[Chris Hegarty]

Looks fine to me John.

-Chris.

On 27/06/2013 17:09, John Zavgren wrote:
> All:
> I just posted a webrev image of the latest changes:
>
>
> http://cr.openjdk.java.net/~jzavgren/8015799/webrev.03/
>
> Thanks!
> John
>
> On 06/26/2013 04:52 PM, Kurchi Hazra wrote:
>> Alright, thanks for the clarification - the source code changes are
>> good as they are then.
>>
>> - Kurchi
>>
>> On 6/26/2013 1:49 PM, Chris Hegarty wrote:
>>> To link this email thread, both in the archives, and for others. The
>>> call for review on this bug started with:
>>> http://mail.openjdk.java.net/pipermail/net-dev/2013-June/006607.html
>>>
>>> On 06/26/2013 08:22 PM, Kurchi Hazra wrote:
>>>>
>>>> On 6/26/2013 12:17 PM, Kurchi Hazra wrote:
>>>>> Hi John,
>>>>>
>>>>> Why not change lines 2810-2811 to:
>>>>> if (value == null || value.length() == 0)
>>>>> return value;
>>>> I meant return null. For other cookie-headers too, is there any reason
>>>> for us not returning null if the length of value is 0?
>>>
>>> In the first webrev John had made this change, but I asked him to
>>> revert it and only change the Set-Cookie(2) headers.
>>>
>>> "Since all header retrieval passes through filterHeaderField, in one
>>> way or another, I'm a little concerned about changing this. Also, as
>>> the only issue we know of is with Set-Cookie(2), maybe you could add
>>> the empty string check to these headers only? ( that is to say, move
>>> the 'value.length() == 0' check into the ' if
>>> (SET_COOKIE.equalsIgnoreCase(name)..... ' "
>>>
>>> The difference is, currently if a header value is non-null and has a
>>> length of 0 ( i.e. empty string ), then empty string is returned.
>>> With the original change then null is returned.
>>>
>>> We have been bitten by subtle changes in this area before. Returning
>>> null from such an API, URLConnection.getHeaderField(s), for cases
>>> where it did not return null before may lead to NPE's in some
>>> applications.
>>>
>>> -Chris.
>>>
>>>>>
>>>>> Also, lots of formatting issue in the test, especially in
>>>>> TestCookieHandler, try-catch block indentation is off in line 54.
>>>>> Its also best to stop the server in a finally clause at line 58.
>>>>> Alternatively, I also liked Andreas' use of autocloseable in his test
>>>>> for 6563286. See [1].
>>>
>>> Yes, please.
>>>
>>> -Chris.
>>>
>>>>>
>>>>> - Kurchi
>>>>>
>>>>> [1]
>>>>> http://cr.openjdk.java.net/~arieber/6563286/webrev.00/test/sun/net/www/http/HttpURLConnection/MalformedFollowRedirect.java.html
>>>>>
>>>>> <http://cr.openjdk.java.net/%7Earieber/6563286/webrev.00/test/sun/net/www/http/HttpURLConnection/MalformedFollowRedirect.java.html>
>>>>>
>>>>>
>>>>> On 6/26/2013 10:54 AM, John Zavgren wrote:
>>>>>> Please consider the following changes to the Java cookie code.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~jzavgren/8015799/webrev.02/
>>>>>> <http://cr.openjdk.java.net/%7Ejzavgren/8015799/webrev.02/>
>>>>>>
>>>>>> The problem I fixed occurs when a server returns an array of cookies
>>>>>> that contains a null cookie.
>>>>>>
>>>>>> Thanks
>>>>>> John
>>>>>> --
>>>>>> John Zavgren
>>>>>> john.zavgren at oracle.com
>>>>>> 603-821-0904
>>>>>> US-Burlington-MA
>>>>>
>>>>
>>>> --
>>>> -Kurchi
>>
>