Skip to content

Commit

Permalink
fix($interpolate): always unescape escaped interpolation markers
Browse files Browse the repository at this point in the history
Previously, whenever `mustHaveExpression` was true (e.g. when compiling a text node),
`$interpolate` would not unescape the escaped interpolation markers if there were no actual
interpolation expressions in the same string.
This commit fixes the problem, by always unescaping the escaped markers (if any).

Due to always checking for the presence of escaped interpolation markers, there is a slight
performance hit.

Fixes angular#14196
  • Loading branch information
gkalpak committed Mar 8, 2016
1 parent db281c1 commit a4c3339
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 14 deletions.
37 changes: 23 additions & 14 deletions src/ng/interpolate.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,19 @@ function $InterpolateProvider() {


this.$get = ['$parse', '$exceptionHandler', '$sce', function($parse, $exceptionHandler, $sce) {
var startSymbolLength = startSymbol.length,
var EVERY_CHAR = /./g,
startSymbolLength = startSymbol.length,
endSymbolLength = endSymbol.length,
escapedStartRegexp = new RegExp(startSymbol.replace(/./g, escape), 'g'),
escapedEndRegexp = new RegExp(endSymbol.replace(/./g, escape), 'g');
escapedStartSymbol = startSymbol.replace(EVERY_CHAR, escapeForString),
escapedEndSymbol = endSymbol.replace(EVERY_CHAR, escapeForString),
escapedStartRegexp = new RegExp(startSymbol.replace(EVERY_CHAR, escapeForRegex), 'g'),
escapedEndRegexp = new RegExp(endSymbol.replace(EVERY_CHAR, escapeForRegex), 'g');

function escape(ch) {
function escapeForString(ch) {
return '\\' + ch;
}

function escapeForRegex(ch) {
return '\\\\\\' + ch;
}

Expand Down Expand Up @@ -194,13 +201,6 @@ function $InterpolateProvider() {
* replacing angle brackets (<, >) with < and > respectively, and replacing all
* interpolation start/end markers with their escaped counterparts.**
*
* Escaped interpolation markers are only replaced with the actual interpolation markers in rendered
* output when the $interpolate service processes the text. So, for HTML elements interpolated
* by {@link ng.$compile $compile}, or otherwise interpolated with the `mustHaveExpression` parameter
* set to `true`, the interpolated text must contain an unescaped interpolation expression. As such,
* this is typically useful only when user-data is used in rendering a template from the server, or
* when otherwise untrusted data is used by a directive.
*
* <example>
* <file name="index.html">
* <div ng-init="username='A user'">
Expand Down Expand Up @@ -232,10 +232,13 @@ function $InterpolateProvider() {
* - `context`: evaluation context for all expressions embedded in the interpolated text
*/
function $interpolate(text, mustHaveExpression, trustedContext, allOrNothing) {
var hasEscapedMarkers = text.length &&
(text.indexOf(escapedStartSymbol) !== -1 || text.indexOf(escapedEndSymbol) !== -1);

// Provide a quick exit and simplified result function for text with no interpolation
if (!text.length || text.indexOf(startSymbol) === -1) {
var constantInterp;
if (!mustHaveExpression) {
if (!mustHaveExpression || hasEscapedMarkers) {
var unescapedText = unescapeText(text);
constantInterp = valueFn(unescapedText);
constantInterp.exp = text;
Expand All @@ -257,8 +260,8 @@ function $InterpolateProvider() {
expressionPositions = [];

while (index < textLength) {
if (((startIndex = text.indexOf(startSymbol, index)) != -1) &&
((endIndex = text.indexOf(endSymbol, startIndex + startSymbolLength)) != -1)) {
if (((startIndex = text.indexOf(startSymbol, index)) !== -1) &&
((endIndex = text.indexOf(endSymbol, startIndex + startSymbolLength)) !== -1)) {
if (index !== startIndex) {
concat.push(unescapeText(text.substring(index, startIndex)));
}
Expand Down Expand Up @@ -332,6 +335,12 @@ function $InterpolateProvider() {
});
}
});
} else if (hasEscapedMarkers) {
return extend(valueFn(unescapeText(text)), {
exp: text,
expressions: [],
$$watchDelegate: constantWatchDelegate
});
}

function parseStringifyInterceptor(value) {
Expand Down
41 changes: 41 additions & 0 deletions test/ng/interpolateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,47 @@ describe('$interpolate', function() {
}));


it('should always unescape markers in uninterpolated strings', inject(function($interpolate) {
// Exercise the "quick exit" path
expect($interpolate('\\{\\{foo\\}\\}', false)(obj)).toBe('{{foo}}');
expect($interpolate('\\{\\{foo\\}\\}', true)(obj)).toBe('{{foo}}');

// Exercise the "slow" path, where we can't immediately tell that there are no expressions
expect($interpolate('}}{{\\{\\{foo\\}\\}', false)(obj)).toBe('}}{{{{foo}}');
expect($interpolate('}}{{\\{\\{foo\\}\\}', true)(obj)).toBe('}}{{{{foo}}');
})
);


it('should always unescape custom markers in uninterpolated strings', function() {
module(function($interpolateProvider) {
$interpolateProvider.startSymbol('[[');
$interpolateProvider.endSymbol(']]');
});
inject(function($interpolate) {
// Exercise the "quick exit" path
expect($interpolate('\\[\\[foo\\]\\]', false)(obj)).toBe('[[foo]]');
expect($interpolate('\\[\\[foo\\]\\]', true)(obj)).toBe('[[foo]]');

// Exercise the "slow" path, where we can't immediately tell that there are no expressions
expect($interpolate(']][[\\[\\[foo\\]\\]', false)(obj)).toBe(']][[[[foo]]');
expect($interpolate(']][[\\[\\[foo\\]\\]', true)(obj)).toBe(']][[[[foo]]');
});
});


it('should not interpolate escaped expressions after unescaping',
inject(function($compile, $rootScope) {
var elem = $compile('<div>\\{\\{foo\\}\\}</div>')($rootScope);
$rootScope.foo = 'bar';
$rootScope.$digest();
$rootScope.$digest();

expect(elem.text()).toBe('{{foo}}');
})
);


// This test demonstrates that the web-server is responsible for escaping every single instance
// of interpolation start/end markers in an expression which they do not wish to evaluate,
// because AngularJS will not protect them from being evaluated (due to the added complexity
Expand Down

0 comments on commit a4c3339

Please sign in to comment.