Skip to content

Commit

Permalink
[BUGFIX beta] Remove DOM Helper monkeypatches
Browse files Browse the repository at this point in the history
The DOM helper used to require a specially patched version of
`protocolForURL` and `setMorphHTML` to work in Node. This commit:

1) Adds a test to verify attributes are correctly sanitized in Node
2) Removes the monkeypatches
  • Loading branch information
tomdale committed Dec 8, 2015
1 parent 3771c93 commit a914541
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 24 deletions.
4 changes: 2 additions & 2 deletions npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"finalhandler": "^0.4.0",
"github": "^0.2.3",
"glob": "~4.3.2",
"htmlbars": "0.14.7",
"htmlbars": "0.14.8",
"qunit-extras": "^1.4.0",
"qunitjs": "^1.19.0",
"route-recognizer": "0.1.5",
Expand Down
30 changes: 30 additions & 0 deletions tests/.jshintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"esnext": false,
"node" : true,
"browser" : false,

"boss" : true,
"curly": false,
"debug": false,
"devel": false,
"eqeqeq": true,
"evil": true,
"forin": false,
"immed": false,
"laxbreak": false,
"newcap": true,
"noarg": true,
"noempty": false,
"nonew": false,
"nomen": false,
"onevar": false,
"plusplus": false,
"regexp": false,
"undef": true,
"sub": true,
"strict": false,
"white": false,
"eqnull": true,
"trailing": true,
"unused": "vars"
}
14 changes: 0 additions & 14 deletions tests/node/helpers/app-module.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ var emberPath = path.join(distPath, 'ember.debug.cjs');
var templateCompilerPath = path.join(distPath, 'ember-template-compiler');
var features = require(path.join(__dirname, '../../../features.json')).features;
var SimpleDOM = require('simple-dom');
var URL = require('url');

/*
* This helper sets up a QUnit test module with all of the environment and
Expand Down Expand Up @@ -108,19 +107,6 @@ module.exports = function(moduleName) {
this.routes = registerRoutes;
this.registry = {};
this.renderToHTML = renderToHTML;

// TODO: REMOVE ME

// Patch DOMHelper
Ember.HTMLBars.DOMHelper.prototype.protocolForURL = function(url) {
var protocol = URL.parse(url).protocol;
return (protocol == null) ? ':' : protocol;
};

Ember.HTMLBars.DOMHelper.prototype.setMorphHTML = function(morph, html) {
var section = this.document.createRawHTMLSection(html);
morph.setNode(section);
};
},

afterEach: function() {
Expand Down
32 changes: 25 additions & 7 deletions tests/node/visit-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ function assertHTMLMatches(assert, actualHTML, expectedHTML) {
function handleError(assert) {
return function(error) {
assert.ok(false, error.stack);
}
};
}

// This is based on what fastboot-server does
Expand Down Expand Up @@ -133,6 +133,24 @@ if (appModule.canRunTests) {
]);
});

QUnit.test('FastBoot: attributes are sanitized', function(assert) {
this.template('application', '<a href={{test}}></a>');

this.controller('application', {
/*jshint scripturl:true*/
test: 'javascript:alert("hello")'
});

var App = this.createApplication();

return RSVP.all([
fastbootVisit(App, '/').then(
assertFastbootResult(assert, { url: '/', body: '<a href="unsafe:javascript:alert\\(&quot;hello&quot;\\)"></a>' }),
handleError(assert)
)
]);
});

QUnit.test('FastBoot: route error', function(assert) {
this.routes(function() {
this.route('a');
Expand All @@ -159,7 +177,7 @@ if (appModule.canRunTests) {
return RSVP.all([
fastbootVisit(App, '/a').then(
function(instance) {
assert.ok(false, 'It should not render')
assert.ok(false, 'It should not render');
instance.destroy();
},
function(error) {
Expand All @@ -168,7 +186,7 @@ if (appModule.canRunTests) {
),
fastbootVisit(App, '/b').then(
function(instance) {
assert.ok(false, 'It should not render')
assert.ok(false, 'It should not render');
instance.destroy();
},
function(error) {
Expand Down Expand Up @@ -203,7 +221,7 @@ if (appModule.canRunTests) {
return this.network.fetch('/a');
},
afterModel: function() {
this.replaceWith('b')
this.replaceWith('b');
}
});

Expand All @@ -212,7 +230,7 @@ if (appModule.canRunTests) {
return this.network.fetch('/b');
},
afterModel: function() {
this.replaceWith('c')
this.replaceWith('c');
}
});

Expand All @@ -227,7 +245,7 @@ if (appModule.canRunTests) {
return this.network.fetch('/d');
},
afterModel: function() {
this.replaceWith('e')
this.replaceWith('e');
}
});

Expand Down Expand Up @@ -283,4 +301,4 @@ if (appModule.canRunTests) {
assert.strictEqual(xFooInstances, 0, 'it should not create any x-foo components');
});
});
}
}

0 comments on commit a914541

Please sign in to comment.