Skip to content

Commit

Permalink
Use stricter binding parsing for efficiency and correctness. Fixes #2705
Browse files Browse the repository at this point in the history 
.
  • Loading branch information
@kevinpschaaf
kevinpschaaf committed Nov 18, 2015
1 parent 0345137 commit 04cd184
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 24 deletions.
59 changes: 40 additions & 19 deletions src/lib/annotations/annotations.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
parseAnnotations: function(template) {
var list = [];
var content = template._content || template.content;
this._parseNodeAnnotations(content, list,
this._parseNodeAnnotations(content, list,
template.hasAttribute('strip-whitespace'));
return list;
},
Expand All @@ -89,7 +89,33 @@
this._parseElementAnnotations(node, list, stripWhiteSpace);
},

_bindingRegex: /([^{[]*)(\{\{|\[\[)(?!\}\}|\]\])(.+?)(?:\]\]|\}\})/g,
_bindingRegex: (function() {
var IDENT = '([\\w\\s-_.:$]+)';
// The following lookahead group (?= ... ) and backreference \\4, etc.
// for repeating identifier characters prevent catastrophic
// backtracking when the argument which contains the identifier itself
// is repeated. This approximates possessive/atomic groups which are
// otherwise unsupported in Javascript. Unfortunately this means each
// usage of identifier must be separated to index the backreference
// correctly.
// v..............v v
var PROPERTY = '(?:(?=' + IDENT + ')\\4)';
var METHOD = '(?:(?=' + IDENT + ')\\5)';
var ARG_IDENT = '(?:(?=' + IDENT + ')\\6)';
var NUMBER = '(?:' + '[0-9]+' + ')';
var SQUOTE_STRING = '(?:' + '\'(?:[^\'\\\\]|\\\\\'|\\\\,)*\'' + ')';
var DQUOTE_STRING = '(?:' + '"(?:[^"\\\\]|\\\\"|\\\\,)*"' + ')';
var STRING = '(?:' + SQUOTE_STRING + '|' + DQUOTE_STRING + ')';
var ARGUMENT = '(?:' + ARG_IDENT + '|' + NUMBER + '|' + STRING + ')';
var ARGUMENT_LIST = '(?:(?:' + ARGUMENT + ',?' + ')*)';
var COMPUTED_FUNCTION = '(?:' + METHOD + '\\(' + ARGUMENT_LIST + '\\)' + ')';
var BINDING = '(' + PROPERTY + '|' + COMPUTED_FUNCTION + ')'; // Group 3
var OPEN_BRACKET = '(\\[\\[|{{)'; // Group 1
var CLOSE_BRACKET = '(?:]]|}})';
var NEGATE = '(!?)'; // Group 2
var EXPRESSION = OPEN_BRACKET + NEGATE + BINDING + CLOSE_BRACKET;
return new RegExp(EXPRESSION, "g");
})(),

// TODO(kschaaf): We could modify this to allow an escape mechanism by
// looking for the escape sequence in each of the matches and converting
Expand All @@ -98,29 +124,24 @@
_parseBindings: function(text) {
var re = this._bindingRegex;
var parts = [];
var m, lastIndex;
// Example: "literal1{{binding1}}literal2[[binding2]]final"
var lastIndex = 0;
var m;
// Example: "literal1{{prop}}literal2[[!compute(foo,bar)]]final"
// Regex matches:
// Iteration 1: Iteration 2:
// m[1]: 'literal1' 'literal2'
// m[2]: '{{' '[['
// m[3]: 'binding1' 'binding2'
// 'final' is manually substring'ed from end
// Iteration 1: Iteration 2:
// m[1]: '{{' '[['
// m[2]: '' '!'
// m[3]: 'prop' 'compute(foo,bar)'
while ((m = re.exec(text)) !== null) {
// Add literal part
if (m[1]) {
parts.push({literal: m[1]});
if (m.index > lastIndex) {
parts.push({literal: text.slice(lastIndex, m.index)});
}
// Add binding part
// Mode (one-way or two)
var mode = m[2][0];
var mode = m[1][0];
var negate = Boolean(m[2]);
var value = m[3].trim();
// Negate
var negate = false;
if (value[0] == '!') {
negate = true;
value = value.substring(1).trim();
}
var customEvent, notifyEvent, colon;
if (mode == '{' && (colon = value.indexOf('::')) > 0) {
notifyEvent = value.substring(colon + 2);
Expand Down Expand Up @@ -242,7 +263,7 @@
}
// if this node didn't get evacipated, parse it.
if (node.parentNode) {
var childAnnotation = this._parseNodeAnnotations(node, list,
var childAnnotation = this._parseNodeAnnotations(node, list,
stripWhiteSpace);
if (childAnnotation) {
childAnnotation.parent = annote;
Expand Down
7 changes: 6 additions & 1 deletion test/unit/bind-elements.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,14 @@
no-computed="{{foobared(noInlineComputed)}}"
compoundAttr1$="{{cpnd1}}{{ cpnd2 }}{{cpnd3.prop}}{{ computeCompound(cpnd4, cpnd5, 'literal')}}"
compoundAttr2$="literal1 {{cpnd1}} literal2 {{cpnd2}}{{cpnd3.prop}} literal3 {{computeCompound(cpnd4, cpnd5, 'literal')}} literal4"
compoundAttr3$="{{computeCompound('world', 'username ', 'Hello {0} ')}}"
compoundAttr3$="[yes/no]: {{cpnd1}}, {{computeCompound('world', 'username ', 'Hello {0} ')}}"
>
Test
<!-- Malformed bindings to be ignored -->
{{really.long.identifier.in.malformed.binding.should.be.ignored]}
{{computeFromLiterals(3, 'really.long.literal.in.malformed.binding.should.be.ignored)]}
<!-- Should still parse -->
{{computeFromLiterals(3, 'foo', bool)}}
</div>
<x-prop id="boundProps"
prop1="{{cpnd1}}{{ cpnd2 }}{{cpnd3.prop}}{{ computeCompound(cpnd4, cpnd5, 'literal')}}"
Expand Down
16 changes: 12 additions & 4 deletions test/unit/bind.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -767,11 +767,11 @@
assert.equal(el.$.boundChild.getAttribute('compoundAttr2'), 'literal1 literal2 literal3 literal literal4');
});

test('compound property attribute with {} in text', function() {
test('compound property attribute with {} and [] in text', function() {
var el = document.createElement('x-basic');

assert.equal(el.$.boundChild.getAttribute('compoundAttr3'), 'Hello {0} username world')
})
el.cpnd1 = 'cpnd1';
assert.equal(el.$.boundChild.getAttribute('compoundAttr3'), '[yes/no]: cpnd1, Hello {0} username world');
});

test('compound adjacent textNode bindings', function() {
var el = document.createElement('x-basic');
Expand Down Expand Up @@ -815,6 +815,14 @@
assert.equal(el.$.compound2.textContent.trim(), 'literal1 literal2 literal3 literal literal4');
});

test('malformed bindings ignroed', function() {
var el = document.createElement('x-basic');
el.bool = true;
assert.isTrue(el.$.boundChild.textContent.indexOf('really.long.identifier.in.malformed.binding.should.be.ignored') >= 0, true);
assert.isTrue(el.$.boundChild.textContent.indexOf('really.long.literal.in.malformed.binding.should.be.ignored') >= 0, true);
assert.isTrue(el.$.boundChild.textContent.indexOf('3foo') >= 0, true);
});

});

</script>
Expand Down

0 comments on commit 04cd184

Please sign in to comment.