diff --git a/crates/wip-https/src/batch.rs b/crates/wip-https/src/batch.rs index 0a0d8ef..f22d89c 100644 --- a/crates/wip-https/src/batch.rs +++ b/crates/wip-https/src/batch.rs @@ -1,6 +1,6 @@ use std::collections::{BTreeMap, BTreeSet}; -use http::header::CONTENT_TYPE; +use http::header::{ACCEPT, CONTENT_TYPE}; use http::{Method, Request, Response, StatusCode}; use serde_json::{Map, Value as Json}; use wip_protocol::{ @@ -11,9 +11,38 @@ use wip_protocol::{ use crate::response::{self, DecodedResponse}; use crate::{ BodyKind, ClientResponseError, CodecError, EncodeHttpError, Endpoint, InvalidResponseKind, - JSON_CONTENT_TYPE, Limits, Route, is_json_content_type, json, + JSON_CONTENT_TYPE, Limits, ResourceLimitKind, Route, is_json_content_type, json, }; +/// A retrieval method accepted by the standard v1 batch endpoint. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BatchMethod { + /// Fetch one object. + Fetch, + /// Fetch one bounded tree. + FetchTree, + /// Fetch one interface descriptor. + FetchInterface, +} + +impl BatchMethod { + fn name(self) -> &'static str { + match self { + Self::Fetch => "fetch", + Self::FetchTree => "fetch_tree", + Self::FetchInterface => "fetch_interface", + } + } + + fn interaction(self) -> ProtocolInteraction { + match self { + Self::Fetch => ProtocolInteraction::Fetch, + Self::FetchTree => ProtocolInteraction::FetchTree, + Self::FetchInterface => ProtocolInteraction::FetchInterface, + } + } +} + /// One retrieval interaction accepted by the standard v1 batch endpoint. #[derive(Debug, Clone, PartialEq, Eq)] pub enum BatchRequestKind { @@ -23,6 +52,13 @@ pub enum BatchRequestKind { FetchTree(FetchTreeRequest), /// Fetch one interface descriptor. FetchInterface(FetchInterfaceRequest), + /// A known method whose `arguments` are not a valid logical request. + Invalid { + /// Method that was successfully identified from the item envelope. + method: BatchMethod, + /// Item-local `invalid_request` to return without rejecting other items. + error: ProtocolError, + }, } impl BatchRequestKind { @@ -31,6 +67,7 @@ impl BatchRequestKind { Self::Fetch(_) => ProtocolInteraction::Fetch, Self::FetchTree(_) => ProtocolInteraction::FetchTree, Self::FetchInterface(_) => ProtocolInteraction::FetchInterface, + Self::Invalid { method, .. } => method.interaction(), } } } @@ -96,7 +133,7 @@ fn validate_ids<'a>(ids: impl Iterator, maximum: usize) -> Resul Ok(()) } -fn request_item_to_json(item: &BatchRequestItem) -> Json { +fn request_item_to_json(item: &BatchRequestItem) -> Result { let (method, arguments) = match &item.request { BatchRequestKind::Fetch(value) => ("fetch", json::fetch_request_to_json(value)), BatchRequestKind::FetchTree(value) => { @@ -106,8 +143,14 @@ fn request_item_to_json(item: &BatchRequestItem) -> Json { "fetch_interface", json::fetch_interface_request_to_json(value), ), + BatchRequestKind::Invalid { .. } => { + return Err(CodecError::InvalidField { + field: "requests".into(), + reason: "item-local invalid requests cannot be encoded as client requests".into(), + }); + } }; - serde_json::json!({"id": item.id, "method": method, "arguments": arguments}) + Ok(serde_json::json!({"id": item.id, "method": method, "arguments": arguments})) } fn request_to_json(request: &BatchRequest, limits: Limits) -> Result { @@ -119,12 +162,15 @@ fn request_to_json(request: &BatchRequest, limits: Limits) -> Result value.validate()?, BatchRequestKind::FetchTree(value) => value.validate()?, - BatchRequestKind::FetchInterface(_) => {} + BatchRequestKind::FetchInterface(_) | BatchRequestKind::Invalid { .. } => {} } } - Ok(serde_json::json!({ - "requests": request.requests.iter().map(request_item_to_json).collect::>() - })) + let requests = request + .requests + .iter() + .map(request_item_to_json) + .collect::, _>>()?; + Ok(serde_json::json!({"requests": requests})) } fn request_from_json(value: &Json, limits: Limits) -> Result { @@ -141,16 +187,12 @@ fn request_from_json(value: &Json, limits: Limits) -> Result BatchRequestKind::Fetch(json::fetch_request_from_json(arguments)?), - "fetch_tree" => { - BatchRequestKind::FetchTree(json::fetch_tree_request_from_json(arguments)?) - } - "fetch_interface" => BatchRequestKind::FetchInterface( - json::fetch_interface_request_from_json(arguments)?, - ), + let method = match method_name.as_str() { + "fetch" => BatchMethod::Fetch, + "fetch_tree" => BatchMethod::FetchTree, + "fetch_interface" => BatchMethod::FetchInterface, _ => { return Err(CodecError::InvalidField { field: format!("{field}.method"), @@ -158,6 +200,23 @@ fn request_from_json(value: &Json, limits: Limits) -> Result { + json::fetch_request_from_json(arguments).map(BatchRequestKind::Fetch) + } + BatchMethod::FetchTree => { + json::fetch_tree_request_from_json(arguments).map(BatchRequestKind::FetchTree) + } + BatchMethod::FetchInterface => json::fetch_interface_request_from_json(arguments) + .map(BatchRequestKind::FetchInterface), + }; + let request = decoded.unwrap_or_else(|error| BatchRequestKind::Invalid { + method, + error: ProtocolError { + code: ProtocolErrorCode::InvalidRequest, + message: format!("invalid {} arguments: {error}", method.name()), + }, + }); requests.push(BatchRequestItem { id, request }); } validate_ids( @@ -178,6 +237,7 @@ pub fn encode_batch_request( .method(Method::POST) .uri(endpoint.route(Route::Batch).as_str()) .header(CONTENT_TYPE, JSON_CONTENT_TYPE) + .header(ACCEPT, "application/json") .body(body)?) } @@ -238,33 +298,29 @@ fn correlate( id: item.id.clone(), }); }; - match &item.outcome { - Ok(BatchSuccess::Fetch(value)) if matches!(kind, BatchRequestKind::Fetch(_)) => { - if let BatchRequestKind::Fetch(request) = kind { - request.validate_response(value)?; - } + match (&item.outcome, *kind) { + (Ok(BatchSuccess::Fetch(value)), BatchRequestKind::Fetch(request)) => { + request.validate_response(value)?; } - Ok(BatchSuccess::FetchTree(value)) - if matches!(kind, BatchRequestKind::FetchTree(_)) => - { - if let BatchRequestKind::FetchTree(request) = kind { - value.validate_for(request)?; - } + (Ok(BatchSuccess::FetchTree(value)), BatchRequestKind::FetchTree(request)) => { + value.validate_for(request)?; } - Ok(BatchSuccess::FetchInterface(value)) - if matches!(kind, BatchRequestKind::FetchInterface(_)) => - { - if let BatchRequestKind::FetchInterface(request) = kind { - value.validate_for(request)?; - } + ( + Ok(BatchSuccess::FetchInterface(value)), + BatchRequestKind::FetchInterface(request), + ) => { + value.validate_for(request)?; } - Ok(_) => { - return Err(CodecError::BatchInteractionMismatch { - id: item.id.clone(), - }); - } - Err(error) if error.code.is_allowed_for(kind.interaction()) => {} - Err(_) => { + ( + Err(error), + BatchRequestKind::Invalid { + error: expected, .. + }, + ) if error == expected => {} + (Err(error), kind) + if !matches!(kind, BatchRequestKind::Invalid { .. }) + && error.code.is_allowed_for(kind.interaction()) => {} + _ => { return Err(CodecError::BatchInteractionMismatch { id: item.id.clone(), }); @@ -301,7 +357,10 @@ pub fn encode_batch_response( /// Encodes a top-level batch-envelope failure. /// /// Item-local failures belong in [`BatchResponseItem`]. Only failures that can -/// reject the batch envelope before item execution are accepted here. +/// reject the batch envelope before item execution are accepted here. A +/// resource-limit failure defaults to the logical-limit status; use +/// [`encode_batch_resource_limit_error_response`] when the concrete class is +/// known. pub fn encode_batch_error_response( error: &ProtocolError, limits: Limits, @@ -325,6 +384,26 @@ pub fn encode_batch_error_response( .body(body)?) } +/// Encodes a class-specific top-level batch `resource_limit_exceeded` failure. +pub fn encode_batch_resource_limit_error_response( + error: &ProtocolError, + kind: ResourceLimitKind, + limits: Limits, +) -> Result>, EncodeHttpError> { + if error.code != ProtocolErrorCode::ResourceLimitExceeded { + return Err(CodecError::InvalidField { + field: "error.code".into(), + reason: "resource-limit encoder requires resource_limit_exceeded".into(), + } + .into()); + } + let body = json::serialize(&response::error_to_json(error), BodyKind::Response, limits)?; + Ok(Response::builder() + .status(response::status_for_resource_limit(kind)) + .header(CONTENT_TYPE, JSON_CONTENT_TYPE) + .body(body)?) +} + fn response_from_json( value: &Json, request: &BatchRequest, @@ -367,8 +446,15 @@ fn response_from_json( } (None, Some(error)) => { let error = response::item_error_from_json(error)?; - if !error.code.is_allowed_for(kind.interaction()) { - return Err(CodecError::BatchInteractionMismatch { id }); + match kind { + BatchRequestKind::Invalid { + error: expected, .. + } if error == *expected => {} + BatchRequestKind::Invalid { .. } => { + return Err(CodecError::BatchInteractionMismatch { id }); + } + kind if error.code.is_allowed_for(kind.interaction()) => {} + _ => return Err(CodecError::BatchInteractionMismatch { id }), } Err(error) } @@ -388,6 +474,9 @@ fn response_from_json( value.validate_for(request)?; BatchSuccess::FetchInterface(value) } + BatchRequestKind::Invalid { .. } => { + return Err(CodecError::BatchInteractionMismatch { id }); + } }), }; responses.push(BatchResponseItem { id, outcome }); diff --git a/crates/wip-https/src/codec.rs b/crates/wip-https/src/codec.rs index 3490c78..d962fa7 100644 --- a/crates/wip-https/src/codec.rs +++ b/crates/wip-https/src/codec.rs @@ -1,5 +1,8 @@ -use http::header::CONTENT_TYPE; -use http::{Method, Request, Response, StatusCode}; +use std::error::Error; +use std::fmt::{self, Display, Formatter}; + +use http::header::{ACCEPT, CONTENT_TYPE}; +use http::{HeaderMap, Method, Request, Response, StatusCode}; use wip_protocol::{ CallOperationRequest, CallOperationResponse, FetchInterfaceRequest, FetchInterfaceResponse, FetchRequest, FetchResponse, FetchTreeRequest, FetchTreeResponse, INTERFACE_FORMAT_V1, @@ -9,7 +12,7 @@ use wip_protocol::{ use crate::response::{self, DecodedResponse}; use crate::{ BodyKind, ClientResponseError, CodecError, Endpoint, InvalidResponseKind, JSON_CONTENT_TYPE, - Limits, Route, json, + Limits, ResourceLimitKind, Route, json, }; fn request( @@ -21,6 +24,7 @@ fn request( .method(Method::POST) .uri(endpoint.route(route).as_str()) .header(CONTENT_TYPE, JSON_CONTENT_TYPE) + .header(ACCEPT, "application/json") .body(body) } @@ -82,27 +86,138 @@ pub fn is_json_content_type(value: &str) -> bool { true } +fn accepts_json(headers: &HeaderMap) -> bool { + let mut present = false; + let mut best_match: Option<(u8, f32)> = None; + for value in headers.get_all(ACCEPT) { + present = true; + let Ok(value) = value.to_str() else { + continue; + }; + for range in value.split(',') { + let mut parts = range.split(';'); + let media_range = parts.next().unwrap_or_default().trim(); + let mut quality = 1.0_f32; + let mut saw_quality = false; + let mut valid = true; + for parameter in parts { + let Some((name, value)) = parameter.trim().split_once('=') else { + valid = false; + break; + }; + if name.trim().eq_ignore_ascii_case("q") { + if saw_quality { + valid = false; + break; + } + saw_quality = true; + match value.trim().parse::() { + Ok(value) if (0.0..=1.0).contains(&value) => quality = value, + _ => { + valid = false; + break; + } + } + } + } + let specificity = if media_range.eq_ignore_ascii_case("application/json") { + Some(2) + } else if media_range.eq_ignore_ascii_case("application/*") { + Some(1) + } else if media_range == "*/*" { + Some(0) + } else { + None + }; + if valid && let Some(specificity) = specificity { + match best_match { + Some((best_specificity, _)) if best_specificity > specificity => {} + Some((best_specificity, best_quality)) if best_specificity == specificity => { + best_match = Some((specificity, best_quality.max(quality))); + } + _ => best_match = Some((specificity, quality)), + } + } + } + } + if present { + best_match.is_some_and(|(_, quality)| quality > 0.0) + } else { + true + } +} + +/// A failure while validating inbound HTTP request metadata. +#[derive(Debug)] +pub enum RequestMetadataError { + /// The method or route does not match the selected WIP interaction. + InvalidRequest(CodecError), + /// The request body does not use UTF-8 `application/json`. + UnsupportedMediaType, + /// The request's `Accept` metadata excludes `application/json`. + NotAcceptable, +} + +impl RequestMetadataError { + /// Returns the HTTP status for this metadata failure. + #[must_use] + pub const fn status(&self) -> StatusCode { + match self { + Self::InvalidRequest(_) => StatusCode::BAD_REQUEST, + Self::UnsupportedMediaType => StatusCode::UNSUPPORTED_MEDIA_TYPE, + Self::NotAcceptable => StatusCode::NOT_ACCEPTABLE, + } + } +} + +impl Display for RequestMetadataError { + fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result { + match self { + Self::InvalidRequest(error) => error.fmt(formatter), + Self::UnsupportedMediaType => { + formatter.write_str("expected UTF-8 application/json Content-Type") + } + Self::NotAcceptable => formatter.write_str("request does not accept application/json"), + } + } +} + +impl Error for RequestMetadataError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::InvalidRequest(error) => Some(error), + Self::UnsupportedMediaType | Self::NotAcceptable => None, + } + } +} + /// Validates mandatory HTTP metadata for an inbound WIP request. /// /// Both absolute-form and origin-form request URIs are accepted, but the path /// must exactly equal the selected endpoint route and a query is forbidden. +/// Unsupported content negotiation is reported as a Transport Binding metadata +/// failure rather than a Core `invalid_request`. pub fn validate_http_request( endpoint: &Endpoint, route: Route, value: &Request, -) -> Result<(), CodecError> { +) -> Result<(), RequestMetadataError> { if value.method() != Method::POST { - return Err(CodecError::InvalidField { - field: "method".into(), - reason: "standard WIP routes require POST".into(), - }); + return Err(RequestMetadataError::InvalidRequest( + CodecError::InvalidField { + field: "method".into(), + reason: "standard WIP routes require POST".into(), + }, + )); } let expected = endpoint.route(route); if value.uri().path() != expected.path() || value.uri().query().is_some() { - return Err(CodecError::InvalidField { - field: "route".into(), - reason: "request URI does not match the selected standard route".into(), - }); + return Err(RequestMetadataError::InvalidRequest( + CodecError::InvalidField { + field: "route".into(), + reason: "request URI does not match the selected standard route".into(), + }, + )); } let valid_content_type = value .headers() @@ -110,10 +225,10 @@ pub fn validate_http_request( .and_then(|value| value.to_str().ok()) .is_some_and(is_json_content_type); if !valid_content_type { - return Err(CodecError::InvalidField { - field: "content-type".into(), - reason: "expected UTF-8 application/json".into(), - }); + return Err(RequestMetadataError::UnsupportedMediaType); + } + if !accepts_json(value.headers()) { + return Err(RequestMetadataError::NotAcceptable); } Ok(()) } @@ -405,17 +520,10 @@ pub fn decode_call_operation_response>( ) } -/// Encodes a canonical Host error envelope using its standard HTTP status. -/// -/// The code must be allowed for `interaction`. This API accepts only -/// [`ProtocolError`], so client-local `InvalidResponse` and -/// `UnsupportedDescriptorFormat` classifications cannot be emitted as Host -/// error envelopes. -pub fn encode_protocol_error_response( +fn validate_protocol_error( interaction: ProtocolInteraction, error: &ProtocolError, - limits: Limits, -) -> Result>, EncodeHttpError> { +) -> Result<(), CodecError> { if !error.code.is_allowed_for(interaction) { return Err(CodecError::InvalidField { field: "error.code".into(), @@ -423,11 +531,63 @@ pub fn encode_protocol_error_response( "code `{}` is not allowed for {interaction:?}", response::error_code_name(error.code) ), + }); + } + Ok(()) +} + +fn encode_protocol_error_with_status( + interaction: ProtocolInteraction, + error: &ProtocolError, + status: StatusCode, + limits: Limits, +) -> Result>, EncodeHttpError> { + validate_protocol_error(interaction, error)?; + let body = json::serialize(&response::error_to_json(error), BodyKind::Response, limits)?; + Ok(response(status, body)?) +} + +/// Encodes a canonical Host error envelope using its standard HTTP status. +/// +/// The code must be allowed for `interaction`. This API accepts only +/// [`ProtocolError`], so client-local `InvalidResponse` and +/// `UnsupportedDescriptorFormat` classifications cannot be emitted as Host +/// error envelopes. Resource-limit errors use the logical-limit status; callers +/// that know the concrete limit class should use +/// [`encode_resource_limit_error_response`]. +pub fn encode_protocol_error_response( + interaction: ProtocolInteraction, + error: &ProtocolError, + limits: Limits, +) -> Result>, EncodeHttpError> { + encode_protocol_error_with_status( + interaction, + error, + response::status_for_error(error.code), + limits, + ) +} + +/// Encodes a `resource_limit_exceeded` envelope with its class-specific status. +pub fn encode_resource_limit_error_response( + interaction: ProtocolInteraction, + error: &ProtocolError, + kind: ResourceLimitKind, + limits: Limits, +) -> Result>, EncodeHttpError> { + if error.code != wip_protocol::ProtocolErrorCode::ResourceLimitExceeded { + return Err(CodecError::InvalidField { + field: "error.code".into(), + reason: "resource-limit encoder requires resource_limit_exceeded".into(), } .into()); } - let body = json::serialize(&response::error_to_json(error), BodyKind::Response, limits)?; - Ok(response(response::status_for_error(error.code), body)?) + encode_protocol_error_with_status( + interaction, + error, + response::status_for_resource_limit(kind), + limits, + ) } /// Failure while building an outbound framework-neutral HTTP message. diff --git a/crates/wip-https/src/error.rs b/crates/wip-https/src/error.rs index 5b2ddff..30d5b94 100644 --- a/crates/wip-https/src/error.rs +++ b/crates/wip-https/src/error.rs @@ -102,6 +102,15 @@ pub enum BodyKind { Response, } +/// The class of a resource limit exceeded by a wire representation. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ResourceLimitKind { + /// A request or response representation exceeded its byte-size limit. + RepresentationBytes, + /// A logical limit such as nesting depth, batch count, or collection size was exceeded. + Logical, +} + /// A malformed or locally disallowed wire representation. #[derive(Debug)] pub enum CodecError { @@ -177,6 +186,20 @@ pub enum CodecError { }, } +impl CodecError { + /// Classifies this failure when it represents a canonical resource limit. + #[must_use] + pub const fn resource_limit_kind(&self) -> Option { + match self { + Self::BodyTooLarge { .. } => Some(ResourceLimitKind::RepresentationBytes), + Self::NestingTooDeep { .. } | Self::TooManyBatchItems { .. } => { + Some(ResourceLimitKind::Logical) + } + _ => None, + } + } +} + impl Display for CodecError { fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result { match self { diff --git a/crates/wip-https/src/lib.rs b/crates/wip-https/src/lib.rs index 31ee7a1..d58771c 100644 --- a/crates/wip-https/src/lib.rs +++ b/crates/wip-https/src/lib.rs @@ -36,25 +36,27 @@ mod json; mod response; pub use batch::{ - BatchRequest, BatchRequestItem, BatchRequestKind, BatchResponse, BatchResponseItem, - BatchSuccess, decode_batch_request, decode_batch_response, encode_batch_error_response, - encode_batch_request, encode_batch_response, + BatchMethod, BatchRequest, BatchRequestItem, BatchRequestKind, BatchResponse, + BatchResponseItem, BatchSuccess, decode_batch_request, decode_batch_response, + encode_batch_error_response, encode_batch_request, encode_batch_resource_limit_error_response, + encode_batch_response, }; pub use codec::{ - CallOperationMetadata, EncodeHttpError, decode_call_operation_metadata, + CallOperationMetadata, EncodeHttpError, RequestMetadataError, decode_call_operation_metadata, decode_call_operation_request, decode_call_operation_response, decode_fetch_interface_request, decode_fetch_interface_response, decode_fetch_request, decode_fetch_response, decode_fetch_tree_request, decode_fetch_tree_response, encode_call_operation_request, encode_call_operation_response, encode_fetch_interface_request, encode_fetch_interface_response, encode_fetch_request, encode_fetch_response, encode_fetch_tree_request, encode_fetch_tree_response, encode_protocol_error_response, - is_json_content_type, validate_http_request, + encode_resource_limit_error_response, is_json_content_type, validate_http_request, }; pub use endpoint::{Endpoint, EndpointError, JSON_CONTENT_TYPE, Route}; -pub use error::{BodyKind, CodecError, LimitConfigurationError, Limits}; +pub use error::{BodyKind, CodecError, LimitConfigurationError, Limits, ResourceLimitKind}; pub use response::{ ClientResponseError, DecodedResponse, InvalidResponseKind, StatusMismatch, TransportBindingFailure, TransportFailureKind, error_code_name, status_for_error, + status_for_resource_limit, }; /// Framework-neutral HTTP types used by this binding's public API. diff --git a/crates/wip-https/src/response.rs b/crates/wip-https/src/response.rs index 016fb9c..8d46899 100644 --- a/crates/wip-https/src/response.rs +++ b/crates/wip-https/src/response.rs @@ -5,7 +5,7 @@ use http::StatusCode; use serde_json::{Map, Value as Json}; use wip_protocol::{ProtocolError, ProtocolErrorCode, ProtocolInteraction}; -use crate::{BodyKind, CodecError, Limits, json}; +use crate::{BodyKind, CodecError, Limits, ResourceLimitKind, json}; /// A successfully classified client response. #[derive(Debug, Clone, PartialEq, Eq)] @@ -59,6 +59,8 @@ pub enum InvalidResponseKind { SuccessBodyOnErrorStatus, /// A valid error envelope used a code forbidden for the interaction. DisallowedProtocolCode, + /// A valid success body used a 2xx status other than canonical HTTP 200. + NonCanonicalSuccessStatus, /// A 2xx body was malformed or semantically invalid. InvalidSuccessBody, /// A batch response did not correlate exactly with its request. @@ -117,6 +119,10 @@ impl Display for ClientResponseError { impl Error for ClientResponseError {} /// Returns the canonical HTTP status for a wire-level protocol error. +/// +/// [`ProtocolErrorCode::ResourceLimitExceeded`] defaults to the logical-limit +/// status. Use [`status_for_resource_limit`] when the concrete limit class is +/// known. #[must_use] pub const fn status_for_error(code: ProtocolErrorCode) -> StatusCode { match code { @@ -132,13 +138,31 @@ pub const fn status_for_error(code: ProtocolErrorCode) -> StatusCode { StatusCode::PRECONDITION_FAILED } ProtocolErrorCode::PermissionDenied => StatusCode::FORBIDDEN, - ProtocolErrorCode::ResourceLimitExceeded => StatusCode::TOO_MANY_REQUESTS, + ProtocolErrorCode::ResourceLimitExceeded => StatusCode::UNPROCESSABLE_ENTITY, ProtocolErrorCode::Internal | ProtocolErrorCode::OperationOutcomeUnknown => { StatusCode::INTERNAL_SERVER_ERROR } } } +/// Returns the canonical HTTP status for a concrete resource-limit class. +#[must_use] +pub const fn status_for_resource_limit(kind: ResourceLimitKind) -> StatusCode { + match kind { + ResourceLimitKind::RepresentationBytes => StatusCode::PAYLOAD_TOO_LARGE, + ResourceLimitKind::Logical => StatusCode::UNPROCESSABLE_ENTITY, + } +} + +fn status_matches_error(status: StatusCode, code: ProtocolErrorCode) -> bool { + match code { + ProtocolErrorCode::ResourceLimitExceeded => { + status == StatusCode::PAYLOAD_TOO_LARGE || status == StatusCode::UNPROCESSABLE_ENTITY + } + _ => status == status_for_error(code), + } +} + /// Returns the stable snake-case wire spelling of a protocol code. #[must_use] pub const fn error_code_name(code: ProtocolErrorCode) -> &'static str { @@ -288,12 +312,14 @@ pub(crate) fn decode( )); } let expected = status_for_error(error.code); - Ok(DecodedResponse::ProtocolFailure { - error, - status_mismatch: (status != expected).then_some(StatusMismatch { + let status_mismatch = + (!status_matches_error(status, error.code)).then_some(StatusMismatch { actual: status, expected, - }), + }); + Ok(DecodedResponse::ProtocolFailure { + error, + status_mismatch, }) } Err(_) if !status.is_success() => Err(transport_failure(status)), @@ -301,16 +327,24 @@ pub(crate) fn decode( InvalidResponseKind::InvalidSuccessBody, error.to_string(), )), - Ok(None) if status.is_success() => { - success(&value) - .map(DecodedResponse::Success) - .map_err(|error| { - ClientResponseError::invalid( - InvalidResponseKind::InvalidSuccessBody, - error.to_string(), - ) - }) - } + Ok(None) if status == StatusCode::OK => success(&value) + .map(DecodedResponse::Success) + .map_err(|error| { + ClientResponseError::invalid( + InvalidResponseKind::InvalidSuccessBody, + error.to_string(), + ) + }), + Ok(None) if status.is_success() => match success(&value) { + Ok(_) => Err(ClientResponseError::invalid( + InvalidResponseKind::NonCanonicalSuccessStatus, + format!("success body carried by non-canonical HTTP {status}"), + )), + Err(error) => Err(ClientResponseError::invalid( + InvalidResponseKind::InvalidSuccessBody, + error.to_string(), + )), + }, Ok(None) => match success(&value) { Ok(_) => Err(ClientResponseError::invalid( InvalidResponseKind::SuccessBodyOnErrorStatus, diff --git a/crates/wip-https/tests/batch.rs b/crates/wip-https/tests/batch.rs index 0ac03e7..d6d88da 100644 --- a/crates/wip-https/tests/batch.rs +++ b/crates/wip-https/tests/batch.rs @@ -4,9 +4,10 @@ use wip_https::wip_protocol::{ InterfaceDescriptor, Object, ProtocolError, ProtocolErrorCode, }; use wip_https::{ - BatchRequest, BatchRequestItem, BatchRequestKind, BatchResponse, BatchResponseItem, - BatchSuccess, ClientResponseError, DecodedResponse, Endpoint, InvalidResponseKind, Limits, - decode_batch_request, decode_batch_response, encode_batch_error_response, encode_batch_request, + BatchMethod, BatchRequest, BatchRequestItem, BatchRequestKind, BatchResponse, + BatchResponseItem, BatchSuccess, ClientResponseError, DecodedResponse, Endpoint, + InvalidResponseKind, Limits, ResourceLimitKind, decode_batch_request, decode_batch_response, + encode_batch_error_response, encode_batch_request, encode_batch_resource_limit_error_response, encode_batch_response, }; @@ -39,6 +40,7 @@ fn batch_round_trips_by_unique_id_with_item_failures_and_validators() { let endpoint = Endpoint::parse("https://example.test/wip").unwrap(); let encoded_request = encode_batch_request(&endpoint, &request, limits()).unwrap(); assert_eq!(encoded_request.uri().path(), "/wip/v1/batch"); + assert_eq!(encoded_request.headers()["accept"], "application/json"); assert_eq!( decode_batch_request(encoded_request.body(), limits()).unwrap(), request @@ -115,8 +117,65 @@ fn batch_interface_validator_round_trips_in_item_body() { ); } +#[test] +fn malformed_known_method_arguments_are_item_local() { + let body = br#"{ + "requests": [ + {"id":"valid","method":"fetch","arguments":{"path":"/item"}}, + {"id":"invalid","method":"fetch_tree","arguments":{"path":"/","depth":"deep","limit":1}} + ] + }"#; + let decoded = decode_batch_request(body, limits()).unwrap(); + assert!(matches!( + decoded.requests[0].request, + BatchRequestKind::Fetch(_) + )); + let invalid = match &decoded.requests[1].request { + BatchRequestKind::Invalid { method, error } => { + assert_eq!(*method, BatchMethod::FetchTree); + assert_eq!(error.code, ProtocolErrorCode::InvalidRequest); + error.clone() + } + other => panic!("expected item-local invalid request, got {other:?}"), + }; + + let response = BatchResponse { + responses: vec![ + BatchResponseItem { + id: "valid".into(), + outcome: Ok(BatchSuccess::Fetch(Object { + name: "item".into(), + description: None, + interfaces: vec![], + r#ref: None, + validator: None, + })), + }, + BatchResponseItem { + id: "invalid".into(), + outcome: Err(invalid), + }, + ], + }; + let encoded = encode_batch_response(&decoded, &response, limits()).unwrap(); + assert_eq!(encoded.status(), StatusCode::OK); + assert_eq!( + decode_batch_response(&decoded, &encoded, limits()).unwrap(), + DecodedResponse::Success(response) + ); +} + #[test] fn invalid_batch_envelopes_are_rejected_at_top_level() { + assert!(decode_batch_request(br#"{}"#, limits()).is_err()); + + let malformed_item = br#"{ + "requests": [ + {"id":"item","method":"fetch","arguments":{"path":"/"},"extra":true} + ] + }"#; + assert!(decode_batch_request(malformed_item, limits()).is_err()); + let duplicate = br#"{ "requests": [ {"id":"same","method":"fetch","arguments":{"path":"/a"}}, @@ -136,6 +195,67 @@ fn invalid_batch_envelopes_are_rejected_at_top_level() { assert!(decode_batch_request(duplicate, too_many).is_err()); } +#[test] +fn batch_resource_limits_use_413_or_422_and_never_429() { + let error = ProtocolError { + code: ProtocolErrorCode::ResourceLimitExceeded, + message: "bounded".into(), + }; + for (kind, status) in [ + ( + ResourceLimitKind::RepresentationBytes, + StatusCode::PAYLOAD_TOO_LARGE, + ), + (ResourceLimitKind::Logical, StatusCode::UNPROCESSABLE_ENTITY), + ] { + let encoded = encode_batch_resource_limit_error_response(&error, kind, limits()).unwrap(); + assert_eq!(encoded.status(), status); + assert_ne!(encoded.status(), StatusCode::TOO_MANY_REQUESTS); + assert_eq!( + decode_batch_response(&request(), &encoded, limits()).unwrap(), + DecodedResponse::ProtocolFailure { + error: error.clone(), + status_mismatch: None, + } + ); + } +} + +#[test] +fn batch_success_body_requires_exactly_http_200() { + let request = request(); + let response = BatchResponse { + responses: vec![ + BatchResponseItem { + id: "object".into(), + outcome: Ok(BatchSuccess::Fetch(Object { + name: "item".into(), + description: None, + interfaces: vec![], + r#ref: None, + validator: None, + })), + }, + BatchResponseItem { + id: "interface".into(), + outcome: Err(ProtocolError { + code: ProtocolErrorCode::InterfaceNotFound, + message: "gone".into(), + }), + }, + ], + }; + let encoded = encode_batch_response(&request, &response, limits()).unwrap(); + for status in [StatusCode::CREATED, StatusCode::ACCEPTED] { + let mut noncanonical = encoded.clone(); + *noncanonical.status_mut() = status; + assert!(matches!( + decode_batch_response(&request, &noncanonical, limits()).unwrap_err(), + ClientResponseError::InvalidResponse { .. } + )); + } +} + #[test] fn top_level_batch_errors_use_the_batch_allowed_code_set() { let invalid = ProtocolError { diff --git a/crates/wip-https/tests/codecs.rs b/crates/wip-https/tests/codecs.rs index 4699c35..0d4929d 100644 --- a/crates/wip-https/tests/codecs.rs +++ b/crates/wip-https/tests/codecs.rs @@ -9,11 +9,12 @@ use wip_https::wip_protocol::{ TypeDeclaration, TypeExpr, UnionCase, Value, }; use wip_https::{ - ClientResponseError, DecodedResponse, Endpoint, InvalidResponseKind, Limits, - decode_call_operation_metadata, decode_call_operation_request, decode_call_operation_response, - decode_fetch_interface_request, decode_fetch_interface_response, decode_fetch_request, - decode_fetch_response, decode_fetch_tree_request, decode_fetch_tree_response, - encode_call_operation_request, encode_call_operation_response, encode_fetch_interface_request, + ClientResponseError, CodecError, DecodedResponse, Endpoint, InvalidResponseKind, Limits, + ResourceLimitKind, decode_call_operation_metadata, decode_call_operation_request, + decode_call_operation_response, decode_fetch_interface_request, + decode_fetch_interface_response, decode_fetch_request, decode_fetch_response, + decode_fetch_tree_request, decode_fetch_tree_response, encode_call_operation_request, + encode_call_operation_response, encode_fetch_interface_request, encode_fetch_interface_response, encode_fetch_request, encode_fetch_response, encode_fetch_tree_request, encode_fetch_tree_response, }; @@ -193,6 +194,36 @@ fn call_request() -> CallOperationRequest { } } +#[test] +fn every_single_interaction_request_accepts_json() { + let fetch = + encode_fetch_request(&endpoint(), &FetchRequest { path: "/".into() }, limits()).unwrap(); + let fetch_tree = encode_fetch_tree_request( + &endpoint(), + &FetchTreeRequest { + path: "/".into(), + depth: 1, + limit: 1, + }, + limits(), + ) + .unwrap(); + let fetch_interface = encode_fetch_interface_request( + &endpoint(), + &FetchInterfaceRequest { + interface: "interface-v1".into(), + }, + limits(), + ) + .unwrap(); + let call = encode_call_operation_request(&endpoint(), &call_request(), &descriptor(), limits()) + .unwrap(); + + for request in [fetch, fetch_tree, fetch_interface, call] { + assert_eq!(request.headers()["accept"], "application/json"); + } +} + #[test] fn fetch_round_trips_direct_body_and_body_validator() { let request = FetchRequest { @@ -362,11 +393,21 @@ fn malformed_unknown_null_and_noncanonical_base64_are_rejected() { #[test] fn caller_limits_bound_bytes_and_nesting() { let tiny = Limits::new(8, 8, 2, 1).unwrap(); - assert!(decode_fetch_request(br#"{"path":"/long"}"#, tiny).is_err()); + let too_large = decode_fetch_request(br#"{"path":"/long"}"#, tiny).unwrap_err(); + assert!(matches!(&too_large, CodecError::BodyTooLarge { .. })); + assert_eq!( + too_large.resource_limit_kind(), + Some(ResourceLimitKind::RepresentationBytes) + ); let nested = br#"{"path":[[["/"]]]}"#; let nesting_limited = Limits::new(1024, 1024, 2, 1).unwrap(); - assert!(decode_fetch_request(nested, nesting_limited).is_err()); + let too_deep = decode_fetch_request(nested, nesting_limited).unwrap_err(); + assert!(matches!(&too_deep, CodecError::NestingTooDeep { .. })); + assert_eq!( + too_deep.resource_limit_kind(), + Some(ResourceLimitKind::Logical) + ); let outbound_limited = Limits::new(1_000_000, 1_000_000, 1, 10).unwrap(); assert!( diff --git a/crates/wip-https/tests/responses.rs b/crates/wip-https/tests/responses.rs index 7456312..1ee4ad1 100644 --- a/crates/wip-https/tests/responses.rs +++ b/crates/wip-https/tests/responses.rs @@ -3,9 +3,10 @@ use wip_https::wip_protocol::{ FetchRequest, ProtocolError, ProtocolErrorCode, ProtocolInteraction, }; use wip_https::{ - ClientResponseError, DecodedResponse, InvalidResponseKind, Limits, TransportFailureKind, - decode_fetch_response, encode_protocol_error_response, error_code_name, is_json_content_type, - status_for_error, + ClientResponseError, DecodedResponse, InvalidResponseKind, Limits, ResourceLimitKind, + TransportFailureKind, decode_fetch_response, encode_protocol_error_response, + encode_resource_limit_error_response, error_code_name, is_json_content_type, status_for_error, + status_for_resource_limit, }; fn limits() -> Limits { @@ -95,7 +96,7 @@ fn every_protocol_code_has_stable_name_and_status() { ( ProtocolErrorCode::ResourceLimitExceeded, "resource_limit_exceeded", - StatusCode::TOO_MANY_REQUESTS, + StatusCode::UNPROCESSABLE_ENTITY, ), ( ProtocolErrorCode::Internal, @@ -115,6 +116,73 @@ fn every_protocol_code_has_stable_name_and_status() { } } +#[test] +fn resource_limit_statuses_encode_and_decode_by_concrete_limit_class() { + let error = ProtocolError { + code: ProtocolErrorCode::ResourceLimitExceeded, + message: "bounded".into(), + }; + let cases = [ + ( + ResourceLimitKind::RepresentationBytes, + StatusCode::PAYLOAD_TOO_LARGE, + ), + (ResourceLimitKind::Logical, StatusCode::UNPROCESSABLE_ENTITY), + ]; + for (kind, status) in cases { + assert_eq!(status_for_resource_limit(kind), status); + let encoded = encode_resource_limit_error_response( + ProtocolInteraction::Fetch, + &error, + kind, + limits(), + ) + .unwrap(); + assert_eq!(encoded.status(), status); + assert_eq!( + decode_fetch_response(&request(), &encoded, limits()).unwrap(), + DecodedResponse::ProtocolFailure { + error: error.clone(), + status_mismatch: None, + } + ); + } + + let default = + encode_protocol_error_response(ProtocolInteraction::Fetch, &error, limits()).unwrap(); + assert_eq!(default.status(), StatusCode::UNPROCESSABLE_ENTITY); + assert_ne!(default.status(), StatusCode::TOO_MANY_REQUESTS); +} + +#[test] +fn rate_limit_status_without_protocol_envelope_stays_transport_failure() { + let response = Response::builder() + .status(StatusCode::TOO_MANY_REQUESTS) + .body(b"rate limited".to_vec()) + .unwrap(); + assert!(matches!( + decode_fetch_response(&request(), &response, limits()).unwrap_err(), + ClientResponseError::TransportBinding(failure) + if failure.status == StatusCode::TOO_MANY_REQUESTS + && failure.kind == TransportFailureKind::HttpStatus + )); +} + +#[test] +fn valid_success_body_requires_exactly_http_200() { + let success_body = br#"{"name":"","interfaces":[]}"#; + for status in [StatusCode::CREATED, StatusCode::ACCEPTED] { + assert!(matches!( + decode_fetch_response(&request(), &response(status, success_body), limits()) + .unwrap_err(), + ClientResponseError::InvalidResponse { + kind: InvalidResponseKind::NonCanonicalSuccessStatus, + .. + } + )); + } +} + #[test] fn server_encoder_rejects_codes_disallowed_for_the_interaction() { let error = ProtocolError { diff --git a/crates/wip-https/tests/routes.rs b/crates/wip-https/tests/routes.rs index ce79b5b..aa2af1a 100644 --- a/crates/wip-https/tests/routes.rs +++ b/crates/wip-https/tests/routes.rs @@ -1,5 +1,5 @@ -use wip_https::http::{Method, Request}; -use wip_https::{Endpoint, Route, validate_http_request}; +use wip_https::http::{Method, Request, StatusCode}; +use wip_https::{Endpoint, RequestMetadataError, Route, validate_http_request}; #[test] fn endpoint_is_https_absolute_queryless_and_canonical() { @@ -72,6 +72,29 @@ fn inbound_http_metadata_enforces_post_route_and_json() { .body(Vec::::new()) .unwrap(); assert!(validate_http_request(&endpoint, Route::Fetch, &wrong_route).is_err()); + + let incompatible_accept = Request::builder() + .method(Method::POST) + .uri("/wip/v1/fetch") + .header("content-type", "application/json") + .header("accept", "text/plain, */*;q=1, application/json;q=0") + .body(Vec::::new()) + .unwrap(); + let error = validate_http_request(&endpoint, Route::Fetch, &incompatible_accept).unwrap_err(); + assert!(matches!(&error, RequestMetadataError::NotAcceptable)); + assert_eq!(error.status(), StatusCode::NOT_ACCEPTABLE); + + let unsupported_media_type = Request::builder() + .method(Method::POST) + .uri("/wip/v1/fetch") + .header("content-type", "text/plain") + .header("accept", "application/json") + .body(Vec::::new()) + .unwrap(); + let error = + validate_http_request(&endpoint, Route::Fetch, &unsupported_media_type).unwrap_err(); + assert!(matches!(&error, RequestMetadataError::UnsupportedMediaType)); + assert_eq!(error.status(), StatusCode::UNSUPPORTED_MEDIA_TYPE); } #[test]