Skip to content

Commit

Permalink
Escape text. Fixes #379.
Browse files Browse the repository at this point in the history 
* 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).
  • Loading branch information
@Krinkle
Krinkle committed Dec 20, 2012
1 parent 94cd3e8 commit 476fb66
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 26 deletions.
71 changes: 45 additions & 26 deletions qunit/qunit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" );
Expand Down Expand Up @@ -142,7 +142,7 @@ Test.prototype = {
var running = id( "qunit-testresult" );

if ( running ) {
running.innerHTML = "Running: <br/>" + this.name;
running.innerHTML = "Running: <br/>" + this.nameHtml;
}

if ( this.async ) {
Expand Down Expand Up @@ -246,7 +246,7 @@ Test.prototype = {

// `b` initialized at top of scope
b = document.createElement( "strong" );
b.innerHTML = this.name + " <b class='counts'>(<b class='failed'>" + bad + "</b>, <b class='passed'>" + good + "</b>, " + this.assertions.length + ")</b>";
b.innerHTML = this.nameHtml + " <b class='counts'>(<b class='failed'>" + bad + "</b>, <b class='passed'>" + good + "</b>, " + this.assertions.length + ")</b>";

addEvent(b, "click", function() {
var next = b.parentNode.lastChild,
Expand Down Expand Up @@ -361,19 +361,19 @@ QUnit = {

test: function( testName, expected, callback, async ) {
var test,
name = "<span class='test-name'>" + escapeInnerText( testName ) + "</span>";
nameHtml = "<span class='test-name'>" + escapeText( testName ) + "</span>";

if ( arguments.length === 2 ) {
callback = expected;
expected = null;
}

if ( config.currentModule ) {
name = "<span class='module-name'>" + config.currentModule + "</span>: " + name;
nameHtml = "<span class='module-name'>" + escapeText( config.currentModule ) + "</span>: " + nameHtml;
}

test = new Test({
name: name,
nameHtml: nameHtml,
testName: testName,
expected: expected,
async: async,
Expand Down Expand Up @@ -485,14 +485,14 @@ assert = {
message: msg
};

msg = escapeInnerText( msg || (result ? "okay" : "failed" ) );
msg = escapeText( msg || (result ? "okay" : "failed" ) );
msg = "<span class='test-message'>" + msg + "</span>";

if ( !result ) {
source = sourceFromStacktrace( 2 );
if ( source ) {
details.source = source;
msg += "<table><tr class='test-source'><th>Source: </th><td><pre>" + escapeInnerText( source ) + "</pre></td></tr></table>";
msg += "<table><tr class='test-source'><th>Source: </th><td><pre>" + escapeText( source ) + "</pre></td></tr></table>";
}
}
runLoggingCallbacks( "log", QUnit, details );
Expand Down Expand Up @@ -774,7 +774,7 @@ extend( QUnit, {

if ( qunit ) {
qunit.innerHTML =
"<h1 id='qunit-header'>" + escapeInnerText( document.title ) + "</h1>" +
"<h1 id='qunit-header'>" + escapeText( document.title ) + "</h1>" +
"<h2 id='qunit-banner'></h2>" +
"<div id='qunit-testrunner-toolbar'></div>" +
"<h2 id='qunit-userAgent'></h2>" +
Expand Down Expand Up @@ -880,13 +880,13 @@ extend( QUnit, {
expected: expected
};

message = escapeInnerText( message ) || ( result ? "okay" : "failed" );
message = escapeText( message ) || ( result ? "okay" : "failed" );
message = "<span class='test-message'>" + message + "</span>";
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 += "<table><tr class='test-expected'><th>Expected: </th><td><pre>" + expected + "</pre></td></tr>";

if ( actual !== expected ) {
Expand All @@ -898,7 +898,7 @@ extend( QUnit, {

if ( source ) {
details.source = source;
output += "<tr class='test-source'><th>Source: </th><td><pre>" + escapeInnerText( source ) + "</pre></td></tr>";
output += "<tr class='test-source'><th>Source: </th><td><pre>" + escapeText( source ) + "</pre></td></tr>";
}

output += "</table>";
Expand All @@ -925,19 +925,19 @@ extend( QUnit, {
message: message
};

message = escapeInnerText( message ) || "error";
message = escapeText( message ) || "error";
message = "<span class='test-message'>" + message + "</span>";
output = message;

output += "<table>";

if ( actual ) {
output += "<tr class='test-actual'><th>Result: </th><td><pre>" + escapeInnerText( actual ) + "</pre></td></tr>";
output += "<tr class='test-actual'><th>Result: </th><td><pre>" + escapeText( actual ) + "</pre></td></tr>";
}

if ( source ) {
details.source = source;
output += "<tr class='test-source'><th>Source: </th><td><pre>" + escapeInnerText( source ) + "</pre></td></tr>";
output += "<tr class='test-source'><th>Source: </th><td><pre>" + escapeText( source ) + "</pre></td></tr>";
}

output += "</table>";
Expand Down Expand Up @@ -1035,14 +1035,24 @@ QUnit.load = function() {
};
}
config[ val.id ] = QUnit.urlParams[ val.id ];
urlConfigHtml += "<input id='qunit-urlconfig-" + val.id + "' name='" + val.id + "' type='checkbox'" + ( config[ val.id ] ? " checked='checked'" : "" ) + " title='" + val.tooltip + "'><label for='qunit-urlconfig-" + val.id + "' title='" + val.tooltip + "'>" + val.label + "</label>";
urlConfigHtml += "<input id='qunit-urlconfig-" + escapeText( val.id ) +
"' name='" + escapeText( val.id ) +
"' type='checkbox'" + ( config[ val.id ] ? " checked='checked'" : "" ) +
" title='" + escapeText( val.tooltip ) +
"'><label for='qunit-urlconfig-" + escapeText( val.id ) +
"' title='" + escapeText( val.tooltip ) + "'>" + val.label + "</label>";
}

moduleFilterHtml += "<label for='qunit-modulefilter'>Module: </label><select id='qunit-modulefilter' name='modulefilter'><option value='' " + ( config.module === undefined ? "selected" : "" ) + ">< All Modules ></option>";
moduleFilterHtml += "<label for='qunit-modulefilter'>Module: </label><select id='qunit-modulefilter' name='modulefilter'><option value='' " +
( config.module === undefined ? "selected='selected'" : "" ) +
">< All Modules ></option>";

for ( i in config.modules ) {
if ( config.modules.hasOwnProperty( i ) ) {
numModules += 1;
moduleFilterHtml += "<option value='" + encodeURIComponent(i) + "' " + ( config.module === i ? "selected" : "" ) + ">" + i + "</option>";
moduleFilterHtml += "<option value='" + escapeText( encodeURIComponent(i) ) + "' " +
( config.module === i ? "selected='selected'" : "" ) +
">" + escapeText(i) + "</option>";
}
}
moduleFilterHtml += "</select>";
Expand Down Expand Up @@ -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 "&amp;";
case "<": return "&lt;";
case ">": return "&gt;";
default: return s;
case '\'':
return '&#039;';
case '"':
return '&quot;';
case '<':
return '&lt;';
case '>':
return '&gt;';
case '&':
return '&amp;';
}
});
}
Expand Down Expand Up @@ -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 );
Expand Down
25 changes: 25 additions & 0 deletions test/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,31 @@ test("module with setup, expect in test call", 2, function() {
ok(true);
});

module("<script id='qunit-unescaped-module'>'module';</script>", {
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("<script id='qunit-unescaped-test'>'test';</script>", 1, function() {
ok(true, "<script id='qunit-unescaped-asassertionsert'>'assertion';</script>");
});

var state;

module("setup/teardown test", {
Expand Down

0 comments on commit 476fb66

Please sign in to comment.