Merge pull request #9448 from keymanapp/feat/core/9119-marker-avoid-surrogate-cliff-epic-ldml

feat(core): limit max marker to 0xD7FF 🙀

Author: srl295

core/include/ldml/keyboardprocessor_ldml.h

@@ -93,10 +93,10 @@
 #define LDML_LENGTH_VARS_ITEM 0x10
 #define LDML_LENGTH_VKEY 0xC
 #define LDML_LENGTH_VKEY_ITEM 0x8
-#define LDML_MARKER_ANY_INDEX 0xFFFE
+#define LDML_MARKER_ANY_INDEX 0xD7FF
 #define LDML_MARKER_CODE 0x8
-#define LDML_MARKER_MAX_COUNT 0xFFFC
-#define LDML_MARKER_MAX_INDEX 0xFFFD
+#define LDML_MARKER_MAX_COUNT 0xD7FE
+#define LDML_MARKER_MAX_INDEX 0xD7FE
 #define LDML_MARKER_MIN_INDEX 0x1
 #define LDML_META_SETTINGS_FALLBACK_OMIT 0x1
 #define LDML_META_SETTINGS_TRANSFORMFAILURE_OMIT 0x2

core/include/ldml/keyboardprocessor_ldml.ts

@@ -622,11 +622,11 @@ class Constants {
   /** minimum usable marker index */
   readonly marker_min_index    = 0x0001;
   /** index value referring to the 'any' marker match */
-  readonly marker_any_index    = 0xFFFE;
+  readonly marker_any_index    = 0xD7FF;
   /** maximum marker index prior to the 'any' value */
   readonly marker_max_index    = this.marker_any_index - 1;
   /** maximum count of markers (not including 'any') */
-  readonly marker_max_count    = this.marker_max_index - this.marker_min_index;
+  readonly marker_max_count    = this.marker_max_index - this.marker_min_index + 1;
 
 };
 

core/src/kmx/kmx_plus.h

@@ -319,6 +319,11 @@ static_assert(LDML_UC_SENTINEL == UC_SENTINEL, "mismatch: LDML_UC_SENTINEL");
 static_assert(LDML_MARKER_CODE == CODE_DEADKEY, "mismatch: LDML_MARKER_CODE");
 static_assert(LDML_MARKER_ANY_INDEX < UC_SENTINEL, "expected LDML_MARKER_ANY_INDEX < UC_SENTINEL");
 
+/** @returns true if a valid marker per spec */
+static inline bool is_valid_marker(KMX_DWORD marker_no) {
+  return ((marker_no == LDML_MARKER_ANY_INDEX) || (marker_no >= LDML_MARKER_MIN_INDEX && marker_no <= LDML_MARKER_MAX_INDEX));
+}
+
 /* ------------------------------------------------------------------
  * bksp section
    ------------------------------------------------------------------ */

core/src/ldml/C9134_ldml_markers.md

@@ -34,8 +34,10 @@ Markers can appear in both 'emitting' and 'matching-only' areas:
 - Keyman already uses `UC_SENTINEL` `U+FFFF` (noncharacter), with `CODE_DEADKEY` (0x0008)
 - The general proposal here is to use the sequence `U+FFFF U+0008 U+XXXX` to represent marker #XXXX (starting with `U+0001`)
 - `U+FFFF` cannot otherwise occur in text, so it is unique
-- `U+FFFF U+0008 U+FFFE` to indicate 'any marker'  corresponds to `\m{.}`
-- This scheme allows for 65,533 (0xFFFD) unique markers, from `U+FFFF U+0008 U+0001` through `U+FFFF U+0008 U+FFFD`
+- `U+FFFF U+0008 U+D7FF` to indicate 'any marker'  corresponds to `\m{.}`
+- The max marker identifier will be `0xD7FE`, with `0xD7FF` reserved to represent 'any marker' if that is needed in the text stream.
+- This scheme allows for 55,294 unique markers, from `U+FFFF U+0008 U+0001` through `U+FFFF U+0008 U+D7FE` inclusive.
+- This scheme avoids the Unicode surrogate space beginning at `U+D800` and other noncharacters.
 
 ## Terminology
 - A marker's "number" is its position in the `markers` list, starting at index 1 (U+0001) being the first element in that list.

core/src/ldml/ldml_processor.cpp

@@ -373,8 +373,6 @@ ldml_processor::emit_text(km_kbp_state *state, const std::u32string &str) {
       it++; // consume LDML_MARKER_CODE
       assert(it < str.end());
       const auto marker_no = *it;
-      assert(marker_no >= LDML_MARKER_MIN_INDEX);
-      assert(marker_no <= LDML_MARKER_ANY_INDEX);
       emit_marker(state, marker_no);
     } else {
       emit_text(state, ch);
@@ -389,10 +387,11 @@ ldml_processor::emit_text(km_kbp_state *state, km_kbp_usv ch) {
   state->actions().push_character(ch);
 }
 
-void ldml_processor::emit_marker(km_kbp_state *state, KMX_DWORD marker_no) {
-    // OK, push the marker
-    state->actions().push_marker(marker_no);
-    state->context().push_marker(marker_no);
+void
+ldml_processor::emit_marker(km_kbp_state *state, KMX_DWORD marker_no) {
+  assert(km::kbp::kmx::is_valid_marker(marker_no));
+  state->actions().push_marker(marker_no);
+  state->context().push_marker(marker_no);
 }
 
 size_t
@@ -406,6 +405,7 @@ ldml_processor::context_to_string(km_kbp_state *state, std::u32string &str) {
       if (last_type == KM_KBP_BT_CHAR) {
         str.insert(0, 1, c->character);
       } else if (last_type == KM_KBP_BT_MARKER) {
+        assert(km::kbp::kmx::is_valid_marker(c->marker));
         prepend_marker(str, c->marker);
       } else {
         break;