#10338 closed bug (wontfix)
getResponseHeader() broken for CORS requests in Firefox 6
Reported by: | anonymous | Owned by: | |
---|---|---|---|
Priority: | low | Milestone: | 1.next |
Component: | ajax | Version: | 1.6.4 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
Due to a bug in Firefox[1] where .getAllResponseHeaders() returns the empty string despite .getResponseHeader("Content-Type") returning a non-empty string, jQuery fails to automatically decode JSON CORS responses in Firefox.
The bug is replicable in jQuery >=1.5.2 but not in jQuery <=1.4.4: http://jsfiddle.net/xeBub/
Change History (19)
comment:1 Changed 11 years ago by
comment:2 Changed 11 years ago by
Thanks for the feedback [1] @dmethvin.
I've re-rolled the patch [2] with better adherence to the style rules.
I've asked Firefox why they haven't patched it [3], though I suspect the answer is that it's low-priority and no-one's felt the need to.
Do you have an idea how I would create a test for this? It seems like it should be the case that the test without this patch fails only in Firefox — but that would require actually making a cross-domain request; which is probably not acceptable for a test-suite that has to work everywhere.
[1] https://github.com/jquery/jquery/pull/517#issuecomment-2184486 [2] https://github.com/ConradIrwin/jquery/commits/fix.cors-firefox [3] https://bugzilla.mozilla.org/show_bug.cgi?id=608735#c5
comment:3 Changed 11 years ago by
I don't like the patch. It's really an ugly workaround in the sense that you list the headers you want supported (open door for people asking for more more more more). I'd live with a notification in the docs about the problem knowing full well that the new Firefox update policy will take care of the problem eventually.
comment:4 Changed 11 years ago by
I see your point @jaubourg. This isn't fixing the problem, it's just making a list of headers that would be nice to get when people try to request all headers. So the patch is creating a false sense that it has gotten all headers when it really hasn't.
It would be better for the requester to know about this problem in Firefox and work around it by requesting the headers *they* expect to receive, even if that turns out to be a large list. Otherwise they will inevitably ask for more headers to be added to our patch list.
comment:5 follow-up: 6 Changed 11 years ago by
The list of headers is complete according to the CORS spec [1].
In order to get access to more headers, you'd have to rely on the Access-Control-Expose-Headers header, which is broken in Chrome and Safari, and broken-by-design in that Javascript can't read it (so we *could* try and read that, and if we can append any headers that are whitelisted to the list we try to read, but it would only work in Firefox and under very limited conditions).
A better fix would be to delegate jqXHR.getResponseHeader() to the builtin getResponseHeader(), though that would break the nice encapsulation of transporters.
If we wanted to document this, and provide a work-around, the best thing I can think of (though it's not very nice) is to do is to override jQuery.ajaxSettings.xhr (something like):
var _super = jQuery.ajaxSettings.xhr; jQuery.ajaxSettings.xhr = function () { var xhr = _super(), getAllResponseHeaders = xhr.getAllResponseHeaders; xhr.getAllResponseHeaders = function () { if ( getAllResponseHeaders() ) { return getAllResponseHeaders(); } var allHeaders = ""; $( ["Cache-Control", "Content-Language", "Content-Type", "Expires", "Last-Modified", "Pragma"] ).each(function (i, header_name) { if ( xhr.getResponseHeader( header_name ) ) { allHeaders += header_name + ": " + xhr.getResponseHeader( header_name ) + "\n"; } return allHeaders; }); }; return xhr; };
[1] """User agents must filter out all response headers other than those that are a simple response header [2] or of which the field name is an ASCII case-insensitive match for one of the values of the Access-Control-Expose-Headers headers (if any), before exposing response headers to APIs defined in CORS API specifications.
E.g. the getResponseHeader() method of XMLHttpRequest will therefore not expose any header not indicated above.""" [2] http://www.w3.org/TR/cors/#simple-response-header
comment:6 Changed 11 years ago by
Keywords: | needsdocs added |
---|
I agree it's not very nice but I honestly prefer something like this over patching for a problem (hopefully) short-lived.
I can't keep count of how many problems we had with CORS so far :/
Replying to [email protected]…:
The list of headers is complete according to the CORS spec [1].
In order to get access to more headers, you'd have to rely on the Access-Control-Expose-Headers header, which is broken in Chrome and Safari, and broken-by-design in that Javascript can't read it (so we *could* try and read that, and if we can append any headers that are whitelisted to the list we try to read, but it would only work in Firefox and under very limited conditions).
A better fix would be to delegate jqXHR.getResponseHeader() to the builtin getResponseHeader(), though that would break the nice encapsulation of transporters.
If we wanted to document this, and provide a work-around, the best thing I can think of (though it's not very nice) is to do is to override jQuery.ajaxSettings.xhr (something like):
var _super = jQuery.ajaxSettings.xhr; jQuery.ajaxSettings.xhr = function () { var xhr = _super(), getAllResponseHeaders = xhr.getAllResponseHeaders; xhr.getAllResponseHeaders = function () { if ( getAllResponseHeaders() ) { return getAllResponseHeaders(); } var allHeaders = ""; $( ["Cache-Control", "Content-Language", "Content-Type", "Expires", "Last-Modified", "Pragma"] ).each(function (i, header_name) { if ( xhr.getResponseHeader( header_name ) ) { allHeaders += header_name + ": " + xhr.getResponseHeader( header_name ) + "\n"; } return allHeaders; }); }; return xhr; };[1] """User agents must filter out all response headers other than those that are a simple response header [2] or of which the field name is an ASCII case-insensitive match for one of the values of the Access-Control-Expose-Headers headers (if any), before exposing response headers to APIs defined in CORS API specifications.
E.g. the getResponseHeader() method of XMLHttpRequest will therefore not expose any header not indicated above.""" [2] http://www.w3.org/TR/cors/#simple-response-header
comment:7 Changed 11 years ago by
Component: | unfiled → ajax |
---|---|
Milestone: | None → 1.next |
Priority: | undecided → low |
Status: | new → open |
comment:10 Changed 11 years ago by
This does not really work. I am getting:
"[Exception... \"Illegal operation on WrappedNative prototype object\" nsresult: \"0x8057000c (NS_ERROR_XPC_BAD_OP_ON_WN_PROTO)\" location: \"JS frame :: http://127.0.0.1:8000/static/project/js/common.js :: <TOP_LEVEL> :: line 225\" data: no]"
Why solution/hack cannot simply reuse native getResponseHeader
? Then there will be no need for a list of supported header names?
comment:12 Changed 11 years ago by
There is a bug existing already there for getResponseHeader
? And I am getting this on a bit old Firefox, 4.0.1. (Want to have support also old Firefox versions.)
But as I understand, calling getResponseHeader
on native xhr would work? How can I access this native getResponseHeader
? Because this broke for me with upgrade to jQuery 1.7. Before I had 1.4.2 and it worked. So obviously it is possible to make getResponseHeader
work independently from Firefox bug.
comment:13 Changed 11 years ago by
Here is an updated work-around I use with jQuery 1.7 and Firefox 8
var _super = $.ajaxSettings.xhr; $.ajaxSetup( { xhr: function () { var xhr = _super(); var getAllResponseHeaders = xhr.getAllResponseHeaders; xhr.getAllResponseHeaders = function () { var allHeaders = getAllResponseHeaders.call( xhr ); if( allHeaders ) { return allHeaders; } allHeaders = ""; $( ["Cache-Control", "Content-Language", "Content-Type", "Expires", "Last-Modified", "Pragma"] ).each( function ( i, header_name ) { if( xhr.getResponseHeader( header_name ) ) { allHeaders += header_name + ": " + xhr.getResponseHeader( header_name ) + "\n"; } } ); return allHeaders; }; return xhr; } } );
comment:14 Changed 11 years ago by
Here's an refinement to the above work-around that makes a distinction between the fixed-list of simple response headers and a custom list for non-simple headers that the CORS server makes accessible via "Access-Control-Expose-Headers" (Location being a common one). It's also a AMD module.
define("jquery-cors-patch", ["jquery"], function ($) { // workaround for Firefox CORS bug - see http://bugs.jquery.com/ticket/10338 var _super = $.ajaxSettings.xhr; $.ajaxSetup({ xhr: function() { var xhr = _super(); var getAllResponseHeaders = xhr.getAllResponseHeaders; xhr.getAllResponseHeaders = function() { var allHeaders = getAllResponseHeaders.call(xhr); if (allHeaders) { return allHeaders; } allHeaders = ""; var concatHeader = function(i, header_name) { if (xhr.getResponseHeader(header_name)) { allHeaders += header_name + ": " + xhr.getResponseHeader( header_name ) + "\n"; } }; // simple headers (fixed set) $(["Cache-Control", "Content-Language", "Content-Type", "Expires", "Last-Modified", "Pragma"]).each(concatHeader); // non-simple headers (add more as required) $(["Location"] ).each(concatHeader); return allHeaders; }; return xhr; } }); });
comment:16 Changed 11 years ago by
In the case that non-simple headers need to be exposed, ex.: Access-Control-Expose-Headers : Location
It might be better to delegate getResponseHeader() to the real xhr object.
diff --git a/src/ajax.js b/src/ajax.js index 2bcc1d0..368c69d 100644 --- a/src/ajax.js +++ b/src/ajax.js @@ -458,7 +458,8 @@ jQuery.extend({ } match = responseHeaders[ key.toLowerCase() ]; } - return match === undefined ? null : match; + // Work around for firefox: xhr.getAllResponseHeaders() retruns "" for cross domain request. + return match === undefined ? ( this.xhr === undefined ? null : this.xhr.getResponseHeader(key) ) : match; }, // Overrides response content-type header @@ -483,7 +484,7 @@ jQuery.extend({ // Callback for when everything is done // It is defined here because jslint complains if it is declared // at the end of the function (which would be more logical and readable) - function done( status, nativeStatusText, responses, headers ) { + function done( status, nativeStatusText, responses, headers, xhr ) { // Called once if ( state === 2 ) { @@ -508,6 +509,9 @@ jQuery.extend({ // Set readyState jqXHR.readyState = status > 0 ? 4 : 0; + // set the real xhr object + jqXHR.xhr = xhr; + var isSuccess, success, error, diff --git a/src/ajax/xhr.js b/src/ajax/xhr.js index 013863d..ec5daca 100644 --- a/src/ajax/xhr.js +++ b/src/ajax/xhr.js @@ -186,7 +186,7 @@ if ( jQuery.support.ajax ) { // Call complete if needed if ( responses ) { - complete( status, statusText, responses, responseHeaders ); + complete( status, statusText, responses, responseHeaders, xhr ); } };
comment:17 Changed 10 years ago by
Resolution: | → wontfix |
---|---|
Status: | open → closed |
This bug is being actively worked by Firefox at this point, so I think we'll wait for their solution rather than bloating both 1.9 and 2.0 with a patch.
comment:18 Changed 10 years ago by
Some additional info on this odd behavior under Firefox.
I just hit up against this issue. A bit of research showed my that the associated Firefox bug is now fixed, scheduled for release version 21. I've just tried out Firefox 21 beta with jQuery 1.9.1 and the issue appears to be fixed.
I'm not sure when Firefox 21 production version will hit.
comment:19 Changed 9 years ago by
BTW, this was fixed in FF21 a while ago as well as in webkit and blink:
https://bugs.webkit.org/show_bug.cgi?id=41210 https://code.google.com/p/chromium/issues/detail?id=87338 https://bugs.webkit.org/show_bug.cgi?id=76419 https://bugzilla.mozilla.org/show_bug.cgi?id=608735
confirmed using http://jsfiddle.net/xeBub/7/ in browserstack
You can get a patch that fixes this at: https://github.com/jquery/jquery/pull/517 .