fix(id): plain_field escapes the shared Unicode 16 Cf set (#2839)

pkgs/id/crates/id-core/src/lib.rs

@@ -25,6 +25,8 @@ pub mod membership;
 pub mod sync_method;
 pub mod time;
 pub mod tuple;
+mod unicode;
+pub use unicode::is_format_char;
 pub mod waiting;
 
 #[cfg(feature = "world")]

pkgs/id/crates/id-core/src/petname.rs

@@ -13,6 +13,7 @@ use anyhow::{Context, Result, ensure};
 use serde::{Deserialize, Serialize};
 
 use crate::Refusal;
+pub use crate::is_format_char;
 
 const MAX_PETNAME_CHARS: usize = 64;
 const FINGERPRINT_CHARS: usize = 6;
@@ -233,38 +234,6 @@ fn looks_like(name: &str) -> anyhow::Error {
     Refusal::Conflict(format!("that name looks too much like {name}")).into()
 }
 
-/// Whether `c` is a Unicode format character (general category Cf):
-/// zero-width spaces and joiners, bidi controls and isolates, the word
-/// joiner, soft hyphen, and the rest of the category. Rust's
-/// `char::is_control` covers only category Cc, and Cf characters are
-/// invisible or reorder the text around them, so a name carrying one
-/// can render exactly like another name. Ranges are the Cf set of
-/// Unicode 16. Shared with the other name boundaries (invite names,
-/// #1770) so every surface refuses the same set.
-pub fn is_format_char(c: char) -> bool {
-    matches!(c,
-        '\u{00AD}'
-        | '\u{0600}'..='\u{0605}'
-        | '\u{06DD}'
-        | '\u{070F}'
-        | '\u{0890}'..='\u{0891}'
-        | '\u{08E2}'
-        | '\u{180E}'
-        | '\u{200B}'..='\u{200F}'
-        | '\u{202A}'..='\u{202E}'
-        | '\u{2060}'..='\u{2064}'
-        | '\u{2066}'..='\u{206F}'
-        | '\u{FEFF}'
-        | '\u{FFF9}'..='\u{FFFB}'
-        | '\u{110BD}'
-        | '\u{110CD}'
-        | '\u{13430}'..='\u{1343F}'
-        | '\u{1BCA0}'..='\u{1BCA3}'
-        | '\u{1D173}'..='\u{1D182}'
-        | '\u{E0001}'
-        | '\u{E0020}'..='\u{E007F}')
-}
-
 /// The name with every format character (category Cf) removed, for the
 /// comparison path: a petname stored before Cf was refused, or an
 /// alleged name carrying Cf, must compare by what it renders as.
@@ -741,6 +710,7 @@ mod tests {
             "Pay\u{200B}Pal",
             "Pay\u{202A}Pal",
             "Pay\u{202E}Pal",
+            "Pay\u{061C}Pal",
             "Pay\u{2060}Pal",
             "Pay\u{FEFF}Pal",
             "Pay\u{00AD}Pal",
@@ -749,6 +719,7 @@ mod tests {
             "Pay\u{FFF9}Pal",
             "Pay\u{E0001}Pal",
             "Pay\u{E0041}Pal",
+            "Pay\u{1D17A}Pal",
         ] {
             assert!(normalize_name(bad).is_err(), "{bad:?} must be refused");
         }

pkgs/id/crates/id-core/src/unicode.rs

@@ -0,0 +1,89 @@
+//! Unicode properties shared across the `id-core` and `id` crates.
+
+/// Whether `c` is a Unicode 16 format character (general category Cf).
+///
+/// This includes zero-width characters, bidi controls and isolates, soft
+/// hyphen, and other format controls. The predicate is shared by name
+/// validation and plain-text rendering so every surface uses the same set.
+#[must_use]
+pub fn is_format_char(c: char) -> bool {
+    matches!(c,
+        '\u{00AD}'
+        | '\u{0600}'..='\u{0605}'
+        | '\u{061C}'
+        | '\u{06DD}'
+        | '\u{070F}'
+        | '\u{0890}'..='\u{0891}'
+        | '\u{08E2}'
+        | '\u{180E}'
+        | '\u{200B}'..='\u{200F}'
+        | '\u{202A}'..='\u{202E}'
+        | '\u{2060}'..='\u{2064}'
+        | '\u{2066}'..='\u{206F}'
+        | '\u{FEFF}'
+        | '\u{FFF9}'..='\u{FFFB}'
+        | '\u{110BD}'
+        | '\u{110CD}'
+        | '\u{13430}'..='\u{1343F}'
+        | '\u{1BCA0}'..='\u{1BCA3}'
+        | '\u{1D173}'..='\u{1D17A}'
+        | '\u{E0001}'
+        | '\u{E0020}'..='\u{E007F}')
+}
+
+#[cfg(test)]
+mod tests {
+    use super::is_format_char;
+
+    #[test]
+    fn format_char_matches_unicode_16_range_boundaries() {
+        for c in [
+            '\u{00AD}',
+            '\u{0600}',
+            '\u{0605}',
+            '\u{061C}',
+            '\u{06DD}',
+            '\u{070F}',
+            '\u{0890}',
+            '\u{0891}',
+            '\u{08E2}',
+            '\u{180E}',
+            '\u{200B}',
+            '\u{200F}',
+            '\u{202A}',
+            '\u{202E}',
+            '\u{2060}',
+            '\u{2064}',
+            '\u{2066}',
+            '\u{206F}',
+            '\u{FEFF}',
+            '\u{FFF9}',
+            '\u{FFFB}',
+            '\u{110BD}',
+            '\u{110CD}',
+            '\u{13430}',
+            '\u{1343F}',
+            '\u{1BCA0}',
+            '\u{1BCA3}',
+            '\u{1D173}',
+            '\u{1D17A}',
+            '\u{E0001}',
+            '\u{E0020}',
+            '\u{E007F}',
+        ] {
+            assert!(is_format_char(c), "{c:?} should be Unicode 16 Cf");
+        }
+
+        for c in [
+            '\u{005F}',
+            '\u{061B}',
+            '\u{061D}',
+            '\u{1D172}',
+            '\u{1D17B}',
+            '\u{1D182}',
+            '\u{E0080}',
+        ] {
+            assert!(!is_format_char(c), "{c:?} should not be Unicode 16 Cf");
+        }
+    }
+}

pkgs/id/src/commands/text.rs

@@ -20,9 +20,10 @@
 //!
 //! [`plain_field`] quotes a value when it contains a character that could
 //! break or blur a **tab-separated** row: any control character (this includes
-//! `LF`, `CR` and `TAB`, plus NUL, DEL, `NEL` and the rest) and the Unicode
-//! line/paragraph separators `U+2028` / `U+2029`. Everything else — including
-//! ASCII space and ordinary Unicode — is printed byte-identically to before.
+//! `LF`, `CR` and `TAB`, plus NUL, DEL, `NEL` and the rest), Unicode format
+//! characters (Cf), and the line/paragraph separators `U+2028` / `U+2029`.
+//! Everything else — including ASCII space and ordinary Unicode — is printed
+//! byte-identically to before.
 //!
 //! The trigger set is deliberately narrower than the `key=value` renderer's
 //! (#1611 also quotes on a space): a space does not delimit a TSV field, so a
@@ -38,11 +39,28 @@ use std::fmt::Display;
 /// Returns `true` for characters that could break a tab-separated row.
 ///
 /// Control characters cover `LF`, `CR`, `TAB`, NUL and every other C0/C1
-/// code point (including `NEL`, `U+0085`); the Unicode line and paragraph
-/// separators are included because some consumers treat them as line breaks
-/// even though they are not control characters.
+/// code point (including `NEL`, `U+0085`); format characters can be invisible
+/// or reorder text; and the Unicode line and paragraph separators are included
+/// because some consumers treat them as line breaks even though they are not
+/// control characters.
 fn breaks_row(c: char) -> bool {
-    c.is_control() || matches!(c, '\u{2028}' | '\u{2029}')
+    c.is_control() || crate::is_format_char(c) || matches!(c, '\u{2028}' | '\u{2029}')
+}
+
+fn push_unicode_escape(rendered: &mut String, c: char) {
+    use std::fmt::Write as _;
+
+    let code_point = u32::from(c);
+    if code_point <= 0xFFFF {
+        // Writing into a String cannot fail.
+        let _ = write!(rendered, "\\u{code_point:04x}");
+    } else {
+        // JSON escapes supplementary scalars as a UTF-16 surrogate pair.
+        let scalar = code_point - 0x1_0000;
+        let high = 0xD800 + (scalar >> 10);
+        let low = 0xDC00 + (scalar & 0x3FF);
+        let _ = write!(rendered, "\\u{high:04x}\\u{low:04x}");
+    }
 }
 
 /// Renders `value` as one row-safe field.
@@ -78,9 +96,7 @@ pub fn plain_field(value: &str) -> String {
             '\r' => rendered.push_str("\\r"),
             '\t' => rendered.push_str("\\t"),
             c if breaks_row(c) => {
-                use std::fmt::Write as _;
-                // Writing into a String cannot fail.
-                let _ = write!(rendered, "\\u{:04x}", u32::from(c));
+                push_unicode_escape(&mut rendered, c);
             }
             c => rendered.push(c),
         }
@@ -143,6 +159,50 @@ mod tests {
         }
     }
 
+    #[test]
+    fn format_characters_are_quoted_and_escaped_alone_or_with_a_tab() {
+        let alone = plain_field("\u{202e}");
+        assert_eq!(alone, r#""\u202e""#);
+        assert!(!alone.contains('\u{202e}'));
+        assert_eq!(serde_json::from_str::<String>(&alone).unwrap(), "\u{202e}");
+
+        let with_tab = plain_field("a\u{202e}\tb");
+        assert_eq!(with_tab, r#""a\u202e\tb""#);
+        assert!(!with_tab.contains('\u{202e}'));
+        assert_eq!(
+            serde_json::from_str::<String>(&with_tab).unwrap(),
+            "a\u{202e}\tb"
+        );
+    }
+
+    #[test]
+    fn every_shared_format_character_is_json_escaped() {
+        let mut count = 0;
+        for code_point in 0..=0x10_FFFF {
+            let Some(c) = char::from_u32(code_point) else {
+                continue;
+            };
+            if !crate::is_format_char(c) {
+                continue;
+            }
+
+            let value = c.to_string();
+            let rendered = plain_field(&value);
+            assert!(rendered.starts_with('"') && rendered.ends_with('"'));
+            assert!(
+                rendered.contains("\\u"),
+                "{c:?} was not escaped: {rendered:?}"
+            );
+            assert!(!rendered.contains(c), "raw format character {c:?} remains");
+            assert_eq!(serde_json::from_str::<String>(&rendered).unwrap(), value);
+            count += 1;
+        }
+        assert!(
+            count > 100,
+            "expected the complete Unicode 16 Cf set, got {count}"
+        );
+    }
+
     #[test]
     fn quotes_and_backslashes_are_escaped_inside_a_quoted_value() {
         assert_eq!(plain_field("a\"b\nc"), r#""a\"b\nc""#);

pkgs/id/src/lib.rs

@@ -227,6 +227,7 @@ pub mod tag_index;
 pub mod tags;
 #[cfg(test)]
 mod test_forgeries;
+pub(crate) use id_core::is_format_char;
 pub(crate) use id_core::time;
 pub use id_core::tuple;
 pub use id_core::waiting;