Skip to content

Commit 63c1d8f

Browse files
committed
fix(compiler): don't report parse error for interpolation inside string in property binding
Currently we check whether a property binding contains an interpolation using a regex so that we can throw an error. The problem is that the regex doesn't account for quotes which means that something like `[prop]="'{{ foo }}'"` will be considered an error, even though it's not actually an interpolation. These changes build on top of the logic from angular#39826 to account for interpolation characters inside quotes. Fixes angular#39601.
1 parent 362f45c commit 63c1d8f

3 files changed

Lines changed: 75 additions & 50 deletions

File tree

‎packages/compiler/src/expression_parser/parser.ts‎

Lines changed: 52 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
import * as chars from '../chars';
1010
import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config';
11-
import {escapeRegExp} from '../util';
1211

1312
import {AbsoluteSourceSpan, AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, ExpressionBinding, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralMapKey, LiteralPrimitive, MethodCall, NonNullAssert, ParserError, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, ThisReceiver, Unary, VariableBinding} from './ast';
1413
import {EOF, isIdentifier, isQuote, Lexer, Token, TokenType} from './lexer';
@@ -30,20 +29,6 @@ export class TemplateBindingParseResult {
3029
public errors: ParserError[]) {}
3130
}
3231

33-
const defaultInterpolateRegExp = _createInterpolateRegExp(DEFAULT_INTERPOLATION_CONFIG);
34-
function _getInterpolateRegExp(config: InterpolationConfig): RegExp {
35-
if (config === DEFAULT_INTERPOLATION_CONFIG) {
36-
return defaultInterpolateRegExp;
37-
} else {
38-
return _createInterpolateRegExp(config);
39-
}
40-
}
41-
42-
function _createInterpolateRegExp(config: InterpolationConfig): RegExp {
43-
const pattern = escapeRegExp(config.start) + '([\\s\\S]*?)' + escapeRegExp(config.end);
44-
return new RegExp(pattern, 'g');
45-
}
46-
4732
export class Parser {
4833
private errors: ParserError[] = [];
4934

@@ -247,7 +232,7 @@ export class Parser {
247232
// parse from starting {{ to ending }} while ignoring content inside quotes.
248233
const fullStart = i;
249234
const exprStart = fullStart + interpStart.length;
250-
const exprEnd = this._getExpressiondEndIndex(input, interpEnd, exprStart);
235+
const exprEnd = this._getInterpolationEndIndex(input, interpEnd, exprStart);
251236
if (exprEnd === -1) {
252237
// Could not find the end of the interpolation; do not parse an expression.
253238
// Instead we should extend the content on the last raw string.
@@ -315,59 +300,76 @@ export class Parser {
315300
return null;
316301
}
317302

318-
private _checkNoInterpolation(
319-
input: string, location: string, interpolationConfig: InterpolationConfig): void {
320-
const regexp = _getInterpolateRegExp(interpolationConfig);
321-
const parts = input.split(regexp);
322-
if (parts.length > 1) {
303+
private _checkNoInterpolation(input: string, location: string, {start, end}: InterpolationConfig):
304+
void {
305+
let startIndex = -1;
306+
let endIndex = -1;
307+
308+
this._forEachUnquotedChar(input, 0, charIndex => {
309+
if (startIndex === -1) {
310+
startIndex = input.indexOf(start, charIndex);
311+
return false;
312+
} else {
313+
endIndex = this._getInterpolationEndIndex(input, end, charIndex);
314+
return endIndex > -1;
315+
}
316+
});
317+
318+
if (startIndex > -1 && endIndex > -1) {
323319
this._reportError(
324-
`Got interpolation (${interpolationConfig.start}${
325-
interpolationConfig.end}) where expression was expected`,
326-
input,
327-
`at column ${this._findInterpolationErrorColumn(parts, 1, interpolationConfig)} in`,
328-
location);
320+
`Got interpolation (${start}${end}) where expression was expected`, input,
321+
`at column ${startIndex} in`, location);
329322
}
330323
}
331324

332-
private _findInterpolationErrorColumn(
333-
parts: string[], partInErrIdx: number, interpolationConfig: InterpolationConfig): number {
334-
let errLocation = '';
335-
for (let j = 0; j < partInErrIdx; j++) {
336-
errLocation += j % 2 === 0 ?
337-
parts[j] :
338-
`${interpolationConfig.start}${parts[j]}${interpolationConfig.end}`;
339-
}
325+
/**
326+
* Finds the index of the end of an interpolation expression
327+
* while ignoring comments and quoted content.
328+
*/
329+
private _getInterpolationEndIndex(input: string, expressionEnd: string, start: number): number {
330+
let result = -1;
331+
332+
this._forEachUnquotedChar(input, start, charIndex => {
333+
if (input.startsWith(expressionEnd, charIndex)) {
334+
result = charIndex;
335+
return true;
336+
}
337+
// Nothing else in the expression matters after we've
338+
// hit a comment so look directly for the end token.
339+
if (input.startsWith('//', charIndex)) {
340+
result = input.indexOf(expressionEnd, charIndex);
341+
return true;
342+
}
343+
return false;
344+
});
340345

341-
return errLocation.length;
346+
return result;
342347
}
343348

344349
/**
345-
* Finds the index of the end of an interpolation expression
346-
* while ignoring comments and quoted content.
350+
* Invokes a callback function for each character of a string that is outside
351+
* of a quote.
352+
* @param input String to loop through.
353+
* @param start Index within the string at which to start.
354+
* @param callback Callback to be invoked for each index. If the callback
355+
* returns `true`, the loop will be broken off.
347356
*/
348-
private _getExpressiondEndIndex(input: string, expressionEnd: string, start: number): number {
357+
private _forEachUnquotedChar(input: string, start: number, callback: (index: number) => boolean):
358+
void {
349359
let currentQuote: string|null = null;
350360
let escapeCount = 0;
351361
for (let i = start; i < input.length; i++) {
352362
const char = input[i];
353-
// Skip the characters inside quotes. Note that we only care about the
354-
// outer-most quotes matching up and we need to account for escape characters.
363+
// Skip the characters inside quotes. Note that we only care about the outer-most
364+
// quotes matching up and we need to account for escape characters.
355365
if (isQuote(input.charCodeAt(i)) && (currentQuote === null || currentQuote === char) &&
356366
escapeCount % 2 === 0) {
357367
currentQuote = currentQuote === null ? char : null;
358-
} else if (currentQuote === null) {
359-
if (input.startsWith(expressionEnd, i)) {
360-
return i;
361-
}
362-
// Nothing else in the expression matters after we've
363-
// hit a comment so look directly for the end token.
364-
if (input.startsWith('//', i)) {
365-
return input.indexOf(expressionEnd, i);
366-
}
368+
} else if (currentQuote === null && callback(i)) {
369+
break;
367370
}
368371
escapeCount = char === '\\' ? escapeCount + 1 : 0;
369372
}
370-
return -1;
371373
}
372374
}
373375

‎packages/compiler/test/expression_parser/parser_spec.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,10 @@ describe('parser', () => {
314314
it('should report when encountering interpolation', () => {
315315
expectActionError('{{a()}}', 'Got interpolation ({{}}) where expression was expected');
316316
});
317+
318+
it('should not report interpolation inside a string', () => {
319+
expect(parseAction('"{{a()}}"').errors).toEqual([]);
320+
});
317321
});
318322

319323
describe('parse spans', () => {
@@ -486,6 +490,10 @@ describe('parser', () => {
486490
expectBindingError('{{a.b}}', 'Got interpolation ({{}}) where expression was expected');
487491
});
488492

493+
it('should not report interpolation inside a string', () => {
494+
expect(parseBinding('"{{exp}}"').errors).toEqual([]);
495+
});
496+
489497
it('should parse conditional expression', () => {
490498
checkBinding('a < b ? a : b');
491499
});
@@ -952,6 +960,10 @@ describe('parser', () => {
952960
'Got interpolation ({{}}) where expression was expected');
953961
});
954962

963+
it('should not report interpolation inside a string', () => {
964+
expect(parseSimpleBinding('"{{exp}}"').errors).toEqual([]);
965+
});
966+
955967
it('should report when encountering field write', () => {
956968
expectError(validate(parseSimpleBinding('a = b')), 'Bindings cannot contain assignments');
957969
});

‎packages/core/test/acceptance/property_binding_spec.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,4 +623,15 @@ describe('property bindings', () => {
623623
fixture.detectChanges();
624624
}).not.toThrow();
625625
});
626+
627+
it('should allow quoted binding syntax inside property binding', () => {
628+
@Component({template: `<span [id]="'{{ id }}'"></span>`})
629+
class Comp {
630+
}
631+
632+
TestBed.configureTestingModule({declarations: [Comp]});
633+
const fixture = TestBed.createComponent(Comp);
634+
fixture.detectChanges();
635+
expect(fixture.nativeElement.querySelector('span').id).toBe('{{ id }}');
636+
});
626637
});

0 commit comments

Comments
 (0)