Merge pull request #12465 from keymanapp/fix/developer/12451-prevent-…

…invalid-string-ids fix(developer): prevent invalid string ids
keymanapp · Sep 25, 2024 · ac2909f · ac2909f
2 parents becba77 + 2c3ae9e
commit ac2909f
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 4 deletions.
diff --git a/developer/src/kmc-ldml/src/compiler/ldml-compiler-messages.ts b/developer/src/kmc-ldml/src/compiler/ldml-compiler-messages.ts
@@ -187,7 +187,13 @@ export class LdmlCompilerMessages {
   static Error_UnparseableReorderSet = (o: { from: string, set: string }) =>
   m(this.ERROR_UnparseableReorderSet, `Illegal UnicodeSet "${def(o.set)}" in reorder "${def(o.from)}`);
 
-  // Available: 0x029
+  static ERROR_InvalidVariableIdentifer = SevError | 0x0029;
+  static Error_InvalidVariableIdentifer = (o: { id: string }) => m(
+    this.ERROR_InvalidVariableIdentifer,
+    `Invalid variable identifier "\\u${def(o.id)}". Identifiers must be between 1 and 32 characters, and can use A-Z, a-z, 0-9, and _.`,
+  );
+
+  // Available: 0x02A-0x2F
 
   static ERROR_InvalidQuadEscape = SevError | 0x0030;
   static Error_InvalidQuadEscape = (o: { cp: number }) =>
@@ -202,9 +208,12 @@ export class LdmlCompilerMessages {
   m(this.ERROR_UnparseableTransformFrom,    `Invalid transform from="${def(o.from)}": "${def(o.message)}"`);
 
   static ERROR_IllegalTransformDollarsign = SevErrorTransform | 0x01;
-  static Error_IllegalTransformDollarsign = (o: { from: string }) =>
-  m(this.ERROR_IllegalTransformDollarsign,  `Invalid transform from="${def(o.from)}": Unescaped dollar-sign ($) is not valid transform syntax.`,
-                                            '**Hint**: Use `\\$` to match a literal dollar-sign.');
+  static Error_IllegalTransformDollarsign = (o: { from: string }) => m(
+    this.ERROR_IllegalTransformDollarsign,
+    `Invalid transform from="${def(o.from)}": Unescaped dollar-sign ($) is not valid transform syntax.`,
+    `**Hint**: Use \`\\$\` to match a literal dollar-sign. If this precedes a variable name, `+
+    `the variable name may not be valid (A-Z, a-z, 0-9, _, 32 character maximum).`
+  );
 
   static ERROR_TransformFromMatchesNothing = SevErrorTransform | 0x02;
   static Error_TransformFromMatchesNothing = (o: { from: string }) =>

diff --git a/developer/src/kmc-ldml/src/compiler/vars.ts b/developer/src/kmc-ldml/src/compiler/vars.ts
@@ -40,6 +40,14 @@ export class VarsCompiler extends SectionCompiler {
     return valid;
   }
 
+  private validateIdentifier(id: string) {
+    if(!id.match(VariableParser.ID)) { // From <string> DTD
+      this.callbacks.reportMessage(LdmlCompilerMessages.Error_InvalidVariableIdentifer({id}));
+      return false;
+    }
+    return true;
+  }
+
   private validateVars(st: Substitutions): boolean {
     let valid = true;
     const variables = this.keyboard3?.variables;
@@ -67,6 +75,10 @@ export class VarsCompiler extends SectionCompiler {
     if (variables) {
       // Strings
       for (const { id, value } of variables.string) {
+        if(!this.validateIdentifier(id)) {
+          valid = false;
+          continue;
+        }
         addId(id);
         const stringrefs = VariableParser.allStringReferences(value);
         for(const ref of stringrefs) {
@@ -81,6 +93,10 @@ export class VarsCompiler extends SectionCompiler {
       }
       // Sets
       for (const { id, value } of variables.set) {
+        if(!this.validateIdentifier(id)) {
+          valid = false;
+          continue;
+        }
         addId(id);
         allSets.add(id);
         // check for illegal references, here.
@@ -103,6 +119,10 @@ export class VarsCompiler extends SectionCompiler {
       }
       // UnicodeSets
       for (const { id, value } of variables.uset) {
+        if(!this.validateIdentifier(id)) {
+          valid = false;
+          continue;
+        }
         addId(id);
         allUnicodeSets.add(id);
         const stringrefs = VariableParser.allStringReferences(value);

diff --git a/developer/src/kmc-ldml/test/fixtures/sections/vars/fail-invalid-identifiers.xml b/developer/src/kmc-ldml/test/fixtures/sections/vars/fail-invalid-identifiers.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
+  <info name="vars-minimal"/>
+
+  <keys />
+
+  <!-- from spec -->
+  <variables>
+    <string id="invalid-string" value="yes" /> <!-- ERROR_InvalidVariableIdentifer -->
+    <set id="invalid-set" value="A B C D E FF" /> <!-- ERROR_InvalidVariableIdentifer -->
+    <uset id="invalid-uset" value="[कसतनमह]" /> <!-- ERROR_InvalidVariableIdentifer -->
+    <set id="a_marker_name_more_than_32_chars_long" value="Y E S" /> <!-- ERROR_InvalidVariableIdentifer -->
+    <string id="😡" value="UPPERCASE" /> <!-- ERROR_InvalidVariableIdentifer -->
+  </variables>
+
+</keyboard3>
diff --git a/developer/src/kmc-ldml/test/test-vars.ts b/developer/src/kmc-ldml/test/test-vars.ts
@@ -121,6 +121,16 @@ describe('vars', function () {
         LdmlCompilerMessages.Error_DuplicateVariable({ids: 'upper, y'})
       ],
     },
+    {
+      subpath: 'sections/vars/fail-invalid-identifiers.xml',
+      errors: [
+        LdmlCompilerMessages.Error_InvalidVariableIdentifer({id: 'invalid-string'}),
+        LdmlCompilerMessages.Error_InvalidVariableIdentifer({id: 'invalid-set'}),
+        LdmlCompilerMessages.Error_InvalidVariableIdentifer({id: 'invalid-uset'}),
+        LdmlCompilerMessages.Error_InvalidVariableIdentifer({id: 'a_marker_name_more_than_32_chars_long'}),
+        LdmlCompilerMessages.Error_InvalidVariableIdentifer({id: '😡'}),
+      ],
+    },
     {
       subpath: 'sections/vars/fail-uset-props1.xml',
       errors: [