Bug Tracker

Modify

Ticket #10338 (closed bug: wontfix)

Opened 20 months ago

Last modified 2 months ago

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:
Blocking: Blocked by:

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

comment:1 Changed 20 months ago by conrad.irwin@…

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

comment:2 Changed 20 months 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 20 months 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 20 months 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 follow-up: ↓ 6 Changed 20 months 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

Last edited 20 months ago by jaubourg (previous) (diff)

comment:6 in reply to: ↑ 5 Changed 20 months 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 20 months ago by jaubourg

  • Priority changed from undecided to low
  • Status changed from new to open
  • Component changed from unfiled to ajax
  • Milestone changed from None to 1.next

comment:8 Changed 19 months ago by addyosmani

  • Keywords needsdocs removed

Docs for this have been added.

comment:9 Changed 18 months ago by mitar

CCing.

comment:10 Changed 18 months 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 18 months ago by dmethvin

Have you tried filing a bug with Firefox?

comment:12 Changed 18 months 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 18 months 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 13 months 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 13 months ago by dmethvin

#11624 is a duplicate of this ticket.

comment:16 Changed 12 months 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 4 months ago by dmethvin

  • Status changed from open to closed
  • Resolution set to wontfix

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 2 months 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.

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.