From 99e25fbd555a9c32f49c82568992a0576ac0b06a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Z=20Sink=C3=B3?= Date: Sat, 17 Oct 2026 13:27:08 +0200 Subject: [PATCH 1/2] feat!: make the invalid Value shapes unrepresentable (#53) #52 made the canonicalizer fail closed on a hand-built BigInt that is not a digit run and on a Map with a repeated key. This removes the possibility of building them at all, so the "closed data set" holds for the public Rust API and not only on the reader path. BREAKING (public Rust API; the README states nothing here is stable): - Value::BigInt now carries BigIntValue, a newtype with a private field. BigIntValue::try_new validates an optional sign followed by ASCII digits; canonical() (A3's normal form) is infallible. The reader builds it through try_new. - Map's storage is private. Map::new, Map::from_pairs (refuses a repeated key), get_mut, iter and IntoIterator replace direct access to the Vec; push is crate-private and debug-asserts the uniqueness its callers claim; insert still replaces. With the types carrying the invariant, the #52 guards in the canonicalizer become unreachable and are removed together with the expect() calls they replaced (canonical, conformance, collection). canonical_integer's logic lives once, in BigIntValue::canonical. The constructors raise E_MALFORMED_VALUE (R-VALUE-BIGINT-DIGITS, R-VALUE-UNIQUE-KEYS, stage read). compile_fail doctests pin that neither type can be built around its constructor; engine/tests/value_invariants.rs covers the constructors and the consumers through the public API. Mutating each guarantee (public field, constructor check off) fails a test or doctest. Closes #53 --- [signing-metadata] key = cic-my-sign-key signature = vault:v1:MEYCIQDHqqlrPL3VQOER1UxHQdaiMvBLZFcfq2bwBJaFfGn0rgIhAPokO42sm4TlNSMguvGaIv3KEkdqq6Wu8eEcBSwtFEpg hash-algorithm = sha256 digest = TPhCmwsBD0+kiaAKAa5rSkhbH85LlkfUREN4Qkrp4Yo= [certificate] -----BEGIN CERTIFICATE----- MIICBjCCAaygAwIBAgIUSnRMR6RPnEbg296XWPOqq/u5PCwwCgYIKoZIzj0EAwIw QzELMAkGA1UEBhMCSFUxGTAXBgNVBAoMEENlbnRyYWxJbmZyYUNvcmUxGTAXBgNV BAMMEENJQyBEZXZlbG9wZXIgQ0EwHhcNMjYwMzIwMTMyMjU5WhcNMjYxMjMxMTMy MjU5WjBFMQswCQYDVQQGEwJIVTEZMBcGA1UECgwQQ2VudHJhbEluZnJhQ29yZTEb MBkGA1UEAwwSR2Fib3IgWm9sdGFuIFNpbmtvMFkwEwYHKoZIzj0CAQYIKoZIzj0D AQcDQgAEIG2CVmTfmLB9pLLclj7YmP2eedAjklpy4LGrU2ijoiy6Xqpuybv7OgJe i+ez31s65NEV8+X/ByeX1cstR988z6N8MHowCQYDVR0TBAIwADAdBgNVHQ4EFgQU yZN6AIX/TNnIJ9GwAa/NRN3ujHAwHwYDVR0jBBgwFoAUXn6CHYzPUqU4JVP8g+OS WeDYjhcwDgYDVR0PAQH/BAQDAgeAMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEF BQcDBDAKBggqhkjOPQQDAgNIADBFAiEA+bFzXRoJ4PCQbhAAtpkcMjt0vNj5rEW0 lOMBGDNyaWkCIB1vmM7PcZzv/c9bIrxF5kqv6QXomouhByUfeNUTbpKW -----END CERTIFICATE----- --- docs/MATERIALIZATION-SPEC.md | 4 +- engine/src/canonical.rs | 87 +++------------ engine/src/collection.rs | 23 ++-- engine/src/conformance.rs | 31 +++-- engine/src/digest_projection.rs | 2 +- engine/src/error.rs | 15 +-- engine/src/plan.rs | 11 +- engine/src/reader.rs | 166 ++++++++++++++++++++++++--- engine/tests/hand_built_values.rs | 180 ------------------------------ engine/tests/value_invariants.rs | 149 +++++++++++++++++++++++++ 10 files changed, 353 insertions(+), 315 deletions(-) delete mode 100644 engine/tests/hand_built_values.rs create mode 100644 engine/tests/value_invariants.rs diff --git a/docs/MATERIALIZATION-SPEC.md b/docs/MATERIALIZATION-SPEC.md index 254e19e..2811996 100644 --- a/docs/MATERIALIZATION-SPEC.md +++ b/docs/MATERIALIZATION-SPEC.md @@ -295,8 +295,8 @@ for both languages from this point forward. had not yet caught in code: `saphyr` (this engine's YAML parser) silently demoted an integer literal beyond `i64` range to a lossy `f64` before canonicalization ever saw it — fixed in `reader.rs`, - not papered over here, via a new `Value::BigInt(String)` carrying - the literal's exact digits. Not yet wired into a pipeline stage — + not papered over here, via a new `Value::BigInt` carrying the + literal's exact digits (a validated `BigIntValue` newtype since #53). Not yet wired into a pipeline stage — `Parse`/`Normalize`/`Resolve`/`Validate`, which would produce a materialized tree to canonicalize, still don't exist — so this is a standalone primitive, correct and tested in isolation, not yet diff --git a/engine/src/canonical.rs b/engine/src/canonical.rs index e13b006..1fb9e44 100644 --- a/engine/src/canonical.rs +++ b/engine/src/canonical.rs @@ -6,6 +6,7 @@ //! session, not assumed from documentation. Number canonicalization (A3) //! already has a proven Rust peer in the `cic-canonical` crate (both its //! `canonical_float` and `canonical_integer`); the logic is ported here +//! (`canonical_integer` as [`crate::reader::BigIntValue::canonical`]) //! rather than taken as a dependency, so this crate's canonicalization //! stays self-contained and this engine does not acquire a cross-repository //! build dependency on `CIC-Relay`. @@ -42,12 +43,9 @@ use sha2::{Digest as _, Sha256}; /// is reachable from authored YAML's own `.nan`/`.inf` tags, not only from /// constructing a [`Value`] directly. /// -/// Returns `E_MALFORMED_VALUE` for the two shapes only a hand-built -/// [`Value`] can have (the reader never produces them): a `Value::BigInt` -/// whose text is not an optional sign followed by ASCII digits -/// (`R-CANON-BIGINT-DIGITS`), and a `Map` holding the same key more than -/// once (`R-CANON-UNIQUE-KEYS`). Neither panics and neither is written out -/// as if it were valid. +/// A `Value` cannot hold a non-digit `BigInt` or a repeated map key (see +/// [`crate::reader::BigIntValue`] and [`crate::reader::Map`]), so the only +/// failure is the non-finite float above. pub fn to_canonical_json(v: &Value) -> Result> { let mut out = Vec::new(); write_value(&mut out, v, "$")?; @@ -78,27 +76,12 @@ fn write_value(out: &mut Vec, v: &Value, path: &str) -> Result<()> { Value::Int(i) => out.extend_from_slice(i.to_string().as_bytes()), // A3's big.Int-equivalent case (review-caught on PR #19: `reader.rs` // used to silently lose this precision to saphyr's own i64/f64 - // fallback -- see `Value::BigInt`'s own doc comment). `reader.rs` - // only guarantees the text is a validated digit run; normalization - // -- sign fold, leading-zero strip, `-0` -> `0` -- happens here, - // ported from `cic-canonical`'s own already-proven - // `canonical_integer`, the same function Go's `number.go` and this - // crate's number canonicalization both already trust. `Value` is - // public, so a caller can hand in a `BigInt` the reader would never - // build: that is an error, not a panic (Go's `CanonicalInteger` - // checks the same way). - Value::BigInt(s) => match canonical_integer(s) { - Some(n) => out.extend_from_slice(n.as_bytes()), - None => { - return Err(Error::new( - code::MALFORMED_VALUE, - "R-CANON-BIGINT-DIGITS", - Stage::Canonicalize, - path, - format!("{path} is a BigInt whose text is not a signed digit run: {s:?}"), - )); - } - }, + // fallback -- see `Value::BigInt`'s own doc comment). The text is a + // digit run by construction (`BigIntValue::try_new`); normalization + // -- sign fold, leading-zero strip, `-0` -> `0` -- is + // `BigIntValue::canonical`, the same rule Go's `number.go` applies via + // `big.Int.SetString(s, 10).String()`. + Value::BigInt(n) => out.extend_from_slice(n.canonical().as_bytes()), Value::Float(f) => { if !f.is_finite() { return Err(Error::new( @@ -121,32 +104,6 @@ fn write_value(out: &mut Vec, v: &Value, path: &str) -> Result<()> { Ok(()) } -/// A3, ported from `cic-canonical`'s own `canonical_integer` (not imported -/// as a dependency — see module docs): sign folded, leading zeros stripped, -/// `-0`/`+0` -> `0` — the same normalization Go's `number.go` applies via -/// `big.Int.SetString(s, 10).String()`, verified empirically against it in -/// this session (`"000000123"` -> `"123"`, `"-00000123"` -> `"-123"`, -/// `"-0"`/`"+0"` -> `"0"`). Returns `None` only if `s` is not actually a -/// validated digit run — [`crate::reader::Value::BigInt`]'s own invariant -/// guarantees this never happens for a value this crate constructed itself. -pub(crate) fn canonical_integer(s: &str) -> Option { - let (neg, digits) = match s.strip_prefix('-') { - Some(rest) => (true, rest), - None => (false, s.strip_prefix('+').unwrap_or(s)), - }; - if digits.is_empty() || !digits.bytes().all(|b| b.is_ascii_digit()) { - return None; - } - let magnitude = digits.trim_start_matches('0'); - Some(if magnitude.is_empty() { - "0".to_string() // all zeros, including "-0"/"+0" - } else if neg { - format!("-{magnitude}") - } else { - magnitude.to_string() - }) -} - /// A3, ported from `cic-canonical`'s own `canonical_float` (not imported as a /// dependency — see module docs): shortest round-trip plain-decimal form, /// `-0.0` folded to `0`. Rust's `f64` `Display` is shortest round-trip and @@ -177,23 +134,11 @@ fn write_array(out: &mut Vec, items: &[Value], path: &str) -> Result<()> { fn write_map(out: &mut Vec, m: &Map, path: &str) -> Result<()> { // Raw UTF-8 byte order -- `str`'s `Ord` is already byte-wise comparison, // matching Go's `sort.Strings` (A2) with no custom comparator needed. - // Sorting the entries (not just the keys) puts any duplicate next to its - // twin, so it is caught here instead of being written twice with the - // first value both times, which is what a key-list + `get` walk did. - let mut entries: Vec<&(String, Value)> = m.0.iter().collect(); - entries.sort_by(|a, b| a.0.cmp(&b.0)); - if let Some(pair) = entries.windows(2).find(|w| w[0].0 == w[1].0) { - let k = &pair[0].0; - return Err(Error::new( - code::MALFORMED_VALUE, - "R-CANON-UNIQUE-KEYS", - Stage::Canonicalize, - format!("{path}.{k}"), - format!("{path} holds the key {k:?} more than once; canonical JSON objects have unique keys"), - )); - } + // Keys are unique by `Map`'s construction, so the order is total. + let mut entries: Vec<(&str, &Value)> = m.iter().collect(); + entries.sort_unstable_by(|a, b| a.0.cmp(b.0)); out.push(b'{'); - for (i, (k, v)) in entries.into_iter().map(|e| (&e.0, &e.1)).enumerate() { + for (i, (k, v)) in entries.into_iter().enumerate() { if i > 0 { out.push(b','); } @@ -249,7 +194,7 @@ fn write_json_string(out: &mut Vec, s: &str) { #[cfg(test)] mod tests { use super::*; - use crate::reader::Map; + use crate::reader::{BigIntValue, Map}; fn json(v: &Value) -> String { String::from_utf8(to_canonical_json(v).expect("value is finite")) @@ -474,7 +419,7 @@ mod tests { ]; for (literal, want) in cases { assert_eq!( - json(&Value::BigInt((*literal).to_string())), + json(&Value::BigInt(BigIntValue::try_new(*literal).unwrap())), *want, "literal {literal:?}" ); diff --git a/engine/src/collection.rs b/engine/src/collection.rs index b4f032e..d4e8dfc 100644 --- a/engine/src/collection.rs +++ b/engine/src/collection.rs @@ -39,7 +39,6 @@ //! floats), kept here only as a defensive, documented exception for a //! `Value` this function might be handed before that stage runs. -use crate::canonical::canonical_integer; use crate::reader::Value; /// Ported from `collection.go`'s `CollectionTopology`. @@ -124,11 +123,9 @@ impl Collection { } /// `fmt.Sprintf("%v", ...)`, ported in full (see this module's own doc -/// comment for the correction history). `None` for a non-finite float, and -/// for the two shapes only a hand-built `Value` can have (a `BigInt` that is -/// not a signed digit run, a `Map` with a repeated key) -- every other -/// `Value` variant, including nested `Seq`/`Map`, has a defined, -/// Go-matching display form. `pub(crate)`: the not-yet-built +/// comment for the correction history). `None` only for a non-finite +/// float -- every other `Value` variant, including nested `Seq`/`Map`, has +/// a defined, Go-matching display form. `pub(crate)`: the not-yet-built /// object-level walker's `resolvePath`-equivalent needs the exact same /// formatting for its own `{key=val}` path-segment matching (`conformance. /// go`'s `resolvePath` uses the identical `fmt.Sprintf("%v", em[key])`), @@ -138,7 +135,7 @@ pub(crate) fn go_display(v: &Value) -> Option { Value::Null => Some("".to_string()), Value::Bool(b) => Some(if *b { "true" } else { "false" }.to_string()), Value::Int(i) => Some(i.to_string()), - Value::BigInt(s) => canonical_integer(s), + Value::BigInt(n) => Some(n.canonical()), Value::Float(f) => go_float_display(*f), Value::Str(s) => Some(s.clone()), Value::Seq(items) => { @@ -151,15 +148,11 @@ pub(crate) fn go_display(v: &Value) -> Option { Value::Map(m) => { // Go's `fmt` package has sorted map keys for deterministic // `%v` output since Go 1.12 (verified empirically) -- A2's - // byte-wise sort, reused, not a second rule. A repeated key - // (impossible in a Go map) has no display form. - let mut entries: Vec<&(String, Value)> = m.0.iter().collect(); - entries.sort_by(|a, b| a.0.cmp(&b.0)); - if entries.windows(2).any(|w| w[0].0 == w[1].0) { - return None; - } + // byte-wise sort, reused, not a second rule. + let mut entries: Vec<(&str, &Value)> = m.iter().collect(); + entries.sort_unstable_by(|a, b| a.0.cmp(b.0)); let mut parts = Vec::with_capacity(entries.len()); - for (k, val) in entries.into_iter().map(|e| (&e.0, &e.1)) { + for (k, val) in entries { parts.push(format!("{k}:{}", go_display(val)?)); } Some(format!("map[{}]", parts.join(" "))) diff --git a/engine/src/conformance.rs b/engine/src/conformance.rs index 51ce153..da5ed7d 100644 --- a/engine/src/conformance.rs +++ b/engine/src/conformance.rs @@ -34,7 +34,7 @@ //! verified against real Go output before being encoded — see that //! function's own doc comment for what each round found and fixed. -use crate::canonical::{canonical_float, canonical_integer, to_canonical_json}; +use crate::canonical::{canonical_float, to_canonical_json}; use crate::reader::Value; use num_bigint::BigInt; use num_rational::BigRational; @@ -192,15 +192,10 @@ fn compare_exact(intent: &Value, observed: &Value) -> (bool, bool) { if i_num != o_num { return (false, false); // number vs non-number -> not comparable } - // A hand-built `BigInt` that is not a digit run has no canonical - // form: not comparable, rather than a panic or a guess. - return match ( - canonical_number_string(intent), - canonical_number_string(observed), - ) { - (Some(a), Some(b)) => (a == b, true), - _ => (false, false), - }; + return ( + canonical_number_string(intent) == canonical_number_string(observed), + true, + ); } if json_kind(intent) != json_kind(observed) { return (false, false); @@ -211,14 +206,12 @@ fn compare_exact(intent: &Value, observed: &Value) -> (bool, bool) { /// Canonical decimal form of a numeric `Value`, reusing section A's own /// number canonicalization (`canonical.rs`) rather than a second /// implementation — the same digits `to_canonical_json` would write for -/// this value, without re-serializing the whole tree. `None` only for a -/// `BigInt` whose text is not a signed digit run, which the reader never -/// builds but a caller of the public `Value` can. -fn canonical_number_string(v: &Value) -> Option { +/// this value, without re-serializing the whole tree. +fn canonical_number_string(v: &Value) -> String { match v { - Value::Int(i) => Some(i.to_string()), - Value::BigInt(s) => canonical_integer(s), - Value::Float(f) => Some(canonical_float(*f)), + Value::Int(i) => i.to_string(), + Value::BigInt(n) => n.canonical(), + Value::Float(f) => canonical_float(*f), _ => unreachable!("canonical_number_string is only called where is_numeric_value(v)"), } } @@ -243,7 +236,9 @@ fn canonical_equal(a: &Value, b: &Value) -> bool { fn as_rational(v: &Value) -> Option { match v { Value::Int(i) => Some(BigRational::from_integer(BigInt::from(*i))), - Value::BigInt(s) => BigInt::from_str(s).ok().map(BigRational::from_integer), + Value::BigInt(n) => BigInt::from_str(n.as_str()) + .ok() + .map(BigRational::from_integer), Value::Float(f) => BigRational::from_f64(*f), Value::Str(s) => rat_from_string(s), Value::Null | Value::Bool(_) | Value::Seq(_) | Value::Map(_) => None, diff --git a/engine/src/digest_projection.rs b/engine/src/digest_projection.rs index bf1705d..1407e13 100644 --- a/engine/src/digest_projection.rs +++ b/engine/src/digest_projection.rs @@ -250,7 +250,7 @@ mod tests { }; let mut fields = BTreeMap::new(); let mut keys = BTreeSet::new(); - for (k, val) in m.0 { + for (k, val) in m { keys.insert(k.clone()); let evidence = evidence_of(&k, val); fields.insert( diff --git a/engine/src/error.rs b/engine/src/error.rs index 2390b0d..2763e8d 100644 --- a/engine/src/error.rs +++ b/engine/src/error.rs @@ -124,13 +124,14 @@ pub mod code { /// Raised at `Stage::Materialize`. pub const INCOMPLETE_OBJECT: &str = "E_INCOMPLETE_OBJECT"; - /// A [`crate::reader::Value`] built by hand (not by the reader) breaks an - /// invariant the reader guarantees: a `BigInt` that is not a signed digit - /// run, or a `Map` holding the same key twice. Raised by `canonical` - /// (`R-CANON-BIGINT-DIGITS`, `R-CANON-UNIQUE-KEYS`) instead of panicking - /// or writing invalid canonical JSON. Not expressible as a YAML vector -- - /// the reader cannot produce either shape -- so the raisers are covered - /// by unit tests in `canonical.rs`. + /// A [`crate::reader::Value`] part built by hand that breaks an invariant + /// the reader guarantees: a `BigInt` that is not a signed digit run + /// (`R-VALUE-BIGINT-DIGITS`, raised by `BigIntValue::try_new`) or a `Map` + /// holding the same key twice (`R-VALUE-UNIQUE-KEYS`, raised by + /// `Map::from_pairs`). These are constructor errors: the types have no + /// other way in, so the canonicalizer and comparators never see either + /// shape. Covered by unit tests in `engine/tests/hand_built_values.rs` + /// (not a YAML vector -- the reader cannot produce them). pub const MALFORMED_VALUE: &str = "E_MALFORMED_VALUE"; } diff --git a/engine/src/plan.rs b/engine/src/plan.rs index c290da7..cb01248 100644 --- a/engine/src/plan.rs +++ b/engine/src/plan.rs @@ -506,7 +506,7 @@ mod tests { }; let mut fields = BTreeMap::new(); let mut keys = BTreeSet::new(); - for (k, val) in m.0 { + for (k, val) in m { keys.insert(k.clone()); let evidence = evidence_of(&k, val); fields.insert( @@ -704,11 +704,7 @@ mod tests { let Value::Map(m) = &mut observed else { unreachable!() }; - let nics_idx = - m.0.iter() - .position(|(k, _)| k == "network_interfaces") - .unwrap(); - let Value::Seq(nics) = &mut m.0[nics_idx].1 else { + let Some(Value::Seq(nics)) = m.get_mut("network_interfaces") else { unreachable!() }; nics[1] = nic("nic-1", "prod-subnet-WRONG", &[]); @@ -736,8 +732,7 @@ mod tests { let Value::Map(m) = &mut observed else { unreachable!() }; - let idx = m.0.iter().position(|(k, _)| k == "memory_gb").unwrap(); - m.0[idx].1 = Value::Str("large".into()); + m.insert("memory_gb", Value::Str("large".into())); let obs = oci_observation(); let verdict = evaluate( diff --git a/engine/src/reader.rs b/engine/src/reader.rs index b7ec4ee..c1b8033 100644 --- a/engine/src/reader.rs +++ b/engine/src/reader.rs @@ -88,22 +88,142 @@ pub enum Value { /// Rust, parsing the raw scalar text directly (via `saphyr`'s own /// `early_parse(false)` loader mode) rather than trusting `saphyr`'s /// default eager resolution to i64/f64. - BigInt(String), + BigInt(BigIntValue), Float(f64), Str(String), Seq(Vec), Map(Map), } +/// An integer literal that does not fit in `i64`, as its validated text: an +/// optional `+`/`-` followed by one or more ASCII digits, nothing else. +/// +/// The field is private and [`BigIntValue::try_new`] is the only way in +/// (besides the reader, which goes through it), so every `BigIntValue` that +/// exists satisfies the invariant the canonicalizer relies on -- it is not a +/// convention a caller can break. Sign folding, leading-zero stripping and +/// `-0` -> `0` still happen at canonicalization ([`BigIntValue::canonical`]). +/// +/// The value cannot be built around the check: +/// +/// ```compile_fail +/// use cic_primitive_engine::reader::{BigIntValue, Value}; +/// let _ = Value::BigInt(BigIntValue("not-a-number".to_string())); +/// ``` +/// +/// ``` +/// use cic_primitive_engine::reader::{BigIntValue, Value}; +/// assert!(BigIntValue::try_new("not-a-number").is_err()); +/// let big = BigIntValue::try_new("9223372036854775808").unwrap(); +/// let _ = Value::BigInt(big); +/// ``` +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BigIntValue(String); + +impl BigIntValue { + /// # Errors + /// `E_MALFORMED_VALUE` (`R-VALUE-BIGINT-DIGITS`) unless `s` is an + /// optional sign followed by one or more ASCII digits. + pub fn try_new(s: impl Into) -> Result { + let s = s.into(); + let digits = s.strip_prefix(['+', '-']).unwrap_or(&s); + if digits.is_empty() || !digits.bytes().all(|b| b.is_ascii_digit()) { + return Err(Error::new( + code::MALFORMED_VALUE, + "R-VALUE-BIGINT-DIGITS", + Stage::Read, + "$", + format!("{s:?} is not an optional sign followed by ASCII digits"), + )); + } + Ok(Self(s)) + } + + /// The validated text, exactly as given (not yet normalized). + #[must_use] + pub fn as_str(&self) -> &str { + &self.0 + } + + /// A3's normal form, ported from `cic-canonical`'s own `canonical_integer` + /// (not imported as a dependency): sign folded, leading zeros stripped, + /// `-0`/`+0` -> `0` -- the same normalization Go's `number.go` applies + /// via `big.Int.SetString(s, 10).String()`, verified empirically against + /// it (`"000000123"` -> `"123"`, `"-00000123"` -> `"-123"`, `"-0"`/`"+0"` + /// -> `"0"`). Infallible because the text is a digit run by construction. + #[must_use] + pub fn canonical(&self) -> String { + let (neg, digits) = match self.0.strip_prefix('-') { + Some(rest) => (true, rest), + None => (false, self.0.strip_prefix('+').unwrap_or(&self.0)), + }; + let magnitude = digits.trim_start_matches('0'); + if magnitude.is_empty() { + "0".to_string() // all zeros, including "-0"/"+0" + } else if neg { + format!("-{magnitude}") + } else { + magnitude.to_string() + } + } +} + +/// An insertion-ordered string-keyed mapping whose keys are unique. +/// +/// The storage is private: a `Map` is built by the reader, by +/// [`Map::from_pairs`] (which refuses a repeated key) or by [`Map::insert`] +/// (which replaces), so no `Map` outside this crate can hold the same key +/// twice -- the canonicalizer and the comparators rely on that. +/// +/// ```compile_fail +/// use cic_primitive_engine::reader::{Map, Value}; +/// let _ = Map(vec![("x".to_string(), Value::Int(1)), ("x".to_string(), Value::Int(2))]); +/// ``` +/// +/// ``` +/// use cic_primitive_engine::reader::{Map, Value}; +/// let pairs = vec![("x".to_string(), Value::Int(1)), ("x".to_string(), Value::Int(2))]; +/// assert!(Map::from_pairs(pairs).is_err()); +/// ``` #[derive(Debug, Clone, Default, PartialEq)] -pub struct Map(pub Vec<(String, Value)>); +pub struct Map(Vec<(String, Value)>); impl Map { + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Build a map from pairs, keeping their order. + /// + /// # Errors + /// `E_MALFORMED_VALUE` (`R-VALUE-UNIQUE-KEYS`) if a key occurs twice. + pub fn from_pairs(pairs: impl IntoIterator) -> Result { + let pairs: Vec<(String, Value)> = pairs.into_iter().collect(); + let mut seen = HashSet::with_capacity(pairs.len()); + for (k, _) in &pairs { + if !seen.insert(k.as_str()) { + return Err(Error::new( + code::MALFORMED_VALUE, + "R-VALUE-UNIQUE-KEYS", + Stage::Read, + format!("$.{k}"), + format!("the key {k:?} occurs more than once"), + )); + } + } + Ok(Self(pairs)) + } + #[must_use] pub fn get(&self, k: &str) -> Option<&Value> { self.0.iter().find(|(key, _)| key == k).map(|(_, v)| v) } #[must_use] + pub fn get_mut(&mut self, k: &str) -> Option<&mut Value> { + self.0.iter_mut().find(|(key, _)| key == k).map(|(_, v)| v) + } + #[must_use] pub fn contains(&self, k: &str) -> bool { self.get(k).is_some() } @@ -112,11 +232,14 @@ impl Map { /// `insert` searches the whole vector on every call, so building an n-key /// mapping with it costs `n(n-1)/2` comparisons — the second quadratic in /// this file, and the one that remained after the duplicate-key scan - /// stopped being the first. The parser can append safely because + /// stopped being the first. The reader can append safely because /// `scan_input` has already refused duplicates, so there is nothing to - /// overwrite. - pub fn push(&mut self, k: impl Into, v: Value) { - self.0.push((k.into(), v)); + /// overwrite. Crate-private because that argument is the caller's to + /// make; debug builds (all tests) check it. + pub(crate) fn push(&mut self, k: impl Into, v: Value) { + let k = k.into(); + debug_assert!(!self.contains(&k), "Map::push with an existing key {k:?}"); + self.0.push((k, v)); } pub fn insert(&mut self, k: impl Into, v: Value) { @@ -127,6 +250,10 @@ impl Map { self.0.push((k, v)); } } + /// The entries in insertion order. + pub fn iter(&self) -> impl Iterator { + self.0.iter().map(|(k, v)| (k.as_str(), v)) + } #[must_use] pub fn keys(&self) -> Vec<&str> { self.0.iter().map(|(k, _)| k.as_str()).collect() @@ -141,6 +268,14 @@ impl Map { } } +impl IntoIterator for Map { + type Item = (String, Value); + type IntoIter = std::vec::IntoIter<(String, Value)>; + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + impl Value { #[must_use] pub fn as_map(&self) -> Option<&Map> { @@ -170,7 +305,7 @@ impl Value { Value::Null => "null".into(), Value::Bool(b) => b.to_string(), Value::Int(i) => i.to_string(), - Value::BigInt(s) => s.clone(), + Value::BigInt(n) => n.as_str().to_string(), Value::Float(f) => f.to_string(), Value::Str(s) => s.clone(), Value::Seq(_) => "".into(), @@ -501,7 +636,7 @@ fn resolve_scalar( if let Some(digits) = base10_integer_literal(text) { return Ok(match digits.parse::() { Ok(i) => Value::Int(i), - Err(_) => Value::BigInt(digits.to_string()), + Err(_) => Value::BigInt(BigIntValue::try_new(digits)?), }); } } @@ -558,11 +693,11 @@ mod tests { ("9223372036854775807", Value::Int(9_223_372_036_854_775_807)), ( "9223372036854775808", - Value::BigInt("9223372036854775808".to_string()), + Value::BigInt(BigIntValue::try_new("9223372036854775808").unwrap()), ), ( "9223372036854775809", - Value::BigInt("9223372036854775809".to_string()), + Value::BigInt(BigIntValue::try_new("9223372036854775809").unwrap()), ), ( "-9223372036854775808", @@ -570,7 +705,7 @@ mod tests { ), ( "-9223372036854775809", - Value::BigInt("-9223372036854775809".to_string()), + Value::BigInt(BigIntValue::try_new("-9223372036854775809").unwrap()), ), ("9007199254740992", Value::Int(9_007_199_254_740_992)), ("9007199254740993", Value::Int(9_007_199_254_740_993)), @@ -580,7 +715,9 @@ mod tests { ("-0", Value::Int(0)), ( "1234567890123456789012345678901234567890", - Value::BigInt("1234567890123456789012345678901234567890".to_string()), + Value::BigInt( + BigIntValue::try_new("1234567890123456789012345678901234567890").unwrap(), + ), ), ]; for (literal, want) in cases { @@ -623,7 +760,10 @@ mod tests { fn explicitly_tagged_int_also_keeps_exact_precision() { let doc = parse_doc("v: !!int 9223372036854775808"); let got = doc.as_map().and_then(|m| m.get("v")).expect("key `v`"); - assert_eq!(got, &Value::BigInt("9223372036854775808".to_string())); + assert_eq!( + got, + &Value::BigInt(BigIntValue::try_new("9223372036854775808").unwrap()) + ); let doc = parse_doc("v: !!int 123"); let got = doc.as_map().and_then(|m| m.get("v")).expect("key `v`"); diff --git a/engine/tests/hand_built_values.rs b/engine/tests/hand_built_values.rs deleted file mode 100644 index ffa4be9..0000000 --- a/engine/tests/hand_built_values.rs +++ /dev/null @@ -1,180 +0,0 @@ -//! `Value` is public, so a caller can build shapes the reader never produces. -//! Nothing reachable through the public API may panic on them or write bytes -//! that are not valid canonical JSON (#52). -//! -//! These cannot be conformance-corpus vectors: the corpus is YAML in, and the -//! reader cannot produce either shape. - -use cic_primitive_engine::reader::{Map, Value}; -use cic_primitive_engine::{ - classify_field_value, code, compare, digest, to_canonical_json, Collection, CollectionTopology, - CompareType, Coverage, FieldVerdict, Stage, -}; - -fn big(s: &str) -> Value { - Value::BigInt(s.to_string()) -} - -fn dup_map() -> Value { - Value::Map(Map(vec![ - ("x".into(), Value::Int(1)), - ("x".into(), Value::Int(2)), - ])) -} - -/// Not a signed digit run: each of these used to hit `expect(...)`. -const BAD_BIGINTS: &[&str] = &[ - "", - "+", - "-", - "not-a-number", - "12 3", - "1e5", - "1.5", - "0x10", - "--1", - "٣", -]; - -#[test] -fn malformed_bigint_is_an_error_not_a_panic() { - for s in BAD_BIGINTS { - let err = to_canonical_json(&big(s)).expect_err(s); - assert_eq!(err.code, code::MALFORMED_VALUE, "{s:?}"); - assert_eq!(err.rule, "R-CANON-BIGINT-DIGITS", "{s:?}"); - assert_eq!(err.stage, Stage::Canonicalize, "{s:?}"); - assert_eq!(err.path, "$", "{s:?}"); - } -} - -#[test] -fn malformed_bigint_error_carries_the_nested_path() { - let v = Value::Map(Map(vec![( - "a".into(), - Value::Seq(vec![Value::Int(1), big("nope")]), - )])); - let err = to_canonical_json(&v).expect_err("nested"); - assert_eq!(err.code, code::MALFORMED_VALUE); - assert!(err.path.starts_with("$.a"), "path was {}", err.path); -} - -#[test] -fn well_formed_bigints_still_canonicalize() { - for (input, want) in [ - ("9223372036854775808", "9223372036854775808"), - ("-9223372036854775809", "-9223372036854775809"), - ("+000123", "123"), - ("-0", "0"), - ] { - let got = to_canonical_json(&big(input)).expect(input); - assert_eq!(String::from_utf8(got).unwrap(), want, "{input}"); - } -} - -#[test] -fn duplicate_key_is_an_error_not_invalid_json() { - let err = to_canonical_json(&dup_map()).expect_err("duplicate key"); - assert_eq!(err.code, code::MALFORMED_VALUE); - assert_eq!(err.rule, "R-CANON-UNIQUE-KEYS"); - assert_eq!(err.stage, Stage::Canonicalize); - assert_eq!(err.path, "$.x"); -} - -#[test] -fn duplicate_key_is_found_wherever_it_sits() { - // Not adjacent in insertion order, and nested inside a sequence. - let inner = Value::Map(Map(vec![ - ("a".into(), Value::Int(1)), - ("b".into(), Value::Int(2)), - ("a".into(), Value::Int(3)), - ])); - let v = Value::Map(Map(vec![( - "k".into(), - Value::Seq(vec![Value::Null, inner]), - )])); - let err = to_canonical_json(&v).expect_err("nested duplicate"); - assert_eq!(err.rule, "R-CANON-UNIQUE-KEYS"); - assert!(err.path.ends_with(".a"), "path was {}", err.path); -} - -#[test] -fn unique_keys_are_still_sorted_bytewise() { - let v = Value::Map(Map(vec![ - ("b".into(), Value::Int(2)), - ("a".into(), Value::Int(1)), - ("B".into(), Value::Int(0)), - ])); - let got = String::from_utf8(to_canonical_json(&v).unwrap()).unwrap(); - assert_eq!(got, r#"{"B":0,"a":1,"b":2}"#); -} - -#[test] -fn digest_input_can_only_come_from_valid_canonical_json() { - // `digest` takes bytes, so the guard is that no bytes are produced at all. - assert!(to_canonical_json(&dup_map()).is_err()); - assert!(to_canonical_json(&big("x")).is_err()); - assert!(digest(b"{}").starts_with("sha256:")); -} - -#[test] -fn compare_does_not_panic_on_a_malformed_bigint() { - let good = Value::Int(5); - for s in BAD_BIGINTS { - let bad = big(s); - // Exact: a malformed number has no canonical form -> not comparable. - assert_eq!( - compare(&bad, &good, CompareType::Exact), - (false, false), - "{s:?}" - ); - assert_eq!( - compare(&good, &bad, CompareType::Exact), - (false, false), - "{s:?}" - ); - assert_eq!( - compare(&bad, &bad, CompareType::Exact), - (false, false), - "{s:?}" - ); - // Numeric already failed closed (BigInt::from_str). - assert_eq!( - compare(&bad, &good, CompareType::Numeric), - (false, false), - "{s:?}" - ); - } -} - -#[test] -fn classification_does_not_panic_on_hand_built_values() { - let v = classify_field_value( - Coverage::Observed, - true, - &big("nope"), - &Value::Int(1), - CompareType::Exact, - ); - assert_eq!(v, FieldVerdict::NotComparable); - // Two equal duplicate-key maps are not "conformant": they have no canonical form. - let v = classify_field_value( - Coverage::Observed, - true, - &dup_map(), - &dup_map(), - CompareType::Exact, - ); - assert_ne!(v, FieldVerdict::Conformant); -} - -#[test] -fn set_element_key_does_not_panic_on_hand_built_values() { - let set = Collection { - topology: CollectionTopology::Set, - keys: vec![], - }; - // No display form -> the empty identity, the same fallback a non-finite float gets. - assert_eq!(set.element_key(&big("nope")), ""); - assert_eq!(set.element_key(&dup_map()), ""); - assert_eq!(set.element_key(&Value::Seq(vec![dup_map()])), ""); -} diff --git a/engine/tests/value_invariants.rs b/engine/tests/value_invariants.rs new file mode 100644 index 0000000..387ee87 --- /dev/null +++ b/engine/tests/value_invariants.rs @@ -0,0 +1,149 @@ +//! `Value` is public, so what a caller can build is part of the engine's +//! contract. The shapes the reader never produces -- a `BigInt` that is not a +//! digit run, a `Map` with a repeated key -- are refused by the constructors +//! (#53; #52 had made the canonicalizer fail closed on them first). Everything +//! below goes through the public API only. +//! +//! "Cannot be built around the constructor" is pinned by the `compile_fail` +//! doctests on `BigIntValue` and `Map` in `reader.rs`. None of this can be a +//! conformance-corpus vector: the corpus is YAML in, and the reader cannot +//! produce these shapes. + +use cic_primitive_engine::reader::{self, BigIntValue, Map, Value}; +use cic_primitive_engine::{ + classify_field_value, code, compare, to_canonical_json, Collection, CollectionTopology, + CompareType, Coverage, FieldVerdict, Stage, +}; + +fn big(s: &str) -> Value { + Value::BigInt(BigIntValue::try_new(s).expect(s)) +} + +fn pairs(kv: &[(&str, i64)]) -> Vec<(String, Value)> { + kv.iter() + .map(|(k, v)| ((*k).to_string(), Value::Int(*v))) + .collect() +} + +fn canon(v: &Value) -> String { + String::from_utf8(to_canonical_json(v).expect("canonical")).unwrap() +} + +/// Not a signed digit run: each of these used to reach an `expect(...)`. +const BAD_BIGINTS: &[&str] = &[ + "", + "+", + "-", + "not-a-number", + "12 3", + "1e5", + "1.5", + "0x10", + "--1", + "+-1", + "٣", + "1\n", +]; + +#[test] +fn bigint_constructor_rejects_anything_but_a_signed_digit_run() { + for s in BAD_BIGINTS { + let err = BigIntValue::try_new(*s).expect_err(s); + assert_eq!(err.code, code::MALFORMED_VALUE, "{s:?}"); + assert_eq!(err.rule, "R-VALUE-BIGINT-DIGITS", "{s:?}"); + assert_eq!(err.stage, Stage::Read, "{s:?}"); + } +} + +#[test] +fn bigint_constructor_accepts_and_normalizes_digit_runs() { + for (input, want) in [ + ("9223372036854775808", "9223372036854775808"), + ("-9223372036854775809", "-9223372036854775809"), + ("+000123", "123"), + ("-000123", "-123"), + ("-0", "0"), + ("+0", "0"), + ("0000", "0"), + ] { + let n = BigIntValue::try_new(input).expect(input); + assert_eq!(n.as_str(), input, "as_str keeps the text as given"); + assert_eq!(n.canonical(), want, "{input}"); + assert_eq!(canon(&Value::BigInt(n)), want, "{input}"); + } +} + +#[test] +fn from_pairs_rejects_a_repeated_key_wherever_it_sits() { + for kv in [ + &[("x", 1), ("x", 2)][..], + &[("a", 1), ("b", 2), ("a", 3)][..], // not adjacent in insertion order + &[("a", 1), ("b", 2), ("c", 3), ("c", 4), ("b", 5)][..], + ] { + let err = Map::from_pairs(pairs(kv)).expect_err("duplicate"); + assert_eq!(err.code, code::MALFORMED_VALUE); + assert_eq!(err.rule, "R-VALUE-UNIQUE-KEYS"); + assert_eq!(err.stage, Stage::Read); + assert!(err.path.starts_with("$."), "path was {}", err.path); + } +} + +#[test] +fn from_pairs_keeps_unique_keys_in_insertion_order() { + let m = Map::from_pairs(pairs(&[("b", 2), ("a", 1), ("B", 0)])).unwrap(); + assert_eq!(m.keys(), ["b", "a", "B"]); + assert_eq!(m.len(), 3); + // Canonical output is sorted bytewise regardless of insertion order. + assert_eq!(canon(&Value::Map(m)), r#"{"B":0,"a":1,"b":2}"#); + assert!(Map::from_pairs(Vec::new()).unwrap().is_empty()); +} + +#[test] +fn insert_replaces_so_uniqueness_survives_mutation() { + let mut m = Map::new(); + m.insert("x", Value::Int(1)); + m.insert("x", Value::Int(2)); + m.insert("y", Value::Int(3)); + assert_eq!(m.len(), 2); + assert_eq!(canon(&Value::Map(m)), r#"{"x":2,"y":3}"#); +} + +#[test] +fn reader_output_satisfies_both_invariants() { + let doc = "big: 9223372036854775808\nneg: -00000000000000000000000000000000000001\nn:\n a: 1\n b: 2\n"; + let v = reader::parse(doc.as_bytes(), Stage::Read, "$", "test").expect("parse"); + assert_eq!( + canon(&v), + r#"{"big":9223372036854775808,"n":{"a":1,"b":2},"neg":-1}"# + ); + // A duplicate key never reaches the type: the reader refuses the document. + let dup = reader::parse(b"x: 1\nx: 2\n", Stage::Read, "$", "test").expect_err("duplicate"); + assert_eq!(dup.code, code::MALFORMED_DOCUMENT); +} + +#[test] +fn comparison_and_classification_handle_every_constructible_bigint() { + let five = Value::Int(5); + let huge = big("9223372036854775808"); + assert_eq!(compare(&huge, &huge, CompareType::Exact), (true, true)); + assert_eq!( + compare(&big("+0009223372036854775808"), &huge, CompareType::Exact), + (true, true) + ); + assert_eq!(compare(&huge, &five, CompareType::Exact), (false, true)); + assert_eq!(compare(&huge, &five, CompareType::Numeric), (false, true)); + assert_eq!( + classify_field_value(Coverage::Observed, true, &huge, &huge, CompareType::Exact), + FieldVerdict::Conformant + ); +} + +#[test] +fn set_element_key_uses_the_canonical_bigint_form() { + let set = Collection { + topology: CollectionTopology::Set, + keys: vec![], + }; + assert_eq!(set.element_key(&big("+0001")), "1"); + assert_eq!(set.element_key(&big("-0")), "0"); +} From deb5b10813fb9e23793381ce28638d1f55a221f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Z=20Sink=C3=B3?= Date: Sat, 17 Oct 2026 13:50:06 +0200 Subject: [PATCH 2/2] fix: enforce Map key uniqueness in every build profile, not only in debug Map::push relied on its caller having proven the key new and checked that with debug_assert!, so a release build would silently keep a repeated key. The one caller that handles arbitrary untrusted keys, the reader, was covered only by scan_input's event-level duplicate refusal. I probed that scan with 15 spellings of a repeated key (plain, quoted, escaped, hex-escaped, flow, explicit, tagged, nested, in a sequence, merge keys, multi-line keys) in a release build: every one is refused, so this was not exploitable. It is hardening so the type's guarantee does not rest on a scan plus a debug-only assertion. - reader: build the mapping through Map::from_pairs (linear, hash-set based) and report a repeat as R-READ-DUPLICATE-KEY like the scan does. It cannot fire today; it makes the uniqueness structural. - Map::push: assert! instead of debug_assert!, documented as crate-private with a linear check. Remaining callers add a handful of literal keys. - test: push_refuses_a_repeated_key_in_every_profile. It passes in debug even with the weak debug_assert! and fails only in release, so the gate needs a release test run (added with the property-test work). --- [signing-metadata] key = cic-my-sign-key signature = vault:v1:MEUCIQCrEZ8M/hZ0u5KPm97BYYgPHk5tLTLQSPM7ovfZGYbzWQIgEdSwl4oqSRzBsHj3JUlEbsYcsHWrCAuBZNuJ9cVkiqQ= hash-algorithm = sha256 digest = IK19+yc0O5DPj/mQ/So02UNQyiAAp0VIv0fdWIpfSnE= [certificate] -----BEGIN CERTIFICATE----- MIICBjCCAaygAwIBAgIUSnRMR6RPnEbg296XWPOqq/u5PCwwCgYIKoZIzj0EAwIw QzELMAkGA1UEBhMCSFUxGTAXBgNVBAoMEENlbnRyYWxJbmZyYUNvcmUxGTAXBgNV BAMMEENJQyBEZXZlbG9wZXIgQ0EwHhcNMjYwMzIwMTMyMjU5WhcNMjYxMjMxMTMy MjU5WjBFMQswCQYDVQQGEwJIVTEZMBcGA1UECgwQQ2VudHJhbEluZnJhQ29yZTEb MBkGA1UEAwwSR2Fib3IgWm9sdGFuIFNpbmtvMFkwEwYHKoZIzj0CAQYIKoZIzj0D AQcDQgAEIG2CVmTfmLB9pLLclj7YmP2eedAjklpy4LGrU2ijoiy6Xqpuybv7OgJe i+ez31s65NEV8+X/ByeX1cstR988z6N8MHowCQYDVR0TBAIwADAdBgNVHQ4EFgQU yZN6AIX/TNnIJ9GwAa/NRN3ujHAwHwYDVR0jBBgwFoAUXn6CHYzPUqU4JVP8g+OS WeDYjhcwDgYDVR0PAQH/BAQDAgeAMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEF BQcDBDAKBggqhkjOPQQDAgNIADBFAiEA+bFzXRoJ4PCQbhAAtpkcMjt0vNj5rEW0 lOMBGDNyaWkCIB1vmM7PcZzv/c9bIrxF5kqv6QXomouhByUfeNUTbpKW -----END CERTIFICATE----- --- engine/src/reader.rs | 49 +++++++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/engine/src/reader.rs b/engine/src/reader.rs index c1b8033..34c13c1 100644 --- a/engine/src/reader.rs +++ b/engine/src/reader.rs @@ -227,18 +227,19 @@ impl Map { pub fn contains(&self, k: &str) -> bool { self.get(k).is_some() } - /// Append without looking for an existing key. + /// Append a key the caller knows is new, without the replace-or-append + /// search `insert` does. /// - /// `insert` searches the whole vector on every call, so building an n-key - /// mapping with it costs `n(n-1)/2` comparisons — the second quadratic in - /// this file, and the one that remained after the duplicate-key scan - /// stopped being the first. The reader can append safely because - /// `scan_input` has already refused duplicates, so there is nothing to - /// overwrite. Crate-private because that argument is the caller's to - /// make; debug builds (all tests) check it. + /// Crate-private: "known to be new" is the caller's claim, and it is + /// checked here in every build profile (`assert!`, not `debug_assert!`), + /// so a wrong claim stops at the call instead of leaving a `Map` with a + /// repeated key. The check is linear per call, which is why the reader -- + /// the one caller that can see arbitrarily many untrusted keys -- builds + /// through [`Map::from_pairs`] instead. Remaining callers add a handful of + /// literal keys. pub(crate) fn push(&mut self, k: impl Into, v: Value) { let k = k.into(); - debug_assert!(!self.contains(&k), "Map::push with an existing key {k:?}"); + assert!(!self.contains(&k), "Map::push with an existing key {k:?}"); self.0.push((k, v)); } @@ -522,7 +523,7 @@ fn convert(y: &Yaml, stage: Stage, path: &str, what: &str) -> Result { .collect::>>()?, )), Yaml::Mapping(m) => { - let mut out = Map::default(); + let mut pairs: Vec<(String, Value)> = Vec::with_capacity(m.len()); for (k, v) in m { // Non-string keys are refused rather than stringified. YAML // allows `1:` and `true:`, and coercing them would let two @@ -551,9 +552,25 @@ fn convert(y: &Yaml, stage: Stage, path: &str, what: &str) -> Result { format!("{what} has a non-string mapping key; node names are strings"), )); }; - out.push(key, convert(v, stage, path, what)?); + pairs.push((key, convert(v, stage, path, what)?)); } - Ok(Value::Map(out)) + // `scan_input` has already refused every spelling of a repeated + // key (plain/quoted/escaped/flow/explicit/tagged), and saphyr's own + // mapping type cannot hold one, so this check does not fire today. + // It is here so the uniqueness of a `Map` the reader hands out + // rests on the type's own constructor in every build profile, not + // on that scan plus a debug-only assertion. Linear, unlike a + // per-key `contains`. + let map = Map::from_pairs(pairs).map_err(|e| { + Error::new( + code::MALFORMED_DOCUMENT, + "R-READ-DUPLICATE-KEY", + stage, + path, + format!("{what}: {}", e.detail), + ) + })?; + Ok(Value::Map(map)) } // Aliases are already refused at the event-scan stage. A tagged // mapping/sequence (a custom, non-core-schema tag — `Yaml::Tagged`) @@ -785,6 +802,14 @@ mod tests { // The existing conformance/reader/ vectors only check accept/reject, // not the resulting tree -- re-verified here that ordinary documents + #[test] + #[should_panic(expected = "Map::push with an existing key")] + fn push_refuses_a_repeated_key_in_every_profile() { + let mut m = Map::default(); + m.push("a", Value::Int(1)); + m.push("a", Value::Int(2)); + } + // still parse to the exact same Value shape as before this rewrite. #[test] fn ordinary_documents_unaffected() {