Bug Tracker

Opened 5 years ago

Closed 5 years ago

#14503 closed bug (fixed)

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?

Change History (10)

comment:1 Changed 5 years ago by 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?

comment:2 in reply to:  1 ; Changed 5 years ago by 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.

comment:3 in reply to:  2 Changed 5 years ago by 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.

comment:4 in reply to:  2 Changed 5 years ago by 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.

Last edited 5 years ago by davidhong (previous) (diff)

comment:5 Changed 5 years ago by gibson042

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

comment:6 Changed 5 years ago by dmethvin

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

comment:7 in reply to:  6 Changed 5 years ago by 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.

comment:9 Changed 5 years ago by hongymagic

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

Changeset: 8d09ee0506794e8a2b7cfae4bb804e4ca59348e1

comment:10 Changed 5 years ago by timmywil

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.