Bug Tracker

Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#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/

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=608735

Change History (19)

comment:1 Changed 8 years ago by conrad.irwin@…

You can get a patch that fixes this at: https://github.com/jquery/jquery/pull/517 .

comment:2 Changed 8 years ago by conrad.irwin@…

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 8 years ago by jaubourg

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 8 years ago by dmethvin

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 Changed 8 years ago by conrad.irwin@…

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

Version 0, edited 8 years ago by conrad.irwin@… (next)

comment:6 in reply to:  5 Changed 8 years ago by jaubourg

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 conrad.irwin@…:

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 8 years ago by jaubourg

Component: unfiledajax
Milestone: None1.next
Priority: undecidedlow
Status: newopen

comment:8 Changed 8 years ago by addyosmani

Keywords: needsdocs removed

Docs for this have been added.

comment:9 Changed 8 years ago by mitar

CCing.

comment:10 Changed 8 years ago by mitar

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:11 Changed 8 years ago by dmethvin

Have you tried filing a bug with Firefox?

comment:12 Changed 8 years ago by mitar

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 8 years ago by volune

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 7 years ago by keith.donald@…

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:15 Changed 7 years ago by dmethvin

#11624 is a duplicate of this ticket.

comment:16 Changed 7 years ago by malix.ren@…

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 6 years ago by dmethvin

Resolution: wontfix
Status: openclosed

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.

https://bugzilla.mozilla.org/show_bug.cgi?id=608735#c38

comment:18 Changed 6 years ago by michael.conway@…

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.

Note: See TracTickets for help on using tickets.