From beeffae9c50f7b1526eeee46ae5a48f98c2f1566 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 30 Sep 2026 06:43:02 +0900 Subject: [PATCH] fix: validate HTTPS binding limits and response ordering --- crates/wip-https/src/batch.rs | 23 --------------- crates/wip-https/src/codec.rs | 7 ----- crates/wip-https/src/json.rs | 40 +++++++++++++++++--------- crates/wip-https/tests/batch.rs | 49 ++++++++++++++++++++++++++++++++ crates/wip-https/tests/codecs.rs | 40 ++++++++++++++++++++++++++ 5 files changed, 115 insertions(+), 44 deletions(-) diff --git a/crates/wip-https/src/batch.rs b/crates/wip-https/src/batch.rs index 93e368e..0a0d8ef 100644 --- a/crates/wip-https/src/batch.rs +++ b/crates/wip-https/src/batch.rs @@ -397,23 +397,6 @@ fn response_from_json( Ok(response) } -fn unsupported_batch_descriptor_format(value: &Json, request: &BatchRequest) -> Option { - let responses = value.as_object()?.get("responses")?.as_array()?; - for item in responses { - let fields = item.as_object()?; - let id = fields.get("id")?.as_str()?; - let request_item = request.requests.iter().find(|item| item.id == id)?; - if matches!(request_item.request, BatchRequestKind::FetchInterface(_)) - && let Some(result) = fields.get("result") - && let Ok(format) = json::fetch_interface_response_format(result) - && format != INTERFACE_FORMAT_V1 - { - return Some(format); - } - } - None -} - /// Decodes a batch response, including item-local failures within HTTP 200. pub fn decode_batch_response>( request: &BatchRequest, @@ -434,12 +417,6 @@ pub fn decode_batch_response>( } return Err(response::transport_failure(value.status())); } - if value.status().is_success() - && let Ok(json) = json::parse(value.body().as_ref(), BodyKind::Response, limits) - && let Some(format) = unsupported_batch_descriptor_format(&json, request) - { - return Err(ClientResponseError::UnsupportedDescriptorFormat { format }); - } let decoded = response::decode( value.status(), value.body().as_ref(), diff --git a/crates/wip-https/src/codec.rs b/crates/wip-https/src/codec.rs index 4ed0fcd..3490c78 100644 --- a/crates/wip-https/src/codec.rs +++ b/crates/wip-https/src/codec.rs @@ -355,13 +355,6 @@ pub fn decode_fetch_interface_response>( limits: Limits, ) -> Result, ClientResponseError> { require_response_content_type(value)?; - if value.status().is_success() - && let Ok(json) = json::parse(value.body().as_ref(), BodyKind::Response, limits) - && let Ok(format) = json::fetch_interface_response_format(&json) - && format != INTERFACE_FORMAT_V1 - { - return Err(ClientResponseError::UnsupportedDescriptorFormat { format }); - } let decoded = response::decode( value.status(), value.body().as_ref(), diff --git a/crates/wip-https/src/json.rs b/crates/wip-https/src/json.rs index 40ad674..3cce5b4 100644 --- a/crates/wip-https/src/json.rs +++ b/crates/wip-https/src/json.rs @@ -8,8 +8,8 @@ use serde_json::{Map, Number, Value as Json}; use wip_protocol::{ CallOperationRequest, CallOperationResponse, Documentation, EnumCase, FetchInterfaceRequest, FetchInterfaceResponse, FetchRequest, FetchTreeRequest, FetchTreeResponse, FieldDeclaration, - InterfaceDescriptor, InterfaceTarget, MAX_SAFE_INTEGER, MIN_SAFE_INTEGER, Object, - ObjectTreeNode, OperationDeclaration, ParameterDeclaration, ReturnDeclaration, Target, + INTERFACE_FORMAT_V1, InterfaceDescriptor, InterfaceTarget, MAX_SAFE_INTEGER, MIN_SAFE_INTEGER, + Object, ObjectTreeNode, OperationDeclaration, ParameterDeclaration, ReturnDeclaration, Target, TypeDeclaration, TypeExpr, UnionCase, Value, }; @@ -119,6 +119,7 @@ pub(crate) fn serialize( kind: BodyKind, limits: Limits, ) -> Result, CodecError> { + check_nesting(value, 0, limits.max_nesting)?; let body = serde_json::to_vec(value).expect("serde_json::Value serialization cannot fail"); let maximum = match kind { BodyKind::Request => limits.max_request_bytes, @@ -759,23 +760,34 @@ pub(crate) fn fetch_interface_response_to_json(response: &FetchInterfaceResponse Json::Object(fields) } -pub(crate) fn fetch_interface_response_format(value: &Json) -> Result { - let fields = exact_object(value, "response", &["interface", "descriptor", "validator"])?; - string(required(fields, "interface")?, "interface")?; - let descriptor = object(required(fields, "descriptor")?, "descriptor")?; - string(required(descriptor, "format")?, "descriptor.format") -} - pub(crate) fn fetch_interface_response_from_json( value: &Json, ) -> Result { let fields = exact_object(value, "response", &["interface", "descriptor", "validator"])?; + let interface = string(required(fields, "interface")?, "interface")?; + let validator = optional(fields, "validator")? + .map(|value| base64_decode(value, "validator")) + .transpose()?; + let descriptor_value = required(fields, "descriptor")?; + let descriptor_fields = object(descriptor_value, "descriptor")?; + let format = string(required(descriptor_fields, "format")?, "descriptor.format")?; + let descriptor = if format == INTERFACE_FORMAT_V1 { + descriptor_from_json(descriptor_value, "descriptor")? + } else { + // Preserve only the format discriminator. The client classifies this + // response after validating common response fields and request + // correspondence, without interpreting an unknown descriptor schema. + InterfaceDescriptor { + format, + documentation: None, + types: Vec::new(), + operations: Vec::new(), + } + }; Ok(FetchInterfaceResponse { - interface: string(required(fields, "interface")?, "interface")?, - descriptor: descriptor_from_json(required(fields, "descriptor")?, "descriptor")?, - validator: optional(fields, "validator")? - .map(|value| base64_decode(value, "validator")) - .transpose()?, + interface, + descriptor, + validator, }) } diff --git a/crates/wip-https/tests/batch.rs b/crates/wip-https/tests/batch.rs index 30d007b..0ac03e7 100644 --- a/crates/wip-https/tests/batch.rs +++ b/crates/wip-https/tests/batch.rs @@ -165,6 +165,55 @@ fn top_level_batch_errors_use_the_batch_allowed_code_set() { )); } +#[test] +fn unsupported_batch_descriptor_waits_for_envelope_correlation() { + let duplicate = wip_https::http::Response::builder() + .status(StatusCode::OK) + .header("content-type", "application/json; charset=utf-8") + .body( + br#"{"responses":[{"id":"interface","result":{"interface":"interface-v1","descriptor":{"format":"wip-interface/2","future":true}}},{"id":"interface","result":{"interface":"interface-v1","descriptor":{"format":"wip-interface/2"}}}]}"# + .to_vec(), + ) + .unwrap(); + assert!(matches!( + decode_batch_response(&request(), &duplicate, limits()).unwrap_err(), + ClientResponseError::InvalidResponse { + kind: InvalidResponseKind::InvalidBatch, + .. + } + )); + + let missing = wip_https::http::Response::builder() + .status(StatusCode::OK) + .header("content-type", "application/json; charset=utf-8") + .body( + br#"{"responses":[{"id":"interface","result":{"interface":"interface-v1","descriptor":{"format":"wip-interface/2"}}}]}"# + .to_vec(), + ) + .unwrap(); + assert!(matches!( + decode_batch_response(&request(), &missing, limits()).unwrap_err(), + ClientResponseError::InvalidResponse { + kind: InvalidResponseKind::InvalidBatch, + .. + } + )); + + let request = BatchRequest { + requests: vec![BatchRequestItem { + id: "interface".into(), + request: BatchRequestKind::FetchInterface(FetchInterfaceRequest { + interface: "interface-v1".into(), + }), + }], + }; + assert!(matches!( + decode_batch_response(&request, &missing, limits()).unwrap_err(), + ClientResponseError::UnsupportedDescriptorFormat { format } + if format == "wip-interface/2" + )); +} + #[test] fn batch_response_must_have_exactly_the_requested_ids() { let request = request(); diff --git a/crates/wip-https/tests/codecs.rs b/crates/wip-https/tests/codecs.rs index 9bbae51..4699c35 100644 --- a/crates/wip-https/tests/codecs.rs +++ b/crates/wip-https/tests/codecs.rs @@ -367,6 +367,27 @@ fn caller_limits_bound_bytes_and_nesting() { let nested = br#"{"path":[[["/"]]]}"#; let nesting_limited = Limits::new(1024, 1024, 2, 1).unwrap(); assert!(decode_fetch_request(nested, nesting_limited).is_err()); + + let outbound_limited = Limits::new(1_000_000, 1_000_000, 1, 10).unwrap(); + assert!( + encode_call_operation_request( + &endpoint(), + &call_request(), + &descriptor(), + outbound_limited, + ) + .is_err() + ); + assert!( + encode_fetch_response( + &FetchRequest { + path: "/item".into(), + }, + &object("item", b"validator"), + outbound_limited, + ) + .is_err() + ); } #[test] @@ -400,6 +421,25 @@ fn unsupported_descriptor_format_is_client_local() { ClientResponseError::UnsupportedDescriptorFormat { format } if format == "wip-interface/2" )); + + for malformed in [ + br#"{"interface":"wrong","descriptor":{"format":"wip-interface/2"}}"#.as_slice(), + br#"{"interface":"interface-v1","descriptor":{"format":"wip-interface/2"},"validator":null}"# + .as_slice(), + ] { + let response = wip_https::http::Response::builder() + .status(StatusCode::OK) + .header("content-type", "application/json; charset=utf-8") + .body(malformed.to_vec()) + .unwrap(); + assert!(matches!( + decode_fetch_interface_response(&request, &response, limits()).unwrap_err(), + ClientResponseError::InvalidResponse { + kind: InvalidResponseKind::InvalidSuccessBody, + .. + } + )); + } } #[test]