Skip to content
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

jQuery.ajax: make dataType: "script" not execute scripts on unsuccessful HTTP status in 3.5.x #4655

Closed
redfox05 opened this issue Apr 6, 2020 · 10 comments

Comments

@redfox05
Copy link

@redfox05 redfox05 commented Apr 6, 2020

Description

Bug Report
When doing an ajax request for a non-existing javascript script file, when used with dataType script, instead of failing and triggering the error handler function, it tries to evaluate whatever it gets back from the server, even if server returns a 404 with a HTML error page.

Expected Result

Error handler function is triggered.

Actual Result

Error handler is not triggered, and jQuery tries to evaluate the 404 error document as a javascript file.

This results in a Uncaught SyntaxError: Unexpected token '<'

This happens for both jQuery.ajax dataType script, and jQuery.getScript, which I imagine is because getScript seems to be just a shortcut for ajax with dataType set to script.

Link to test case

This issue was already logged previous and fixed for jQuery 4.0.0. in #4250 but this is still a problem in 3.4.1. Why was this not included as a fix for the 3.x.x versions?

The example issue can be see on StackOverflow, but this is a bug report, not a support issue.
https://stackoverflow.com/questions/61056023/jquery-ajax-unexpected-token-error-on-404-instead-of-normal-ajax-error-handler

Notes

If the fix has to stay with the 4.0.0 release, can we get a clear reason why, and perhaps suggestion for a workaround fix, or can we implement the fix ourselves?

@redfox05 redfox05 changed the title jQuery.ajax: make dataType: "script" not execute scripts on unsuccessful HTTP status in 3.4.x jQuery.ajax: make dataType: "script" not execute scripts on unsuccessful HTTP status in 3.5.x Apr 6, 2020
@mgol
Copy link
Member

@mgol mgol commented Apr 6, 2020

Thanks for the report. It was scheduled for jQuery 4.0.0 as there may be some code out there that depends on executing scripts for 404 responses. That said, it does seem like an edge case & incorrect usage, really, so I'm open to considering backporting it.

We'll discuss this at the core meeting today.

@mgol mgol added Ajax Discuss in Meeting 3.x-only labels Apr 6, 2020
@redfox05
Copy link
Author

@redfox05 redfox05 commented Apr 6, 2020

Thank you for the consideration. Is there any other way an error handler could catch this, or intercept using a pre-execution hook of some sort before the script execution? Then there could be more flexible if as you say, others rely on execution.

Although thinking again, a 404 script couldn't be executed if nothing was returned? Unless the error handling page was for some reason required to be parsed. But then still the error callback should be called before the execution stage or the success? From what I understood, success was only run on a 200 or 20x? So would have thought anything else would trigger error? At least some kind of callback somewhere I would have thought, as none of my error or success callbacks triggered.

Eagerly await the results of your meeting though, and if it does not go ahead, some suggestions on alternatives would be appreciated.

@mgol
Copy link
Member

@mgol mgol commented Apr 6, 2020

@redfox05 We've decided to backport the fix, thanks for making us re-evaluate this!

@mgol mgol removed the Discuss in Meeting label Apr 6, 2020
@mgol mgol added this to the 3.5.0 milestone Apr 6, 2020
@redfox05
Copy link
Author

@redfox05 redfox05 commented Apr 6, 2020

Great thanks for the quick updates!

@redfox05
Copy link
Author

@redfox05 redfox05 commented Apr 6, 2020

@mgol Is there a rough date for 3.5.0? Is it days, weeks or months? Just a rough idea if possible will help for our planning and reminder to upgrade.

@mgol
Copy link
Member

@mgol mgol commented Apr 6, 2020

@redfox05 If nothing unexpected happens, the release should be out within a week.

mgol pushed a commit that referenced this issue Apr 6, 2020
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
Copy link
Member

@mgol mgol commented Apr 6, 2020

Backported to 3.x-stable in da3dd85.

@mgol mgol closed this as completed Apr 6, 2020
@mgol
Copy link
Member

@mgol mgol commented Apr 10, 2020

jQuery 3.5.0 released: https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/

@fschwahn
Copy link

@fschwahn fschwahn commented Jun 4, 2020

FWIW, this change prevents us from updating to jQuery 3.5.x - so we seem to be the odd one out here. What we do is return HTTP 422 in case a validation fails on the server, and re-render the form with validation error messages via the returned script. We think da3dd85 is responsible that this no longer works.

@mgol
Copy link
Member

@mgol mgol commented Jun 6, 2020

@fschwahn Many bug fixes change something around edge cases some people may depend on, unfortunately. You can work around this change by executing the script directly, something like:

$.ajax({
	method: 'GET',
	dataType: 'text',
	url: 'http://jquery.l/test/data/mock.php?action=errorWithScript&withScriptContentType',
}).catch(function (jqXHR) {
	return jqXHR.responseText;
}).then(function (data) {
	$.globalEval(data);
});

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

No branches or pull requests

3 participants