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.
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.
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 :)
(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.
thiago, they need you (see comment :)
If this is a workaround for a QTNetwork issue, shouldn't it have a link to the QtNetwork bug in a comment?
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.
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.
Would be nice to add a comment above the change as well. Something along the lines of what Thiago said.
Created attachment 42782 [details] Patch Added regression test and a comment for this patch.
Comment on attachment 42782 [details] Patch Clearing flags on attachment: 42782 Committed r50679: <http://trac.webkit.org/changeset/50679>
All reviewed patches have been landed. Closing bug.