Fixed CORS response headers in Firefox #881

Closed
wants to merge 1 commit into
from

7 participants

@rwaldron
jQuery Foundation member

Thanks got the contribution! In #10338 the issue reported exists in FF4.0.x — does the issue still exist in the latest Firefox? We'll need tests added to support the fix and you'll need to read and review the contribution guides on the main readme file. Specifically, there are style guide conformance issues throughput the code.

@rkatic rkatic commented on the diff Aug 26, 2012
src/ajax/xhr.js
@@ -138,6 +141,33 @@ if ( jQuery.support.ajax ) {
} else {
status = xhr.status;
responseHeaders = xhr.getAllResponseHeaders();
+
+ /* #10338 */
+ if ( s.crossDomain && !responseHeaders ) {
+ /* Access-Control-Expose-Headers MUST be exposed by itself */
+ var exposeHeader = xhr.getResponseHeader("Access-Control-Expose-Headers"),
+ exposedHeaders = [];
+
+ if ( exposeHeader ) {
+ exposedHeaders = jQuery.map(
+ exposeHeader.split(','),
+ function(token) {
+ return jQuery.trim(token);
+ }
@rkatic
rkatic added a line comment Aug 26, 2012

Why not simply passing jQuery.trim to .map() directly?

@dmethvin
jQuery Foundation member
dmethvin added a line comment Aug 26, 2012

Perhaps exposeHeaders = jQuery.trim(exposeHeader).split(/\s*,\s*/) instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@staabm staabm commented on the diff Aug 26, 2012
src/ajax/xhr.js
+ /* Access-Control-Expose-Headers MUST be exposed by itself */
+ var exposeHeader = xhr.getResponseHeader("Access-Control-Expose-Headers"),
+ exposedHeaders = [];
+
+ if ( exposeHeader ) {
+ exposedHeaders = jQuery.map(
+ exposeHeader.split(','),
+ function(token) {
+ return jQuery.trim(token);
+ }
+ );
+ }
+
+ var headers = simpleHTTPHeaders.concat(exposedHeaders);
+
+ jQuery.each(headers, function(i, header) {
@staabm
staabm added a line comment Aug 26, 2012

Iterating using a simple for-loop is much fast..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mikesherov
jQuery Foundation member

This is cool and all, but I'd love to get a way to unit test CORS into core... anyone have any suggestions?

@dmethvin
jQuery Foundation member

We'd like to have some way to test this. Ideas?

@dmethvin
jQuery Foundation member

I don't feel comfortable landing this at the moment. We don't have a way to test, it is a large amount of code, and I'm not sure the issue is common enough to warrant this big of a workaround.

@dmethvin dmethvin closed this Nov 24, 2012
@yiminghe

@mikesherov

The following is what I do to test CROS:

  1. First starting two web servers (recommend nodejs) listening on different port (such as 8888,9999),

  2. Then perform a xhr from 127.0.0.1:8888 to 127.0.0.1:9999.

it is also easy to be integrated into CI system (travis-ci) .

@staabm

Since jquery already uses php, you might be interessted in the php builtin webserver, which is made also for ci and automated testing purposes..

http://php.net/manual/en/features.commandline.webserver.php

@staabm

@dmethvin if you want me to prepare a php script which bootstraps a inmemory webserver, so another person could start with the actual CORS test, just give ma a ping,... I will have some time at the end of the week,..

@rjfranco

Has the status of this issue changed? I'm looking for a workaround to this issue now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment