Permalink
Browse files

refactor($interpolate): split .parts into .expressions and .separators

BREAKING CHANGE: the function returned by $interpolate
no longer has a `.parts` array set on it.
It has been replaced by two arrays:
* `.expressions`, an array of the expressions in the
  interpolated text. The expressions are parsed with
  $parse, with an extra layer converting them to strings
  when computed
* `.separators`, an array of strings representing the
  separations between interpolations in the text.
  This array is **always** 1 item longer than the
  `.expressions` array for easy merging with it
  • Loading branch information...
1 parent 21f9316 commit 88c2193c71954b9e7e7e4bdf636a2b168d36300d @rodyhaddad rodyhaddad committed with IgorMinar Mar 6, 2014
Showing with 118 additions and 80 deletions.
  1. +61 −37 src/ng/interpolate.js
  2. +2 −2 src/ngScenario/Scenario.js
  3. +55 −41 test/ng/interpolateSpec.js
View
@@ -125,32 +125,36 @@ function $InterpolateProvider() {
var startIndex,
endIndex,
index = 0,
- parts = [],
- length = text.length,
+ separators = [],
+ expressions = [],
+ textLength = text.length,
hasInterpolation = false,
+ hasText = false,
fn,
exp,
concat = [];
- while(index < length) {
+ while(index < textLength) {
if ( ((startIndex = text.indexOf(startSymbol, index)) != -1) &&
((endIndex = text.indexOf(endSymbol, startIndex + startSymbolLength)) != -1) ) {
- (index != startIndex) && parts.push(text.substring(index, startIndex));
- parts.push(fn = $parse(exp = text.substring(startIndex + startSymbolLength, endIndex)));
+ if (index !== startIndex) hasText = true;
+ separators.push(text.substring(index, startIndex));
+ expressions.push(fn = interpolateParse(exp = text.substring(startIndex + startSymbolLength, endIndex)));
fn.exp = exp;
index = endIndex + endSymbolLength;
hasInterpolation = true;
} else {
- // we did not find anything, so we have to add the remainder to the parts array
- (index != length) && parts.push(text.substring(index));
- index = length;
+ // we did not find an interpolation, so we have to add the remainder to the separators array
+ if (index !== textLength) {
+ hasText = true;
+ separators.push(text.substring(index));
+ }
+ break;
}
}
- if (!(length = parts.length)) {
- // we added, nothing, must have been an empty string.
- parts.push('');
- length = 1;
+ if (separators.length === expressions.length) {
+ separators.push('');
}
// Concatenating expressions makes it hard to reason about whether some combination of
@@ -159,44 +163,64 @@ function $InterpolateProvider() {
// that's used is assigned or constructed by some JS code somewhere that is more testable or
// make it obvious that you bound the value to some user controlled value. This helps reduce
// the load when auditing for XSS issues.
- if (trustedContext && parts.length > 1) {
+ if (trustedContext && hasInterpolation && (hasText || expressions.length > 1)) {
throw $interpolateMinErr('noconcat',
"Error while interpolating: {0}\nStrict Contextual Escaping disallows " +
"interpolations that concatenate multiple expressions when a trusted value is " +
"required. See http://docs.angularjs.org/api/ng.$sce", text);
}
- if (!mustHaveExpression || hasInterpolation) {
- concat.length = length;
+ if (!mustHaveExpression || hasInterpolation) {
+ concat.length = separators.length + expressions.length;
+ var computeFn = function (values, context) {
+ for(var i = 0, ii = expressions.length; i < ii; i++) {
+ concat[2*i] = separators[i];
+ concat[(2*i)+1] = values ? values[i] : expressions[i](context);
+ }
+ concat[2*ii] = separators[ii];
+ return concat.join('');
+ };
+
fn = function(context) {
+ return computeFn(null, context);
+ };
+ fn.exp = text;
+
+ // hack in order to preserve existing api
+ fn.$$invoke = function (listener) {
+ return function (values, oldValues, scope) {
+ var current = computeFn(values, scope);
+ listener(current, this.$$lastInter == null ? current : this.$$lastInter, scope);
+ this.$$lastInter = current;
+ };
+ };
+ fn.separators = separators;
+ fn.expressions = expressions;
+ return fn;
+ }
+
+ function interpolateParse(expression) {
+ var exp = $parse(expression);
+ return function (scope) {
try {
- for(var i = 0, ii = length, part; i<ii; i++) {
- if (typeof (part = parts[i]) == 'function') {
- part = part(context);
- if (trustedContext) {
- part = $sce.getTrusted(trustedContext, part);
- } else {
- part = $sce.valueOf(part);
- }
- if (part === null || isUndefined(part)) {
- part = '';
- } else if (typeof part != 'string') {
- part = toJson(part);
- }
- }
- concat[i] = part;
+ var value = exp(scope);
+ if (trustedContext) {
+ value = $sce.getTrusted(trustedContext, value);
+ } else {
+ value = $sce.valueOf(value);
}
- return concat.join('');
- }
- catch(err) {
+ if (value === null || isUndefined(value)) {
+ value = '';
+ } else if (typeof value != 'string') {
+ value = toJson(value);
+ }
+ return value;
+ } catch(err) {
var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text,
- err.toString());
+ err.toString());
$exceptionHandler(newErr);
}
};
- fn.exp = text;
- fn.parts = parts;
- return fn;
}
}
@@ -314,8 +314,8 @@ _jQuery.fn.bindings = function(windowJquery, bindExp) {
}
for(var fns, j=0, jj=binding.length; j<jj; j++) {
fns = binding[j];
- if (fns.parts) {
- fns = fns.parts;
+ if (fns.expressions) {
+ fns = fns.expressions;
} else {
fns = [fns];
}
@@ -123,77 +123,84 @@ describe('$interpolate', function() {
}));
it('should not get confused with same markers', inject(function($interpolate) {
- expect($interpolate('---').parts).toEqual(['---']);
+ expect($interpolate('---').separators).toEqual(['---']);
+ expect($interpolate('---').expressions).toEqual([]);
expect($interpolate('----')()).toEqual('');
expect($interpolate('--1--')()).toEqual('1');
}));
});
-
describe('parseBindings', function() {
it('should Parse Text With No Bindings', inject(function($interpolate) {
- var parts = $interpolate("a").parts;
- expect(parts.length).toEqual(1);
- expect(parts[0]).toEqual("a");
+ expect($interpolate("a").separators).toEqual(['a']);
+ expect($interpolate("a").expressions).toEqual([]);
}));
it('should Parse Empty Text', inject(function($interpolate) {
- var parts = $interpolate("").parts;
- expect(parts.length).toEqual(1);
- expect(parts[0]).toEqual("");
+ expect($interpolate("").separators).toEqual(['']);
+ expect($interpolate("").expressions).toEqual([]);
}));
it('should Parse Inner Binding', inject(function($interpolate) {
- var parts = $interpolate("a{{b}}C").parts;
- expect(parts.length).toEqual(3);
- expect(parts[0]).toEqual("a");
- expect(parts[1].exp).toEqual("b");
- expect(parts[1]({b:123})).toEqual(123);
- expect(parts[2]).toEqual("C");
+ var interpolateFn = $interpolate("a{{b}}C"),
+ separators = interpolateFn.separators, expressions = interpolateFn.expressions;
+ expect(separators).toEqual(['a', 'C']);
+ expect(expressions.length).toEqual(1);
+ expect(expressions[0].exp).toEqual('b');
+ expect(expressions[0]({b:123})).toEqual('123');
}));
it('should Parse Ending Binding', inject(function($interpolate) {
- var parts = $interpolate("a{{b}}").parts;
- expect(parts.length).toEqual(2);
- expect(parts[0]).toEqual("a");
- expect(parts[1].exp).toEqual("b");
- expect(parts[1]({b:123})).toEqual(123);
+ var interpolateFn = $interpolate("a{{b}}"),
+ separators = interpolateFn.separators, expressions = interpolateFn.expressions;
+ expect(separators).toEqual(['a', '']);
+ expect(expressions.length).toEqual(1);
+ expect(expressions[0].exp).toEqual('b');
+ expect(expressions[0]({b:123})).toEqual('123');
}));
it('should Parse Begging Binding', inject(function($interpolate) {
- var parts = $interpolate("{{b}}c").parts;
- expect(parts.length).toEqual(2);
- expect(parts[0].exp).toEqual("b");
- expect(parts[1]).toEqual("c");
+ var interpolateFn = $interpolate("{{b}}c"),
+ separators = interpolateFn.separators, expressions = interpolateFn.expressions;
+ expect(separators).toEqual(['', 'c']);
+ expect(expressions.length).toEqual(1);
+ expect(expressions[0].exp).toEqual('b');
+ expect(expressions[0]({b:123})).toEqual('123');
}));
it('should Parse Loan Binding', inject(function($interpolate) {
- var parts = $interpolate("{{b}}").parts;
- expect(parts.length).toEqual(1);
- expect(parts[0].exp).toEqual("b");
+ var interpolateFn = $interpolate("{{b}}"),
+ separators = interpolateFn.separators, expressions = interpolateFn.expressions;
+ expect(separators).toEqual(['', '']);
+ expect(expressions.length).toEqual(1);
+ expect(expressions[0].exp).toEqual('b');
+ expect(expressions[0]({b:123})).toEqual('123');
}));
it('should Parse Two Bindings', inject(function($interpolate) {
- var parts = $interpolate("{{b}}{{c}}").parts;
- expect(parts.length).toEqual(2);
- expect(parts[0].exp).toEqual("b");
- expect(parts[1].exp).toEqual("c");
+ var interpolateFn = $interpolate("{{b}}{{c}}"),
+ separators = interpolateFn.separators, expressions = interpolateFn.expressions;
+ expect(separators).toEqual(['', '', '']);
+ expect(expressions.length).toEqual(2);
+ expect(expressions[0].exp).toEqual('b');
+ expect(expressions[1].exp).toEqual('c');
}));
it('should Parse Two Bindings With Text In Middle', inject(function($interpolate) {
- var parts = $interpolate("{{b}}x{{c}}").parts;
- expect(parts.length).toEqual(3);
- expect(parts[0].exp).toEqual("b");
- expect(parts[1]).toEqual("x");
- expect(parts[2].exp).toEqual("c");
+ var interpolateFn = $interpolate("{{b}}x{{c}}"),
+ separators = interpolateFn.separators, expressions = interpolateFn.expressions;
+ expect(separators).toEqual(['', 'x', '']);
+ expect(expressions.length).toEqual(2);
+ expect(expressions[0].exp).toEqual('b');
+ expect(expressions[1].exp).toEqual('c');
}));
it('should Parse Multiline', inject(function($interpolate) {
- var parts = $interpolate('"X\nY{{A\n+B}}C\nD"').parts;
- expect(parts.length).toEqual(3);
- expect(parts[0]).toEqual('"X\nY');
- expect(parts[1].exp).toEqual('A\n+B');
- expect(parts[2]).toEqual('C\nD"');
+ var interpolateFn = $interpolate('"X\nY{{A\n+B}}C\nD"'),
+ separators = interpolateFn.separators, expressions = interpolateFn.expressions;
+ expect(separators).toEqual(['"X\nY', 'C\nD"']);
+ expect(expressions.length).toEqual(1);
+ expect(expressions[0].exp).toEqual('A\n+B');
}));
});
@@ -208,6 +215,12 @@ describe('$interpolate', function() {
"Contextual Escaping disallows interpolations that concatenate multiple expressions " +
"when a trusted value is required. See http://docs.angularjs.org/api/ng.$sce");
expect(function() {
+ $interpolate('{{var}}/constant', true, isTrustedContext);
+ }).toThrowMinErr(
+ "$interpolate", "noconcat", "Error while interpolating: {{var}}/constant\nStrict " +
+ "Contextual Escaping disallows interpolations that concatenate multiple expressions " +
+ "when a trusted value is required. See http://docs.angularjs.org/api/ng.$sce");
+ expect(function() {
$interpolate('{{foo}}{{bar}}', true, isTrustedContext);
}).toThrowMinErr(
"$interpolate", "noconcat", "Error while interpolating: {{foo}}{{bar}}\nStrict " +
@@ -248,7 +261,8 @@ describe('$interpolate', function() {
});
inject(function($interpolate) {
- expect($interpolate('---').parts).toEqual(['---']);
+ expect($interpolate('---').separators).toEqual(['---']);
+ expect($interpolate('---').expressions).toEqual([]);
expect($interpolate('----')()).toEqual('');
expect($interpolate('--1--')()).toEqual('1');
});

0 comments on commit 88c2193

Please sign in to comment.