RFR JDK8015799

[John Zavgren]

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 
>

-- 
John Zavgren
john.zavgren at oracle.com
603-821-0904
US-Burlington-MA