Skip to content

Commit

Permalink
[Glimmer 2] Unskip tests related to inline styles and unsafe hrefs
Browse files Browse the repository at this point in the history
  • Loading branch information
chadhietala committed Jul 17, 2016
1 parent 62d0a35 commit fd367c6
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 16 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"express": "^4.5.0",
"finalhandler": "^0.4.0",
"github": "^0.2.3",
"glimmer-engine": "tildeio/glimmer#b618846",
"glimmer-engine": "tildeio/glimmer#d2feca1",
"glob": "^5.0.13",
"htmlbars": "0.14.24",
"mocha": "^2.4.5",
Expand Down
31 changes: 29 additions & 2 deletions packages/ember-glimmer/lib/environment.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import lookupPartial, { hasPartial } from 'ember-views/system/lookup_partial';
import {
Environment as GlimmerEnvironment,
HelperSyntax
HelperSyntax,
AttributeChangeList,
isSafeString
} from 'glimmer-runtime';
import Dict from 'ember-metal/empty_object';
import { assert } from 'ember-metal/debug';
import { assert, warn } from 'ember-metal/debug';
import { CurlyComponentSyntax, CurlyComponentDefinition } from './syntax/curly-component';
import { DynamicComponentSyntax } from './syntax/dynamic-component';
import { RenderSyntax } from './syntax/render';
import { OutletSyntax } from './syntax/outlet';
import lookupComponent from 'ember-views/utils/lookup-component';
import { STYLE_WARNING } from 'ember-views/system/utils';
import createIterable from './utils/iterable';
import {
ConditionalReference,
Expand Down Expand Up @@ -53,6 +56,22 @@ function buildTextFieldSyntax({ args, templates }, getDefinition) {
return new CurlyComponentSyntax({ args, definition, templates });
}

const StyleAttributeChangeList = {
setAttribute(dom, element, attr, value) {
warn(STYLE_WARNING, (() => {
if (value === null || value === undefined || isSafeString(value)) {
return true;
}
return false;
})(), { id: 'ember-htmlbars.style-xss-warning' });
AttributeChangeList.setAttribute(...arguments);
},

updateAttribute() {
this.setAttribute(...arguments);
}
};

const builtInDynamicComponents = {
input({ key, args, templates }, getDefinition) {
if (args.named.has('type')) {
Expand Down Expand Up @@ -245,6 +264,14 @@ export default class Environment extends GlimmerEnvironment {
return hasPartial(this, name[0]);
}

attributeFor(element, attr, reference, isTrusting) {
if (attr === 'style' && !isTrusting) {
return StyleAttributeChangeList;
}

return super.attributeFor(...arguments);
}

lookupPartial(name) {
let partial = {
template: lookupPartial(this, name[0]).spec
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ moduleFor('Attribute bindings integration', class extends RenderingTest {
}, /You cannot use class as an attributeBinding, use classNameBindings instead./i);
}

['@htmlbars blacklists href bindings based on protocol']() {
['@test blacklists href bindings based on protocol']() {
/* jshint scripturl:true */

let FooBarComponent = Component.extend({
Expand Down
8 changes: 4 additions & 4 deletions packages/ember-glimmer/tests/integration/content-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import EmberObject from 'ember-runtime/system/object';
import ObjectProxy from 'ember-runtime/system/object_proxy';
import { classes } from '../utils/test-helpers';
import { getDebugFunction, setDebugFunction } from 'ember-metal/debug';
import { styleWarning } from 'ember-htmlbars/morphs/attr-morph';
import { STYLE_WARNING } from 'ember-views/system/utils';
import { Component } from '../utils/helpers';
import { SafeString } from 'ember-htmlbars/utils/string';

Expand Down Expand Up @@ -939,7 +939,7 @@ moduleFor('Dynamic content tests (integration)', class extends RenderingTest {
if (!EmberDev.runningProdBuild) {
let warnings, originalWarn;

moduleFor('@htmlbars Inline style tests', class extends RenderingTest {
moduleFor('Inline style tests', class extends RenderingTest {
constructor() {
super(...arguments);
warnings = [];
Expand All @@ -961,7 +961,7 @@ if (!EmberDev.runningProdBuild) {
userValue: 'width: 42px'
});

assert.deepEqual(warnings, [styleWarning]);
assert.deepEqual(warnings, [STYLE_WARNING]);
}

['@test specifying `attributeBindings: ["style"]` generates a warning'](assert) {
Expand All @@ -975,7 +975,7 @@ if (!EmberDev.runningProdBuild) {
userValue: 'width: 42px'
});

assert.deepEqual(warnings, [styleWarning]);
assert.deepEqual(warnings, [STYLE_WARNING]);
}

['@test specifying `<div style={{{userValue}}}></div>` works properly without a warning'](assert) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ moduleFor('Helpers test: {{unbound}}', class extends RenderingTest {
this.assertHTML('<a href="BORK"></a>');
}

['@htmlbars should property escape unsafe hrefs']() {
['@test should property escape unsafe hrefs']() {
let unsafeUrls = emberA([
{
name: 'Bob',
Expand Down
9 changes: 2 additions & 7 deletions packages/ember-htmlbars/lib/morphs/attr-morph.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
import { warn, debugSeal } from 'ember-metal/debug';
import DOMHelper from 'dom-helper';
import isNone from 'ember-metal/is_none';
import { STYLE_WARNING } from 'ember-views/system/utils';

const HTMLBarsAttrMorph = DOMHelper.prototype.AttrMorphClass;

export var styleWarning = '' +
'Binding style attributes may introduce cross-site scripting vulnerabilities; ' +
'please ensure that values being bound are properly escaped. For more information, ' +
'including how to disable this warning, see ' +
'http://emberjs.com/deprecations/v1.x/#toc_binding-style-attributes.';

let proto = HTMLBarsAttrMorph.prototype;

proto.didInit = function() {
Expand All @@ -20,7 +15,7 @@ proto.didInit = function() {

function deprecateEscapedStyle(morph, value) {
warn(
styleWarning,
STYLE_WARNING,
(function(name, value, escaped) {
// SafeString
if (isNone(value) || (value && value.toHTML)) {
Expand Down
6 changes: 6 additions & 0 deletions packages/ember-views/lib/system/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ export function isSimpleClick(event) {
return !modifier && !secondaryClick;
}

export const STYLE_WARNING = '' +
'Binding style attributes may introduce cross-site scripting vulnerabilities; ' +
'please ensure that values being bound are properly escaped. For more information, ' +
'including how to disable this warning, see ' +
'http://emberjs.com/deprecations/v1.x/#toc_binding-style-attributes.';

/**
@private
@method getViewRange
Expand Down

0 comments on commit fd367c6

Please sign in to comment.