From d1bd4ec651b30f58a5889d1527499b53496509e2 Mon Sep 17 00:00:00 2001 From: "Dr Mark C. Sinclair" Date: Fri, 1 Nov 2024 10:28:13 +0000 Subject: [PATCH 1/6] fix(developer): remove virtual key series --- developer/src/kmcmplib/src/Compiler.cpp | 2 ++ developer/src/kmcmplib/tests/gtest-compiler-test.cpp | 11 ++++------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/developer/src/kmcmplib/src/Compiler.cpp b/developer/src/kmcmplib/src/Compiler.cpp index 57702ee1f0f..656a6e85840 100644 --- a/developer/src/kmcmplib/src/Compiler.cpp +++ b/developer/src/kmcmplib/src/Compiler.cpp @@ -2561,6 +2561,8 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX } while (iswspace(*q)) q++; + if (*q != ']') return KmnCompilerMessages::ERROR_InvalidToken; + break; /* out of while loop */ } tstr[mx++] = UC_SENTINEL_EXTENDEDEND; tstr[mx] = 0; diff --git a/developer/src/kmcmplib/tests/gtest-compiler-test.cpp b/developer/src/kmcmplib/tests/gtest-compiler-test.cpp index e6a5a0f513f..d9a3fb7526d 100644 --- a/developer/src/kmcmplib/tests/gtest-compiler-test.cpp +++ b/developer/src/kmcmplib/tests/gtest-compiler-test.cpp @@ -2029,13 +2029,10 @@ TEST_F(CompilerTest, GetXStringImpl_type_osb_test) { EXPECT_EQ(KmnCompilerMessages::WARN_VirtualKeyWithMnemonicLayout, msgproc_errors[0].errorCode); msgproc_errors.clear(); - // virtual key, in VKeyNames, multiple keys, valid (see #12307) - // fileKeyboard.version = VERSION_90; - // u16cpy(str, u"[K_A K_B K_C]"); - // EXPECT_EQ(STATUS_Success, GetXStringImpl(tstr, &fileKeyboard, str, u"", output, 80, 0, &newp, FALSE)); - // sFlag = ISVIRTUALKEY; - // KMX_WCHAR tstr_virtual_key_multi_valid[] = { UC_SENTINEL, CODE_EXTENDED, sFlag, 65, 66, 67, UC_SENTINEL_EXTENDEDEND, 0 }; - // EXPECT_EQ(0, u16cmp(tstr_virtual_key_multi_valid, tstr)); + // virtual key, in VKeyNames, multiple keys, KmnCompilerMessages::ERROR_InvalidToken (see #12307) + fileKeyboard.version = VERSION_90; + u16cpy(str, u"[K_A K_B K_C]"); + EXPECT_EQ(KmnCompilerMessages::ERROR_InvalidToken, GetXStringImpl(tstr, &fileKeyboard, str, u"", output, 80, 0, &newp, FALSE)); } // KMX_DWORD process_baselayout(PFILE_KEYBOARD fk, PKMX_WCHAR q, PKMX_WCHAR tstr, int *mx) From 86a6bf617ba4cef526bb19e919409afd7862605d Mon Sep 17 00:00:00 2001 From: "Dr Mark C. Sinclair" Date: Fri, 1 Nov 2024 10:40:58 +0000 Subject: [PATCH 2/6] fix(developer): remove redundant virtual key series while loop --- developer/src/kmcmplib/src/Compiler.cpp | 48 ++++++++++--------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/developer/src/kmcmplib/src/Compiler.cpp b/developer/src/kmcmplib/src/Compiler.cpp index 656a6e85840..8ea18abeac0 100644 --- a/developer/src/kmcmplib/src/Compiler.cpp +++ b/developer/src/kmcmplib/src/Compiler.cpp @@ -2498,27 +2498,22 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX return KmnCompilerMessages::ERROR_InvalidToken; // I3137 - key portion of VK is missing e.g. "[CTRL ALT]", this generates invalid kmx file that can crash Keyman or compiler later on // I3511 } - while (*q != ']') - { - if (*q == '\'' || *q == '"') - { - if(!VerifyKeyboardVersion(fk, VERSION_60)) { - return KmnCompilerMessages::ERROR_60FeatureOnly_VirtualCharKey; - } - if (!kmcmp::FMnemonicLayout) { - ReportCompilerMessage(KmnCompilerMessages::WARN_VirtualCharKeyWithPositionalLayout); - } - KMX_WCHAR chQuote = *q; - q++; if (*q == chQuote || *q == '\n' || *q == 0) return KmnCompilerMessages::ERROR_InvalidToken; - tstr[mx - 1] |= VIRTUALCHARKEY; - tstr[mx++] = *q; - q++; if (*q != chQuote) return KmnCompilerMessages::ERROR_InvalidToken; - q++; - while (iswspace(*q)) q++; - if (*q != ']') return KmnCompilerMessages::ERROR_InvalidToken; - break; /* out of while loop */ + if (*q == '\'' || *q == '"') { + if(!VerifyKeyboardVersion(fk, VERSION_60)) { + return KmnCompilerMessages::ERROR_60FeatureOnly_VirtualCharKey; } - + if (!kmcmp::FMnemonicLayout) { + ReportCompilerMessage(KmnCompilerMessages::WARN_VirtualCharKeyWithPositionalLayout); + } + KMX_WCHAR chQuote = *q; + q++; if (*q == chQuote || *q == '\n' || *q == 0) return KmnCompilerMessages::ERROR_InvalidToken; + tstr[mx - 1] |= VIRTUALCHARKEY; + tstr[mx++] = *q; + q++; if (*q != chQuote) return KmnCompilerMessages::ERROR_InvalidToken; + q++; + while (iswspace(*q)) q++; + if (*q != ']') return KmnCompilerMessages::ERROR_InvalidToken; + } else { for (j = 0; !iswspace(*q) && *q != ']' && *q != 0; q++, j++); if (*q == 0) return KmnCompilerMessages::ERROR_InvalidToken; @@ -2532,17 +2527,14 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX if (u16icmp(vkname, u"K_NPENTER") == 0) i = 5; // I649 - K_NPENTER hack - else - { - for (i = 0; i <= VK__MAX; i++) - { + else { + for (i = 0; i <= VK__MAX; i++) { if (u16icmp(vkname, VKeyNames[i]) == 0 || u16icmp(vkname, VKeyISO9995Names[i]) == 0) break; } } - if (i == VK__MAX + 1) - { + if (i == VK__MAX + 1) { if(!VerifyKeyboardVersion(fk, VERSION_90)) { return KmnCompilerMessages::ERROR_InvalidToken; } @@ -2552,8 +2544,6 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX return KmnCompilerMessages::ERROR_InvalidToken; } - p = q; - tstr[mx++] = (int)i; if (kmcmp::FMnemonicLayout && (i <= VK__MAX) && VKeyMayBeVCKey[i]) { @@ -2562,8 +2552,8 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX while (iswspace(*q)) q++; if (*q != ']') return KmnCompilerMessages::ERROR_InvalidToken; - break; /* out of while loop */ } + tstr[mx++] = UC_SENTINEL_EXTENDEDEND; tstr[mx] = 0; //printf("--EXTENDEDEND--\n"); From 72a27464f5a167f5db7c50afeafcee9ea83f5710 Mon Sep 17 00:00:00 2001 From: "Dr Mark C. Sinclair" Date: Fri, 1 Nov 2024 10:48:49 +0000 Subject: [PATCH 3/6] fix(developer): refactor virtual key whitespace loop and improve layout --- developer/src/kmcmplib/src/Compiler.cpp | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/developer/src/kmcmplib/src/Compiler.cpp b/developer/src/kmcmplib/src/Compiler.cpp index 8ea18abeac0..1e3b457c27b 100644 --- a/developer/src/kmcmplib/src/Compiler.cpp +++ b/developer/src/kmcmplib/src/Compiler.cpp @@ -2502,17 +2502,22 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX if(!VerifyKeyboardVersion(fk, VERSION_60)) { return KmnCompilerMessages::ERROR_60FeatureOnly_VirtualCharKey; } + if (!kmcmp::FMnemonicLayout) { ReportCompilerMessage(KmnCompilerMessages::WARN_VirtualCharKeyWithPositionalLayout); } + KMX_WCHAR chQuote = *q; + q++; if (*q == chQuote || *q == '\n' || *q == 0) return KmnCompilerMessages::ERROR_InvalidToken; + tstr[mx - 1] |= VIRTUALCHARKEY; tstr[mx++] = *q; - q++; if (*q != chQuote) return KmnCompilerMessages::ERROR_InvalidToken; - q++; - while (iswspace(*q)) q++; - if (*q != ']') return KmnCompilerMessages::ERROR_InvalidToken; + + q++; // skip key + if (*q != chQuote) + return KmnCompilerMessages::ERROR_InvalidToken; + q++; // skip quote } else { for (j = 0; !iswspace(*q) && *q != ']' && *q != 0; q++, j++); @@ -2549,11 +2554,13 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX if (kmcmp::FMnemonicLayout && (i <= VK__MAX) && VKeyMayBeVCKey[i]) { ReportCompilerMessage(KmnCompilerMessages::WARN_VirtualKeyWithMnemonicLayout); // I3438 } - - while (iswspace(*q)) q++; - if (*q != ']') return KmnCompilerMessages::ERROR_InvalidToken; } + while (iswspace(*q)) q++; + + if (*q != ']') + return KmnCompilerMessages::ERROR_InvalidToken; + tstr[mx++] = UC_SENTINEL_EXTENDEDEND; tstr[mx] = 0; //printf("--EXTENDEDEND--\n"); From 94019b92b5c3c157985b2b9eb74fca8be90f79f2 Mon Sep 17 00:00:00 2001 From: "Dr Mark C. Sinclair" Date: Fri, 1 Nov 2024 11:43:16 +0000 Subject: [PATCH 4/6] fix(developer): add wsRequired flag to check for whitespace between modifiers and modifier(s) and key --- developer/src/kmcmplib/src/Compiler.cpp | 33 ++++++++++--------- .../kmcmplib/tests/gtest-compiler-test.cpp | 30 ++++++++--------- 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/developer/src/kmcmplib/src/Compiler.cpp b/developer/src/kmcmplib/src/Compiler.cpp index 1e3b457c27b..b7b17725224 100644 --- a/developer/src/kmcmplib/src/Compiler.cpp +++ b/developer/src/kmcmplib/src/Compiler.cpp @@ -2034,6 +2034,7 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX int type, mx = 0, n, n1, n2, tokenFound = FALSE, z, sFlag = 0, j; KMX_DWORD i; KMX_BOOL finished = FALSE; + KMX_BOOL wsRequired = FALSE; KMX_WCHAR c; *tstr = 0; @@ -2412,50 +2413,53 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX } continue; case 11: - p++; sFlag = ISVIRTUALKEY /* 0 */; finished = FALSE; + p++; sFlag = ISVIRTUALKEY /* 0 */; finished = FALSE; wsRequired = FALSE; //printf("--EXTENDEDSTRING--\n"); do { + if (wsRequired && !iswspace(*p)) + return KmnCompilerMessages::ERROR_InvalidToken; // #12307 + while (iswspace(*p)) p++; switch (towupper(*p)) { case 'N': if (u16nicmp(p, u"NCAPS", 5) == 0) - sFlag |= NOTCAPITALFLAG, p += 5; + sFlag |= NOTCAPITALFLAG, wsRequired = TRUE, p += 5; else finished = TRUE; break; case 'L': if (u16nicmp(p, u"LALT", 4) == 0) - sFlag |= LALTFLAG, p += 4; + sFlag |= LALTFLAG, wsRequired = TRUE, p += 4; else if (u16nicmp(p, u"LCTRL", 5) == 0) - sFlag |= LCTRLFLAG, p += 5; + sFlag |= LCTRLFLAG, wsRequired = TRUE, p += 5; else finished = TRUE; break; case 'R': if (u16nicmp(p, u"RALT", 4) == 0) - sFlag |= RALTFLAG, p += 4; + sFlag |= RALTFLAG, wsRequired = TRUE, p += 4; else if (u16nicmp(p, u"RCTRL", 5) == 0) - sFlag |= RCTRLFLAG, p += 5; + sFlag |= RCTRLFLAG, wsRequired = TRUE, p += 5; else finished = TRUE; break; case 'A': if (u16nicmp(p, u"ALT", 3) == 0) - sFlag |= K_ALTFLAG, p += 3; + sFlag |= K_ALTFLAG, wsRequired = TRUE, p += 3; else finished = TRUE; break; case 'C': if (u16nicmp(p, u"CTRL", 4) == 0) - sFlag |= K_CTRLFLAG, p += 4; + sFlag |= K_CTRLFLAG, wsRequired = TRUE, p += 4; else if (u16nicmp(p, u"CAPS", 4) == 0) - sFlag |= CAPITALFLAG, p += 4; + sFlag |= CAPITALFLAG, wsRequired = TRUE, p += 4; else finished = TRUE; break; case 'S': if (u16nicmp(p, u"SHIFT", 5) == 0) - sFlag |= K_SHIFTFLAG, p += 5; + sFlag |= K_SHIFTFLAG, wsRequired = TRUE, p += 5; else finished = TRUE; break; default: @@ -2489,12 +2493,9 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX tstr[mx++] = CODE_EXTENDED; tstr[mx++] = sFlag; - while (iswspace(*p)) p++; - q = p; - if (*q == ']') - { + if (*q == ']') { return KmnCompilerMessages::ERROR_InvalidToken; // I3137 - key portion of VK is missing e.g. "[CTRL ALT]", this generates invalid kmx file that can crash Keyman or compiler later on // I3511 } @@ -2509,7 +2510,9 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX KMX_WCHAR chQuote = *q; - q++; if (*q == chQuote || *q == '\n' || *q == 0) return KmnCompilerMessages::ERROR_InvalidToken; + q++; // skip quote + if (*q == chQuote || *q == '\n' || *q == 0) + return KmnCompilerMessages::ERROR_InvalidToken; tstr[mx - 1] |= VIRTUALCHARKEY; tstr[mx++] = *q; diff --git a/developer/src/kmcmplib/tests/gtest-compiler-test.cpp b/developer/src/kmcmplib/tests/gtest-compiler-test.cpp index d9a3fb7526d..ba66e313656 100644 --- a/developer/src/kmcmplib/tests/gtest-compiler-test.cpp +++ b/developer/src/kmcmplib/tests/gtest-compiler-test.cpp @@ -1807,25 +1807,25 @@ TEST_F(CompilerTest, GetXStringImpl_type_osb_test) { u16cpy(str, u"[CTRL ALT]"); EXPECT_EQ(KmnCompilerMessages::ERROR_InvalidToken, GetXStringImpl(tstr, &fileKeyboard, str, u"", output, 80, 0, &newp, FALSE)); + // virtual key, modifiers only (no key portion), space after, KmnCompilerMessages::ERROR_InvalidToken + fileKeyboard.version = VERSION_90; + u16cpy(str, u"[CTRL ALT ]"); + EXPECT_EQ(KmnCompilerMessages::ERROR_InvalidToken, GetXStringImpl(tstr, &fileKeyboard, str, u"", output, 80, 0, &newp, FALSE)); + // virtual key, in VKeyNames, no space between modifier and key portion, ERROR_InvalidToken (see #12307) - // fileKeyboard.version = VERSION_90; - // u16cpy(str, u"[CTRLK_A]"); - // EXPECT_EQ(KmnCompilerMessages::ERROR_InvalidToken, GetXStringImpl(tstr, &fileKeyboard, str, u"", output, 80, 0, &newp, FALSE)); + fileKeyboard.version = VERSION_90; + u16cpy(str, u"[CTRLK_A]"); + EXPECT_EQ(KmnCompilerMessages::ERROR_InvalidToken, GetXStringImpl(tstr, &fileKeyboard, str, u"", output, 80, 0, &newp, FALSE)); // virtual key, in VKeyNames, no space between modifiers, ERROR_InvalidToken (see #12307) - // fileKeyboard.version = VERSION_90; - // u16cpy(str, u"[CTRLALT K_A]"); - // EXPECT_EQ(KmnCompilerMessages::ERROR_InvalidToken, GetXStringImpl(tstr, &fileKeyboard, str, u"", output, 80, 0, &newp, FALSE)); + fileKeyboard.version = VERSION_90; + u16cpy(str, u"[CTRLALT K_A]"); + EXPECT_EQ(KmnCompilerMessages::ERROR_InvalidToken, GetXStringImpl(tstr, &fileKeyboard, str, u"", output, 80, 0, &newp, FALSE)); // virtual key, '_' between modifier and key portion', ERROR_InvalidToken (see #12307) - // fileKeyboard.version = VERSION_90; - // u16cpy(str, u"[CTRL_K_A]"); - // EXPECT_EQ(STATUS_Success, GetXStringImpl(tstr, &fileKeyboard, str, u"", output, 80, 0, &newp, FALSE)); - // tstr_virtual_key_valid[2] = ISVIRTUALKEY | K_CTRLFLAG; - // tstr_virtual_key_valid[3] = 259; // VK__MAX + 4 - // EXPECT_EQ(0, u16cmp(tstr_virtual_key_valid, tstr)); - // EXPECT_EQ(0, msgproc_errors.size()); - // tstr_virtual_key_valid[3] = 65; + fileKeyboard.version = VERSION_90; + u16cpy(str, u"[CTRL_K_A]"); + EXPECT_EQ(KmnCompilerMessages::ERROR_InvalidToken, GetXStringImpl(tstr, &fileKeyboard, str, u"", output, 80, 0, &newp, FALSE)); // virtual char key, single quote, ERROR_60FeatureOnly_VirtualCharKey fileKeyboard.version = VERSION_50; @@ -2031,7 +2031,7 @@ TEST_F(CompilerTest, GetXStringImpl_type_osb_test) { // virtual key, in VKeyNames, multiple keys, KmnCompilerMessages::ERROR_InvalidToken (see #12307) fileKeyboard.version = VERSION_90; - u16cpy(str, u"[K_A K_B K_C]"); + u16cpy(str, u"[K_A K_B]"); EXPECT_EQ(KmnCompilerMessages::ERROR_InvalidToken, GetXStringImpl(tstr, &fileKeyboard, str, u"", output, 80, 0, &newp, FALSE)); } From ed42594b1aaff02bfc01cf6a4aa59f0507028a50 Mon Sep 17 00:00:00 2001 From: "Dr Mark C. Sinclair" Date: Fri, 1 Nov 2024 12:10:39 +0000 Subject: [PATCH 5/6] fix(developer): minor layout changes --- developer/src/kmcmplib/src/Compiler.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/developer/src/kmcmplib/src/Compiler.cpp b/developer/src/kmcmplib/src/Compiler.cpp index b7b17725224..e878e4690ec 100644 --- a/developer/src/kmcmplib/src/Compiler.cpp +++ b/developer/src/kmcmplib/src/Compiler.cpp @@ -2524,13 +2524,15 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX } else { for (j = 0; !iswspace(*q) && *q != ']' && *q != 0; q++, j++); - if (*q == 0) return KmnCompilerMessages::ERROR_InvalidToken; + if (*q == 0) + return KmnCompilerMessages::ERROR_InvalidToken; KMX_WCHAR vkname[SZMAX_VKDICTIONARYNAME]; // I3438 - if (j >= SZMAX_VKDICTIONARYNAME) return KmnCompilerMessages::ERROR_InvalidToken; + if (j >= SZMAX_VKDICTIONARYNAME) + return KmnCompilerMessages::ERROR_InvalidToken; - u16ncpy(vkname, p, j); // I3481 + u16ncpy(vkname, p, j); // I3481 vkname[j] = 0; if (u16icmp(vkname, u"K_NPENTER") == 0) From a33037b4df5da9eea38868ce83dc5a7a2c1e6005 Mon Sep 17 00:00:00 2001 From: "Dr Mark C. Sinclair" Date: Wed, 13 Nov 2024 16:54:59 +0000 Subject: [PATCH 6/6] fix(developer): add basic_kbdcherp and basic_kbdolch to list of excluded keyboards --- developer/src/kmcmplib/tests/get-test-source.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/developer/src/kmcmplib/tests/get-test-source.sh b/developer/src/kmcmplib/tests/get-test-source.sh index a195fc271dc..fe6cded2f19 100755 --- a/developer/src/kmcmplib/tests/get-test-source.sh +++ b/developer/src/kmcmplib/tests/get-test-source.sh @@ -9,8 +9,10 @@ set -eu find "$1" -name '*.kmn' | \ grep -E '(release|experimental)/([a-z0-9_]+)/([a-z0-9_]+)/source/\3\.kmn$' | \ - grep -vE 'masaram_gondi|anii|sil_kmhmu|fv_statimcets|fv_nuucaanul' + grep -vE 'masaram_gondi|anii|sil_kmhmu|fv_statimcets|fv_nuucaanul|basic_kbdcherp|basic_kbdolch' # #12623: exclude masaram_gondi due to #11806 # #12631: exclude anii, sil_kmhmu as ico references have mismatching case # #12631: exclude fv_statimcets, fv_nuucaanul as these include U+2002 which is not -# treated as whitespace on mac arch \ No newline at end of file +# treated as whitespace on mac arch +# #12604: exclude basic_kbdcherp, basic_kbdolch as these do not include now necessary +# whitespace, see also issue #12307