From 8c2d8428d50ebe410e458ab8aba7176bdf9fa4c7 Mon Sep 17 00:00:00 2001
From: Andrew Kushnir <akushnir@google.com>
Date: Fri, 6 Mar 2020 16:33:09 -0800
Subject: [PATCH] fix(core): verify parsed ICU expression at runtime before
 executing it (#35923)

Prior to this commit, i18n runtime logic relied on the assumption that provided translation is syntactically correct, specifically around ICU syntax. However provided translations might contain some errors that lead to parsing failure. Specifically when translation contains curly braces, runtime i18n logic tries to parse them as an ICU expression and fails. This commit validates ICU parsing result (making sure it was parsed correctly) and throws an error if parsing error happens. The error that is thrown also contains translated message text for easier debugging.

Note: the check and the error message introduced in this PR is a safeguard against the problem that led to unhandled i18n runtime logic crash. So the framework behavior remains the same, we just improve the error message and it should be safe to merge to the patch branch.

Resolves #35689.

PR Close #35923
---
 integration/_payload-limits.json           |  4 +-
 packages/core/src/render3/i18n.ts          | 10 ++++-
 packages/core/test/acceptance/i18n_spec.ts | 49 ++++++++++++++++++++++
 3 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json
index 21cb9fbcb1..54704a01b4 100644
--- a/integration/_payload-limits.json
+++ b/integration/_payload-limits.json
@@ -30,7 +30,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 136777,
+        "main-es2015": 137320,
         "polyfills-es2015": 37334
       }
     }
@@ -64,4 +64,4 @@
       }
     }
   }
-}
\ No newline at end of file
+}
diff --git a/packages/core/src/render3/i18n.ts b/packages/core/src/render3/i18n.ts
index 04888681e5..5bcda795a8 100644
--- a/packages/core/src/render3/i18n.ts
+++ b/packages/core/src/render3/i18n.ts
@@ -441,6 +441,15 @@ function i18nStartFirstPass(
       for (let j = 0; j < parts.length; j++) {
         if (j & 1) {
           // Odd indexes are ICU expressions
+          const icuExpression = parts[j] as IcuExpression;
+
+          // Verify that ICU expression has the right shape. Translations might contain invalid
+          // constructions (while original messages were correct), so ICU parsing at runtime may not
+          // succeed (thus `icuExpression` remains a string).
+          if (typeof icuExpression !== 'object') {
+            throw new Error(`Unable to parse ICU expression in "${templateTranslation}" message.`);
+          }
+
           // Create the comment node that will anchor the ICU expression
           const icuNodeIndex = startIndex + i18nVarsCount++;
           createOpCodes.push(
@@ -448,7 +457,6 @@ function i18nStartFirstPass(
               parentIndex << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild);
 
           // Update codes for the ICU expression
-          const icuExpression = parts[j] as IcuExpression;
           const mask = getBindingMask(icuExpression);
           icuStart(icuExpressions, icuExpression, icuNodeIndex, icuNodeIndex);
           // Since this is recursive, the last TIcu that was pushed is the one we want
diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts
index 1a45e7cacc..c32c56f02b 100644
--- a/packages/core/test/acceptance/i18n_spec.ts
+++ b/packages/core/test/acceptance/i18n_spec.ts
@@ -2119,6 +2119,55 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => {
     });
   });
 
+  describe('invalid translations handling', () => {
+    it('should throw in case invalid ICU is present in a template', () => {
+      // Error message is produced by Compiler.
+      expect(() => initWithTemplate(AppComp, '{count, select, 10 {ten} other {other}'))
+          .toThrowError(
+              /Invalid ICU message. Missing '}'. \("{count, select, 10 {ten} other {other}\[ERROR ->\]"\)/);
+    });
+
+    it('should throw in case invalid ICU is present in translation', () => {
+      loadTranslations({
+        [computeMsgId('{VAR_SELECT, select, 10 {ten} other {other}}')]:
+            // Missing "}" at the end of translation.
+            '{VAR_SELECT, select, 10 {dix} other {autre}'
+      });
+
+      // Error message is produced at runtime.
+      expect(() => initWithTemplate(AppComp, '{count, select, 10 {ten} other {other}}'))
+          .toThrowError(
+              /Unable to parse ICU expression in "{�0�, select, 10 {dix} other {autre}" message./);
+    });
+
+    it('should throw in case unescaped curly braces are present in a template', () => {
+      // Error message is produced by Compiler.
+      expect(() => initWithTemplate(AppComp, 'Text { count }'))
+          .toThrowError(
+              /Do you have an unescaped "{" in your template\? Use "{{ '{' }}"\) to escape it/);
+    });
+
+    it('should throw in case curly braces are added into translation', () => {
+      loadTranslations({
+        // Curly braces which were not present in a template were added into translation.
+        [computeMsgId('Text')]: 'Text { count }',
+      });
+      expect(() => initWithTemplate(AppComp, '<div i18n>Text</div>'))
+          .toThrowError(/Unable to parse ICU expression in "Text { count }" message./);
+    });
+  });
+
+  it('should handle extra HTML in translation as plain text', () => {
+    loadTranslations({
+      // Translation contains HTML tags that were not present in original message.
+      [computeMsgId('Text')]: 'Text <div *ngIf="true">Extra content</div>',
+    });
+    const fixture = initWithTemplate(AppComp, '<div i18n>Text</div>');
+
+    const element = fixture.nativeElement;
+    expect(element).toHaveText('Text <div *ngIf="true">Extra content</div>');
+  });
+
   it('should reflect lifecycle hook changes in text interpolations in i18n block', () => {
     @Directive({selector: 'input'})
     class InputsDir {