Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat($interpolate): enable escaping interpolated expressions #7511

Closed
wants to merge 2 commits into from
Closed

feat($interpolate): enable escaping interpolated expressions #7511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2965129
feat($interpolate): enable escaping interpolated expressions
caitp May 19, 2014
3fd7836
fix($interpolate): don't unescape incomplete interpolation escape mar…
caitp May 19, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 111 additions & 30 deletions src/ng/interpolate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ var $interpolateMinErr = minErr('$interpolate');
function $InterpolateProvider() {
var startSymbol = '{{';
var endSymbol = '}}';
var escapedStartSymbol = '{{{{';
var escapedEndSymbol = '}}}}';

/**
* @ngdoc method
Expand All @@ -49,11 +51,15 @@ function $InterpolateProvider() {
* Symbol to denote start of expression in the interpolated string. Defaults to `{{`.
*
* @param {string=} value new value to set the starting symbol to.
* @param {string=} escaped new value to set the escaped starting symbol to.
* @returns {string|self} Returns the symbol when used as getter and self if used as setter.
*/
this.startSymbol = function(value){
this.startSymbol = function(value, escaped){
if (value) {
startSymbol = value;
if (escaped) {
escapedStartSymbol = escaped;
}
return this;
} else {
return startSymbol;
Expand All @@ -69,9 +75,12 @@ function $InterpolateProvider() {
* @param {string=} value new value to set the ending symbol to.
* @returns {string|self} Returns the symbol when used as getter and self if used as setter.
*/
this.endSymbol = function(value){
this.endSymbol = function(value, escaped){
if (value) {
endSymbol = value;
if (escaped) {
escapedEndSymbol = escaped;
}
return this;
} else {
return endSymbol;
Expand All @@ -81,7 +90,33 @@ function $InterpolateProvider() {

this.$get = ['$parse', '$exceptionHandler', '$sce', function($parse, $exceptionHandler, $sce) {
var startSymbolLength = startSymbol.length,
endSymbolLength = endSymbol.length;
endSymbolLength = endSymbol.length,
escapedStartLength = escapedStartSymbol.length,
escapedEndLength = escapedEndSymbol.length,
escapedLength = escapedStartLength + escapedEndLength,
ESCAPED_EXPR_REGEXP = new RegExp(lit(escapedStartSymbol) + '((?:.|\n)*?)' + lit(escapedEndSymbol), 'm'),
EXPR_REGEXP = new RegExp(lit(startSymbol) + '((?:.|\n)*?)' + lit(endSymbol), 'm');

function lit(str) {
return str.replace(/([\(\)\[\]\{\}\+\\\^\$\.\!\?\*\=\:\|\-])/g, function(op) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

escaping regexp operators, so that you don't get unexpected surprises/syntax errors with markers like [[/]] or (((/)))

return '\\' + op;
});
}

function Piece(text, isExpr) {
this.text = text;
this.isExpr = isExpr;
}

function addPiece(text, isExpr, pieces) {
var lastPiece = pieces.length ? pieces[pieces.length - 1] : null;
if (!isExpr && lastPiece && !lastPiece.isExpr) {
lastPiece.text += text;
} else {
pieces.push(new Piece(text, isExpr));
}
return pieces[pieces.length-1];
}

/**
* @ngdoc service
Expand Down Expand Up @@ -152,34 +187,84 @@ function $InterpolateProvider() {
textLength = text.length,
hasInterpolation = false,
hasText = false,
exp,
concat = [],
lastValuesCache = { values: {}, results: {}};

while(index < textLength) {
if ( ((startIndex = text.indexOf(startSymbol, index)) != -1) &&
((endIndex = text.indexOf(endSymbol, startIndex + startSymbolLength)) != -1) ) {
if (index !== startIndex) hasText = true;
separators.push(text.substring(index, startIndex));
exp = text.substring(startIndex + startSymbolLength, endIndex);
expressions.push(exp);
parseFns.push($parse(exp));
index = endIndex + endSymbolLength;
exp = text,
concat,
lastValuesCache = { values: {}, results: {}},
pieces = [],
piece,
indexes = [],
last;

while (text.length) {
var expr = EXPR_REGEXP.exec(text);
var escaped = ESCAPED_EXPR_REGEXP.exec(text);
var until = text.length;
var chunk;
var from = 0;
var start;

if (!escaped && expr) {
index = 0;
from = expr.index;
while ((start = text.indexOf(escapedStartSymbol, index)) >= 0 && start <= from) {
index = until = start + escapedStartLength;
expr = null;
}
}

if (expr) {
until = expr.index;
}

if (escaped && escaped.index <= until) {
from = escaped.index;
until = escaped.index + escaped[0].length;
escaped = startSymbol + escaped[1] + endSymbol;
expr = null;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this block is hit, the escaped expression precedes the non-escaped expression (if any), so it should be ignored. unescaping is a simple string concatenation

}

if (until > 0) {
chunk = isString(escaped) ? text.substring(0, from) + escaped : text.substring(0, until);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this block is hit, it means there was text prior to an expression (escaped or otherwise), or the text begins with an escaped expression. So, if there was an escaped expression, it gets included with the preceeding text, otherwise the text is grabbed until the next marker.

This is used to add a separator piece

text = text.slice(until);
var newChunk = addPiece(chunk, false, pieces);
if (last && !last.isExpr && separators.length) {
separators[separators.length - 1] += chunk;
} else {
separators.push(chunk);
}
last = newChunk;
newChunk = null;
hasText = true;
} else {
separators.push('');
}

if (expr) {
text = text.slice(expr[0].length);
last = addPiece(expr[1], true, pieces);
expr = null;
hasInterpolation = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally, if an expression is found, it gets added to the set of expressions

}
}

concat = new Array(pieces.length);
for (index = 0; index < pieces.length; ++index) {
piece = pieces[index];
if (piece.isExpr) {
parseFns.push($parse(piece.text));
expressions.push(piece.text);
indexes.push(index);
} else {
// 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;
concat[index] = piece.text;
}
}

if (separators.length === expressions.length) {
separators.push('');
}

last = pieces = null;

// Concatenating expressions makes it hard to reason about whether some combination of
// concatenated values are unsafe to use and could easily lead to XSS. By requiring that a
// single expression be used for iframe[src], object[src], etc., we ensure that the value
Expand All @@ -190,18 +275,14 @@ function $InterpolateProvider() {
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);
"required. See http://docs.angularjs.org/api/ng.$sce", exp);
}

if (!mustHaveExpression || hasInterpolation) {
concat.length = separators.length + expressions.length;

var compute = function(values) {
for(var i = 0, ii = expressions.length; i < ii; i++) {
concat[2*i] = separators[i];
concat[(2*i)+1] = values[i];
concat[indexes[i]] = values[i];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

concatenation is much simpler now, it's simply replacing the old values with new ones. Yes, currently this does use an array of indices rather than clever math, but I think it's easier to reason about it this way, and it is less work. Plus, short of not having existing tests fail, there's no reason to append an extra empty string to the end of separators, or to have separators at all

}
concat[2*ii] = separators[ii];
return concat.join('');
};

Expand Down Expand Up @@ -278,15 +359,15 @@ function $InterpolateProvider() {
lastValuesCache.results[scopeId] = lastResult = compute(values);
}
} catch(err) {
var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text,
var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", exp,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since text got clobbered, it needed to be replaced with the original value. This could be refactored to not clobber text, and there may be a slight performance increase, but I think it would also be more complex code.

err.toString());
$exceptionHandler(newErr);
}

return lastResult;
}, {
// all of these properties are undocumented for now
exp: text, //just for compatibility with regular watchers created via $watch
exp: exp, //just for compatibility with regular watchers created via $watch
separators: separators,
expressions: expressions
});
Expand Down
48 changes: 48 additions & 0 deletions test/ng/interpolateSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,54 @@ describe('$interpolate', function() {
}));


describe('interpolation escaping', function() {
var obj;

beforeEach(function() {
obj = {foo: 'Hello', bar: 'World'};
});

it('should support escaping interpolation signs', inject(function($interpolate) {
expect($interpolate('{{foo}} {{{{bar}}}}')(obj)).toBe('Hello {{bar}}');
expect($interpolate('{{{{foo}}}} {{bar}}')(obj)).toBe('{{foo}} World');
}));


it('should unescape multiple expressions', inject(function($interpolate) {
expect($interpolate('{{{{foo}}}}{{{{bar}}}} {{foo}}')(obj)).toBe('{{foo}}{{bar}} Hello');
}));


it('should support customizing escape signs', function() {
module(function($interpolateProvider) {
$interpolateProvider.startSymbol('{{', '[[');
$interpolateProvider.endSymbol('}}', ']]');
});
inject(function($interpolate) {
expect($interpolate('{{foo}} [[bar]]')(obj)).toBe('Hello {{bar}}');
});
});

it('should support customizing escape signs which contain interpolation signs', function() {
module(function($interpolateProvider) {
$interpolateProvider.startSymbol('{{', '-{{-');
$interpolateProvider.endSymbol('}}', '-}}-');
});
inject(function($interpolate) {
expect($interpolate('{{foo}} -{{-bar-}}-')(obj)).toBe('Hello {{bar}}');
});
});


it('should not unescape incomplete escape expressions', inject(function($interpolate) {
expect($interpolate('{{{{foo{{foo}}')(obj)).toBe('{{{{fooHello');
expect($interpolate('}}}}foo{{foo}}')(obj)).toBe('}}}}fooHello');
expect($interpolate('foo{{foo}}{{{{')(obj)).toBe('fooHello{{{{');
expect($interpolate('foo{{foo}}}}}}')(obj)).toBe('fooHello}}}}');
}));
});


describe('interpolating in a trusted context', function() {
var sce;
beforeEach(function() {
Expand Down