From 8787073c40fba198a77db820bfacaaaff63dd8a0 Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Tue, 13 Feb 2024 12:26:46 -0600
Subject: [PATCH] =?UTF-8?q?feat(developer):=20errs=20on=20\uXXXX=20escapes?=
 =?UTF-8?q?=20=F0=9F=99=80?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- note, some older test cases had to be fixed also.

For: #10389
---
 common/web/types/src/util/util.ts             |  2 +-
 .../src/kmc-ldml/src/compiler/messages.ts     |  4 ++
 developer/src/kmc-ldml/src/compiler/tran.ts   | 39 +++++++++++++++----
 .../sections/strs/invalid-illegal.xml         |  2 +-
 ...bad-reorder.xml => fail-bad-reorder-1.xml} |  0
 .../sections/tran/fail-bad-reorder-2.xml      | 14 +++++++
 .../sections/tran/fail-bad-reorder-3.xml      | 14 +++++++
 .../sections/tran/fail-bad-tran-2.xml         | 14 +++++++
 .../src/kmc-ldml/test/test-compiler-e2e.ts    |  2 +-
 developer/src/kmc-ldml/test/test-tran.ts      | 20 +++++++++-
 10 files changed, 100 insertions(+), 11 deletions(-)
 rename developer/src/kmc-ldml/test/fixtures/sections/tran/{fail-bad-reorder.xml => fail-bad-reorder-1.xml} (100%)
 create mode 100644 developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder-2.xml
 create mode 100644 developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder-3.xml
 create mode 100644 developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-tran-2.xml

diff --git a/common/web/types/src/util/util.ts b/common/web/types/src/util/util.ts
index 1e088c588a..13057bc338 100644
--- a/common/web/types/src/util/util.ts
+++ b/common/web/types/src/util/util.ts
@@ -36,7 +36,7 @@ export class UnescapeError extends Error {
  * @param hex one codepoint in hex, such as '0127'
  * @returns the unescaped codepoint
  */
-function unescapeOne(hex: string): string {
+export function unescapeOne(hex: string): string {
   const codepoint = Number.parseInt(hex, 16);
   return String.fromCodePoint(codepoint);
 }
diff --git a/developer/src/kmc-ldml/src/compiler/messages.ts b/developer/src/kmc-ldml/src/compiler/messages.ts
index 224dc9b02c..b1d8b2dbfe 100644
--- a/developer/src/kmc-ldml/src/compiler/messages.ts
+++ b/developer/src/kmc-ldml/src/compiler/messages.ts
@@ -173,6 +173,10 @@ export class CompilerMessages {
   static Error_UnparseableTransformFrom = (o: { from: string }) =>
   m(this.ERROR_UnparseableTransformFrom, `Invalid transfom from "${o.from}"`);
   static ERROR_UnparseableTransformFrom = SevError | 0x0029;
+
+  static Error_InvalidQuadEscape = (o: { cp: number }) =>
+  m(this.ERROR_InvalidQuadEscape, `Invalid escape "\\u${util.hexQuad(o?.cp || 0)}", use "\\u{${o?.cp?.toString(16)}}" instead.`);
+  static ERROR_InvalidQuadEscape = SevError | 0x0030;
 }
 
 
diff --git a/developer/src/kmc-ldml/src/compiler/tran.ts b/developer/src/kmc-ldml/src/compiler/tran.ts
index d73182ba90..709049d885 100644
--- a/developer/src/kmc-ldml/src/compiler/tran.ts
+++ b/developer/src/kmc-ldml/src/compiler/tran.ts
@@ -144,8 +144,11 @@ export abstract class TransformCompiler<T extends TransformCompilerType, TranBas
     // don't unescape literals here, because we are going to pass them through into the regex
     cookedFrom = util.unescapeStringToRegex(cookedFrom);
 
-    // cook the ranges also
+    cookedFrom = this.checkEscapes(cookedFrom); // check for \uXXXX escapes before normalizing
+    if (cookedFrom === null) return null; // error
+
     cookedFrom = this.checkRanges(cookedFrom); // check before normalizing
+    if (cookedFrom === null) return null; // error
 
     if (!sections?.meta?.normalizationDisabled) {
       // nfd here.
@@ -156,7 +159,7 @@ export abstract class TransformCompiler<T extends TransformCompilerType, TranBas
       new RegExp(cookedFrom, 'ug');
     } catch (e) {
       this.callbacks.reportMessage(CompilerMessages.Error_UnparseableTransformFrom({ from: transform.from }));
-      return null;
+      return null; // error
     }
 
     // cookedFrom is cooked above, since there's some special treatment
@@ -185,6 +188,12 @@ export abstract class TransformCompiler<T extends TransformCompilerType, TranBas
 
   private compileReorder(sections: DependencySections, reorder: LKReorder): TranReorder {
     let result = new TranReorder();
+    if (reorder.from && this.checkEscapes(reorder.from) === null) {
+      return null; // error'ed
+    }
+    if (reorder.before && this.checkEscapes(reorder.before) === null) {
+      return null; // error'ed
+    }
     result.elements = sections.elem.allocElementString(sections, reorder.from, reorder.order, reorder.tertiary, reorder.tertiaryBase, reorder.preBase);
     result.before = sections.elem.allocElementString(sections, reorder.before);
     if (!result.elements || !result.before) {
@@ -216,9 +225,27 @@ export abstract class TransformCompiler<T extends TransformCompilerType, TranBas
     return defaults;
   }
 
-  /** true if ok */
-  private checkRanges(cookedFrom: string): string {
+  /** returns ===null on error */
+  private checkEscapes(cookedFrom: string): string | null {
     if (!cookedFrom) return cookedFrom;
+
+    // should not follow marker prefix, nor marker prefix with range
+    const anyQuad = /(?<!\\uffff\\u0008(?:\[[0-9a-fA-F\\u-]*)?)\\u([0-9a-fA-F]{4})/g;
+
+    for (const [, sub] of cookedFrom.matchAll(anyQuad)) {
+      const s = util.unescapeOne(sub);
+      if (s !== '\uffff' && s !== '\u0008') { // markers
+        this.callbacks.reportMessage(CompilerMessages.Error_InvalidQuadEscape({ cp: s.codePointAt(0) }));
+        return null; // exit on the first error
+      }
+    }
+    return cookedFrom;
+  }
+
+  /** returns ===null on error */
+  private checkRanges(cookedFrom: string): string | null {
+    if (!cookedFrom) return cookedFrom;
+
     // extract all of the potential ranges - but don't match any-markers!
     const anyRange = /(?<!\\uffff\\u0008)\[([^\]]+)\]/g;
     const ranges = cookedFrom.matchAll(anyRange);
@@ -226,7 +253,7 @@ export abstract class TransformCompiler<T extends TransformCompilerType, TranBas
     if (!ranges) return cookedFrom;
 
     // extract inner members of a range (inside the [])
-    const rangeRegex = /(\\u[0-9a-fA-F]{4}|.)-(\\u[0-9a-fA-F]{4}|.)|./g;
+    const rangeRegex = /(\\u\{[0-9a-fA-F]\}{1,6}|.)-(\\u\{[0-9a-fA-F]\}{1,6}|.)|./g;
 
     const rangeExplicit = new util.NFDAnalyzer();
     const rangeImplicit = new util.NFDAnalyzer();
@@ -235,8 +262,6 @@ export abstract class TransformCompiler<T extends TransformCompilerType, TranBas
     function processExplicit(s: string) {
       if (s.startsWith('\\u{')) {
         s = util.unescapeString(s);
-      } else if(s.startsWith('\\u')) {
-        s = util.unescapeOneQuadString(s);
       }
       rangeExplicit.add(s);
       return s;
diff --git a/developer/src/kmc-ldml/test/fixtures/sections/strs/invalid-illegal.xml b/developer/src/kmc-ldml/test/fixtures/sections/strs/invalid-illegal.xml
index 6362ac7fec..034d86b345 100644
--- a/developer/src/kmc-ldml/test/fixtures/sections/strs/invalid-illegal.xml
+++ b/developer/src/kmc-ldml/test/fixtures/sections/strs/invalid-illegal.xml
@@ -47,7 +47,7 @@
 
     <transformGroup>
       <!-- Northern Thai example from spec -->
-      <reorder before="\u{1A6B}" from="\u{1A60}[\u1A75-\u1A79]\u{1A45}" order="10 55 10" />
+      <reorder before="\u{1A6B}" from="\u{1A60}[\u{1A75}-\u{1A79}]\u{1A45}" order="10 55 10" />
     </transformGroup>
   </transforms>
 
diff --git a/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder.xml b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder-1.xml
similarity index 100%
rename from developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder.xml
rename to developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder-1.xml
diff --git a/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder-2.xml b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder-2.xml
new file mode 100644
index 0000000000..2f54cb328a
--- /dev/null
+++ b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder-2.xml
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!DOCTYPE keyboard3 SYSTEM "../../../../../../../resources/standards-data/ldml-keyboards/techpreview/dtd/ldmlKeyboard3.dtd">
+<keyboard3 locale="mt" conformsTo="techpreview">
+  <info name="fail-bad-reorder"/>
+
+  <keys />
+
+  <transforms type="simple">
+    <transformGroup>
+      <reorder before="\u1A6B" from="\u{1A60}[\u{1A75}-\u{1A79}]\u{1A45}" order="10 55 10" />
+    </transformGroup>
+  </transforms>
+</keyboard3>
diff --git a/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder-3.xml b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder-3.xml
new file mode 100644
index 0000000000..68add1ff7b
--- /dev/null
+++ b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-reorder-3.xml
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!DOCTYPE keyboard3 SYSTEM "../../../../../../../resources/standards-data/ldml-keyboards/techpreview/dtd/ldmlKeyboard3.dtd">
+<keyboard3 locale="mt" conformsTo="techpreview">
+  <info name="fail-bad-reorder"/>
+
+  <keys />
+
+  <transforms type="simple">
+    <transformGroup>
+      <reorder from="\u1A60[\u1A75-\u1A79]\u1A45" order="10 55 10" />
+    </transformGroup>
+  </transforms>
+</keyboard3>
diff --git a/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-tran-2.xml b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-tran-2.xml
new file mode 100644
index 0000000000..48b1580193
--- /dev/null
+++ b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-tran-2.xml
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!DOCTYPE keyboard3 SYSTEM "../../../../../../../resources/standards-data/ldml-keyboards/techpreview/dtd/ldmlKeyboard3.dtd">
+<keyboard3 locale="mt" conformsTo="techpreview">
+  <info name="fail-bad-tran-2"/>
+
+  <keys />
+
+  <transforms type="simple">
+    <transformGroup>
+      <transform from="\u0127" to="x"/> <!-- fail: use \u{1234} instead -->
+    </transformGroup>
+  </transforms>
+</keyboard3>
diff --git a/developer/src/kmc-ldml/test/test-compiler-e2e.ts b/developer/src/kmc-ldml/test/test-compiler-e2e.ts
index 829c8c2e16..aa014e15a6 100644
--- a/developer/src/kmc-ldml/test/test-compiler-e2e.ts
+++ b/developer/src/kmc-ldml/test/test-compiler-e2e.ts
@@ -71,7 +71,7 @@ describe('compiler-tests', function() {
     const source = k.load(filename);
     assert.notOk(source, `Trying to loadTestData(${filename})`);
   });
-  it('should fail on illegal chars', async function() {
+  it('should fail on illegal chars - sections/strs/invalid-illegal.xml', async function() {
     const inputFilename = makePathToFixture('sections/strs/invalid-illegal.xml');
     const kmx = await compileKeyboard(inputFilename, { ...compilerTestOptions, saveDebug: true, shouldAddCompilerVersion: false },
       [
diff --git a/developer/src/kmc-ldml/test/test-tran.ts b/developer/src/kmc-ldml/test/test-tran.ts
index f576fc67b8..5c5fd74018 100644
--- a/developer/src/kmc-ldml/test/test-tran.ts
+++ b/developer/src/kmc-ldml/test/test-tran.ts
@@ -265,11 +265,23 @@ describe('tran', function () {
       ],
     },
     {
-      subpath: 'sections/tran/fail-bad-reorder.xml',
+      subpath: 'sections/tran/fail-bad-reorder-1.xml',
       errors: [
         KmnOtherCompilerMessages.Error_UnicodeSetSyntaxError()
       ],
     },
+    {
+      subpath: 'sections/tran/fail-bad-reorder-2.xml',
+      errors: [
+        CompilerMessages.Error_InvalidQuadEscape({ cp: 0x1a6b }),
+      ],
+    },
+    {
+      subpath: 'sections/tran/fail-bad-reorder-3.xml',
+      errors: [
+        CompilerMessages.Error_InvalidQuadEscape({ cp: 0x1a60 }),
+      ],
+    },
     // error due to bad regex
     {
       subpath: `sections/tran/fail-bad-tran-1.xml`,
@@ -277,6 +289,12 @@ describe('tran', function () {
         CompilerMessages.Error_UnparseableTransformFrom({ from: 'AB(now if only I would terminate this group..' }),
       ],
     },
+    {
+      subpath: `sections/tran/fail-bad-tran-2.xml`,
+      errors: [
+        CompilerMessages.Error_InvalidQuadEscape({ cp: 295 }),
+      ],
+    },
   ], tranDependencies);
 });