From fea7565613118b3a50c0d68b7577559679c97a71 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Mon, 31 Aug 2026 01:12:43 +0000 Subject: [PATCH] fix(adapters): bound import/call error messages from spec-derived lists (OAI-17) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Import errors echoed unbounded spec-derived strings: a 100k-path servers-override list produced a multi-megabyte SchemaParse message. forward::bounded_join caps list echoes at 8 items / 128 chars per item with a ', … (+N more)' suffix, and is applied at the servers/callbacks /security location lists, the unbound-placeholder and unbound-remnant lists in build_registration/forward, and the call-time declared-keys echo. resolve_ref caps interpolated $ref strings at 128 chars. Tests: 100k-path servers fixture asserts message < 4 KiB (linear, completes fast); bounded_join unit test pins count+width truncation with unchanged small-case output. --- src/adapters/forward.rs | 41 +++++++++++++++++- src/adapters/from_openapi.rs | 14 +++---- src/adapters/openapi_spec.rs | 81 +++++++++++++++++++++++++++++++++--- 3 files changed, 122 insertions(+), 14 deletions(-) diff --git a/src/adapters/forward.rs b/src/adapters/forward.rs index 11a38c0..a77a4a9 100644 --- a/src/adapters/forward.rs +++ b/src/adapters/forward.rs @@ -177,6 +177,43 @@ pub(crate) const GATEWAY_BODY_KEY: &str = "body"; pub(crate) const HEADER_PARAM_IN_MARKER: &str = "wire"; pub(crate) const HEADER_PARAM_MARKER_VALUE: &str = "header"; +/// Upper bound on how many list items an adapter error message echoes +/// (review 002 OAI-17), and the per-item string cap. A spec-derived list +/// (servers locations, placeholder names, declared keys) can be +/// arbitrarily large; an error echoing all of it turns a 100k-path +/// document into a multi-megabyte message. The shape is "first N + count +/// of the rest". +pub(crate) const ERROR_LIST_ITEMS: usize = 8; +pub(crate) const ERROR_ITEM_STRING_CAP: usize = 128; + +/// Joins list items into an error-message fragment bounded in both item +/// count and item width: at most [`ERROR_LIST_ITEMS`] entries, each +/// truncated to [`ERROR_ITEM_STRING_CAP`] chars with a `…` marker, plus +/// a `, … (+N more)` suffix naming how many were suppressed. +pub(crate) fn bounded_join(items: &[String]) -> String { + let shown: Vec = items + .iter() + .take(ERROR_LIST_ITEMS) + .map(|item| { + if item.chars().count() > ERROR_ITEM_STRING_CAP { + let truncated: String = item.chars().take(ERROR_ITEM_STRING_CAP).collect(); + format!("{truncated}…") + } else { + item.clone() + } + }) + .collect(); + if items.len() > ERROR_LIST_ITEMS { + format!( + "{}, … (+{} more)", + shown.join(", "), + items.len() - ERROR_LIST_ITEMS + ) + } else { + shown.join(", ") + } +} + /// Import-time path-template validation shared by `from_jsonschema` /// (review 001 OAI-09) and `from_openapi` (review 002 JS-02): every /// `{placeholder}` must terminate with a `}` and carry a name. A @@ -447,7 +484,7 @@ fn enforce_input_schema( let declared_list = if declared.is_empty() { "none".to_string() } else { - declared.join(", ") + bounded_join(&declared) }; return Err(CallError::invalid_input(format!( "input key `{first}` is not declared by the operation's input schema \ @@ -621,7 +658,7 @@ pub(crate) fn render_path_template( if !unresolved.is_empty() { return Err(CallError::internal(format!( "path template `{template}` references unbound placeholder(s): {}", - unresolved.join(", ") + bounded_join(&unresolved) ))); } Ok(out) diff --git a/src/adapters/from_openapi.rs b/src/adapters/from_openapi.rs index 8f28f10..23decce 100644 --- a/src/adapters/from_openapi.rs +++ b/src/adapters/from_openapi.rs @@ -27,8 +27,8 @@ use async_trait::async_trait; use serde_json::Value; use super::forward::{ - forward, forward_stream, validate_path_template, HttpServiceConfig, GATEWAY_BODY_KEY, - HEADER_PARAM_IN_MARKER, HEADER_PARAM_MARKER_VALUE, + bounded_join, forward, forward_stream, validate_path_template, HttpServiceConfig, + GATEWAY_BODY_KEY, HEADER_PARAM_IN_MARKER, HEADER_PARAM_MARKER_VALUE, }; use super::openapi_spec::{ collect_ignored_schema_keys, OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS, @@ -378,16 +378,16 @@ impl FromOpenAPI { // diagnosis is not a dead end. return Err(AdapterError::SchemaParse { message: format!( - "path {method} {path_template} declares placeholder(s) {} with no \ + "path {method} {} declares placeholder(s) {} with no \ matching parameter in the operation's resolved input schema. Every \ `parameters` source was merged (path-item level and operation level, \ review 002 OAI-13); a placeholder left unbound after the merge means \ a declared parameter was dropped — check that each entry under \ - `paths.{path_template}.parameters` (and the path-item's shared \ + `paths` for this path's `parameters` (and the path-item's shared \ list) has `name` and `in`, and that its $ref, if any, resolves. \ - The placeholder would otherwise render as a literal `{}` path segment", - unbound.join(", "), - unbound[0] + The placeholder would otherwise render as a literal `{{}}` path segment", + bounded_join(&[path_template.to_string()]), + bounded_join(&unbound) ), }); } diff --git a/src/adapters/openapi_spec.rs b/src/adapters/openapi_spec.rs index 92e6f6f..03c25a9 100644 --- a/src/adapters/openapi_spec.rs +++ b/src/adapters/openapi_spec.rs @@ -46,6 +46,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; +use crate::adapters::forward::bounded_join; use alkcall::client::AdapterError; use serde_json::Value; use yaml_serde::Value as YamlValue; @@ -484,7 +485,7 @@ impl OpenAPISpec { configured at assembly time and cannot honor per-location `servers` — \ remove the `servers` entries or split the service into one import per \ base URL (review 001 OAI-06)", - servers_locations.join(", ") + bounded_join(&servers_locations) ), }); } @@ -633,7 +634,7 @@ impl OpenAPISpec { single-endpoint HTTP adapter does not model — remove the \ `callbacks` entries or split those operations into their own \ service definition (review 002 OAI-14)", - callback_locations.join(", ") + bounded_join(&callback_locations) ), }); } @@ -646,7 +647,7 @@ impl OpenAPISpec { security requirements would silently change nothing at call time — \ remove the `security` blocks or set the `auth` field on the service \ config instead (review 002 OAI-14)", - security_locations.join(", ") + bounded_join(&security_locations) ), }); } @@ -654,15 +655,22 @@ impl OpenAPISpec { } pub(crate) fn resolve_ref(&self, reference: &str) -> Result { + let bounded = |r: &str| { + if r.chars().count() > 128 { + format!("{}…", r.chars().take(128).collect::()) + } else { + r.to_string() + } + }; if !reference.starts_with("#/") { return Err(AdapterError::SchemaParse { - message: format!("external $ref not supported: {reference}"), + message: format!("external $ref not supported: {}", bounded(reference)), }); } let mut current: &Value = &self.raw; for part in reference.trim_start_matches("#/").split('/') { current = current.get(part).ok_or_else(|| AdapterError::SchemaParse { - message: format!("cannot resolve $ref: {reference}"), + message: format!("cannot resolve $ref: {}", bounded(reference)), })?; } Ok(current.clone()) @@ -2104,6 +2112,69 @@ mod tests { } } + // --- OAI-17: bounded import error messages ------------------------------- + + #[test] + fn hundred_thousand_servers_overrides_produce_bounded_error_message() { + let mut paths = String::from("{"); + for i in 0..100_000 { + let entry = r#""/p0": {"servers": [{"url": "https://h.example.com"}], "get": {"operationId": "op0", "responses": {"200": {"content": {"application/json": {"schema": {}}}}}}},"# + .replace("p0", &format!("p{i}")) + .replace("h.example.com", &format!("h{i}.example.com")) + .replace("op0", &format!("op{i}")); + paths.push_str(&entry); + } + paths.push_str(r#""/final": {"servers": [{"url": "https://z.example.com"}]}}"#); + let doc = format!( + r#"{{"openapi": "3.0.0", "info": {{"title": "T", "version": "1"}}, "paths": {paths}}}"# + ); + let started = std::time::Instant::now(); + match OpenAPISpec::from_json(&doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.len() < 4096, + "servers error must stay bounded (OAI-17), got {} bytes", + message.len() + ); + assert!( + message.contains('+') && message.contains("more"), + "the bounded join must name the suppressed count: {message}" + ); + } + Ok(_) => panic!("100k servers overrides must fail import"), + other => panic!("expected SchemaParse, got {other:?}"), + } + assert!( + started.elapsed().as_secs() < 30, + "the fixtures stay linear/bounded; this took {:?}", + started.elapsed() + ); + } + + #[test] + fn bounded_join_truncates_both_count_and_width() { + let many: Vec = (0..50).map(|i| format!("item{i}")).collect(); + let joined = bounded_join(&many); + assert!( + joined.contains("item0") && joined.contains("item7"), + "first 8 shown: {joined}" + ); + assert!(!joined.contains("item8,"), "9th item suppressed: {joined}"); + assert!(joined.contains("+42 more"), "count named: {joined}"); + + let wide = vec!["x".repeat(500)]; + let joined = bounded_join(&wide); + assert!( + joined.len() < 200, + "each item truncated to the width cap: {} bytes", + joined.len() + ); + assert!(joined.ends_with('…'), "truncation marker: {joined}"); + + let small = vec!["a".to_string(), "b".to_string()]; + assert_eq!(bounded_join(&small), "a, b", "under cap is unchanged"); + } + // --- OAI-12: YAML input normalization ----------------------------------- const OAI12_HEADER: &str = r#"