Skip to content

Commit

Permalink
Moved escape regex to modules/Escape.
Browse files Browse the repository at this point in the history
Removed unnecessary escaping in query/createVirtual we parse the dom so
the content has to be trusted already.

Fixed tests.
  • Loading branch information
Kanaye committed Oct 2, 2016
1 parent cc1e93f commit 7e2cf5c
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 61 deletions.
14 changes: 8 additions & 6 deletions src/modules/Escape.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ function () {
'>': '>',
'"': '"',
'\'': ''',
'/': '&x2F;'
'/': '/'
};

var htmlEscapeRegEx = (function () {
Expand All @@ -17,18 +17,20 @@ function () {
return new RegExp('(' + entities.join('|') + ')', 'g');
})();

function internalHTMLEscapeReplacer(entity) {
return htmlEntityMap[entity];
}

var Escape = {
// moved from modules/escapeRegEx
forRegEx: function escapeRegEx(string) {
return string.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, '\\$&');
},
forHTML: function (value) {
if (!blocks.isString(value)) {
return value;
if (blocks.isString(value)) {
return value.replace(htmlEscapeRegEx, internalHTMLEscapeReplacer);
}
return value.replace(htmlEscapeRegEx, function (entity) {
return htmlEntityMap[entity];
});
return value;
},
// This is only valid because jsblocks forces (inserts itself) double quotes for attributes
// don't use this in other cases
Expand Down
6 changes: 3 additions & 3 deletions src/modules/Router.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
define([
'../core',
'./escapeRegEx'
], function (blocks, escapeRegEx) {
'../modules/Escape'
], function (blocks, Escape) {
blocks.route = function (route) {
return Route(route);
};
Expand Down Expand Up @@ -272,7 +272,7 @@
containsParameter = false;
allOptionalBetweenForwardSlash = true;
}
regExString += escapeRegEx(split.string);
regExString += Escape.forRegEx(split.string);
}
});

Expand Down
7 changes: 0 additions & 7 deletions src/modules/escapeRegEx.js

This file was deleted.

6 changes: 3 additions & 3 deletions src/mvc/History.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
define([
'../core',
'../modules/escapeRegEx',
'../modules/Escape',
'../modules/Events',
'../query/addListener'
], function (blocks, escapeRegEx, Events, addListener) {
], function (blocks, Escape, Events, addListener) {
var routeStripper = /^[#\/]|\s+$/g;
var rootStripper = /^\/+|\/+$/g;
var isExplorer = /msie [\w.]+/;
Expand All @@ -27,7 +27,7 @@ define([
this._fragment = this._getFragment();
this._wants = this._options.history === true ? HASH : this._options.history;
this._use = this._wants == PUSH_STATE && (this._history && this._history.pushState) ? PUSH_STATE : HASH;
this._hostRegEx = new RegExp(escapeRegEx(this._location.host));
this._hostRegEx = new RegExp(Escape.forRegEx(this._location.host));
}

History.prototype = {
Expand Down
6 changes: 3 additions & 3 deletions src/query/Expression.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
define([
'../core',
'./var/parameterQueryCache',
'./escapeValue',
'../modules/Escape',
'./ElementsData',
'./Observer'
], function (blocks, parameterQueryCache, escapeValue, ElementsData, Observer) {
], function (blocks, parameterQueryCache, Escape, ElementsData, Observer) {
var Expression = {
Html: 0,
ValueOnly: 2,
Expand Down Expand Up @@ -159,7 +159,7 @@ define([
isObservable = blocks.isObservable(value);
result = isObservable ? value() : value;
result = result == null ? '' : result.toString();
result = escapeValue(result);
result = attributeName ? Escape.forHTMLAttributes(result) : Escape.forHTML(result);

observables = Observer.stopObserving();

Expand Down
8 changes: 4 additions & 4 deletions src/query/VirtualElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ define([
'./var/dataQueryAttr',
'./getClassIndex',
'./setClass',
'./escapeValue',
'./createFragment',
'./dom',
'./Expression',
'./ElementsData'
], function (blocks, hasOwn, keys, Escape, virtualElementIdentity, classAttr, dataIdAttr, dataQueryAttr, getClassIndex, setClass, escapeValue, createFragment, dom,
], function (blocks, hasOwn, keys, Escape, virtualElementIdentity, classAttr, dataIdAttr, dataQueryAttr, getClassIndex, setClass, createFragment, dom,
Expression, ElementsData) {

function VirtualElement(tagName) {
Expand Down Expand Up @@ -91,6 +90,7 @@ define([
}
return this;
}
// TODO strip tags of the html
return this.html();
},

Expand Down Expand Up @@ -158,7 +158,7 @@ define([
if (blocks.isArray(attributeValue)) {
attributeValue = attributeValue.indexOf(value) !== -1 ? 'checked' : null;
} else {
attributeValue = attributeValue ? 'checked' : null;
attributeValue = attributeValue ? 'checked' : null;
}
}
} else if (attributeName == 'disabled') {
Expand Down Expand Up @@ -629,7 +629,7 @@ define([
if (value === '') {
html += ' ' + key;
} else if (value != null) {
html += ' ' + key + '="' + value + '"';
html += ' ' + key + '="' + Escape.forHTMLAttributes(value) + '"';
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/query/createVirtual.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
define([
'../var/trimRegExp',
'./var/dataQueryAttr',
'./escapeValue',
'./browser',
'./Expression',
'./VirtualElement',
'./VirtualComment'
], function (trimRegExp, dataQueryAttr, escapeValue, browser, Expression, VirtualElement, VirtualComment) {
], function (trimRegExp, dataQueryAttr, browser, Expression, VirtualElement, VirtualComment) {
function createVirtual(htmlElement, parentElement) {
var serverData = window.__blocksServerData__;
var elements = [];
Expand Down Expand Up @@ -66,7 +65,7 @@ define([
//if (htmlElement.data.replace(trimRegExp, '').replace(/(\r\n|\n|\r)/gm, '') !== '') {
//
//}
data = escapeValue(htmlElement.data);
data = htmlElement.data;
elements.push(Expression.Create(data, null, htmlElement) || data);
} else if (nodeType == 8) {
// Comment
Expand Down
10 changes: 5 additions & 5 deletions src/query/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ define([
'./animation',
'./createFragment'
], function (blocks, trimRegExp, keys, dataIdAttr, on, browser, setClass, animation, createFragment) {

var dom = blocks.dom = {
valueTagNames: {
input: true,
Expand Down Expand Up @@ -62,13 +62,13 @@ define([

addClass: function (element, className) {
if (element) {
setClass('add', element, className);
setClass('add', element, className);
}
},

removeClass: function (element, className) {
if (element) {
setClass('remove', element, className);
setClass('remove', element, className);
}
},

Expand Down Expand Up @@ -113,7 +113,7 @@ define([

removeAttr: function (element, attributeName) {
if (element && attributeName) {
dom.attr(element, attributeName, null);
dom.attr(element, attributeName, null);
}
},

Expand All @@ -127,7 +127,7 @@ define([
!element) {
return;
}

if (element.nodeType == 8) {
dom.comment.attr(element, attributeName, attributeValue);
return;
Expand Down
12 changes: 0 additions & 12 deletions src/query/escapeValue.js

This file was deleted.

3 changes: 1 addition & 2 deletions src/query/queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ define([
'./var/queries',
'./addListener',
'./getClassIndex',
'./escapeValue',
'./animation',
'./createFragment',
'./createVirtual',
Expand All @@ -16,7 +15,7 @@ define([
'./Expression',
'./VirtualElement',
'./dom'
], function (blocks, slice, trimRegExp, keys, classAttr, queries, addListener, getClassIndex, escapeValue, animation, createFragment, createVirtual,
], function (blocks, slice, trimRegExp, keys, classAttr, queries, addListener, getClassIndex, animation, createFragment, createVirtual,
ElementsData, DomQuery, Expression, VirtualElement, dom) {

/**
Expand Down
25 changes: 12 additions & 13 deletions test/spec/query/expressions.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@
query({
html: html
});

expect($('#testElement')).toHaveText('<div>content</div>');
expect($('#testElement').children().length).toBe(0);

Expand All @@ -220,24 +219,24 @@
expect($('#testElement')).toHaveText('<input />');
expect($('#testElement').children().length).toBe(0);
});

it('multiple observables and non observables are synced correctly', function () {
var observable = blocks.observable(2);

appendText('{{nonObservable}}{{observable}}{{observable}}{{nonObservable}}{{nonObservable}}{{observable}}');

query({
nonObservable: 1,
observable: observable
});

expect($('#testElement')).toHaveText('122112');

observable(3);

expect($('#testElement')).toHaveText('133113');
});

it('multiple observables and non observables (with spaces between them) are synced correctly', function () {
var observable = blocks.observable(2);

Expand Down Expand Up @@ -384,22 +383,22 @@
});

it('attribute is escaped successfully', function () {
var html = blocks.observable('<div>content</div>');
var html = blocks.observable('"><div>content</div>');

setAttr('data-value', '{{html}}');

query({
html: html
});

expect($('#testElement')).toHaveAttr('data-value', '&lt;div>content&lt;/div>');
expect($('#testElement')).toHaveAttr('data-value', '&quot;><div>content</div>');

html('<input />&nbsp;');
html('<"input />&nbsp;');

expect($('#testElement')).toHaveAttr('data-value', '&lt;input />&amp;nbsp;');
expect($('#testElement')).toHaveAttr('data-value', '<&quot;input />&nbsp;');
});
});

describe('array observables', function () {

it('deletes the right number of nodes on reset', function () {
Expand Down

0 comments on commit 7e2cf5c

Please sign in to comment.