XMLHttpRequest: setRequestHeader() does not throw SYNTAX_ERR exception if the header field name is empty Per the XHR spec: client . setRequestHeader(header, value) Throws a SYNTAX_ERR exception if header is not a valid HTTP header field name or if value is not a valid HTTP header field value. Test: http://tc.labs.opera.com/apis/XMLHttpRequest/setrequestheader-bogus-header.htm Note that this test is not 100% correct because it checks against INVALID_STATE_ERR instead of SYNTAX_ERR. However, after fixing this, we do have one bug with not throwing SYNTAX_ERR exception if the header field name is empty
The fix appears to be changing XMLHttpRequest::isValidToken() to match http://www.ietf.org/rfc/rfc2616.txt section 2.2 by rejecting zero-length tokens. The question is: will throwing an exception here as mandated by the spec break existing web sites?
> The fix appears to be changing XMLHttpRequest::isValidToken() Note that this function is also used to check HTTP method, so this will also fix part of bug 46008. I think that this is unlikely to break web sites. Seems fine to make the change, as long as at least one of IE/Firefox matches the spec here. Given that we're going to import the whole test suite sooner or later, and that this is such an inconsequential change, it may even be OK to land it without a regression test.
Created attachment 68490 [details] Patch
Does IE or Firefox match the spec here?
Firefox and IE did both throw an exception in the test case in the patch attached to this bug. (once I changed it to use innerHTML) The conversion from the caught exception to a string did give different results on each browser though, so I'm not sure if they are returning SYNTAX_ERR or some other exception.
Comment on attachment 68490 [details] Patch Code changes look fine, but you should test both issues fixed here - or for neither.
@Alexey, is the second issue you want tested https://bugs.webkit.org/show_bug.cgi?id=46008, or something else?
Yes, the empty HTTP method one. Even though it was originally split into a different bug, it makes most sense to land regression tests in the same commit as code changes.
Created attachment 68502 [details] Patch
Comment on attachment 68502 [details] Patch Clearing flags on attachment: 68502 Committed r68124: <http://trac.webkit.org/changeset/68124>
All reviewed patches have been landed. Closing bug.