Hi,
Please find an alternative fix below. Chris and Daniel suggested to add
a new method MessageHeader::isRequestline rather than a separate data
field for the requestline. A whitebox test was added with the method
testMessageHeader.
Updated webrev: http://cr.openjdk.java.net/~michaelm/jboes/8185898/webrev.4/
Regards,
Julia
On 06/08/2019 11:03, Julia Boes wrote:
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
