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

[Jaikiran Pai]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20190803/13e7a036/attachment.html>