From a545c8e5360292169f10562ac72508d072b8c60a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mauricio=20Pe=C3=B1aranda?= Date: Fri, 31 Jul 2026 09:42:40 -0400 Subject: [PATCH] fix(validator): compile primitive regexes with ASCII (XSD) class semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FHIR primitive `pattern` facets are XSD regexes, and XSD defines `\s` as ASCII-only (`[#x20#x9#xD#xA]`). Rust's `regex` crate makes `\s`/`\S` Unicode-aware, so a value containing U+00A0 (non-breaking space) — Unicode whitespace but not XSD whitespace — was wrongly rejected: it matches neither the literal `[ \r\n\t]` branch nor `\S` in the R4/R4B `string`/`markdown` pattern `[ \r\n\t\S]+`, so the anchored match fails on a valid FHIR string. The same Unicode `\s` also affected `code` (`[^\s]+...`) on every version. Fix at the compile site in `engine/primitives.rs::compiled()`: compile with `unicode(false)` so the shorthand classes carry XSD/ASCII semantics. Because `unicode(false)` on the `&str` engine refuses any program that could match invalid UTF-8 (which `\S`/`[^\s]` can), compile on the `regex::bytes` engine and match the value's UTF-8 bytes. The value is always valid UTF-8, so the only observable effect is ASCII shorthand classes. Across every primitive pattern in all four versions, `\s`/`\S` are the only shorthands present (no `.`, `\d`, `\w`), so byte matching is exactly XSD-equivalent here. Packs stay spec-literal (no regeneration) and no storage/DB is touched. Fixes #425 --- .../fhir-validator/src/engine/primitives.rs | 121 +++++++++++++++++- 1 file changed, 115 insertions(+), 6 deletions(-) diff --git a/crates/fhir-validator/src/engine/primitives.rs b/crates/fhir-validator/src/engine/primitives.rs index 31115e8aa..33398394f 100644 --- a/crates/fhir-validator/src/engine/primitives.rs +++ b/crates/fhir-validator/src/engine/primitives.rs @@ -9,11 +9,26 @@ //! //! FHIR spec regexes are implicitly anchored: they must match the whole //! value, so patterns are compiled as `^(?:...)$`. +//! +//! The patterns are XSD `pattern` facets published by FHIR, and XSD defines the +//! shorthand classes as ASCII (`\s` == `[#x20#x9#xD#xA]`). Rust's `regex` crate, +//! by contrast, makes `\s`/`\S` Unicode-aware, so a value containing U+00A0 +//! (non-breaking space) — Unicode whitespace but *not* XSD whitespace — is +//! wrongly rejected by `string`/`markdown` (`[ \r\n\t\S]+`) on R4/R4B and by +//! `code` (`[^\s]+...`) on every version (issue #425). We therefore compile with +//! ASCII class semantics (`unicode(false)`) so `\s`/`\S`/`\d`/`\w` mean what the +//! XSD author intended. Because `unicode(false)` on the `&str` engine rejects any +//! program that could match invalid UTF-8 (which `\S`/`[^\s]` can), the patterns +//! are compiled on the `regex::bytes` engine and matched against the value's +//! UTF-8 bytes; the value is always valid UTF-8, so the only observable effect is +//! that the shorthand classes become ASCII. Across every primitive pattern in +//! all four versions, `\s` and `\S` are the *only* shorthands that appear (no +//! `.`, `\d`, or `\w`), so byte matching is exactly equivalent to XSD here. use super::errors::{self, ErrorKind}; use super::walk::WalkCtx; use crate::schema::FhirSchema; -use regex::Regex; +use regex::bytes::{Regex, RegexBuilder}; use serde_json::Value; use std::collections::HashMap; use std::sync::{Arc, OnceLock, RwLock}; @@ -41,7 +56,7 @@ pub(super) fn validate_primitive(ctx: &mut WalkCtx<'_>, schema: &FhirSchema, dat if let Some(pattern) = &schema.regex && let Some(s) = data.as_str() && let Some(re) = compiled(pattern) - && !re.is_match(s) + && !re.is_match(s.as_bytes()) { ctx.error( ErrorKind::PrimitiveValue, @@ -87,6 +102,11 @@ fn expected_json_class(type_name: &str) -> Option { /// every resource). Invalid patterns are cached as misses so they are only /// reported... never: they are simply skipped — the converter emits spec /// patterns, and a bad pattern must not fail validation. +/// +/// Patterns are compiled on the byte engine with `unicode(false)` so the +/// shorthand classes carry XSD/ASCII semantics (see the module docs). The cache +/// key is the raw pattern string: every entry is compiled under the same fixed +/// flag set, so the pattern alone is a total key. fn compiled(pattern: &str) -> Option> { static CACHE: OnceLock>>>> = OnceLock::new(); let cache = CACHE.get_or_init(|| RwLock::new(HashMap::new())); @@ -94,7 +114,11 @@ fn compiled(pattern: &str) -> Option> { if let Some(hit) = cache.read().expect("regex cache lock").get(pattern) { return hit.clone(); } - let compiled = Regex::new(&format!("^(?:{pattern})$")).ok().map(Arc::new); + let compiled = RegexBuilder::new(&format!("^(?:{pattern})$")) + .unicode(false) + .build() + .ok() + .map(Arc::new); cache .write() .expect("regex cache lock") @@ -106,16 +130,101 @@ fn compiled(pattern: &str) -> Option> { mod tests { use super::*; + /// Convenience: does the compiled, anchored pattern match the whole value? + /// Mirrors the engine, which matches the value's UTF-8 bytes. + fn matches(pattern: &str, value: &str) -> bool { + compiled(pattern).unwrap().is_match(value.as_bytes()) + } + #[test] fn anchoring_is_whole_value() { let re = compiled("[0-9]+").unwrap(); - assert!(re.is_match("123")); - assert!(!re.is_match("a123")); - assert!(!re.is_match("123b")); + assert!(re.is_match(b"123")); + assert!(!re.is_match(b"a123")); + assert!(!re.is_match(b"123b")); } #[test] fn invalid_pattern_is_skipped() { assert!(compiled("([unclosed").is_none()); } + + // The FHIR primitive regexes are XSD `pattern` facets, and XSD `\s` is + // ASCII-only (`[#x20#x9#xD#xA]`). Rust's `regex` makes `\s`/`\S` + // Unicode-aware by default, which wrongly rejected values containing U+00A0 + // (non-breaking space) — a valid FHIR string. #425. The engine now compiles + // with ASCII class semantics, so these pin the XSD behaviour directly. + + /// The R4/R4B `string`/`markdown` pattern. `\S` must be ASCII, so U+00A0 is + /// a non-whitespace codepoint and a valid string. This is the exact case + /// from the issue (`...Program\u{a0}`). + #[test] + fn string_markdown_r4_accepts_non_breaking_space() { + let p = r"[ \r\n\t\S]+"; + assert!(matches(p, "Acquired Brain Injury (ABI) Program\u{a0}")); + assert!(matches(p, "a\u{a0}b")); + assert!(matches(p, "\u{a0}")); // U+00A0 on its own is a valid string + assert!(matches(p, "a b")); // an ordinary space is still allowed + assert!(matches(p, "a\tb")); + assert!(!matches(p, "")); // but empty is not a valid string + } + + /// The R5/R6 `string`/`markdown` pattern (`[\s\S]` = any codepoint) was + /// never affected; behaviour must be unchanged either way. + #[test] + fn string_markdown_r5_unchanged() { + let p = r"^[\s\S]+$"; + assert!(matches(p, "anything\u{a0}")); + assert!(matches(p, "plain")); + assert!(!matches(p, "")); + } + + /// `code` (all versions) also used `[^\s]`, which under Unicode excluded + /// U+00A0. With ASCII semantics U+00A0 is content, so a token containing it + /// is one token; real ASCII whitespace still delimits/rejects as before. + #[test] + fn code_treats_non_breaking_space_as_content() { + for p in [r"[^\s]+(\s[^\s]+)*", r"[^\s]+( [^\s]+)*"] { + assert!(matches(p, "a\u{a0}b"), "U+00A0 is content in {p}"); + assert!(matches(p, "abc")); + assert!(matches(p, "ab cd")); // space-separated tokens are valid + assert!(!matches(p, " abc")); // leading real space is not + assert!(!matches(p, "abc ")); // trailing real space is not + } + } + + /// `uri`/`url`/`canonical` is `\S*`: a real space is rejected, U+00A0 is + /// accepted (XSD non-whitespace), and empty is allowed by `*`. + #[test] + fn uri_star_s_semantics() { + let p = r"\S*"; + assert!(matches(p, "http://example.org/a\u{a0}b")); + assert!(!matches(p, "a b")); + assert!(matches(p, "")); + } + + /// The fix must NOT blanket-accept non-ASCII: patterns built from explicit + /// ASCII classes still reject a stray U+00A0. Guards against over-permissive + /// byte matching. + #[test] + fn explicit_ascii_patterns_still_reject_non_breaking_space() { + // date (R4): U+00A0 is not part of any date class. + let date = r"([0-9]([0-9]([0-9][1-9]|[1-9]0)|[1-9]00)|[1-9]000)(-(0[1-9]|1[0-2])(-(0[1-9]|[1-2][0-9]|3[0-1]))?)?"; + assert!(matches(date, "2013-06-08")); + assert!(!matches(date, "2013-06-08\u{a0}")); + // id: `[A-Za-z0-9\-\.]{1,64}` — U+00A0 is not allowed. + let id = r"[A-Za-z0-9\-\.]{1,64}"; + assert!(matches(id, "abc-1.2")); + assert!(!matches(id, "abc\u{a0}")); + } + + /// `base64Binary` (R4/R4B) uses a `{4}` quantifier over an ASCII class; byte + /// counting equals char counting here, so grouping is unaffected. + #[test] + fn base64_quantifier_unaffected() { + let p = r"(\s*([0-9a-zA-Z\+/=]){4}\s*)+"; + assert!(matches(p, "AAAA")); + assert!(matches(p, "aGVsbG8=")); + assert!(!matches(p, "AA")); // not a multiple of 4 + } }