IE8 null-values in `headers` object causes Ajax to drop headers properties

[hongymagic]

Environment:

IE8 with "Enable native XMLHTTP support" unchecked (i.e., disabled). This bug could also apply to IE6/IE7 under similar circumstances.

Tested on:

IE8 on Windows XP and Windows 7.

Bug:

When calling ​$.ajax, if supplied "headers" object contains null-value all subsequent headers, including the null-value property, are not attached.

Reduced case:

​http://jsbin.com/uBaNeTa/4/edit

Please check it out on other browsers as well to see which headers get sent across.

Fix:

​https://github.com/jquery/jquery/blob/1.x-master/src/ajax/xhr.js#L97

This try and catch swallows the Type Mismatch exception in IE8 when trying to set request header to a null-value. This means if it fails at the first key, rest of the values won't be set at all.

It seems like most desktop browsers tend to convert null to "null" (string) when setting request header to a null-value. jQuery could either provide the same sort of mechanism or swallow exception at setRequestHeader level not at the loop.

I would prefer the former approach where null is converted to "null" when setting the request header.

Any thoughts on this?

[jaubourg]

What would you think of testing if the header value is trueish, then not set it if it isn't. I really fail to see the interest of sending headers that have "null" as their value. Do you actually process that server-side?

[gibson042]

Replying to jaubourg:

What would you think of testing if the header value is trueish, then not set it if it isn't.

Wouldn't that fail for empty-string values? Judging from other XMLHttpRequest implementations, the conventional behavior would be skipping undefined and otherwise casting to string.

[jaubourg]

Wouldn't that fail for empty-string values? Judging from other XMLHttpRequest implementations, the conventional behavior would be skipping undefined and otherwise casting to string.

Seems reasonable to me.

[davidhong]

Replying to gibson042:

Judging from other XMLHttpRequest implementations, the conventional behavior would be skipping undefined and otherwise casting to string.

Yes that is correct. Something like ​this commit.

[gibson042]

Yeah, that's perfect after some style guide cleanup. Why not sign the CLA and open a PR?

[dmethvin]

Let's give you credit David, please file a pull request.

[davidhong]

Replying to gibson042:

Yeah, that's perfect after some style guide cleanup. Why not sign the CLA and open a PR?

I wanted to discuss the options before filing a PR. I will be fixing the single quotes and etc. Also seems like the code has changed since I've started working on it. The outer try/catch is no longer there, meaning it may just throw an exception and crash (which maybe a good thing)!

Replying to dmethvin:

Let's give you credit David, please file a pull request.

Yep will do.

[davidhong]

Pull request: ​https://github.com/jquery/jquery/pull/1427

[hongymagic]

Ref #14503: Cherry-pick tests. (cherry picked from commit 27b22f4ef5f3f291204f0e0f9f414ac503f6c8a8) (cherry picked from commit 8dc0f2ea84e1861d8d8dfa7699268368c659f8e9) (cherry picked from commit 936126f10de193e2bb1c8280dbc2361ca5d62e29)

Changeset: 8d09ee0506794e8a2b7cfae4bb804e4ca59348e1