Merge pull request #11806 from keymanapp/fix/developer/11643-prevent-…

…non-bmp-chars-in-key-part-of-rule

fix(developer): prevent non-BMP characters in key part of rule

developer/src/common/include/kmn_compiler_errors.h

@@ -179,6 +179,7 @@
 #define CERR_ExtendedStringTooLong                         0x00004076
 #define CERR_VirtualKeyExpansionTooLong                    0x00004077
 #define CERR_CharacterRangeTooLong                         0x00004078
+#define CERR_NonBMPCharactersNotSupportedInKeySection      0x00004079
 
 #define CWARN_TooManyWarnings                              0x00002080
 #define CWARN_OldVersion                                   0x00002081

developer/src/kmc-kmn/src/compiler/kmn-compiler-messages.ts

@@ -571,6 +571,8 @@ export class KmnCompilerMessages {
   static ERROR_CharacterRangeTooLong                          = SevError | 0x078;
   static Error_CharacterRangeTooLong                          = () => m(this.ERROR_CharacterRangeTooLong, `Character range is too large and cannot be expanded`);
 
+  static ERROR_NonBMPCharactersNotSupportedInKeySection       = SevError | 0x079;
+  static Error_NonBMPCharactersNotSupportedInKeySection       = () => m(this.ERROR_NonBMPCharactersNotSupportedInKeySection, `Characters with codepoints over U+FFFF are not supported in the key part of the rule`);
 
   static WARN_TooManyWarnings                                 = SevWarn | 0x080;
   static Warn_TooManyWarnings                                 = () => m(this.WARN_TooManyWarnings, `Too many warnings or errors`);

developer/src/kmcmplib/src/CompMsg.cpp

@@ -111,6 +111,7 @@ std::map<KMX_DWORD, const KMX_CHAR*> CompilerErrorMap = {
     { CERR_ExtendedStringTooLong                         , "Extended string is too long" },
     { CERR_VirtualKeyExpansionTooLong                    , "Virtual key expansion is too large" },
     { CERR_CharacterRangeTooLong                         , "Character range is too large and cannot be expanded" },
+    { CERR_NonBMPCharactersNotSupportedInKeySection      , "Characters with codepoints over U+FFFF are not supported in the key part of the rule" },
 
     { CHINT_UnreachableRule                              , "This rule will never be matched as another rule takes precedence"},
     { CHINT_NonUnicodeFile                               , "Keyman Developer has detected that the file has ANSI encoding. Consider converting this file to UTF-8"},

developer/src/kmcmplib/src/Compiler.cpp

@@ -1211,7 +1211,7 @@ int GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store) {
 KMX_BOOL IsValidKeyboardVersion(KMX_WCHAR *dpString) {   // I4140
   /**
     version format: /^\d+(\.\d+)*$/
-    e.g. 9.0.3, 1.0, 1.2.3.4, 6.2.1.4.6.4, 11.22.3 are all ok; 
+    e.g. 9.0.3, 1.0, 1.2.3.4, 6.2.1.4.6.4, 11.22.3 are all ok;
     empty string is not permitted; whitespace is not permitted
   */
 
@@ -1472,7 +1472,14 @@ KMX_DWORD ProcessKeyLineImpl(PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX_BOOL IsUnico
 
     str = p + 1;
     if ((msg = GetXString(fk, str, u">", pklKey, GLOBAL_BUFSIZE - 1, (int)(str - pp), &p, TRUE, IsUnicode)) != CERR_None) return msg;
+
     if (pklKey[0] == 0) return CERR_ZeroLengthString;
+
+    if(Uni_IsSurrogate1(pklKey[0])) {
+      // #11643: non-BMP characters do not makes sense for key codes
+      return CERR_NonBMPCharactersNotSupportedInKeySection;
+    }
+
     if (xstrlen(pklKey) > 1) AddWarning(CWARN_KeyBadLength);
   } else {
     if ((msg = GetXString(fk, str, u">", pklIn, GLOBAL_BUFSIZE - 1, (int)(str - pp), &p, TRUE, IsUnicode)) != CERR_None) return msg;
@@ -1654,9 +1661,10 @@ KMX_DWORD ExpandKp(PFILE_KEYBOARD fk, PFILE_KEY kpp, KMX_DWORD storeIndex)
       default:
         return CERR_CodeInvalidInKeyStore;
       }
-    }
-    else
-    {
+    } else if(Uni_IsSurrogate1(*pn)) {
+      // #11643: non-BMP characters do not makes sense for key codes
+      return CERR_NonBMPCharactersNotSupportedInKeySection;
+    } else {
       k->Key = *pn;				// set the key to store offset.
       k->ShiftFlags = 0;
     }

developer/src/kmcmplib/tests/gtest-compiler-test.cpp

@@ -19,6 +19,7 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX
 );
 KMX_DWORD GetRHS(PFILE_KEYBOARD fk, PKMX_WCHAR p, PKMX_WCHAR buf, int bufsize, int offset, int IsUnicode);
 bool hasPreamble(std::u16string result);
+KMX_DWORD ProcessKeyLineImpl(PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX_BOOL IsUnicode, PKMX_WCHAR pklIn, PKMX_WCHAR pklKey, PKMX_WCHAR pklOut);
 
 extern kmcmp_CompilerMessageProc msgproc;
 
@@ -84,6 +85,20 @@ class CompilerTest : public testing::Test {
             fk.extra             = nullptr;
         }
 
+        void initFileGroupArray(FILE_KEYBOARD &fk, KMX_BOOL fUsingKeys) {
+            fk.dpGroupArray = new FILE_GROUP[1];
+            fk.cxGroupArray = 1;
+
+            fk.dpGroupArray->szName[0] = 0;
+            fk.dpGroupArray->cxKeyArray = 0;
+            fk.dpGroupArray->dpKeyArray = nullptr;
+            fk.dpGroupArray->dpMatch = nullptr;
+            fk.dpGroupArray->dpNoMatch = nullptr;
+            fk.dpGroupArray->fUsingKeys = fUsingKeys;
+            fk.dpGroupArray->fReadOnly = FALSE;
+            fk.dpGroupArray->Line = 0;
+        }
+
         void deleteFileKeyboard(FILE_KEYBOARD &fk) {
             if (fk.dpStoreArray)   { delete[] fk.dpStoreArray;   }
             if (fk.dpGroupArray)   { delete[] fk.dpGroupArray;   }
@@ -266,7 +281,32 @@ TEST_F(CompilerTest, IsValidKeyboardVersion_test) {
 // KMX_DWORD InjectContextToReadonlyOutput(PKMX_WCHAR pklOut)
 // KMX_DWORD CheckOutputIsReadonly(const PFILE_KEYBOARD fk, const PKMX_WCHAR output)
 // KMX_DWORD ProcessKeyLine(PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX_BOOL IsUnicode)
-// KMX_DWORD ProcessKeyLineImpl(PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX_BOOL IsUnicode, PKMX_WCHAR pklIn, PKMX_WCHAR pklKey, PKMX_WCHAR pklOut)
+
+TEST_F(CompilerTest, ProcessKeyLineImpl_test) {
+    initFileGroupArray(fileKeyboard, TRUE);
+
+    PKMX_WCHAR pklIn, pklKey, pklOut;
+    KMX_WCHAR str[128];
+
+    pklIn  = new KMX_WCHAR[GLOBAL_BUFSIZE];
+    pklKey = new KMX_WCHAR[GLOBAL_BUFSIZE];
+    pklOut = new KMX_WCHAR[GLOBAL_BUFSIZE];
+
+    // #11643: non-BMP characters do not makes sense for key codes
+    u16cpy(str, u"+ 'A' > 'test'\n"); // baseline
+    EXPECT_EQ(CERR_None, ProcessKeyLineImpl(&fileKeyboard, str, TRUE, pklIn, pklKey, pklOut));
+
+    u16cpy(str, u"+ '\U00010000' > 'test'\n"); // surrogate pair
+    EXPECT_EQ(CERR_NonBMPCharactersNotSupportedInKeySection, ProcessKeyLineImpl(&fileKeyboard, str, TRUE, pklIn, pklKey, pklOut));
+
+    delete[] pklIn;
+    delete[] pklKey;
+    delete[] pklOut;
+
+    // TODO: other tests for this function
+
+}
+
 // KMX_DWORD ExpandKp_ReplaceIndex(PFILE_KEYBOARD fk, PFILE_KEY k, KMX_DWORD keyIndex, int nAnyIndex)
 // KMX_DWORD ExpandKp(PFILE_KEYBOARD fk, PFILE_KEY kpp, KMX_DWORD storeIndex)
 // PKMX_WCHAR GetDelimitedString(PKMX_WCHAR *p, KMX_WCHAR const * Delimiters, KMX_WORD Flags)