Skip to content

/jquery

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jqXHR.getResponseHeader #3403

Closed
PSPvZ opened this Issue Nov 18, 2016 · 4 comments

Comments

Assignees
No one assigned
Labels
Ajax Bug Has Pull Request Web Standards help wanted
Projects
None yet
Milestone
  3.4.0
7 participants
@PSPvZ @gibson042 @pankajpatel @Andrei15193 @timmywil @markelog @mgol
@PSPvZ
Copy link

commented Nov 18, 2016

Description

jqXHR.getResponseHeader fails to combine multiple HTTP headers with the same name. Instead, it only returns the value of the last HTTP header. This clashes with the specification of XMLHttpRequest.getResponseHeader, which states: "If there are multiple response headers with the same name, then their values are returned as a single concatenated string, where each value is separated from the previous one by a pair of comma and space."

The issue seems to occur only on Internet Explorer (I encountered it on IE11); calling IE's own XMLHttpRequest.getRequestHeader implementation does return the correct result,

From what I can tell Firefox 50 already combined the two headers into one before calling the XMLHttpRequest completion callback, inadvertently allowing jQuery to return the correct result.

Tested in jQuery 3.1.1.

Link to test case

I have no test-environment that can easily set multiple headers with the same name, unfortunately.
@gibson042

This comment has been minimized.

Sign in to view
Copy link
Member

commented Nov 21, 2016

The problem seems to be misimplementation of getAllResponseHeaders (on which we rely) in at least IE11, but I otherwise agree with this assessment.

@PSPvZ, would you be willing to submit a PR updating our getResponseHeader to something like responseHeaders[ match[ 1 ].toLowerCase() ] = (responseHeaders[ match[ 1 ].toLowerCase() ] || []).concat( match[ 2 ] ); … return match == null ? null : match.join( ", " )?

@gibson042 gibson042 added Ajax Web Standards labels Nov 21, 2016

@timmywil timmywil added this to the 4.0.0 milestone Nov 21, 2016

@pankajpatel

This comment has been minimized.

Sign in to view
Copy link

commented Nov 23, 2016 •
edited

Hello,
I tried to create a test for this which can be seen here:
$getJSON: http://jsbin.com/vofipib/edit?js,console
$get: http://jsbin.com/texago/edit?js,console
XMLHttpRequest: http://jsbin.com/pugemo/edit?js,console
Can I contribute on this issue?
@gibson042

This comment has been minimized.

Sign in to view
Copy link
Member

commented Nov 23, 2016

Can I contribute on this issue?

Yes! https://contribute.jquery.org/commits-and-pull-requests/

@markelog markelog removed this from the 4.0.0 milestone Dec 5, 2016

@markelog markelog added the Needs review label Dec 5, 2016

@timmywil timmywil added help wanted and removed Needs review labels Dec 19, 2016

@timmywil timmywil added this to the Future milestone Dec 19, 2016

@gibson042 gibson042 added the Bug label Dec 19, 2016

@timmywil timmywil modified the milestones: 3.2.0, Future Dec 19, 2016

@timmywil timmywil modified the milestones: 3.2.0, 3.3.0 Mar 6, 2017

@timmywil timmywil modified the milestones: Future, 3.3.0 Mar 27, 2017

@Andrei15193 Andrei15193 referenced this issue Sep 11, 2018

Merged

Fix getResponseHeader(key) for IE11 #4173

4 of 4 tasks complete
@Andrei15193

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Sep 13, 2018

The problem seems to be misimplementation of getAllResponseHeaders (on which we rely) in at least IE11, but I otherwise agree with this assessment.

@PSPvZ, would you be willing to submit a PR updating our getResponseHeader to something like responseHeaders[ match[ 1 ].toLowerCase() ] = (responseHeaders[ match[ 1 ].toLowerCase() ] || []).concat( match[ 2 ] ); … return match == null ? null : match.join( ", " )?

@gibson042, done: #4173

@mgol mgol added the Has Pull Request label Nov 3, 2018

@timmywil timmywil modified the milestones: Future, 3.4.0 Nov 26, 2018

@timmywil timmywil closed this in e0d9411 Nov 26, 2018

mgol added a commit to mgol/jquery that referenced this issue Dec 14, 2018

@mgol
Tests: Exclude Android 4.x from repeated headers test 
Android Browser only returns the last value for each header so there's no way
for jQuery get all parts.

Ref jquerygh-3403
Ref jquerygh-4173
1891e50

mgol added a commit to mgol/jquery that referenced this issue Dec 14, 2018

@mgol
Tests: Exclude Android 4.x from repeated header names test 
Android Browser only returns the last value for each header so there's no way
for jQuery get all parts.

Ref jquerygh-3403
Ref jquerygh-4173
68e2135

mgol added a commit to mgol/jquery that referenced this issue Dec 14, 2018

@mgol
Tests: Exclude Android 4.x from repeated header names test 
Android Browser only returns the last value for each header so there's no way
for jQuery get all parts.

Ref jquerygh-3403
Ref jquerygh-4173
8760f1a

@mgol mgol referenced this issue Dec 14, 2018

Merged

Tests: Exclude Android 4.x from repeated header names test #4259

2 of 2 tasks complete

mgol added a commit that referenced this issue Dec 14, 2018

@mgol
Tests: Exclude Android 4.x from repeated header names test 
Android Browser only returns the last value for each header so there's no way
for jQuery get all parts.

Closes gh-4259
Ref gh-3403
Ref gh-4173
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
9cb162f

@GulajavaMinistudio GulajavaMinistudio referenced this issue Dec 15, 2018

Merged

Auto-update dependencies. #70

0 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.