Skip to content

Commit

Permalink
fix(compiler): don't report parse error for interpolation inside stri…
Browse files Browse the repository at this point in the history
…ng 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.
  • Loading branch information
crisbeto committed Dec 26, 2020
1 parent 362f45c commit 63c1d8f
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 50 deletions.
102 changes: 52 additions & 50 deletions packages/compiler/src/expression_parser/parser.ts
Expand Up @@ -8,7 +8,6 @@

import * as chars from '../chars';
import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config';
import {escapeRegExp} from '../util';

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';
import {EOF, isIdentifier, isQuote, Lexer, Token, TokenType} from './lexer';
Expand All @@ -30,20 +29,6 @@ export class TemplateBindingParseResult {
public errors: ParserError[]) {}
}

const defaultInterpolateRegExp = _createInterpolateRegExp(DEFAULT_INTERPOLATION_CONFIG);
function _getInterpolateRegExp(config: InterpolationConfig): RegExp {
if (config === DEFAULT_INTERPOLATION_CONFIG) {
return defaultInterpolateRegExp;
} else {
return _createInterpolateRegExp(config);
}
}

function _createInterpolateRegExp(config: InterpolationConfig): RegExp {
const pattern = escapeRegExp(config.start) + '([\\s\\S]*?)' + escapeRegExp(config.end);
return new RegExp(pattern, 'g');
}

export class Parser {
private errors: ParserError[] = [];

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

private _checkNoInterpolation(
input: string, location: string, interpolationConfig: InterpolationConfig): void {
const regexp = _getInterpolateRegExp(interpolationConfig);
const parts = input.split(regexp);
if (parts.length > 1) {
private _checkNoInterpolation(input: string, location: string, {start, end}: InterpolationConfig):
void {
let startIndex = -1;
let endIndex = -1;

this._forEachUnquotedChar(input, 0, charIndex => {
if (startIndex === -1) {
startIndex = input.indexOf(start, charIndex);
return false;
} else {
endIndex = this._getInterpolationEndIndex(input, end, charIndex);
return endIndex > -1;
}
});

if (startIndex > -1 && endIndex > -1) {
this._reportError(
`Got interpolation (${interpolationConfig.start}${
interpolationConfig.end}) where expression was expected`,
input,
`at column ${this._findInterpolationErrorColumn(parts, 1, interpolationConfig)} in`,
location);
`Got interpolation (${start}${end}) where expression was expected`, input,
`at column ${startIndex} in`, location);
}
}

private _findInterpolationErrorColumn(
parts: string[], partInErrIdx: number, interpolationConfig: InterpolationConfig): number {
let errLocation = '';
for (let j = 0; j < partInErrIdx; j++) {
errLocation += j % 2 === 0 ?
parts[j] :
`${interpolationConfig.start}${parts[j]}${interpolationConfig.end}`;
}
/**
* Finds the index of the end of an interpolation expression
* while ignoring comments and quoted content.
*/
private _getInterpolationEndIndex(input: string, expressionEnd: string, start: number): number {
let result = -1;

this._forEachUnquotedChar(input, start, charIndex => {
if (input.startsWith(expressionEnd, charIndex)) {
result = charIndex;
return true;
}
// Nothing else in the expression matters after we've
// hit a comment so look directly for the end token.
if (input.startsWith('//', charIndex)) {
result = input.indexOf(expressionEnd, charIndex);
return true;
}
return false;
});

return errLocation.length;
return result;
}

/**
* Finds the index of the end of an interpolation expression
* while ignoring comments and quoted content.
* Invokes a callback function for each character of a string that is outside
* of a quote.
* @param input String to loop through.
* @param start Index within the string at which to start.
* @param callback Callback to be invoked for each index. If the callback
* returns `true`, the loop will be broken off.
*/
private _getExpressiondEndIndex(input: string, expressionEnd: string, start: number): number {
private _forEachUnquotedChar(input: string, start: number, callback: (index: number) => boolean):
void {
let currentQuote: string|null = null;
let escapeCount = 0;
for (let i = start; i < input.length; i++) {
const char = input[i];
// Skip the characters inside quotes. Note that we only care about the
// outer-most quotes matching up and we need to account for escape characters.
// Skip the characters inside quotes. Note that we only care about the outer-most
// quotes matching up and we need to account for escape characters.
if (isQuote(input.charCodeAt(i)) && (currentQuote === null || currentQuote === char) &&
escapeCount % 2 === 0) {
currentQuote = currentQuote === null ? char : null;
} else if (currentQuote === null) {
if (input.startsWith(expressionEnd, i)) {
return i;
}
// Nothing else in the expression matters after we've
// hit a comment so look directly for the end token.
if (input.startsWith('//', i)) {
return input.indexOf(expressionEnd, i);
}
} else if (currentQuote === null && callback(i)) {
break;
}
escapeCount = char === '\\' ? escapeCount + 1 : 0;
}
return -1;
}
}

Expand Down
12 changes: 12 additions & 0 deletions packages/compiler/test/expression_parser/parser_spec.ts
Expand Up @@ -314,6 +314,10 @@ describe('parser', () => {
it('should report when encountering interpolation', () => {
expectActionError('{{a()}}', 'Got interpolation ({{}}) where expression was expected');
});

it('should not report interpolation inside a string', () => {
expect(parseAction('"{{a()}}"').errors).toEqual([]);
});
});

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

it('should not report interpolation inside a string', () => {
expect(parseBinding('"{{exp}}"').errors).toEqual([]);
});

it('should parse conditional expression', () => {
checkBinding('a < b ? a : b');
});
Expand Down Expand Up @@ -952,6 +960,10 @@ describe('parser', () => {
'Got interpolation ({{}}) where expression was expected');
});

it('should not report interpolation inside a string', () => {
expect(parseSimpleBinding('"{{exp}}"').errors).toEqual([]);
});

it('should report when encountering field write', () => {
expectError(validate(parseSimpleBinding('a = b')), 'Bindings cannot contain assignments');
});
Expand Down
11 changes: 11 additions & 0 deletions packages/core/test/acceptance/property_binding_spec.ts
Expand Up @@ -623,4 +623,15 @@ describe('property bindings', () => {
fixture.detectChanges();
}).not.toThrow();
});

it('should allow quoted binding syntax inside property binding', () => {
@Component({template: `<span [id]="'{{ id }}'"></span>`})
class Comp {
}

TestBed.configureTestingModule({declarations: [Comp]});
const fixture = TestBed.createComponent(Comp);
fixture.detectChanges();
expect(fixture.nativeElement.querySelector('span').id).toBe('{{ id }}');
});
});

0 comments on commit 63c1d8f

Please sign in to comment.