fix(adapters): bound import/call error messages from spec-derived lists (OAI-17)
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.
This commit is contained in:
+39
-2
@@ -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_IN_MARKER: &str = "wire";
|
||||||
pub(crate) const HEADER_PARAM_MARKER_VALUE: &str = "header";
|
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<String> = 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`
|
/// Import-time path-template validation shared by `from_jsonschema`
|
||||||
/// (review 001 OAI-09) and `from_openapi` (review 002 JS-02): every
|
/// (review 001 OAI-09) and `from_openapi` (review 002 JS-02): every
|
||||||
/// `{placeholder}` must terminate with a `}` and carry a name. A
|
/// `{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() {
|
let declared_list = if declared.is_empty() {
|
||||||
"none".to_string()
|
"none".to_string()
|
||||||
} else {
|
} else {
|
||||||
declared.join(", ")
|
bounded_join(&declared)
|
||||||
};
|
};
|
||||||
return Err(CallError::invalid_input(format!(
|
return Err(CallError::invalid_input(format!(
|
||||||
"input key `{first}` is not declared by the operation's input schema \
|
"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() {
|
if !unresolved.is_empty() {
|
||||||
return Err(CallError::internal(format!(
|
return Err(CallError::internal(format!(
|
||||||
"path template `{template}` references unbound placeholder(s): {}",
|
"path template `{template}` references unbound placeholder(s): {}",
|
||||||
unresolved.join(", ")
|
bounded_join(&unresolved)
|
||||||
)));
|
)));
|
||||||
}
|
}
|
||||||
Ok(out)
|
Ok(out)
|
||||||
|
|||||||
@@ -27,8 +27,8 @@ use async_trait::async_trait;
|
|||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
|
|
||||||
use super::forward::{
|
use super::forward::{
|
||||||
forward, forward_stream, validate_path_template, HttpServiceConfig, GATEWAY_BODY_KEY,
|
bounded_join, forward, forward_stream, validate_path_template, HttpServiceConfig,
|
||||||
HEADER_PARAM_IN_MARKER, HEADER_PARAM_MARKER_VALUE,
|
GATEWAY_BODY_KEY, HEADER_PARAM_IN_MARKER, HEADER_PARAM_MARKER_VALUE,
|
||||||
};
|
};
|
||||||
use super::openapi_spec::{
|
use super::openapi_spec::{
|
||||||
collect_ignored_schema_keys, OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS,
|
collect_ignored_schema_keys, OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS,
|
||||||
@@ -378,16 +378,16 @@ impl FromOpenAPI {
|
|||||||
// diagnosis is not a dead end.
|
// diagnosis is not a dead end.
|
||||||
return Err(AdapterError::SchemaParse {
|
return Err(AdapterError::SchemaParse {
|
||||||
message: format!(
|
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 \
|
matching parameter in the operation's resolved input schema. Every \
|
||||||
`parameters` source was merged (path-item level and operation level, \
|
`parameters` source was merged (path-item level and operation level, \
|
||||||
review 002 OAI-13); a placeholder left unbound after the merge means \
|
review 002 OAI-13); a placeholder left unbound after the merge means \
|
||||||
a declared parameter was dropped — check that each entry under \
|
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. \
|
list) has `name` and `in`, and that its $ref, if any, resolves. \
|
||||||
The placeholder would otherwise render as a literal `{}` path segment",
|
The placeholder would otherwise render as a literal `{{}}` path segment",
|
||||||
unbound.join(", "),
|
bounded_join(&[path_template.to_string()]),
|
||||||
unbound[0]
|
bounded_join(&unbound)
|
||||||
),
|
),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -46,6 +46,7 @@
|
|||||||
|
|
||||||
use std::collections::{BTreeMap, HashMap, HashSet};
|
use std::collections::{BTreeMap, HashMap, HashSet};
|
||||||
|
|
||||||
|
use crate::adapters::forward::bounded_join;
|
||||||
use alkcall::client::AdapterError;
|
use alkcall::client::AdapterError;
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
use yaml_serde::Value as YamlValue;
|
use yaml_serde::Value as YamlValue;
|
||||||
@@ -484,7 +485,7 @@ impl OpenAPISpec {
|
|||||||
configured at assembly time and cannot honor per-location `servers` — \
|
configured at assembly time and cannot honor per-location `servers` — \
|
||||||
remove the `servers` entries or split the service into one import per \
|
remove the `servers` entries or split the service into one import per \
|
||||||
base URL (review 001 OAI-06)",
|
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 \
|
single-endpoint HTTP adapter does not model — remove the \
|
||||||
`callbacks` entries or split those operations into their own \
|
`callbacks` entries or split those operations into their own \
|
||||||
service definition (review 002 OAI-14)",
|
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 — \
|
security requirements would silently change nothing at call time — \
|
||||||
remove the `security` blocks or set the `auth` field on the service \
|
remove the `security` blocks or set the `auth` field on the service \
|
||||||
config instead (review 002 OAI-14)",
|
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<Value, AdapterError> {
|
pub(crate) fn resolve_ref(&self, reference: &str) -> Result<Value, AdapterError> {
|
||||||
|
let bounded = |r: &str| {
|
||||||
|
if r.chars().count() > 128 {
|
||||||
|
format!("{}…", r.chars().take(128).collect::<String>())
|
||||||
|
} else {
|
||||||
|
r.to_string()
|
||||||
|
}
|
||||||
|
};
|
||||||
if !reference.starts_with("#/") {
|
if !reference.starts_with("#/") {
|
||||||
return Err(AdapterError::SchemaParse {
|
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;
|
let mut current: &Value = &self.raw;
|
||||||
for part in reference.trim_start_matches("#/").split('/') {
|
for part in reference.trim_start_matches("#/").split('/') {
|
||||||
current = current.get(part).ok_or_else(|| AdapterError::SchemaParse {
|
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())
|
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<String> = (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 -----------------------------------
|
// --- OAI-12: YAML input normalization -----------------------------------
|
||||||
|
|
||||||
const OAI12_HEADER: &str = r#"
|
const OAI12_HEADER: &str = r#"
|
||||||
|
|||||||
Reference in New Issue
Block a user