Merge pull request #13365 from keymanapp/fix/developer/13348-handle-non-bmp-numeric-entities-in-xml-reader

fix(developer): handle non bmp numeric entities in xml reader

developer/src/common/web/utils/package.json

@@ -12,7 +12,7 @@
     "@keymanapp/common-types": "*",
     "@sentry/node": "^7.57.0",
     "eventemitter3": "^5.0.0",
-    "fast-xml-parser": "^4.5.0",
+    "fast-xml-parser": "^5.0.9",
     "path-browserify": "^1.0.1",
     "restructure": "^3.0.1",
     "sax": ">=0.6.0",

developer/src/common/web/utils/src/xml-utils.ts

@@ -6,7 +6,7 @@
  * Abstraction for XML reading and writing
  */
 
-import { XMLParser, XMLBuilder } from 'fast-xml-parser';
+import { XMLParser, XMLBuilder, XmlBuilderOptions, X2jOptions } from 'fast-xml-parser';
 
 export type KeymanXMLType =
   'keyboard3'           // LDML <keyboard3>
@@ -17,102 +17,87 @@ export type KeymanXMLType =
   ;
 
 /** Bag of options, maximally one for each KeymanXMLType */
-type KeymanXMLOptionsBag = {
-  [key in KeymanXMLType]?: any
+type KeymanXMLParserOptionsBag = {
+  [key in KeymanXMLType]?: X2jOptions;
+};
+
+const PARSER_COMMON_OPTIONS: X2jOptions = {
+  attributeNamePrefix: '$', // causes remapping into $: { … } objects
+  htmlEntities: true,
+  ignoreAttributes: false,
+  ignorePiTags: true,
+  numberParseOptions: { // TODO: query is this option really necessary?
+    eNotation: null,
+    hex: null,
+    leadingZeros: null,
+    skipLike: /(?:)/, // parse numbers as strings
+  },
+  textNodeName: '_',
 };
 
 /** map of options for the XML parser */
-const PARSER_OPTIONS: KeymanXMLOptionsBag = {
+const PARSER_OPTIONS: KeymanXMLParserOptionsBag = {
   'keyboard3': {
-    ignoreAttributes: false, // We'd like attributes, please
     attributeNamePrefix: '@__', // We'll use this to convert attributes to strings and subobjects to arrays, when empty.
-    trimValues: false, // preserve spaces, but:
     htmlEntities: true,
-    tagValueProcessor: (tagName: string, tagValue: string /*, jPath, hasAttributes, isLeafNode*/) => {
+    ignoreAttributes: false, // We'd like attributes, please
+    tagValueProcessor: (_tagName: string, tagValue: string /*, jPath, hasAttributes, isLeafNode*/) => {
       // since trimValues: false, we need to zap any element values that would be trimmed.
       // currently, the LDML spec doesn't have any element values, but this
       // future-proofs us a little in that element values are allowed, just trimmed.
       // if we do need elements in the future, we'd check the preserve-space attribute here.
       return tagValue?.trim();
     },
+    trimValues: false, // preserve spaces, but see tagValueProcessor
   },
   'keyboardTest3': {
-    ignorePiTags: true,
+    attributeNamePrefix: '', // avoid @_
     htmlEntities: true,
     ignoreAttributes: false, // We'd like attributes, please
-    attributeNamePrefix: '', // avoid @_
+    ignorePiTags: true,
     preserveOrder: true,     // Gives us a 'special' format
   },
   'kps': {
-    ignorePiTags: true,
-    ignoreAttributes: false,
-    htmlEntities: true,
-    attributeNamePrefix: '$', // causes remapping into $: { … } objects
-    textNodeName: '_',
-    numberParseOptions: {
-      skipLike: /(?:)/, // parse numbers as strings
-      hex: null,
-      leadingZeros: null,
-      eNotation: null,
-    },
+    ...PARSER_COMMON_OPTIONS,
   },
   'kpj': {
-    ignorePiTags: true,
-    textNodeName: '_',
-    htmlEntities: true,
-    ignoreAttributes: false, // We'd like attributes, please
+    ...PARSER_COMMON_OPTIONS,
     attributeNamePrefix: '', // to avoid '@_' prefixes
-    numberParseOptions: {
-      skipLike: /(?:)/, // parse numbers as strings
-      hex: null,
-      leadingZeros: null,
-      eNotation: null,
-    },
   },
   'kvks': {
-    ignorePiTags: true,
-    textNodeName: '_',
-    htmlEntities: true,
-    ignoreAttributes: false, // We'd like attributes, please
-    attributeNamePrefix: '$', // causes remapping into $: { … } objects
-    numberParseOptions: {
-      skipLike: /(?:)/, // parse numbers as strings
-      hex: null,
-      leadingZeros: null,
-      eNotation: null,
-    },
-    trimValues: false, // preserve spaces, but:
-    tagValueProcessor: (tagName: string, tagValue: string, jPath: string, hasAttributes: string, isLeafNode: boolean) : string | undefined => {
+    ...PARSER_COMMON_OPTIONS,
+    tagValueProcessor: (_tagName: string, tagValue: string, _jPath: string, _hasAttributes: boolean, isLeafNode: boolean) : string | undefined => {
       if (!isLeafNode) {
         return tagValue?.trim(); // trimmed value
       } else {
         return null;  // no change to leaf nodes
       }
     },
+    trimValues: false, // preserve spaces
   },
 };
 
-const GENERATOR_OPTIONS: KeymanXMLOptionsBag = {
+type KeymanXMLGeneratorOptionsBag = {
+  [key in KeymanXMLType]?: XmlBuilderOptions
+};
+
+const GENERATOR_COMMON_OPTIONS: XmlBuilderOptions = {
+  attributeNamePrefix: '$',
+  ignoreAttributes: false,
+  format: true,
+  textNodeName: '_',
+  suppressEmptyNode: true,
+};
+
+const GENERATOR_OPTIONS: KeymanXMLGeneratorOptionsBag = {
   kvks: {
-    attributeNamePrefix: '$',
-    ignoreAttributes: false,
-    format: true,
-    textNodeName: '_',
-    suppressEmptyNode: true,
+    ...GENERATOR_COMMON_OPTIONS,
   },
   kpj: {
-    attributeNamePrefix: '$',
-    ignoreAttributes: false,
-    format: true,
-    textNodeName: '_',
-    suppressEmptyNode: true,
+    ...GENERATOR_COMMON_OPTIONS,
   },
   kps: {
-    attributeNamePrefix: '$',
-    ignoreAttributes: false,
-    format: true,
-    textNodeName: '_',
-    suppressEmptyNode: true,
+    ...GENERATOR_COMMON_OPTIONS,
   },
 };
 
@@ -263,9 +248,6 @@ export class KeymanXMLReader {
       throw Error(`Internal error: unhandled XML type ${this.type}`);
     }
     options = Object.assign({}, options); // TODO: xml2js likes to mutate the options here. Shallow clone the object.
-    if (options.emptyTag) {
-        options.emptyTag = {}; // TODO: xml2js likes to mutate the options here. Reset it.
-    }
     return new XMLParser(options);
   }
 }

developer/src/common/web/utils/test/fixtures/kvks/hex_escape.kvks

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="utf-8"?>
+<visualkeyboard>
+  <header>
+    <version>10.0</version>
+    <kbdname>hex_escape</kbdname>
+    <flags/>
+  </header>
+  <encoding name="unicode" fontname="arial" fontsize="-12">
+    <layer shift="">
+      <key vkey="K_1">&#x1234;</key>
+      <key vkey="K_2">&#256;</key>
+      <key vkey="K_3">&#65536;</key>
+      <key vkey="K_4">&#x12345;</key>
+    </layer>
+  </encoding>
+</visualkeyboard>

developer/src/common/web/utils/test/fixtures/ldml-keyboard/numeric-id.xml

@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="46">
+  <info author="srl295" indicator="🙀" layout="qwerty"  name="TestKbd"/>
+
+  <keys>
+    <key id="1" output="ħ" />
+    <key id="2" output="ថា" />
+  </keys>
+
+</keyboard3>

developer/src/common/web/utils/test/fixtures/ldml-keyboard/test-fr.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<!DOCTYPE keyboardTest3 SYSTEM "../../../../../resources/standards-data/ldml-keyboards/46/dtd/ldmlKeyboardTest3.dtd">
+<!DOCTYPE keyboardTest3 SYSTEM "../../../../../../../../resources/standards-data/ldml-keyboards/46/dtd/ldmlKeyboardTest3.dtd">
 <keyboardTest3 conformsTo="techpreview">
     <!--
       Read by:

developer/src/common/web/utils/test/kvks/kvks-file.tests.ts

@@ -40,6 +40,21 @@ describe('kvks-file-reader', function() {
     const reader = new KvksFileReader();
     assert.throws(() => reader.read(input), 'File appears to be a binary .kvk file');
   });
+
+  it('should read non-bmp hex escapes correctly', function() {
+    const path = makePathToFixture('kvks', 'hex_escape.kvks');
+    const input = fs.readFileSync(path);
+
+    const reader = new KvksFileReader();
+    const kvks = reader.read(input);
+    const invalidVkeys: string[] = [];
+    const vk = reader.transform(kvks, invalidVkeys);
+    assert.isEmpty(invalidVkeys);
+    assert.equal(vk.keys[0].text, '\u{1234}');
+    assert.equal(vk.keys[1].text, '\u{100}');
+    assert.equal(vk.keys[2].text, String.fromCodePoint(0x10000));
+    assert.equal(vk.keys[3].text, String.fromCodePoint(0x12345));
+  });
 });
 
 describe('kvks-file-writer', function() {

developer/src/common/web/utils/test/ldml/ldml-keyboard-testdata-reader.tests.ts

@@ -2,7 +2,7 @@ import { constants } from '@keymanapp/ldml-keyboard-constants';
 import { assert } from 'chai';
 import 'mocha';
 import { testTestdataReaderCases } from '../helpers/reader-callback-test.js';
-import { LKTAnyAction } from '../../src/types/ldml-keyboard/ldml-keyboard-testdata-xml.js';
+import { LKTAnyAction } from '../types/ldml-keyboard/ldml-keyboard-testdata-xml.js';
 
 describe('ldml keyboard xml reader tests', function () {
   this.slow(500); // 0.5 sec -- json schema validation takes a while

developer/src/common/web/utils/test/ldml/ldml-keyboard-xml-reader.tests.ts

@@ -1,9 +1,9 @@
-import { LKKey, ImportStatus } from '../../src/types/ldml-keyboard/ldml-keyboard-xml.js';
 import 'mocha';
 import {assert} from 'chai';
-import { CommonTypesMessages } from '../../src/common-messages.js';
-import { testReaderCases } from '../helpers/reader-callback-test.js';
 import { Constants } from '@keymanapp/common-types';
+import { CommonTypesMessages } from '../../src/common-messages.js';
+import { LKKey, ImportStatus } from '../../src/types/ldml-keyboard/ldml-keyboard-xml.js';
+import { testReaderCases } from '../helpers/reader-callback-test.js';
 
 import CLDRScanToVkey = Constants.CLDRScanToVkey;
 import CLDRScanToKeyMap = Constants.CLDRScanToKeyMap;
@@ -201,6 +201,25 @@ describe('ldml keyboard xml reader tests', function () {
         }),
       ],
     },
+    {
+      subpath: 'numeric-id.xml',
+      callback: (data, source, subpath, callbacks) => {
+        assert.ok(source?.keyboard3?.keys);
+        const k = pluckKeysFromKeybag(source?.keyboard3?.keys.key, ['1', '2']);
+        assert.sameDeepOrderedMembers(k.map((entry) => {
+          // Drop the Symbol members from the returned keys; assertions may expect their presence.
+          return {
+            id: entry.id,
+            output: entry.output
+          };
+        }), [
+          {id: '1', output: '1'}, //default import
+          {id: '2', output: '2'}, //default import
+          {id: '1', output: 'ħ'}, //override
+          {id: '2', output: 'ថា'}, //override
+        ]);
+      },
+    },
   ]);
 });
 

package-lock.json

@@ -342,7 +342,7 @@
         "@keymanapp/common-types": "*",
         "@sentry/node": "^7.57.0",
         "eventemitter3": "^5.0.0",
-        "fast-xml-parser": "^4.5.0",
+        "fast-xml-parser": "^5.0.9",
         "path-browserify": "^1.0.1",
         "restructure": "^3.0.1",
         "sax": ">=0.6.0",
@@ -8962,22 +8962,18 @@
       "dev": true
     },
     "node_modules/fast-xml-parser": {
-      "version": "4.5.0",
-      "resolved": "https://registry.npmjs.org/fast-xml-parser/-/fast-xml-parser-4.5.0.tgz",
-      "integrity": "sha512-/PlTQCI96+fZMAOLMZK4CWG1ItCbfZ/0jx7UIJFChPNrx7tcEgerUgWbeieCM9MfHInUDyK8DWYZ+YrywDJuTg==",
+      "version": "5.0.9",
+      "resolved": "https://registry.npmjs.org/fast-xml-parser/-/fast-xml-parser-5.0.9.tgz",
+      "integrity": "sha512-2mBwCiuW3ycKQQ6SOesSB8WeF+fIGb6I/GG5vU5/XEptwFFhp9PE8b9O7fbs2dpq9fXn4ULR3UsfydNUCntf5A==",
       "funding": [
         {
           "type": "github",
           "url": "https://github.com/sponsors/NaturalIntelligence"
-        },
-        {
-          "type": "paypal",
-          "url": "https://paypal.me/naturalintelligence"
         }
       ],
       "license": "MIT",
       "dependencies": {
-        "strnum": "^1.0.5"
+        "strnum": "^2.0.5"
       },
       "bin": {
         "fxparser": "src/cli/cli.js"
@@ -13804,9 +13800,15 @@
       "link": true
     },
     "node_modules/strnum": {
-      "version": "1.0.5",
-      "resolved": "https://registry.npmjs.org/strnum/-/strnum-1.0.5.tgz",
-      "integrity": "sha512-J8bbNyKKXl5qYcR36TIO8W3mVGVHrmmxsd5PAItGkmyzwJvybiw2IVq5nqd0i4LSNSkB/sx9VHllbfFdr9k1JA==",
+      "version": "2.0.5",
+      "resolved": "https://registry.npmjs.org/strnum/-/strnum-2.0.5.tgz",
+      "integrity": "sha512-YAT3K/sgpCUxhxNMrrdhtod3jckkpYwH6JAuwmUdXZsmzH1wUyzTMrrK2wYCEEqlKwrWDd35NeuUkbBy/1iK+Q==",
+      "funding": [
+        {
+          "type": "github",
+          "url": "https://github.com/sponsors/NaturalIntelligence"
+        }
+      ],
       "license": "MIT"
     },
     "node_modules/supports-color": {