Hello Julia,
I'm not a reviewer, but I noticed this change:
src/java.base/share/classes/sun/net/www/MessageHeader.java
public synchronized String toString() {
String result = super.toString() + nkeys + " pairs: ";
+ String reqLine = this.requestLine;
for (int i = 0; i < keys.length && i < nkeys; i++) {
result += "{"+keys[i]+": "+values[i]+"}";
}
- return result;
+ return reqLine + result;
}
Given that "this.requestLine" can be null (based on some of the null
checks I see in the rest of this patch), the return value of this
toString() can now end up prefixing "null" literal to this result. I
think a null check here would be needed to decide whether or not to do a
"reqLine + result" here, isn't it?
Furthermore, even when the requestLine is non-null, do you think the
return statement should perhaps be:
return reqLine + " " + result;
so that the output looks something like "<snip> GET /foo/bar HTTP/1.1
{h1:v1}" instead of "<snip> GET /foo/bar HTTP/1.1{h1:v1}"
-Jaikiran
On 31/07/19 4:26 PM, 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
>
