From 1728b4881e6d4966295407c101c15c931757d574 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Sun, 30 Aug 2026 22:37:34 +0000 Subject: [PATCH] =?UTF-8?q?feat(adapters):=20YAML=20input=20normalization?= =?UTF-8?q?=20=E2=80=94=20duplicates,=20.inf/.nan,=20merge=20keys,=20non-s?= =?UTF-8?q?tring=20keys=20(OAI-12)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit from_yaml previously went straight yaml_serde::from_str:: with zero post-parse normalization. Three verified corruptions flowed through unimpeded: - duplicate mapping keys silently last-won (serde_json's visit_map insert semantics — verified against 1.0.151: the JSON path last-wins too, so the YAML path is now deliberately the stricter one); - .inf/-.inf/.nan scalars silently became Value::Null because serde_json::Number::from_f64(non-finite) is None; - YAML 1.1 merge keys (<<: *anchor) survived as literal '<<' properties; - non-string mapping keys (null keys, collection keys) either stringified through YAML's debug rendering or panicked the conversion. The new from_yaml pipeline: explicit yaml_serde::Value parse (native loud duplicate-key rejection with line/column) -> apply_merge() (merge keys applied, shallow per yaml_serde semantics; scalar/invalid merge values fail loudly) -> one structural normalization pass into serde_json::Value that rejects non-finite floats and non-string keys with the offending JSON pointer, and stringifies scalar keys exactly as the YAML 1.2 core schema renders them (200: -> "200", matching the JSON path's {"200": ...}). The walk stays inside yaml_serde's own parse-time bounds (recursion limit 128, alias jump limit, RepetitionLimitExceeded); no new budgets and no resolver changes. 14 seam tests added at the from_yaml boundary. Verification: cargo test 366 passed / 0 failed; clippy --all-targets -D warnings clean; fmt --check clean (scripts/verify.sh). --- src/adapters/openapi_spec.rs | 541 ++++++++++++++++++++++++++++++++++- 1 file changed, 540 insertions(+), 1 deletion(-) diff --git a/src/adapters/openapi_spec.rs b/src/adapters/openapi_spec.rs index e57de4b..fb4a5c3 100644 --- a/src/adapters/openapi_spec.rs +++ b/src/adapters/openapi_spec.rs @@ -3,11 +3,41 @@ //! (produce). JSON + YAML parsing (ADR-051: `yaml_serde` 0.10.x is YAML //! 1.2; JSON-first detection under `from_str`), plus `$ref` resolution //! against the raw document. +//! +//! # YAML/JSON parity contract (review 002 OAI-12) +//! +//! Both entry points must mean the same thing for the same document, or +//! the difference must fail loudly: +//! +//! - **Duplicate keys** — rejected on the YAML path, loudly, with the +//! key and its line/column. The JSON path inherits `serde_json`'s +//! `Map::insert` semantics and silently last-wins; the YAML path is +//! deliberately stricter, so a duplicate-key document can never +//! silently mean different things through the two entry points (the +//! YAML side fails instead of agreeing with the last value). +//! - **Merge keys** (`<<: *anchor`) — YAML 1.1-style merge keys are +//! *applied* via `apply_merge()` and never advertised as literal +//! properties. This is a deliberate, tested deviation from strict +//! YAML 1.2 core-schema processing. +//! - **Non-finite floats** (`.inf`/`-.inf`/`.nan`) — rejected loudly. +//! `serde_json::Number` cannot represent them; the naive passthrough +//! silently nulls the value, so [`from_yaml`](OpenAPISpec::from_yaml) +//! errors with the JSON pointer of the offending value instead. +//! - **Non-string mapping keys** — a non-string key that has no +//! round-trip-stable string form (e.g. `~`, `[a]: 1`) is rejected; +//! scalar keys are stringified exactly as the YAML 1.2 core schema +//! renders them (`200:` → `"200"`), matching what the JSON path +//! requires (`{"200": ...}`) — so response codes, the shape OpenAPI +//! overwhelmingly uses, mean the same thing in both formats. +//! - **Bare `yes`/`no`/`on`/`off` and tags** — identical behavior on +//! both paths by YAML 1.2 core schema (strings, `!!str 200` → +//! `"200"`); unknown tags fail loudly on both. use std::collections::{BTreeMap, HashMap, HashSet}; use alkcall::client::AdapterError; use serde_json::Value; +use yaml_serde::Value as YamlValue; /// Maximum structural nesting depth for recursive `$ref` resolution /// (review 001 OAI-01). Bounds schema object/array height so a @@ -138,6 +168,105 @@ pub struct Components { pub request_bodies: HashMap, } +/// Converts a merge-applied YAML document into the shared +/// `serde_json::Value` representation, rejecting loudly what the +/// conversion would otherwise silently corrupt (review 002 OAI-12): +/// +/// - **Duplicate keys** are rejected before conversion — the input +/// comes from an explicit `yaml_serde::Value` parse, whose `Mapping` +/// insertion rejects duplicates natively (with line/column) where the +/// direct `serde_json::Value` path silently last-wins. +/// - **Non-finite floats** (`yaml_serde::Number` carries +/// `.inf`/`-.inf`/`.nan`, `serde_json::Number` cannot) fail with the +/// value's JSON pointer instead of silently becoming `null`. +/// - **Non-string mapping keys** are stringified exactly as the YAML 1.2 +/// core schema renders the scalar (`200:` → `"200"`, matching the JSON +/// path's `{"200": ...}`); keys with no round-trip-stable string form +/// (null keys, sequence/map keys) fail with the key's JSON pointer. +/// +/// Recursion follows document structure (already bounded by +/// yaml_serde's parse-time recursion limit) and terminates because +/// YAML mappings are acyclic after alias expansion. +fn yaml_to_json_value(value: &YamlValue) -> Result { + match value { + YamlValue::Null => Ok(Value::Null), + YamlValue::Bool(b) => Ok(Value::Bool(*b)), + YamlValue::Number(n) => yaml_number_to_json(n), + YamlValue::String(s) => Ok(Value::String(s.clone())), + YamlValue::Tagged(tagged) => yaml_to_json_value(&tagged.value), + YamlValue::Sequence(items) => { + let mut out = Vec::with_capacity(items.len()); + for (index, item) in items.iter().enumerate() { + out.push( + yaml_to_json_value(item) + .map_err(|detail| format!("/: {detail}"))?, + ); + } + Ok(Value::Array(out)) + } + YamlValue::Mapping(map) => { + let mut out = serde_json::Map::new(); + for (key, val) in map { + let ptr = yaml_key_pointer(key)?; + let converted = + yaml_to_json_value(val).map_err(|detail| format!("{ptr}: {detail}"))?; + out.insert(ptr, converted); + } + Ok(Value::Object(out)) + } + } +} + +/// Converts one YAML number into a `serde_json::Number`, rejecting +/// non-finite floats loudly (OAI-12): `Number::from_f64(∞)` is `None`, +/// so an unguarded conversion would advertise `null` where the document +/// declares `maximum: .inf`. +fn yaml_number_to_json(number: &yaml_serde::Number) -> Result { + if let Some(integer) = number.as_i64() { + return Ok(Value::Number(serde_json::Number::from(integer))); + } + if let Some(unsigned) = number.as_u64() { + return Ok(Value::Number(serde_json::Number::from(unsigned))); + } + let float = number.as_f64().ok_or_else(|| { + "number is out of range for JSON (neither i64, u64, nor a finite f64)".to_string() + })?; + let finite = serde_json::Number::from_f64(float) + .ok_or_else(|| format!("{float} is not representable in JSON (.inf/.nan)"))?; + Ok(Value::Number(finite)) +} + +/// Renders one YAML mapping key as its `serde_json::Map` string key, +/// preserving the YAML 1.2 core schema's scalar rendering so numeric +/// keys match what the JSON path requires (`200:` → `"200"`). Keys +/// without a round-trip-stable string form are rejected loudly: +/// nullish keys (`~`, empty) and collection keys (sequences, mappings). +fn yaml_key_pointer(key: &YamlValue) -> Result { + match key { + YamlValue::String(s) => Ok(s.clone()), + YamlValue::Bool(b) => Ok(b.to_string()), + YamlValue::Number(n) => { + if let Some(integer) = n.as_i64() { + return Ok(integer.to_string()); + } + if let Some(unsigned) = n.as_u64() { + return Ok(unsigned.to_string()); + } + match n.as_f64().and_then(serde_json::Number::from_f64) { + Some(finite) => Ok(finite.to_string()), + None => Err(format!( + "mapping key {n:?} is a non-finite number with no string form" + )), + } + } + YamlValue::Tagged(tagged) => yaml_key_pointer(&tagged.value), + other => Err(format!( + "mapping key {other:?} has no string form; only string, number, and boolean \ + keys can be represented in an OpenAPI document" + )), + } +} + fn index_component_map(raw: Option<&Value>) -> HashMap { let mut map = HashMap::new(); if let Some(obj) = raw.and_then(|m| m.as_object()) { @@ -186,10 +315,33 @@ impl OpenAPISpec { /// to a `serde_json::Value` and then fed through /// [`from_value`](Self::from_value), so there is one internal /// `OpenAPISpec` representation shared with the JSON path. + /// + /// The parse runs in two steps, both bounded by `yaml_serde`'s own + /// recursion/alias limits (OAI-12): an explicit + /// `yaml_serde::Value` parse — so duplicate keys are rejected + /// natively with line/column, instead of silently last-winning + /// through the direct-to-`serde_json::Value` path — followed by + /// [`apply_merge`](yaml_serde::Value::apply_merge) (YAML 1.1-style + /// `<<: *anchor` merge keys are **applied**, matching the + /// merge-key.html behavior users authoring YAML anchors expect; ADR-051 + /// declares YAML 1.2 core schema for scalars, and this is the one + /// deliberate deviation), then a normalization pass into + /// `serde_json::Value` that rejects loudly what `serde_json::Number` + /// cannot represent: non-finite floats (`.inf`/`.nan` would silently + /// null) and non-string mapping keys with no round-trip-stable + /// string form. Every rejection names the JSON pointer of the + /// offending value. pub fn from_yaml(doc: &str) -> Result { - let raw: Value = yaml_serde::from_str(doc).map_err(|e| AdapterError::SchemaParse { + let yaml: YamlValue = yaml_serde::from_str(doc).map_err(|e| AdapterError::SchemaParse { message: format!("invalid YAML: {e}"), })?; + let mut yaml = yaml; + yaml.apply_merge().map_err(|e| AdapterError::SchemaParse { + message: format!("invalid YAML merge key: {e}"), + })?; + let raw = yaml_to_json_value(&yaml).map_err(|message| AdapterError::SchemaParse { + message: format!("invalid YAML: {message}"), + })?; Self::from_value(raw) } @@ -1551,6 +1703,393 @@ mod tests { ); } + // --- OAI-12: YAML input normalization ----------------------------------- + + const OAI12_HEADER: &str = r#" +openapi: 3.0.0 +info: + title: T + version: "1" +"#; + + #[test] + fn from_yaml_duplicate_keys_fail_loudly_not_last_win() { + let doc = r#" +openapi: 3.0.0 +info: + title: T + version: "1" + description: first + description: second +paths: + /x: + get: + operationId: x + responses: + "200": + content: + application/json: + schema: {} +"#; + match OpenAPISpec::from_yaml(doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("duplicate"), + "the error must name the duplicate: {message}" + ); + assert!( + message.contains("line") || message.contains("column"), + "the error must carry the document position: {message}" + ); + } + Ok(_) => panic!("duplicate YAML keys must fail loudly, not last-win"), + other => panic!("expected SchemaParse, got {other:?}"), + } + } + + #[test] + fn from_yaml_duplicate_keys_stricter_than_json_path() { + let dup_json = r#"{"a":1,"a":2}"#; + let json_last_wins = serde_json::from_str::(dup_json) + .expect("serde_json's visit_map last-wins (verified against 1.0.151)"); + assert_eq!( + json_last_wins["a"], 2, + "the JSON path silently last-wins; this assertion documents the asymmetry \ + the YAML path refuses to reproduce" + ); + let yaml_doc = "a: 1\na: 2"; + match OpenAPISpec::from_yaml(yaml_doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!(message.contains("duplicate"), "message was: {message}"); + } + Ok(_) => panic!("YAML duplicate keys must fail loudly, not last-win"), + other => panic!("expected SchemaParse, got {other:?}"), + } + } + + #[test] + fn from_yaml_inf_maximum_fails_loudly_not_silent_null() { + let doc = format!( + r#"{OAI12_HEADER} +components: + schemas: + Price: + type: object + properties: + cap: + type: number + maximum: .inf +"# + ); + match OpenAPISpec::from_yaml(&doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains(".inf") || message.contains("inf"), + "the error must name the non-finite value: {message}" + ); + assert!( + message.contains("maximum") || message.contains("schemas"), + "the error must carry the value's JSON pointer: {message}" + ); + } + Ok(spec) => { + let maximum = + &spec.raw["components"]["schemas"]["Price"]["properties"]["cap"]["maximum"]; + assert_ne!( + *maximum, + Value::Null, + "maximum: .inf silently nulled — the corruption OAI-12 fixes" + ); + panic!("non-finite float must fail loudly, not null"); + } + other => panic!("expected SchemaParse, got {other:?}"), + } + } + + #[test] + fn from_yaml_nan_value_fails_loudly() { + let doc = format!( + r#"{OAI12_HEADER} +components: + schemas: + Ratio: + type: object + properties: + value: + type: number + example: .nan +"# + ); + match OpenAPISpec::from_yaml(&doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains(".nan") || message.contains("NaN"), + "the error must name the non-finite value: {message}" + ); + } + Ok(_) => panic!(".nan must fail loudly, not null"), + other => panic!("expected SchemaParse, got {other:?}"), + } + } + + #[test] + fn from_yaml_merge_key_is_applied_not_advertised() { + let doc = r#" +openapi: 3.0.0 +info: + title: T + version: "1" +components: + schemas: + Base: &Base + type: object + required: [id] + properties: + id: + type: string + Widget: + <<: *Base + title: Widget +paths: + /x: + get: + operationId: x + responses: + "200": + content: + application/json: + schema: {} +"#; + let spec = OpenAPISpec::from_yaml(doc).expect("merge keys apply"); + let widget = &spec.raw["components"]["schemas"]["Widget"]; + assert!( + widget.get("<<").is_none(), + "<< must never survive as a literal property" + ); + assert_eq!(widget["type"], "object", "merged from Base"); + let required = widget["required"].as_array().expect("required merged"); + assert_eq!(required[0], "id"); + let props = widget["properties"].as_object().expect("props merged"); + assert_eq!(props["id"]["type"], "string", "base properties merged in"); + assert_eq!( + widget["title"], "Widget", + "the referencing mapping's own keys are kept" + ); + } + + #[test] + fn from_yaml_merge_key_local_override_wins_over_base() { + let doc = format!( + r#"{OAI12_HEADER} +components: + schemas: + Base: &Base + type: object + description: base description + properties: + id: + type: string + Widget: + <<: *Base + description: widget description +paths: {{}} +"# + ); + let spec = OpenAPISpec::from_yaml(&doc).expect("merge keys apply"); + let widget = &spec.raw["components"]["schemas"]["Widget"]; + assert_eq!(widget["description"], "widget description"); + assert_eq!( + widget["type"], "object", + "keys the referencing mapping does not declare come from the merge" + ); + } + + #[test] + fn from_yaml_scalar_merge_value_fails_loudly() { + let doc = format!( + r#"{OAI12_HEADER} +components: + schemas: + Broken: + <<: 5 +paths: {{}} +"# + ); + match OpenAPISpec::from_yaml(&doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("merge"), + "the error must name the merge-key failure: {message}" + ); + } + Ok(_) => panic!("`<<:` with a scalar value must fail loudly"), + other => panic!("expected SchemaParse, got {other:?}"), + } + } + + #[test] + fn from_yaml_quoted_status_keys_survive_unchanged() { + let doc = r#" +openapi: 3.0.0 +info: + title: T + version: "1" +paths: + /x: + get: + operationId: x + responses: + "200": + description: ok + content: + application/json: + schema: {} + "404": + description: missing +"#; + let spec = OpenAPISpec::from_yaml(doc).expect("quoted keys unchanged"); + let responses = &spec.paths["/x"].operations[0].1.responses; + assert!(responses.contains_key("200")); + assert!(responses.contains_key("404")); + assert_eq!( + &spec.raw["paths"]["/x"]["get"]["responses"]["200"]["description"], + "ok" + ); + } + + #[test] + fn from_yaml_unquoted_status_keys_match_json_path_shape() { + let doc = format!( + r#"{OAI12_HEADER} +paths: + /x: + get: + operationId: x + responses: + 200: + description: ok + 404: + description: missing +"# + ); + let spec = OpenAPISpec::from_yaml(&doc).expect("bare numeric keys round-trip"); + let responses = &spec.paths["/x"].operations[0].1.responses; + assert!( + responses.contains_key("200") && responses.contains_key("404"), + "unquoted response codes must mean the same thing as quoted/JSON ones" + ); + } + + #[test] + fn from_yaml_null_and_collection_keys_fail_loudly() { + let doc = "~: 1"; + match OpenAPISpec::from_yaml(doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("no string form") || message.contains("key"), + "the error must name the unusable key: {message}" + ); + } + Ok(_) => panic!("null mapping key must fail loudly, not stringily as \"~\""), + other => panic!("expected SchemaParse, got {other:?}"), + } + } + + #[test] + fn from_yaml_float_key_preserves_rendering() { + let doc = format!( + r#"{OAI12_HEADER} +components: + schemas: + Mapped: + type: object + additionalProperties: + type: string + x-key-map: + 1.5: one-point-five +paths: {{}} +"# + ); + let spec = OpenAPISpec::from_yaml(&doc).expect("finite float key stringifies"); + let key_map = &spec.raw["components"]["schemas"]["Mapped"]["x-key-map"]; + assert_eq!(key_map["1.5"], "one-point-five"); + } + + #[test] + fn from_yaml_bool_key_stringifies() { + let doc = format!( + r#"{OAI12_HEADER} +components: + schemas: + Mapped: + type: object + x-flags: + true: enabled +paths: {{}} +"# + ); + let spec = OpenAPISpec::from_yaml(&doc).expect("bool key stringifies"); + assert_eq!( + &spec.raw["components"]["schemas"]["Mapped"]["x-flags"]["true"], + "enabled" + ); + } + + #[test] + fn from_yaml_plain_scalars_unaffected_by_normalization() { + let doc = format!( + r#"{OAI12_HEADER} +components: + schemas: + All: + type: object + properties: + yes: + type: string + default: yes + n: + type: integer + maximum: 9007199254740993 + f: + type: number + example: 1.5 + s: + type: string + example: "200" +paths: {{}} +"# + ); + let spec = OpenAPISpec::from_yaml(&doc).expect("normal doc unaffected"); + let props = &spec.raw["components"]["schemas"]["All"]["properties"]; + assert_eq!(props["yes"]["default"], "yes"); + assert_eq!( + props["n"]["maximum"], 9007199254740993i64, + "i64 range survives exactly" + ); + assert_eq!(props["f"]["example"], 1.5); + assert_eq!(props["s"]["example"], "200"); + } + + #[test] + fn from_yaml_untagged_bare_scalars_still_round_trip() { + let doc = format!( + r#"{OAI12_HEADER} +components: + schemas: + T: + type: object + properties: + a: + type: string + default: !!str 200 +paths: {{}} +"# + ); + let spec = OpenAPISpec::from_yaml(&doc).expect("tagged scalar passes through"); + let props = &spec.raw["components"]["schemas"]["T"]["properties"]; + assert_eq!(props["a"]["default"], "200"); + } + // --- OAI-13: path-item parameters, response wildcards, webhooks --- #[test]