From 66d4dd3a5dfbc70368733709f950d404eb196855 Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Sun, 1 Nov 2026 18:28:40 +0000 Subject: [PATCH] =?UTF-8?q?CP4.2:=20review=20=E2=80=94=20pack=20names=20dr?= =?UTF-8?q?op=20bidi/format=20characters,=20licence=20texts=20are=20capped?= =?UTF-8?q?=20in=20all,=20code()=20documented=20for=20Pack?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FypgCX8LsYYE8fsj4byBm2 --- crates/proof/README.md | 4 ++-- crates/proof/src/lang.rs | 10 +++++++--- crates/proof/src/lib.rs | 20 ++++++++++++++++++++ crates/proof/src/packs.rs | 38 ++++++++++++++++++++++++++++++++++++-- 4 files changed, 65 insertions(+), 7 deletions(-) diff --git a/crates/proof/README.md b/crates/proof/README.md index d62facba..cd0e3f7d 100644 --- a/crates/proof/README.md +++ b/crates/proof/README.md @@ -92,8 +92,8 @@ through the shared affix engine with default tuning, and hyphenated from its pac `hyphenation.pat`; it has no grammar rules and the generic English spelling message. Without a dictionary or pack it behaves exactly as `Other` did (no marks, English hyphenation fallback). `has_dictionary(tag)` / `ProofLang::is_proofed()` say whether a `Pack` language has a dictionary -now (read on first use, cached). Loaded `Pack` dictionaries share a 256 MB budget (estimated from their files); the least recently used goes first, but never one used in the paragraph being checked, and languages with no dictionary take no room. Downstream `match`es on `ProofLang` need a `Pack(_)` arm; `code()` is `""` for it, as -for `Other` (use `pack_locale()`). +now (read on first use, cached). Loaded `Pack` dictionaries share a 256 MB budget (estimated from their files); the least recently used goes first, but never one used in the paragraph being checked, and languages with no dictionary take no room. Downstream `match`es on `ProofLang` need a `Pack(_)` arm; `code()` stays `&'static str` +and is `""` for it, as for `Other`: use `pack_locale()` for its locale. **Which pack serves a language:** its locale, then shorter prefixes (`fr-ca` → `fr`), then another region's pack of the same language (`nl` → `nl-be`), never another script's (`sr-latn` never reads diff --git a/crates/proof/src/lang.rs b/crates/proof/src/lang.rs index 4586a746..f6d77443 100644 --- a/crates/proof/src/lang.rs +++ b/crates/proof/src/lang.rs @@ -174,9 +174,13 @@ impl ProofLang { matches!(self, ProofLang::Es | ProofLang::EsMx | ProofLang::Es419 | ProofLang::EsUy) } - /// The primary language subtag (`en`, `pl`, `bg`, `tr`, `ru`, `cs`, `id`, `de`, `es`); empty for [`ProofLang::Other`]. + /// The primary language subtag (`en`, `pl`, `bg`, `tr`, `ru`, `cs`, `id`, `de`, `es`); empty for + /// [`ProofLang::Other`] and for a [`ProofLang::Pack`] language, whose locale isn't `'static`: + /// use [`ProofLang::pack_locale`] (`PackLocale::as_str`) for it. Kept `'static` so existing + /// callers (`PROOFED.map(ProofLang::code)`) don't break. pub fn code(self) -> &'static str { match self { + ProofLang::Pack(_) => "", ProofLang::En | ProofLang::EnGb | ProofLang::EnCa => "en", ProofLang::Pl => "pl", ProofLang::Bg => "bg", @@ -189,7 +193,7 @@ impl ProofLang { ProofLang::Id => "id", ProofLang::Es | ProofLang::EsMx | ProofLang::Es419 | ProofLang::EsUy => "es", ProofLang::De => "de", - ProofLang::Pack(_) | ProofLang::Other => "", + ProofLang::Other => "", } } @@ -480,7 +484,7 @@ mod tests { for tag in ["fr-FR", "dex", "ja-JP", "plx", "bgx", "trx", "csx", "idx", "inx", "esx", "spax"] { let lang = ProofLang::from_tag(Some(tag)); assert_eq!(lang.pack_locale().map(|l| l.to_string()), Some(tag.to_ascii_lowercase()), "{tag}"); - assert_eq!(lang.code(), "", "{tag}"); + assert_eq!(lang.code(), "", "{tag}: a pack language's locale is pack_locale()"); } assert_tags(&["x-none", "e", "-", "und", "zxx", "i-klingon"], ProofLang::Other); } diff --git a/crates/proof/src/lib.rs b/crates/proof/src/lib.rs index 60e5a491..9e7c0a6a 100644 --- a/crates/proof/src/lib.rs +++ b/crates/proof/src/lib.rs @@ -1585,6 +1585,26 @@ mod tests { let _ = std::fs::remove_dir_all(&root); } + /// Review of #485: licence texts are read up to a total, however many packs hold big files. + #[test] + fn licence_texts_are_capped_in_all() { + let _guard = DICTIONARY_TESTS.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + set_dictionaries_folder(Some(shared_test_folder())); + let root = std::env::temp_dir().join(format!("wc-proof-licence-cap-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + let big = vec![b'x'; 4 << 20]; + for l in ["qa", "qb", "qc", "qd", "qe"] { + let dir = write_pack(&root, l, &[]); + std::fs::write(dir.join("LICENCES").join("BIG"), &big).unwrap(); + } + packs::set_packs_dir(Some(root.clone())); + let from_packs: usize = packs::licence_texts().iter().filter(|(t, _)| t.contains(": BIG")).map(|(_, x)| x.len()).sum(); + assert!(from_packs > 0 && from_packs <= 16 << 20, "{from_packs}"); + packs::set_packs_dir(None); + set_dictionaries_folder(Some(shared_test_folder())); + let _ = std::fs::remove_dir_all(&root); + } + /// Pack languages for the cache tests: `qa`, `qb`… each knowing `word`. fn many_packs(root: &std::path::Path, n: usize) -> Vec { (0..n) diff --git a/crates/proof/src/packs.rs b/crates/proof/src/packs.rs index 1cc4ca56..66368d95 100644 --- a/crates/proof/src/packs.rs +++ b/crates/proof/src/packs.rs @@ -150,14 +150,23 @@ pub fn licence_texts() -> Vec<(String, String)> { } out.push(("English core word list".into(), CORE_LIST_NOTICE.into())); out.extend(crate::data::EMBEDDED_LICENCES.iter().map(|(t, x)| ((*t).to_string(), (*x).to_string()))); - for (p, _) in &scan().packs { + let mut total: u64 = 0; + 'packs: for (p, _) in &scan().packs { for (file, text) in licence_files(&p.path) { + total = total.saturating_add(text.len() as u64); + if total > MAX_LICENCE_TOTAL { + log::warn!("Language pack licences: over {MAX_LICENCE_TOTAL} bytes in all; the rest aren't shown"); + break 'packs; + } out.push((format!("{} ({}): {file}", p.name, p.locale), text)); } } out } +/// Most bytes of pack licence text [`licence_texts`] reads in all. +const MAX_LICENCE_TOTAL: u64 = 16 << 20; + /// The notice of the embedded Moby word list (craft-i18n `language-data/en-us/en-us.dic`). const MOBY_NOTICE: &str = "The Moby Hyphenator II word list by Grady Ward (Project Gutenberg eBook #3204) is in the public domain. \ The English hyphenation patterns were trained from it for the Craft apps and are distributed with them under MIT OR Apache-2.0."; @@ -263,10 +272,26 @@ fn parse_pack_toml(text: &str) -> Result { return Err(format!("pack.toml: {locale:?} isn't a lowercase language tag")); } let licence = string("licence").or_else(|| string("license")).filter(|l| !l.trim().is_empty()).ok_or("pack.toml: no licence")?; - let name = string("name").filter(|n| !n.trim().is_empty() && n.chars().count() <= 200 && !n.chars().any(char::is_control)); + let name = string("name").map(|n| plain_text(&n)).filter(|n| !n.is_empty()); Ok(PackInfo { locale, name, licence: licence.chars().take(200).collect() }) } +/// Text from a pack shown to people (its name): without control, bidirectional or other format +/// characters (U+202E would reverse what follows), trimmed, at most 200 characters. +pub(crate) fn plain_text(s: &str) -> String { + s.chars().filter(|&c| !c.is_control() && !is_format(c)).take(200).collect::().trim().to_string() +} + +/// Unicode format characters (general category Cf): bidirectional controls, zero-width ones, +/// the soft hyphen, tags. +fn is_format(c: char) -> bool { + matches!(c, + '\u{ad}' | '\u{600}'..='\u{605}' | '\u{61c}' | '\u{6dd}' | '\u{70f}' | '\u{890}'..='\u{891}' | '\u{8e2}' | '\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}') +} + /// Deepest nesting of tables and arrays checked in `pack.toml` (deeper is refused). const MAX_TOML_DEPTH: usize = 16; @@ -487,6 +512,15 @@ fn load_patterns(path: &Path) -> Result { mod tests { use super::*; + /// Review of #485: a pack's name loses bidirectional and other format characters. + #[test] + fn pack_names_are_plain_text() { + let t = "locale = \"nl\"\nlicence = \"MIT\"\nname = \"Neder\\u202elands\\u200b\\u2066 \\u0007\"\n"; + assert_eq!(parse_pack_toml(t).unwrap().name.as_deref(), Some("Nederlands")); + let t = "locale = \"nl\"\nlicence = \"MIT\"\nname = \"\\u202e\\u200f\"\n"; + assert_eq!(parse_pack_toml(t).unwrap().name, None, "nothing left: the locale is shown"); + } + #[test] fn locales_are_lowercase_tags() { for ok in ["nl", "pt-br", "sr-latn", "es-419", "de-1996", "ca-valencia"] {