From a6247aafa135267cd9734cd24adbd7a334c15a00 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 15 Nov 2019 16:25:59 +0000 Subject: [PATCH] fix(ivy): i18n - support "\", "`" and "${" sequences in i18n messages (#33820) Since i18n messages are mapped to `$localize` tagged template strings, the "raw" version must be properly escaped. Otherwise TS will throw an error such as: ``` Error: Debug Failure. False expression: Expected argument 'text' to be the normalized (i.e. 'cooked') version of argument 'rawText'. ``` This commit ensures that we properly escape these raw strings before creating TS AST nodes from them. PR Close #33820 --- .../src/ngtsc/translator/src/translator.ts | 8 +- .../test/compliance/mock_compile.ts | 9 ++- .../compliance/r3_view_compiler_i18n_spec.ts | 34 +++++++- .../compiler/src/output/abstract_emitter.ts | 14 +--- packages/compiler/src/output/output_ast.ts | 66 ++++++++++++++++ .../compiler/src/render3/view/i18n/meta.ts | 52 ------------- .../compiler/test/render3/view/i18n_spec.ts | 77 ++++++++++++------- 7 files changed, 156 insertions(+), 104 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index 63f43730c3..49c791255f 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -8,7 +8,6 @@ import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinType, BuiltinTypeName, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, Type, TypeVisitor, TypeofExpr, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler'; import {LocalizedString} from '@angular/compiler/src/output/output_ast'; -import {serializeI18nHead, serializeI18nTemplatePart} from '@angular/compiler/src/render3/view/i18n/meta'; import * as ts from 'typescript'; import {DefaultImportRecorder, ImportRewriter, NOOP_DEFAULT_IMPORT_RECORDER, NoopImportRewriter} from '../../imports'; @@ -547,7 +546,7 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor { */ function visitLocalizedString(ast: LocalizedString, context: Context, visitor: ExpressionVisitor) { let template: ts.TemplateLiteral; - const metaBlock = serializeI18nHead(ast.metaBlock, ast.messageParts[0]); + const metaBlock = ast.serializeI18nHead(); if (ast.messageParts.length === 1) { template = ts.createNoSubstitutionTemplateLiteral(metaBlock.cooked, metaBlock.raw); } else { @@ -555,9 +554,8 @@ function visitLocalizedString(ast: LocalizedString, context: Context, visitor: E const spans: ts.TemplateSpan[] = []; for (let i = 1; i < ast.messageParts.length; i++) { const resolvedExpression = ast.expressions[i - 1].visitExpression(visitor, context); - const templatePart = - serializeI18nTemplatePart(ast.placeHolderNames[i - 1], ast.messageParts[i]); - const templateMiddle = ts.createTemplateMiddle(templatePart); + const templatePart = ast.serializeI18nTemplatePart(i); + const templateMiddle = ts.createTemplateMiddle(templatePart.cooked, templatePart.raw); spans.push(ts.createTemplateSpan(resolvedExpression, templateMiddle)); } if (spans.length > 0) { diff --git a/packages/compiler-cli/test/compliance/mock_compile.ts b/packages/compiler-cli/test/compliance/mock_compile.ts index 7fc3fc6b87..859451d81d 100644 --- a/packages/compiler-cli/test/compliance/mock_compile.ts +++ b/packages/compiler-cli/test/compliance/mock_compile.ts @@ -14,15 +14,15 @@ import {NgtscProgram} from '../../src/ngtsc/program'; const IDENTIFIER = /[A-Za-z_$ɵ][A-Za-z0-9_$]*/; const OPERATOR = - /!|\?|%|\*|\/|\^|&&?|\|\|?|\(|\)|\{|\}|\[|\]|:|;|<=?|>=?|={1,3}|!==?|=>|\+\+?|--?|@|,|\.|\.\.\./; + /!|\?|%|\*|\/|\^|&&?|\|\|?|\(|\)|\{|\}|\[|\]|:|;|<=?|>=?|={1,3}|!==?|=>|\+\+?|--?|@|,|\.|\.\.\.|`|\\'/; const STRING = /'(\\'|[^'])*'|"(\\"|[^"])*"/; -const BACKTICK_STRING = /\\`(([\s\S]*?)(\$\{[^}]*?\})?)*?\\`/; +const BACKTICK_STRING = /\\`(([\s\S]*?)(\$\{[^}]*?\})?)*?[^\\]\\`/; const BACKTICK_INTERPOLATION = /(\$\{[^}]*\})/; const NUMBER = /\d+/; const ELLIPSIS = '…'; const TOKEN = new RegExp( - `\\s*((${IDENTIFIER.source})|(${OPERATOR.source})|(${STRING.source})|(${BACKTICK_STRING.source})|${NUMBER.source}|${ELLIPSIS})\\s*`, + `\\s*((${IDENTIFIER.source})|(${BACKTICK_STRING.source})|(${OPERATOR.source})|(${STRING.source})|${NUMBER.source}|${ELLIPSIS})\\s*`, 'y'); type Piece = string | RegExp; @@ -76,6 +76,9 @@ function tokenize(text: string): Piece[] { */ function tokenizeBackTickString(str: string): Piece[] { const pieces: Piece[] = ['`']; + // Unescape backticks that are inside the backtick string + // (we had to double escape them in the test string so they didn't look like string markers) + str = str.replace(/\\\\\\`/, '\\`'); const backTickPieces = str.slice(2, -2).split(BACKTICK_INTERPOLATION); backTickPieces.forEach((backTickPiece) => { if (BACKTICK_INTERPOLATION.test(backTickPiece)) { diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts index 9614315155..30002b1b0b 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts @@ -111,6 +111,15 @@ const verifyUniqueConsts = (output: string) => { return true; }; +/** + * Escape the template string for being placed inside a backtick string literal. + * + * * "\" would erroneously indicate a control character + * * "`" and "${" strings would erroneously indicate the end of a message part + */ +const escapeTemplate = (template: string) => + template.replace(/\\/g, '\\\\').replace(/`/g, '\\`').replace(/\$\{/g, '$\\{'); + const getAppFilesWithTemplate = (template: string, args: any = {}) => ({ app: { 'spec.ts': ` @@ -120,7 +129,7 @@ const getAppFilesWithTemplate = (template: string, args: any = {}) => ({ selector: 'my-component', ${args.preserveWhitespaces ? 'preserveWhitespaces: true,' : ''} ${args.interpolation ? 'interpolation: ' + JSON.stringify(args.interpolation) + ', ' : ''} - template: \`${template}\` + template: \`${escapeTemplate(template)}\` }) export class MyComponent {} @@ -180,7 +189,8 @@ describe('i18n support in the template compiler', () => {
Content D
Content E
Content F
-
Content G
+
Content G
+
Content H
`; const output = String.raw ` @@ -258,15 +268,28 @@ describe('i18n support in the template compiler', () => { var $I18N_23$; if (ngI18nClosureMode) { /** - * @desc [BACKUP_MESSAGE_ID:idH]desc + * @desc [BACKUP_$` + + String.raw `{MESSAGE}_ID:idH]` + + '`' + String.raw `desc */ const $MSG_EXTERNAL_idG$$APP_SPEC_TS_24$ = goog.getMsg("Title G"); $I18N_23$ = $MSG_EXTERNAL_idG$$APP_SPEC_TS_24$; } else { - $I18N_23$ = $localize \`:[BACKUP_MESSAGE_ID\:idH]desc@@idG:Title G\`; + $I18N_23$ = $localize \`:[BACKUP_$\{MESSAGE}_ID\:idH]\\\`desc@@idG:Title G\`; } const $_c25$ = ["title", $I18N_23$]; + var $I18N_20$; + if (ngI18nClosureMode) { + /** + * @desc Some text \' [BACKUP_MESSAGE_ID: xxx] + */ + const $MSG_EXTERNAL_idG$$APP_SPEC_TS_21$ = goog.getMsg("Content H"); + $I18N_20$ = $MSG_EXTERNAL_idG$$APP_SPEC_TS_21$; + } + else { + $I18N_20$ = $localize \`:Some text \\' [BACKUP_MESSAGE_ID\: xxx]:Content H\`; + } … consts: [[${AttributeMarker.I18n}, "title"]], template: function MyComponent_Template(rf, ctx) { @@ -298,6 +321,9 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵi18nAttributes(18, $_c25$); $r3$.ɵɵtext(19, "Content G"); $r3$.ɵɵelementEnd(); + $r3$.ɵɵelementStart(20, "div"); + $r3$.ɵɵi18n(21, $I18N_20$); + $r3$.ɵɵelementEnd(); } } `; diff --git a/packages/compiler/src/output/abstract_emitter.ts b/packages/compiler/src/output/abstract_emitter.ts index 9c505c9d18..ff10e36a4a 100644 --- a/packages/compiler/src/output/abstract_emitter.ts +++ b/packages/compiler/src/output/abstract_emitter.ts @@ -7,8 +7,6 @@ */ import {ParseSourceSpan} from '../parse_util'; -import {serializeI18nHead, serializeI18nTemplatePart} from '../render3/view/i18n/meta'; - import * as o from './output_ast'; import {SourceMapGenerator} from './source_map'; @@ -363,14 +361,12 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex } visitLocalizedString(ast: o.LocalizedString, ctx: EmitterVisitorContext): any { - const head = serializeI18nHead(ast.metaBlock, ast.messageParts[0]); - ctx.print(ast, '$localize `' + escapeBackticks(head.raw)); + const head = ast.serializeI18nHead(); + ctx.print(ast, '$localize `' + head.raw); for (let i = 1; i < ast.messageParts.length; i++) { ctx.print(ast, '${'); ast.expressions[i - 1].visitExpression(this, ctx); - ctx.print( - ast, - `}${escapeBackticks(serializeI18nTemplatePart(ast.placeHolderNames[i - 1], ast.messageParts[i]))}`); + ctx.print(ast, `}${ast.serializeI18nTemplatePart(i).raw}`); } ctx.print(ast, '`'); return null; @@ -560,7 +556,3 @@ function _createIndent(count: number): string { } return res; } - -function escapeBackticks(str: string): string { - return str.replace(/`/g, '\\`'); -} diff --git a/packages/compiler/src/output/output_ast.ts b/packages/compiler/src/output/output_ast.ts index 35f53e3a47..1071ae554e 100644 --- a/packages/compiler/src/output/output_ast.ts +++ b/packages/compiler/src/output/output_ast.ts @@ -499,8 +499,74 @@ export class LocalizedString extends Expression { visitExpression(visitor: ExpressionVisitor, context: any): any { return visitor.visitLocalizedString(this, context); } + + /** + * Serialize the given `meta` and `messagePart` into "cooked" and "raw" strings that can be used + * in a `$localize` tagged string. The format of the metadata is the same as that parsed by + * `parseI18nMeta()`. + * + * @param meta The metadata to serialize + * @param messagePart The first part of the tagged string + */ + serializeI18nHead(): {cooked: string, raw: string} { + let metaBlock = this.metaBlock.description || ''; + if (this.metaBlock.meaning) { + metaBlock = `${this.metaBlock.meaning}|${metaBlock}`; + } + if (this.metaBlock.customId || this.metaBlock.legacyId) { + metaBlock = `${metaBlock}@@${this.metaBlock.customId || this.metaBlock.legacyId}`; + } + return createCookedRawString(metaBlock, this.messageParts[0]); + } + + /** + * Serialize the given `placeholderName` and `messagePart` into "cooked" and "raw" strings that + * can be used in a `$localize` tagged string. + * + * @param placeholderName The placeholder name to serialize + * @param messagePart The following message string after this placeholder + */ + serializeI18nTemplatePart(partIndex: number): {cooked: string, raw: string} { + const placeholderName = this.placeHolderNames[partIndex - 1]; + const messagePart = this.messageParts[partIndex]; + return createCookedRawString(placeholderName, messagePart); + } } +const escapeSlashes = (str: string): string => str.replace(/\\/g, '\\\\'); +const escapeStartingColon = (str: string): string => str.replace(/^:/, '\\:'); +const escapeColons = (str: string): string => str.replace(/:/g, '\\:'); +const escapeForMessagePart = (str: string): string => + str.replace(/`/g, '\\`').replace(/\${/g, '$\\{'); + +/** + * Creates a `{cooked, raw}` object from the `metaBlock` and `messagePart`. + * + * The `raw` text must have various character sequences escaped: + * * "\" would otherwise indicate that the next character is a control character. + * * "`" and "${" are template string control sequences that would otherwise prematurely indicate + * the end of a message part. + * * ":" inside a metablock would prematurely indicate the end of the metablock. + * * ":" at the start of a messagePart with no metablock would erroneously indicate the start of a + * metablock. + * + * @param metaBlock Any metadata that should be prepended to the string + * @param messagePart The message part of the string + */ +function createCookedRawString(metaBlock: string, messagePart: string) { + if (metaBlock === '') { + return { + cooked: messagePart, + raw: escapeForMessagePart(escapeStartingColon(escapeSlashes(messagePart))) + }; + } else { + return { + cooked: `:${metaBlock}:${messagePart}`, + raw: escapeForMessagePart( + `:${escapeColons(escapeSlashes(metaBlock))}:${escapeSlashes(messagePart)}`) + }; + } +} export class ExternalExpr extends Expression { constructor( diff --git a/packages/compiler/src/render3/view/i18n/meta.ts b/packages/compiler/src/render3/view/i18n/meta.ts index 0b78536955..a1122d0357 100644 --- a/packages/compiler/src/render3/view/i18n/meta.ts +++ b/packages/compiler/src/render3/view/i18n/meta.ts @@ -234,50 +234,6 @@ export function parseI18nMeta(meta?: string): I18nMeta { return {customId, meaning, description}; } -/** - * Serialize the given `meta` and `messagePart` "cooked" and "raw" strings that can be used in a - * `$localize` tagged string. The format of the metadata is the same as that parsed by - * `parseI18nMeta()`. - * - * @param meta The metadata to serialize - * @param messagePart The first part of the tagged string - */ -export function serializeI18nHead( - meta: I18nMeta, messagePart: string): {cooked: string, raw: string} { - let metaBlock = meta.description || ''; - if (meta.meaning) { - metaBlock = `${meta.meaning}|${metaBlock}`; - } - if (meta.customId || meta.legacyId) { - metaBlock = `${metaBlock}@@${meta.customId || meta.legacyId}`; - } - if (metaBlock === '') { - // There is no metaBlock, so we must ensure that any starting colon is escaped. - return {cooked: messagePart, raw: escapeStartingColon(messagePart)}; - } else { - return { - cooked: `:${metaBlock}:${messagePart}`, - raw: `:${escapeColons(metaBlock)}:${messagePart}` - }; - } -} - -/** - * Serialize the given `placeholderName` and `messagePart` into strings that can be used in a - * `$localize` tagged string. - * - * @param placeholderName The placeholder name to serialize - * @param messagePart The following message string after this placeholder - */ -export function serializeI18nTemplatePart(placeholderName: string, messagePart: string): string { - if (placeholderName === '') { - // There is no placeholder name block, so we must ensure that any starting colon is escaped. - return escapeStartingColon(messagePart); - } else { - return `:${placeholderName}:${messagePart}`; - } -} - // Converts i18n meta information for a message (id, description, meaning) // to a JsDoc statement formatted as expected by the Closure compiler. export function i18nMetaToDocStmt(meta: I18nMeta): o.JSDocCommentStmt|null { @@ -290,11 +246,3 @@ export function i18nMetaToDocStmt(meta: I18nMeta): o.JSDocCommentStmt|null { } return tags.length == 0 ? null : new o.JSDocCommentStmt(tags); } - -export function escapeStartingColon(str: string): string { - return str.replace(/^:/, '\\:'); -} - -export function escapeColons(str: string): string { - return str.replace(/:/g, '\\:'); -} \ No newline at end of file diff --git a/packages/compiler/test/render3/view/i18n_spec.ts b/packages/compiler/test/render3/view/i18n_spec.ts index d446349e73..a4660926ce 100644 --- a/packages/compiler/test/render3/view/i18n_spec.ts +++ b/packages/compiler/test/render3/view/i18n_spec.ts @@ -15,7 +15,7 @@ import {I18nContext} from '../../../src/render3/view/i18n/context'; import {serializeI18nMessageForGetMsg} from '../../../src/render3/view/i18n/get_msg_utils'; import {serializeIcuNode} from '../../../src/render3/view/i18n/icu_serializer'; import {serializeI18nMessageForLocalize} from '../../../src/render3/view/i18n/localize_utils'; -import {I18nMeta, parseI18nMeta, serializeI18nHead, serializeI18nTemplatePart} from '../../../src/render3/view/i18n/meta'; +import {I18nMeta, parseI18nMeta} from '../../../src/render3/view/i18n/meta'; import {formatI18nPlaceholderName} from '../../../src/render3/view/i18n/util'; import {parseR3 as parse} from './util'; @@ -210,47 +210,66 @@ describe('Utils', () => { }); it('serializeI18nHead()', () => { - expect(serializeI18nHead(meta(), '')).toEqual({cooked: '', raw: ''}); - expect(serializeI18nHead(meta('', '', 'desc'), '')) + expect(o.localizedString(meta(), [''], [], []).serializeI18nHead()) + .toEqual({cooked: '', raw: ''}); + expect(o.localizedString(meta('', '', 'desc'), [''], [], []).serializeI18nHead()) .toEqual({cooked: ':desc:', raw: ':desc:'}); - expect(serializeI18nHead(meta('id', '', 'desc'), '')) + expect(o.localizedString(meta('id', '', 'desc'), [''], [], []).serializeI18nHead()) .toEqual({cooked: ':desc@@id:', raw: ':desc@@id:'}); - expect(serializeI18nHead(meta('', 'meaning', 'desc'), '')) + expect(o.localizedString(meta('', 'meaning', 'desc'), [''], [], []).serializeI18nHead()) .toEqual({cooked: ':meaning|desc:', raw: ':meaning|desc:'}); - expect(serializeI18nHead(meta('id', 'meaning', 'desc'), '')) + expect(o.localizedString(meta('id', 'meaning', 'desc'), [''], [], []).serializeI18nHead()) .toEqual({cooked: ':meaning|desc@@id:', raw: ':meaning|desc@@id:'}); - expect(serializeI18nHead(meta('id', '', ''), '')).toEqual({cooked: ':@@id:', raw: ':@@id:'}); + expect(o.localizedString(meta('id', '', ''), [''], [], []).serializeI18nHead()) + .toEqual({cooked: ':@@id:', raw: ':@@id:'}); // Escaping colons (block markers) - expect(serializeI18nHead(meta('id:sub_id', 'meaning', 'desc'), '')) + expect( + o.localizedString(meta('id:sub_id', 'meaning', 'desc'), [''], [], []).serializeI18nHead()) .toEqual({cooked: ':meaning|desc@@id:sub_id:', raw: ':meaning|desc@@id\\:sub_id:'}); - expect(serializeI18nHead(meta('id', 'meaning:sub_meaning', 'desc'), '')).toEqual({ - cooked: ':meaning:sub_meaning|desc@@id:', - raw: ':meaning\\:sub_meaning|desc@@id:' - }); - expect(serializeI18nHead(meta('id', 'meaning', 'desc:sub_desc'), '')) + expect(o.localizedString(meta('id', 'meaning:sub_meaning', 'desc'), [''], [], []) + .serializeI18nHead()) + .toEqual( + {cooked: ':meaning:sub_meaning|desc@@id:', raw: ':meaning\\:sub_meaning|desc@@id:'}); + expect(o.localizedString(meta('id', 'meaning', 'desc:sub_desc'), [''], [], []) + .serializeI18nHead()) .toEqual({cooked: ':meaning|desc:sub_desc@@id:', raw: ':meaning|desc\\:sub_desc@@id:'}); - expect(serializeI18nHead(meta('id', 'meaning', 'desc'), 'message source')).toEqual({ - cooked: ':meaning|desc@@id:message source', - raw: ':meaning|desc@@id:message source' - }); - expect(serializeI18nHead(meta('id', 'meaning', 'desc'), ':message source')).toEqual({ - cooked: ':meaning|desc@@id::message source', - raw: ':meaning|desc@@id::message source' - }); - expect(serializeI18nHead(meta('', '', ''), 'message source')) + expect(o.localizedString(meta('id', 'meaning', 'desc'), ['message source'], [], []) + .serializeI18nHead()) + .toEqual({ + cooked: ':meaning|desc@@id:message source', + raw: ':meaning|desc@@id:message source' + }); + expect(o.localizedString(meta('id', 'meaning', 'desc'), [':message source'], [], []) + .serializeI18nHead()) + .toEqual({ + cooked: ':meaning|desc@@id::message source', + raw: ':meaning|desc@@id::message source' + }); + expect(o.localizedString(meta('', '', ''), ['message source'], [], []).serializeI18nHead()) .toEqual({cooked: 'message source', raw: 'message source'}); - expect(serializeI18nHead(meta('', '', ''), ':message source')) + expect(o.localizedString(meta('', '', ''), [':message source'], [], []).serializeI18nHead()) .toEqual({cooked: ':message source', raw: '\\:message source'}); }); it('serializeI18nPlaceholderBlock()', () => { - expect(serializeI18nTemplatePart('', '')).toEqual(''); - expect(serializeI18nTemplatePart('abc', '')).toEqual(':abc:'); - expect(serializeI18nTemplatePart('', 'message')).toEqual('message'); - expect(serializeI18nTemplatePart('abc', 'message')).toEqual(':abc:message'); - expect(serializeI18nTemplatePart('', ':message')).toEqual('\\:message'); - expect(serializeI18nTemplatePart('abc', ':message')).toEqual(':abc::message'); + expect(o.localizedString(meta('', '', ''), ['', ''], [''], []).serializeI18nTemplatePart(1)) + .toEqual({cooked: '', raw: ''}); + expect( + o.localizedString(meta('', '', ''), ['', ''], ['abc'], []).serializeI18nTemplatePart(1)) + .toEqual({cooked: ':abc:', raw: ':abc:'}); + expect(o.localizedString(meta('', '', ''), ['', 'message'], [''], []) + .serializeI18nTemplatePart(1)) + .toEqual({cooked: 'message', raw: 'message'}); + expect(o.localizedString(meta('', '', ''), ['', 'message'], ['abc'], []) + .serializeI18nTemplatePart(1)) + .toEqual({cooked: ':abc:message', raw: ':abc:message'}); + expect(o.localizedString(meta('', '', ''), ['', ':message'], [''], []) + .serializeI18nTemplatePart(1)) + .toEqual({cooked: ':message', raw: '\\:message'}); + expect(o.localizedString(meta('', '', ''), ['', ':message'], ['abc'], []) + .serializeI18nTemplatePart(1)) + .toEqual({cooked: ':abc::message', raw: ':abc::message'}); }); function meta(customId?: string, meaning?: string, description?: string): I18nMeta {