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

Jump to bottom

jQuery.ajax: make `dataType: "script"` not execute scripts on unsuccessful HTTP status #4250

Closed
mgol opened this issue Dec 3, 2018 · 5 comments
Closed

jQuery.ajax: make `dataType: "script"` not execute scripts on unsuccessful HTTP status #4250

mgol opened this issue Dec 3, 2018 · 5 comments
Labels
Ajax Behavior Change
Milestone
3.5.0

Comments

@mgol
Copy link
Member

@mgol mgol commented Dec 3, 2018

Description

A followup of PR #4243 and an extension of issue #4126. We should stop evaluating scripts returned with an unsuccessful HTTP status when loaded via jQuery.ajax with dataType: "script". The fix in #4243 only targets DOM manipulation.

Link to test case
@mgol mgol added this to the 4.0.0 milestone Dec 3, 2018
@mgol mgol changed the title jQuery.ajax: make `dataType: "script"` not execute scripts on unsuccessfull HTTP status jQuery.ajax: make `dataType: "script"` not execute scripts on unsuccessful HTTP status Dec 3, 2018
@mgol mgol removed the Needs review label Dec 3, 2018
skrobinson added a commit to skrobinson/jquery that referenced this issue Apr 19, 2019
@skrobinson
Ajax: Do not execute scripts on unsuccessful HTTP status
62d48b6 
The content returned in an error response may not be valid Javascript.  I
first saw this when trying to retrieve a missing script file, the server
replied with HTTP 404 and an HTML page detailing the error.  So the error
which appeared on the console was "SyntaxError: expected expression,
got '<'" because the browser was attempting to parse the error page as
a script file.

Fixes jquerygh-4250

Signed-off-by: Sean Robinson <sean.robinson@scottsdalecc.edu>
skrobinson added a commit to skrobinson/jquery that referenced this issue Apr 19, 2019
@skrobinson
Ajax: Do not execute scripts on unsuccessful HTTP status
6716b75 
The content returned in an error response may not be valid Javascript.  I
first saw this when trying to retrieve a missing script file, the server
replied with HTTP 404 and an HTML page detailing the error.  So the error
which appeared on the console was "SyntaxError: expected expression,
got '<'" because the browser was attempting to parse the error page as
a script file.

Fixes jquerygh-4250

Signed-off-by: Sean Robinson <sean.robinson@scottsdalecc.edu>
@skrobinson
Copy link
Contributor

@skrobinson skrobinson commented Apr 19, 2019

I have a possible fix and a test case in commit 6716b75.
@mgol
Copy link
Member Author

@mgol mgol commented Apr 23, 2019

@skrobinson Your patch skips converters completely for unsuccessful responses; this is not right, it doesn't pass our test suite.

The proper fix will most likely require generalizing what was done in #4243 so that the logic is applied to all requests with dataType: "script".
@mgol mgol added the Needs review label Apr 25, 2019
skrobinson added a commit to skrobinson/jquery that referenced this issue Apr 26, 2019
@skrobinson
Ajax: Use noop converter for unsuccessful HTTP responses
Loading status checks…
2cb0639 
Unsuccessful responses may not contain text convertible to the intended
type (e.g. json, script, etc.) using the normal converter.  The two added
test cases only check for `dataType: "script"`.

Fixes jquerygh-4250

Signed-off-by: Sean Robinson <sean.robinson@scottsdalecc.edu>
@skrobinson
Copy link
Contributor

@skrobinson skrobinson commented Apr 26, 2019

@mgol Thank you for your feedback. I've re-read #4243 and #4258 and tried again using the pattern in #4258. I've also improved my testing and have a new attempt at a fix.
@mgol
Copy link
Member Author

@mgol mgol commented Apr 27, 2019

@skrobinson Let us know if you need any help/direction! (or if you don't have time to submit a PR so that we know we should look at it ourselves)
@mgol mgol removed the Needs review label Apr 27, 2019
@skrobinson skrobinson mentioned this issue Apr 30, 2019
Closed
2 of 4 tasks complete
skrobinson added a commit to skrobinson/jquery that referenced this issue Jun 12, 2019
@skrobinson
Add failing JSONP service
4f0a90f 
While trying to improve my fix for jquerygh-4250, I needed a failing service.
This will be parked on a side branch until it is ever needed.  Which may
be never.

Signed-off-by: Sean Robinson <sean.robinson@scottsdalecc.edu>
skrobinson added a commit to skrobinson/jquery that referenced this issue Sep 6, 2019
@skrobinson
Ajax: Use noop converter for unsuccessful HTTP responses
f336363 
Unsuccessful responses may not contain text convertible to the intended
type (e.g. json, script, etc.) using the normal converter.  The two added
test cases only check for `dataType: "script"`.

Fixes jquerygh-4250

Signed-off-by: Sean Robinson <sean.robinson@scottsdalecc.edu>
skrobinson added a commit to skrobinson/jquery that referenced this issue Sep 6, 2019
@skrobinson
Ajax: Use noop converter for unsuccessful HTTP responses
8d2ee2e 
Unsuccessful responses may not contain text convertible to the intended
type (e.g. json, script, etc.) using the normal converter.  The two added
test cases only check for `dataType: "script"`.

Fixes jquerygh-4250

Signed-off-by: Sean Robinson <sean.robinson@scottsdalecc.edu>
@mgol mgol closed this in 50871a5 Sep 26, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 27, 2020
mgol added a commit that referenced this issue Apr 6, 2020
@skrobinson @mgol
Ajax: Do not execute scripts for unsuccessful HTTP responses
Loading status checks…
da3dd85 
The script transport used to evaluate fetched script sources which is
undesirable for unsuccessful HTTP responses. This is different to other data
types where such a convention was fine (e.g. in case of JSON).

(cherry picked from 50871a5)

Fixes gh-4250
Fixes gh-4655
Closes gh-4379
@mgol mgol modified the milestones: 4.0.0, 3.5.0 Apr 6, 2020
@mgol
Copy link
Member Author

@mgol mgol commented Apr 6, 2020

Landed on master in 50871a5 last year & now backported to 3.x-stable in da3dd85.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Ajax Behavior Change
Projects
None yet
Milestone
3.5.0
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
@mgol @skrobinson