From 476fb66cd8a1883e52b009968fce14b53e5c616d Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Thu, 20 Dec 2012 03:11:26 +0100 Subject: [PATCH] Escape text. Fixes #379. * Rename escapeInnerText to escapeText and include singe quotes and double quotes. * Consistent use of `bool="bool"` instead of `bool`. * Add regression tests for unescaped names in module, test and assertion. * Escape everything that does not explicitly support HTML (even urlConfig.id for example). In DOM methods text is default and HTML needs parsing. But when writing HTML it is the opposite: text needs escaping and html is default. So for security we need to reverse that and ensure we're escaping stuff by default. * Rename Test..name to Test..nameHtml (because it is). --- qunit/qunit.js | 71 ++++++++++++++++++++++++++++++++------------------ test/test.js | 25 ++++++++++++++++++ 2 files changed, 70 insertions(+), 26 deletions(-) diff --git a/qunit/qunit.js b/qunit/qunit.js index 4f6b03cec..a96e44949 100644 --- a/qunit/qunit.js +++ b/qunit/qunit.js @@ -69,7 +69,7 @@ Test.prototype = { if ( tests ) { b = document.createElement( "strong" ); - b.innerHTML = this.name; + b.innerHTML = this.nameHtml; // `a` initialized at top of scope a = document.createElement( "a" ); @@ -142,7 +142,7 @@ Test.prototype = { var running = id( "qunit-testresult" ); if ( running ) { - running.innerHTML = "Running:
" + this.name; + running.innerHTML = "Running:
" + this.nameHtml; } if ( this.async ) { @@ -246,7 +246,7 @@ Test.prototype = { // `b` initialized at top of scope b = document.createElement( "strong" ); - b.innerHTML = this.name + " (" + bad + ", " + good + ", " + this.assertions.length + ")"; + b.innerHTML = this.nameHtml + " (" + bad + ", " + good + ", " + this.assertions.length + ")"; addEvent(b, "click", function() { var next = b.parentNode.lastChild, @@ -361,7 +361,7 @@ QUnit = { test: function( testName, expected, callback, async ) { var test, - name = "" + escapeInnerText( testName ) + ""; + nameHtml = "" + escapeText( testName ) + ""; if ( arguments.length === 2 ) { callback = expected; @@ -369,11 +369,11 @@ QUnit = { } if ( config.currentModule ) { - name = "" + config.currentModule + ": " + name; + nameHtml = "" + escapeText( config.currentModule ) + ": " + nameHtml; } test = new Test({ - name: name, + nameHtml: nameHtml, testName: testName, expected: expected, async: async, @@ -485,14 +485,14 @@ assert = { message: msg }; - msg = escapeInnerText( msg || (result ? "okay" : "failed" ) ); + msg = escapeText( msg || (result ? "okay" : "failed" ) ); msg = "" + msg + ""; if ( !result ) { source = sourceFromStacktrace( 2 ); if ( source ) { details.source = source; - msg += "
Source: 
" + escapeInnerText( source ) + "
"; + msg += "
Source: 
" + escapeText( source ) + "
"; } } runLoggingCallbacks( "log", QUnit, details ); @@ -774,7 +774,7 @@ extend( QUnit, { if ( qunit ) { qunit.innerHTML = - "

" + escapeInnerText( document.title ) + "

" + + "

" + escapeText( document.title ) + "

" + "

" + "
" + "

" + @@ -880,13 +880,13 @@ extend( QUnit, { expected: expected }; - message = escapeInnerText( message ) || ( result ? "okay" : "failed" ); + message = escapeText( message ) || ( result ? "okay" : "failed" ); message = "" + message + ""; output = message; if ( !result ) { - expected = escapeInnerText( QUnit.jsDump.parse(expected) ); - actual = escapeInnerText( QUnit.jsDump.parse(actual) ); + expected = escapeText( QUnit.jsDump.parse(expected) ); + actual = escapeText( QUnit.jsDump.parse(actual) ); output += ""; if ( actual !== expected ) { @@ -898,7 +898,7 @@ extend( QUnit, { if ( source ) { details.source = source; - output += ""; + output += ""; } output += "
Expected: 
" + expected + "
Source: 
" + escapeInnerText( source ) + "
Source: 
" + escapeText( source ) + "
"; @@ -925,19 +925,19 @@ extend( QUnit, { message: message }; - message = escapeInnerText( message ) || "error"; + message = escapeText( message ) || "error"; message = "" + message + ""; output = message; output += ""; if ( actual ) { - output += ""; + output += ""; } if ( source ) { details.source = source; - output += ""; + output += ""; } output += "
Result: 
" + escapeInnerText( actual ) + "
Result: 
" + escapeText( actual ) + "
Source: 
" + escapeInnerText( source ) + "
Source: 
" + escapeText( source ) + "
"; @@ -1035,14 +1035,24 @@ QUnit.load = function() { }; } config[ val.id ] = QUnit.urlParams[ val.id ]; - urlConfigHtml += ""; + urlConfigHtml += ""; } - moduleFilterHtml += ""; + for ( i in config.modules ) { if ( config.modules.hasOwnProperty( i ) ) { numModules += 1; - moduleFilterHtml += ""; + moduleFilterHtml += ""; } } moduleFilterHtml += ""; @@ -1335,17 +1345,27 @@ function sourceFromStacktrace( offset ) { } } -function escapeInnerText( s ) { +/** + * Escape text for attribute or text content. + */ +function escapeText( s ) { if ( !s ) { return ""; } s = s + ""; - return s.replace( /[\&<>]/g, function( s ) { + // Both single quotes and double quotes (for attributes) + return s.replace( /['"<>&]/g, function( s ) { switch( s ) { - case "&": return "&"; - case "<": return "<"; - case ">": return ">"; - default: return s; + case '\'': + return '''; + case '"': + return '"'; + case '<': + return '<'; + case '>': + return '>'; + case '&': + return '&'; } }); } @@ -1502,7 +1522,6 @@ function registerLoggingCallback( key ) { // Supports deprecated method of completely overwriting logging callbacks function runLoggingCallbacks( key, scope, args ) { - //debugger; var i, callbacks; if ( QUnit.hasOwnProperty( key ) ) { QUnit[ key ].call(scope, args ); diff --git a/test/test.js b/test/test.js index e896f9d73..5b2e16dd9 100644 --- a/test/test.js +++ b/test/test.js @@ -96,6 +96,31 @@ test("module with setup, expect in test call", 2, function() { ok(true); }); +module("", { + setup: function() { + }, + teardown: function() { + // We can't use ok(false) inside script tags since some browsers + // don't evaluate script tags inserted through innerHTML after domready. + // Counting them before/after doesn't cover everything either as qunit-modulefilter + // is created before any test is ran. So use ids instead. + if (document.getElementById('qunit-unescaped-module')) { + // This can either be from in #qunit-modulefilter or #qunit-testresult + ok(false, 'Unscaped module name'); + } + if (document.getElementById('qunit-unescaped-test')) { + ok(false, 'Unscaped test name'); + } + if (document.getElementById('qunit-unescaped-assertion')) { + ok(false, 'Unscaped test name'); + } + } +}); + +test("", 1, function() { + ok(true, ""); +}); + var state; module("setup/teardown test", {