JSONP and parsing responseText upon 409 Conflict #4771

Activity

1. mgol commented on Aug 18, 2020

   

   mgol

   on Aug 18, 2020

   Member

   More actions

   Thanks for the report.

   We considered #4379 to be a bug fix but JSONP is a bit special, indeed - under the hood it's a script but it simulates JSON responses in an environment without a CORS setup and sending JSON payloads on error responses is quite typical there. Maybe we need to revisit the JSONP case.

   It'd help if you could provide a minimal test case or - even better - a test case for the jQuery test suite, something like

   jquery/test/unit/ajax.js

   Lines 809 to 922 in 82b87f6

   	ajaxTest( "jQuery.ajax() - do not execute scripts from unsuccessful responses (gh-4250)", 11, function( assert ) {
   	var globalEval = jQuery.globalEval;
   	var failConverters = {
   	"text script": function() {
   	assert.ok( false, "No converter for unsuccessful response" );
   	}
   	};
   	function request( title, options ) {
   	var testMsg = title + ": expected file missing status";
   	return jQuery.extend( {
   	beforeSend: function() {
   	jQuery.globalEval = function() {
   	assert.ok( false, "Should not eval" );
   	};
   	},
   	complete: function() {
   	jQuery.globalEval = globalEval;
   	},
   	// error is the significant assertion
   	error: function( xhr ) {
   	assert.strictEqual( xhr.status, 404, testMsg );
   	},
   	success: function() {
   	assert.ok( false, "Unanticipated success" );
   	}
   	}, options );
   	}
   	return [
   	request(
   	"HTML reply",
   	{
   	url: url( "404.txt" )
   	}
   	),
   	request(
   	"HTML reply with dataType",
   	{
   	dataType: "script",
   	url: url( "404.txt" )
   	}
   	),
   	request(
   	"script reply",
   	{
   	url: url( "mock.php?action=errorWithScript&withScriptContentType" )
   	}
   	),
   	request(
   	"non-script reply",
   	{
   	url: url( "mock.php?action=errorWithScript" )
   	}
   	),
   	request(
   	"script reply with dataType",
   	{
   	dataType: "script",
   	url: url( "mock.php?action=errorWithScript&withScriptContentType" )
   	}
   	),
   	request(
   	"non-script reply with dataType",
   	{
   	dataType: "script",
   	url: url( "mock.php?action=errorWithScript" )
   	}
   	),
   	request(
   	"script reply with converter",
   	{
   	converters: failConverters,
   	url: url( "mock.php?action=errorWithScript&withScriptContentType" )
   	}
   	),
   	request(
   	"non-script reply with converter",
   	{
   	converters: failConverters,
   	url: url( "mock.php?action=errorWithScript" )
   	}
   	),
   	request(
   	"script reply with converter and dataType",
   	{
   	converters: failConverters,
   	dataType: "script",
   	url: url( "mock.php?action=errorWithScript&withScriptContentType" )
   	}
   	),
   	request(
   	"non-script reply with converter and dataType",
   	{
   	converters: failConverters,
   	dataType: "script",
   	url: url( "mock.php?action=errorWithScript" )
   	}
   	),
   	request(
   	"JSONP reply with dataType",
   	{
   	dataType: "jsonp",
   	url: url( "mock.php?action=errorWithScript" ),
   	beforeSend: function() {
   	jQuery.globalEval = function( response ) {
   	assert.ok( /"status": 404, "msg": "Not Found"/.test( response ), "Error object returned" );
   	};
   	}
   	}
   	)
   	];
   	} );

   but for JSONP & a different expected result. Would you want to try that?
2. mgol

   added

   Discuss in MeetingReserved for Issues and PRs that anyone would like to discuss in the weekly meeting.Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting.

   Ajax

   on Aug 18, 2020
3. fras2560 commented on Aug 18, 2020

   

   fras2560

   on Aug 18, 2020

   ContributorAuthor

   More actions

   @mgol Thanks for the quick response. I will give it a try to make a test case or at the very least a minimal test case. I hope I correctly described the situation but will confirm for sure (bit complicated with Backbone, Server, and JQuery).
4. mgol commented on Aug 18, 2020

   

   mgol

   on Aug 18, 2020 via email

   Member

   More actions

   It’d help if the test case didn’t contain external libraries. If this is just about JSONP in error scenarios, any 4xx or 5xx status code should work for the test case. Thanks! -- Michał Gołębiowski-Owczarek
5. fras2560 commented on Aug 20, 2020

   

   fras2560

   on Aug 20, 2020

   ContributorAuthor

   More actions

   @mgol I think I have fix for the issue and wrote up a simple test. However, I am not too familar with PHP so I might need some help with this. I am not even sure how to run it locally. Also, need some help with response body

   #4773
6. mgol

   added

   Has Pull Request

   on Aug 20, 2020
7. mgol

   added this to the 3.6.0 milestone on Aug 22, 2020
8. timmywil

   removed

   Discuss in MeetingReserved for Issues and PRs that anyone would like to discuss in the weekly meeting.Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting.

   on Aug 24, 2020
9. mgol

   closed this as completedin #4773on Aug 25, 2020
10. mgol

    added 4 commits that reference this issue on Aug 25, 2020

    Ajax: Execute JSONP error script responses

    ...

    This issue will close once commit a1e619b is merged into the 'main' branch.

    a1e619b

    Ajax: Execute JSONP error script responses

    ...

    This issue will close once commit 3bae54a is merged into the 'main' branch.

    3bae54a

    Ajax: Make responseJSON work for erroneous same-domain JSONP requests

    ...

    This issue will close once commit fc13a09 is merged into the 'main' branch.

    fc13a09

    Ajax: Make responseJSON work for erroneous same-domain JSONP requests

    ...

    This issue will close once commit 68b4ec5 is merged into the 'main' branch.

    68b4ec5
11. jquery

    deleted a comment from on Feb 2, 2025
12. github-actions

    locked as resolved and limited conversation to collaborators on Sep 24, 2025