Skip to main content

Bug Tracker

Side navigation

#14503 closed bug (fixed)

Opened November 01, 2013 12:05AM UTC

Closed December 06, 2013 05:29PM UTC

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

Reported by: hongymagic Owned by:
Priority: undecided Milestone: None
Component: unfiled Version: 1.10.2
Keywords: Cc:
Blocked by: Blocking:
Description

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?

Attachments (0)
Change History (10)

Changed November 08, 2013 02:25PM UTC by jaubourg comment:1

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?

Changed November 08, 2013 04:24PM UTC by gibson042 comment:2

Replying to [comment:1 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.

Changed November 08, 2013 05:54PM UTC by jaubourg comment:3

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.

Changed November 10, 2013 11:11AM UTC by davidhong comment:4

_comment0: Replying to [comment:2 gibson042]: \ > Judging from other XMLHttpRequest implementations, the conventional behavior would be skipping `undefined` and otherwise casting to string. \ \ Yes that is correct. Something like [https://github.com/hongymagic/jquery/commit/398f0a42e0e5df2e127937963f6620616cc9b74b this commit].1384138739543721

Replying to [comment:2 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.

Changed November 10, 2013 01:59PM UTC by gibson042 comment:5

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

Changed November 10, 2013 07:10PM UTC by dmethvin comment:6

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

Changed November 11, 2013 12:28AM UTC by davidhong comment:7

Replying to [comment:5 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 [comment:6 dmethvin]:

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

Yep will do.

Changed November 11, 2013 09:46AM UTC by davidhong comment:8

Changed November 15, 2013 05:34PM UTC by hongymagic comment:9

Ref #14503: Cherry-pick tests.

(cherry picked from commit 27b22f4ef5f3f291204f0e0f9f414ac503f6c8a8)

(cherry picked from commit 8dc0f2ea84e1861d8d8dfa7699268368c659f8e9)

(cherry picked from commit 936126f10de193e2bb1c8280dbc2361ca5d62e29)

Changeset: 8d09ee0506794e8a2b7cfae4bb804e4ca59348e1

Changed December 06, 2013 05:29PM UTC by timmywil comment:10

resolution: → fixed
status: newclosed