RFR: 8185898: setRequestProperty(key, null) results in HTTP header without colon in request

[Julia Boes]

Hi Michael and Daniel,

Thanks for your comments. I made the following changes to B8185898:

- Added checks for URLConnection::getRequestProperties, 
::getHeaderField(0), ::getHeaderFieldKey(0)

- Included a test of MessageHeader::print and ::toString. Added a null 
check for the requestLine in MessageHeader::toString (Thanks Jaikiran!)

Updated webrev: http://cr.openjdk.java.net/~michaelm/jboes/8185898/webrev.2/

Best,

Julia


On 31/07/2019 15:34, Daniel Fuchs wrote:
> Hi Michael,
>
> On 31/07/2019 14:47, Michael McMahon wrote:
>> Daniel
>>
>> I don't think the change affects the usage of response headers,
>> but it might be useful to include in the test a call to 
>> getHeaderField(0)
>> to verify that.
>
> Oh - right - but it will/might affect the toString() of the
> response headers, if that is used for logging, isn't it?
> Wouldn't the status line now be printed with an additional colon
> at the end?
>
> best,
>
> -- daniel
>
>>
>> Michael.
>>
>> On 31/07/2019, 12:30, Daniel Fuchs wrote:
>>> Hi Julia,
>>>
>>> Could you verify that `HttpURLConnection::getHeaderField(0)` and
>>> `HttpURLConnection::getHeaderFieldKey(0)` return the same thing
>>> before and after the fix, for all implementations of
>>> `HttpURLConnection` in the JDK?
>>>
>>> I believe your test is missing that.
>>>
>>> More specifically, I'm referring to this:
>>> https://download.java.net/java/early_access/jdk13/docs/api/java.base/java/net/HttpURLConnection.html#getHeaderFieldKey(int) 
>>>
>>>
>>> "Some implementations may treat the 0th header field as special,
>>> i.e. as the status line returned by the HTTP server. In this case,
>>> getHeaderField(0) returns the status line, but getHeaderFieldKey(0)
>>> returns null."
>>>
>>> I wonder if the JDK itself was one of those implementations before
>>> the fix. We do not want to change the behaviour of these methods.
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>> On 31/07/2019 11:56, Julia Boes wrote:
>>>> Hi,
>>>>
>>>> Please find below a patch for:
>>>>
>>>> 8185898: setRequestProperty(key, null) results in HTTP header 
>>>> without colon in request
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8185898
>>>>
>>>> According to RFC 2616 
>>>> <https://tools.ietf.org/html/rfc2616#section-4>, message headers of 
>>>> a HTTP message must adhere to the format:
>>>>
>>>> field-name ":" [ field-value ]
>>>>
>>>> Previously, the request line was handled like a message header and 
>>>> MessageHeader::print would omit ":" for message headers with value 
>>>> == null to account for the request line. However, this can result 
>>>> in a regular message header missing the colon if its value is null. 
>>>> To fix this, MessageHeader.requestLine was added, which is used 
>>>> only for the status line of a request. Any use of 
>>>> MessageHeader.set(0) and MessageHeader.prepend() in 
>>>> HttpURLConnection was replaced by MessageHeader::setRequestLine.
>>>>
>>>> Webrev:
>>>>
>>>> http://cr.openjdk.java.net/~michaelm/jboes/8185898/webrev.1/
>>>>
>>>>
>>>> Cheers,
>>>>
>>>> Julia
>>>>
>>>
>