From: Richard Gibson Date: Thu, 31 May 2012 15:31:13 +0000 (-0700) Subject: Fix #11743: Don't mask script errors in jQuery.ajax, closes gh-795. X-Git-Tag: 1.8b1~92 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=742872984e000ff8e13b9a23e74852d1b549f161;p=jquery.git Fix #11743: Don't mask script errors in jQuery.ajax, closes gh-795. --- diff --git a/src/ajax.js b/src/ajax.js index 80e3b11df..c402f2153 100644 --- a/src/ajax.js +++ b/src/ajax.js @@ -319,6 +319,7 @@ jQuery.extend({ username: null, password: null, cache: null, + throws: false, traditional: false, headers: {}, */ @@ -480,6 +481,8 @@ jQuery.extend({ // It is defined here because jslint complains if it is declared // at the end of the function (which would be more logical and readable) function done( status, nativeStatusText, responses, headers ) { + var isSuccess, success, error, response, modified, + statusText = nativeStatusText; // Called once if ( state === 2 ) { @@ -504,13 +507,10 @@ jQuery.extend({ // Set readyState jqXHR.readyState = status > 0 ? 4 : 0; - var isSuccess, - success, - error, - statusText = nativeStatusText, - response = responses ? ajaxHandleResponses( s, jqXHR, responses ) : undefined, - lastModified, - etag; + // Get response data + if ( responses ) { + response = ajaxHandleResponses( s, jqXHR, responses ); + } // If successful, handle type chaining if ( status >= 200 && status < 300 || status === 304 ) { @@ -518,11 +518,13 @@ jQuery.extend({ // Set the If-Modified-Since and/or If-None-Match header, if in ifModified mode. if ( s.ifModified ) { - if ( ( lastModified = jqXHR.getResponseHeader( "Last-Modified" ) ) ) { - jQuery.lastModified[ ifModifiedKey ] = lastModified; + modified = jqXHR.getResponseHeader("Last-Modified"); + if ( modified ) { + jQuery.lastModified[ ifModifiedKey ] = modified; } - if ( ( etag = jqXHR.getResponseHeader( "Etag" ) ) ) { - jQuery.etag[ ifModifiedKey ] = etag; + modified = jqXHR.getResponseHeader("Etag"); + if ( modified ) { + jQuery.etag[ ifModifiedKey ] = modified; } } @@ -535,15 +537,11 @@ jQuery.extend({ // If we have data } else { - try { - success = ajaxConvert( s, response ); - statusText = "success"; - isSuccess = true; - } catch(e) { - // We have a parsererror - statusText = "parsererror"; - error = e; - } + isSuccess = ajaxConvert( s, response ); + statusText = isSuccess.state; + success = isSuccess.data; + error = isSuccess.error; + isSuccess = !error; } } else { // We extract error from statusText @@ -913,86 +911,87 @@ function ajaxHandleResponses( s, jqXHR, responses ) { // Chain conversions given the request and the original response function ajaxConvert( s, response ) { + var conv, conv2, current, tmp, + // Work with a copy of dataTypes in case we need to modify it for conversion + dataTypes = s.dataTypes.slice(), + prev = dataTypes[ 0 ], + converters = {}, + i = 0; + // Apply the dataFilter if provided if ( s.dataFilter ) { response = s.dataFilter( response, s.dataType ); } - var dataTypes = s.dataTypes, - converters = {}, - i, - key, - length = dataTypes.length, - tmp, - // Current and previous dataTypes - current = dataTypes[ 0 ], - prev, - // Conversion expression - conversion, - // Conversion function - conv, - // Conversion functions (transitive conversion) - conv1, - conv2; - - // For each dataType in the chain - for ( i = 1; i < length; i++ ) { - - // Create converters map - // with lowercased keys - if ( i === 1 ) { - for ( key in s.converters ) { - if ( typeof key === "string" ) { - converters[ key.toLowerCase() ] = s.converters[ key ]; - } - } + // Create converters map with lowercased keys + if ( dataTypes[ 1 ] ) { + for ( conv in s.converters ) { + converters[ conv.toLowerCase() ] = s.converters[ conv ]; } + } + + // Convert to each sequential dataType, tolerating list modification + for ( ; (current = dataTypes[++i]); ) { + + // There's only work to do if current dataType is non-auto + if ( current !== "*" ) { + + // Convert response if prev dataType is non-auto and differs from current + if ( prev !== "*" && prev !== current ) { + + // Seek a direct converter + conv = converters[ prev + " " + current ] || converters[ "* " + current ]; - // Get the dataTypes - prev = current; - current = dataTypes[ i ]; - - // If current is auto dataType, update it to prev - if ( current === "*" ) { - current = prev; - // If no auto and dataTypes are actually different - } else if ( prev !== "*" && prev !== current ) { - - // Get the converter - conversion = prev + " " + current; - conv = converters[ conversion ] || converters[ "* " + current ]; - - // If there is no direct converter, search transitively - if ( !conv ) { - conv2 = undefined; - for ( conv1 in converters ) { - tmp = conv1.split( " " ); - if ( tmp[ 0 ] === prev || tmp[ 0 ] === "*" ) { - conv2 = converters[ tmp[1] + " " + current ]; - if ( conv2 ) { - conv1 = converters[ conv1 ]; - if ( conv1 === true ) { - conv = conv2; - } else if ( conv2 === true ) { - conv = conv1; + // If none found, seek a pair + if ( !conv ) { + for ( conv2 in converters ) { + + // If conv2 outputs current + tmp = conv2.split(" "); + if ( tmp[ 1 ] === current ) { + + // If prev can be converted to accepted input + conv = converters[ prev + " " + tmp[ 0 ] ] || + converters[ "* " + tmp[ 0 ] ]; + if ( conv ) { + // Condense equivalence converters + if ( conv === true ) { + conv = converters[ conv2 ]; + + // Otherwise, insert the intermediate dataType + } else if ( converters[ conv2 ] !== true ) { + current = tmp[ 0 ]; + dataTypes.splice( i--, 0, current ); + } + + break; } - break; + } + } + } + + // Apply converter (if not an equivalence) + if ( conv !== true ) { + + // Unless errors are allowed to bubble, catch and return them + if ( conv && s.throws ) { + response = conv( response ); + } else { + try { + response = conv( response ); + } catch ( e ) { + return { state: "parsererror", error: conv ? e : "No conversion from " + prev + " to " + current }; } } } } - // If we found no converter, dispatch an error - if ( !( conv || conv2 ) ) { - jQuery.error( "No conversion from " + conversion.replace(" "," to ") ); - } - // If found converter is not an equivalence - if ( conv !== true ) { - // Convert with 1 or 2 converters accordingly - response = conv ? conv( response ) : conv2( conv1(response) ); - } + + // Update prev for next iteration + prev = current; } } - return response; + + return { state: "success", data: response }; } })( jQuery ); diff --git a/src/manipulation.js b/src/manipulation.js index 2c1f0d2b3..a6b8edc6f 100644 --- a/src/manipulation.js +++ b/src/manipulation.js @@ -343,11 +343,12 @@ jQuery.fn.extend({ jQuery.each( scripts, function( i, elem ) { if ( elem.src ) { jQuery.ajax({ - type: "GET", - global: false, url: elem.src, + type: "GET", + dataType: "script", async: false, - dataType: "script" + global: false, + throws: true }); } else { jQuery.globalEval( ( elem.text || elem.textContent || elem.innerHTML || "" ).replace( rcleanScript, "" ) ); diff --git a/test/data/badjson.js b/test/data/badjson.js index ec41ee5d6..ad2de7a39 100644 --- a/test/data/badjson.js +++ b/test/data/badjson.js @@ -1 +1 @@ -{bad: 1} +{bad: toTheBone} diff --git a/test/unit/ajax.js b/test/unit/ajax.js index 62677697b..878b311eb 100644 --- a/test/unit/ajax.js +++ b/test/unit/ajax.js @@ -1746,6 +1746,28 @@ test("jQuery.ajax() - malformed JSON", function() { }); }); +test("jQuery.ajax() - script, throws exception (#11743)", function() { + expect(1); + + raises(function() { + jQuery.ajax({ + url: "data/badjson.js", + dataType: "script", + throws: true, + // TODO find a way to test this asynchronously, too + async: false, + // Global events get confused by the exception + global: false, + success: function() { + ok( false, "Success." ); + }, + error: function() { + ok( false, "Error." ); + } + }); + }, "exception bubbled" ); +}); + test("jQuery.ajax() - script by content-type", function() { expect(2); diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js index a6f418522..0e419a8bb 100644 --- a/test/unit/manipulation.js +++ b/test/unit/manipulation.js @@ -1796,3 +1796,17 @@ test("Ensure oldIE creates a new set on appendTo (#8894)", function() { strictEqual( jQuery("").clone().addClass("test").appendTo("
").end().hasClass("test"), false, "Check jQuery.fn.appendTo after clone html5 element" ); strictEqual( jQuery("

").appendTo("

").end().length, jQuery("

test

").appendTo("
").end().length, "Elements created with createElement and with createDocumentFragment should be treated alike" ); }); + +test("html() - script exceptions bubble (#11743)", function() { + expect(2); + + raises(function() { + jQuery("#qunit-fixture").html(""); + ok( false, "error ignored" ); + }, "exception bubbled from inline script" ); + + raises(function() { + jQuery("#qunit-fixture").html(""); + ok( false, "error ignored" ); + }, "exception bubbled from remote script" ); +});