From 63c1d8f062ba995c9f6dd488b9517d221fba38ed Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sat, 26 Dec 2020 12:45:40 +0200 Subject: [PATCH] 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 #39826 to account for interpolation characters inside quotes. Fixes #39601. --- .../compiler/src/expression_parser/parser.ts | 102 +++++++++--------- .../test/expression_parser/parser_spec.ts | 12 +++ .../test/acceptance/property_binding_spec.ts | 11 ++ 3 files changed, 75 insertions(+), 50 deletions(-) diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index feb3c3c3c78b9..20ce2f1ab09da 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -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'; @@ -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[] = []; @@ -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. @@ -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; } } diff --git a/packages/compiler/test/expression_parser/parser_spec.ts b/packages/compiler/test/expression_parser/parser_spec.ts index 2acde1d89fa22..c82400fff0cb7 100644 --- a/packages/compiler/test/expression_parser/parser_spec.ts +++ b/packages/compiler/test/expression_parser/parser_spec.ts @@ -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', () => { @@ -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'); }); @@ -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'); }); diff --git a/packages/core/test/acceptance/property_binding_spec.ts b/packages/core/test/acceptance/property_binding_spec.ts index ddb9a18ffeed6..55530f2e4cff6 100644 --- a/packages/core/test/acceptance/property_binding_spec.ts +++ b/packages/core/test/acceptance/property_binding_spec.ts @@ -623,4 +623,15 @@ describe('property bindings', () => { fixture.detectChanges(); }).not.toThrow(); }); + + it('should allow quoted binding syntax inside property binding', () => { + @Component({template: ``}) + class Comp { + } + + TestBed.configureTestingModule({declarations: [Comp]}); + const fixture = TestBed.createComponent(Comp); + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector('span').id).toBe('{{ id }}'); + }); });