Skip to content

Commit 7428729

Browse files
gibson042dmethvin
authored andcommitted
Fix #11743: Don't mask script errors in jQuery.ajax, closes jquerygh-795.
1 parent 2d37b6c commit 7428729

File tree

5 files changed

+126
-90
lines changed

5 files changed

+126
-90
lines changed

src/ajax.js

Lines changed: 85 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,7 @@ jQuery.extend({
319319
username: null,
320320
password: null,
321321
cache: null,
322+
throws: false,
322323
traditional: false,
323324
headers: {},
324325
*/
@@ -480,6 +481,8 @@ jQuery.extend({
480481
// It is defined here because jslint complains if it is declared
481482
// at the end of the function (which would be more logical and readable)
482483
function done( status, nativeStatusText, responses, headers ) {
484+
var isSuccess, success, error, response, modified,
485+
statusText = nativeStatusText;
483486

484487
// Called once
485488
if ( state === 2 ) {
@@ -504,25 +507,24 @@ jQuery.extend({
504507
// Set readyState
505508
jqXHR.readyState = status > 0 ? 4 : 0;
506509

507-
var isSuccess,
508-
success,
509-
error,
510-
statusText = nativeStatusText,
511-
response = responses ? ajaxHandleResponses( s, jqXHR, responses ) : undefined,
512-
lastModified,
513-
etag;
510+
// Get response data
511+
if ( responses ) {
512+
response = ajaxHandleResponses( s, jqXHR, responses );
513+
}
514514

515515
// If successful, handle type chaining
516516
if ( status >= 200 && status < 300 || status === 304 ) {
517517

518518
// Set the If-Modified-Since and/or If-None-Match header, if in ifModified mode.
519519
if ( s.ifModified ) {
520520

521-
if ( ( lastModified = jqXHR.getResponseHeader( "Last-Modified" ) ) ) {
522-
jQuery.lastModified[ ifModifiedKey ] = lastModified;
521+
modified = jqXHR.getResponseHeader("Last-Modified");
522+
if ( modified ) {
523+
jQuery.lastModified[ ifModifiedKey ] = modified;
523524
}
524-
if ( ( etag = jqXHR.getResponseHeader( "Etag" ) ) ) {
525-
jQuery.etag[ ifModifiedKey ] = etag;
525+
modified = jqXHR.getResponseHeader("Etag");
526+
if ( modified ) {
527+
jQuery.etag[ ifModifiedKey ] = modified;
526528
}
527529
}
528530

@@ -535,15 +537,11 @@ jQuery.extend({
535537
// If we have data
536538
} else {
537539

538-
try {
539-
success = ajaxConvert( s, response );
540-
statusText = "success";
541-
isSuccess = true;
542-
} catch(e) {
543-
// We have a parsererror
544-
statusText = "parsererror";
545-
error = e;
546-
}
540+
isSuccess = ajaxConvert( s, response );
541+
statusText = isSuccess.state;
542+
success = isSuccess.data;
543+
error = isSuccess.error;
544+
isSuccess = !error;
547545
}
548546
} else {
549547
// We extract error from statusText
@@ -913,86 +911,87 @@ function ajaxHandleResponses( s, jqXHR, responses ) {
913911
// Chain conversions given the request and the original response
914912
function ajaxConvert( s, response ) {
915913

914+
var conv, conv2, current, tmp,
915+
// Work with a copy of dataTypes in case we need to modify it for conversion
916+
dataTypes = s.dataTypes.slice(),
917+
prev = dataTypes[ 0 ],
918+
converters = {},
919+
i = 0;
920+
916921
// Apply the dataFilter if provided
917922
if ( s.dataFilter ) {
918923
response = s.dataFilter( response, s.dataType );
919924
}
920925

921-
var dataTypes = s.dataTypes,
922-
converters = {},
923-
i,
924-
key,
925-
length = dataTypes.length,
926-
tmp,
927-
// Current and previous dataTypes
928-
current = dataTypes[ 0 ],
929-
prev,
930-
// Conversion expression
931-
conversion,
932-
// Conversion function
933-
conv,
934-
// Conversion functions (transitive conversion)
935-
conv1,
936-
conv2;
937-
938-
// For each dataType in the chain
939-
for ( i = 1; i < length; i++ ) {
940-
941-
// Create converters map
942-
// with lowercased keys
943-
if ( i === 1 ) {
944-
for ( key in s.converters ) {
945-
if ( typeof key === "string" ) {
946-
converters[ key.toLowerCase() ] = s.converters[ key ];
947-
}
948-
}
926+
// Create converters map with lowercased keys
927+
if ( dataTypes[ 1 ] ) {
928+
for ( conv in s.converters ) {
929+
converters[ conv.toLowerCase() ] = s.converters[ conv ];
949930
}
931+
}
932+
933+
// Convert to each sequential dataType, tolerating list modification
934+
for ( ; (current = dataTypes[++i]); ) {
935+
936+
// There's only work to do if current dataType is non-auto
937+
if ( current !== "*" ) {
938+
939+
// Convert response if prev dataType is non-auto and differs from current
940+
if ( prev !== "*" && prev !== current ) {
941+
942+
// Seek a direct converter
943+
conv = converters[ prev + " " + current ] || converters[ "* " + current ];
950944

951-
// Get the dataTypes
952-
prev = current;
953-
current = dataTypes[ i ];
954-
955-
// If current is auto dataType, update it to prev
956-
if ( current === "*" ) {
957-
current = prev;
958-
// If no auto and dataTypes are actually different
959-
} else if ( prev !== "*" && prev !== current ) {
960-
961-
// Get the converter
962-
conversion = prev + " " + current;
963-
conv = converters[ conversion ] || converters[ "* " + current ];
964-
965-
// If there is no direct converter, search transitively
966-
if ( !conv ) {
967-
conv2 = undefined;
968-
for ( conv1 in converters ) {
969-
tmp = conv1.split( " " );
970-
if ( tmp[ 0 ] === prev || tmp[ 0 ] === "*" ) {
971-
conv2 = converters[ tmp[1] + " " + current ];
972-
if ( conv2 ) {
973-
conv1 = converters[ conv1 ];
974-
if ( conv1 === true ) {
975-
conv = conv2;
976-
} else if ( conv2 === true ) {
977-
conv = conv1;
945+
// If none found, seek a pair
946+
if ( !conv ) {
947+
for ( conv2 in converters ) {
948+
949+
// If conv2 outputs current
950+
tmp = conv2.split(" ");
951+
if ( tmp[ 1 ] === current ) {
952+
953+
// If prev can be converted to accepted input
954+
conv = converters[ prev + " " + tmp[ 0 ] ] ||
955+
converters[ "* " + tmp[ 0 ] ];
956+
if ( conv ) {
957+
// Condense equivalence converters
958+
if ( conv === true ) {
959+
conv = converters[ conv2 ];
960+
961+
// Otherwise, insert the intermediate dataType
962+
} else if ( converters[ conv2 ] !== true ) {
963+
current = tmp[ 0 ];
964+
dataTypes.splice( i--, 0, current );
965+
}
966+
967+
break;
978968
}
979-
break;
969+
}
970+
}
971+
}
972+
973+
// Apply converter (if not an equivalence)
974+
if ( conv !== true ) {
975+
976+
// Unless errors are allowed to bubble, catch and return them
977+
if ( conv && s.throws ) {
978+
response = conv( response );
979+
} else {
980+
try {
981+
response = conv( response );
982+
} catch ( e ) {
983+
return { state: "parsererror", error: conv ? e : "No conversion from " + prev + " to " + current };
980984
}
981985
}
982986
}
983987
}
984-
// If we found no converter, dispatch an error
985-
if ( !( conv || conv2 ) ) {
986-
jQuery.error( "No conversion from " + conversion.replace(" "," to ") );
987-
}
988-
// If found converter is not an equivalence
989-
if ( conv !== true ) {
990-
// Convert with 1 or 2 converters accordingly
991-
response = conv ? conv( response ) : conv2( conv1(response) );
992-
}
988+
989+
// Update prev for next iteration
990+
prev = current;
993991
}
994992
}
995-
return response;
993+
994+
return { state: "success", data: response };
996995
}
997996

998997
})( jQuery );

src/manipulation.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -343,11 +343,12 @@ jQuery.fn.extend({
343343
jQuery.each( scripts, function( i, elem ) {
344344
if ( elem.src ) {
345345
jQuery.ajax({
346-
type: "GET",
347-
global: false,
348346
url: elem.src,
347+
type: "GET",
348+
dataType: "script",
349349
async: false,
350-
dataType: "script"
350+
global: false,
351+
throws: true
351352
});
352353
} else {
353354
jQuery.globalEval( ( elem.text || elem.textContent || elem.innerHTML || "" ).replace( rcleanScript, "" ) );

test/data/badjson.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{bad: 1}
1+
{bad: toTheBone}

test/unit/ajax.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1746,6 +1746,28 @@ test("jQuery.ajax() - malformed JSON", function() {
17461746
});
17471747
});
17481748

1749+
test("jQuery.ajax() - script, throws exception (#11743)", function() {
1750+
expect(1);
1751+
1752+
raises(function() {
1753+
jQuery.ajax({
1754+
url: "data/badjson.js",
1755+
dataType: "script",
1756+
throws: true,
1757+
// TODO find a way to test this asynchronously, too
1758+
async: false,
1759+
// Global events get confused by the exception
1760+
global: false,
1761+
success: function() {
1762+
ok( false, "Success." );
1763+
},
1764+
error: function() {
1765+
ok( false, "Error." );
1766+
}
1767+
});
1768+
}, "exception bubbled" );
1769+
});
1770+
17491771
test("jQuery.ajax() - script by content-type", function() {
17501772
expect(2);
17511773

test/unit/manipulation.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1796,3 +1796,17 @@ test("Ensure oldIE creates a new set on appendTo (#8894)", function() {
17961796
strictEqual( jQuery("<bdi/>").clone().addClass("test").appendTo("<div/>").end().hasClass("test"), false, "Check jQuery.fn.appendTo after clone html5 element" );
17971797
strictEqual( jQuery("<p/>").appendTo("<div/>").end().length, jQuery("<p>test</p>").appendTo("<div/>").end().length, "Elements created with createElement and with createDocumentFragment should be treated alike" );
17981798
});
1799+
1800+
test("html() - script exceptions bubble (#11743)", function() {
1801+
expect(2);
1802+
1803+
raises(function() {
1804+
jQuery("#qunit-fixture").html("<script>undefined(); ok( false, 'error not thrown' );</script>");
1805+
ok( false, "error ignored" );
1806+
}, "exception bubbled from inline script" );
1807+
1808+
raises(function() {
1809+
jQuery("#qunit-fixture").html("<script src='data/badjson.js'></script>");
1810+
ok( false, "error ignored" );
1811+
}, "exception bubbled from remote script" );
1812+
});

0 commit comments

Comments
 (0)