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 (#40267)

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 #39826 to account for interpolation
characters inside quotes.

Fixes #39601.

PR Close #40267
  • Loading branch information
crisbeto authored and josephperrott committed Jan 5, 2021
1 parent 6a9d7e5 commit 6abc133
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 48 deletions.
93 changes: 45 additions & 48 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,71 @@ 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;

for (const charIndex of this._forEachUnquotedChar(input, 0)) {
if (startIndex === -1) {
if (input.startsWith(start)) {
startIndex = charIndex;
}
} else {
endIndex = this._getInterpolationEndIndex(input, end, charIndex);
if (endIndex > -1) {
break;
}
}
}

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 {
for (const charIndex of this._forEachUnquotedChar(input, start)) {
if (input.startsWith(expressionEnd, charIndex)) {
return charIndex;
}

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

return errLocation.length;
return -1;
}

/**
* Finds the index of the end of an interpolation expression
* while ignoring comments and quoted content.
* Generator used to iterate over the character indexes of a string that are outside of quotes.
* @param input String to loop through.
* @param start Index within the string at which to start.
*/
private _getExpressiondEndIndex(input: string, expressionEnd: string, start: number): number {
private * _forEachUnquotedChar(input: string, start: number) {
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);
}
yield i;
}
escapeCount = char === '\\' ? escapeCount + 1 : 0;
}
return -1;
}
}

Expand Down
21 changes: 21 additions & 0 deletions packages/compiler/test/expression_parser/parser_spec.ts
Expand Up @@ -314,6 +314,13 @@ 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([]);
expect(parseAction(`'{{a()}}'`).errors).toEqual([]);
expect(parseAction(`"{{a('\\"')}}"`).errors).toEqual([]);
expect(parseAction(`'{{a("\\'")}}'`).errors).toEqual([]);
});
});

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

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

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

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

it('should report when encountering field write', () => {
expectError(validate(parseSimpleBinding('a = b')), 'Bindings cannot contain assignments');
});
Expand Down
22 changes: 22 additions & 0 deletions packages/core/test/acceptance/property_binding_spec.ts
Expand Up @@ -623,4 +623,26 @@ 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 }}');
});

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

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

0 comments on commit 6abc133

Please sign in to comment.