Bug 31140 – [Qt] Allow setting HTTP headers with empty value in XMLHTTPRequest

• Status: RESOLVED
• Resolution: FIXED
• Component: XML

[Yael]

The following script fragment should add a request header to xhr, but in Qt it does not
   xhr.setRequestHeader("Custom", "");

The reason is that a NULL value causes QNetworkRequest::setRawHeader to remove the header, instead of adding it.

[Yael]

Created attachment 42518 [details]
Patch

This patch converts the header value from null string to empty string before passing the header to QtNetwork. QtNetwork would add headers with empty strings, but not with null strings.

[Simon Hausmann]

Hm, we should talk to Thiago about this. Clearly calling setRawHeader() with a null value is currently the only way to _remove_ a header field from a request. But I think it would be cleaner if QNetworkRequest had a removeRawHeader() function and would allow empty header values.

Your use-case is a good example of why supporting it would make sense, and therefore it would be nice to not have to work around Qt's behaviour in WebKit if possible.

But yes, the last resort is your patch, which looks fine otherwise :)

[Yael]

(In reply to comment #2)
> Hm, we should talk to Thiago about this. Clearly calling setRawHeader() with a
> null value is currently the only way to _remove_ a header field from a request.
> But I think it would be cleaner if QNetworkRequest had a removeRawHeader()
> function and would allow empty header values.
> 
> Your use-case is a good example of why supporting it would make sense, and
> therefore it would be nice to not have to work around Qt's behaviour in WebKit
> if possible.
> 
> But yes, the last resort is your patch, which looks fine otherwise :)

Simon, I completely agree with you that this is a workaround for a QtNetwork issue. However, if we change QtNetwork, we will break its backwards compatibility, and I did not think Thiago will agree to that.

[Antonio Gomes]

thiago, they need you (see comment :)

[Eric Seidel (no email)]

If this is a workaround for a QTNetwork issue, shouldn't it have a link to the QtNetwork bug in a comment?

[Holger Freyther]

Comment on attachment 42518 [details]
Patch

I agree with Eric that we at least need some documentation here, and it would be nice if you could come up with a simple http test. E.g. I think we already have a perl script that will return the http header... we should have a dump as text test for that.

[Thiago Macieira]

setSomething with a null value is the way to remove that something. This pattern appears in multiple places in Qt.

If you need an empty value, please use an empty-but-not-null QString.

So attachment 42518 [details] looks correct for me.

[Kenneth Rohde Christiansen]

Would be nice to add a comment above the change as well. Something along the lines of what Thiago said.

[Yael]

Created attachment 42782 [details]
Patch

Added regression test and a comment for this patch.

[WebKit Commit Bot]

Comment on attachment 42782 [details]
Patch

Clearing flags on attachment: 42782

Committed r50679: <http://trac.webkit.org/changeset/50679>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.