diff --git a/crates/wip-client/tests/adapter.rs b/crates/wip-client/tests/adapter.rs index 3875bd1..aec63c5 100644 --- a/crates/wip-client/tests/adapter.rs +++ b/crates/wip-client/tests/adapter.rs @@ -145,16 +145,37 @@ fn valid_error_envelope_is_authoritative_and_status_mismatch_is_diagnostic() { } #[test] -fn non_success_without_valid_envelope_remains_transport_failure() { +fn resource_envelope_on_413_uses_code_and_records_canonical_422_mismatch() { let (mut client, session) = setup(); let request = client.refresh_observed(&session, "/item", 0).unwrap(); - let error = client - .complete( - request.id, - response(StatusCode::BAD_GATEWAY, b"gateway html"), - ) - .unwrap_err(); - assert_eq!(error.kind, ClientErrorKind::Transport); + let error = ProtocolError { + code: ProtocolErrorCode::ResourceLimitExceeded, + message: "logical observation limit exceeded".into(), + }; + let mut encoded = + encode_protocol_error_response(ProtocolInteraction::Observe, &error, limits()).unwrap(); + *encoded.status_mut() = StatusCode::PAYLOAD_TOO_LARGE; + + assert_eq!( + client.complete(request.id, encoded).unwrap(), + Completion::ProtocolFailure(error) + ); + assert_eq!( + client.diagnostics(&session).unwrap().back().unwrap().kind, + DiagnosticKind::HttpStatusMismatch + ); +} + +#[test] +fn non_success_without_valid_envelope_remains_transport_failure() { + for status in [StatusCode::BAD_GATEWAY, StatusCode::PAYLOAD_TOO_LARGE] { + let (mut client, session) = setup(); + let request = client.refresh_observed(&session, "/item", 0).unwrap(); + let error = client + .complete(request.id, response(status, b"infrastructure failure")) + .unwrap_err(); + assert_eq!(error.kind, ClientErrorKind::Transport); + } } #[test] @@ -250,3 +271,80 @@ fn malformed_call_success_after_dispatch_is_outcome_unknown_and_stales_object() ); assert_eq!(client.in_flight_len(), 0); } + +#[test] +fn plain_413_after_call_dispatch_is_unknown_transport_outcome() { + let (mut client, session) = setup(); + let observation = client.refresh_observed(&session, "/item", 0).unwrap(); + client + .complete( + observation.id, + encode_observe_response( + &ObserveRequest { + path: "/item".into(), + depth: 0, + }, + &ObjectObservation { + object: object(), + children: None, + }, + limits(), + ) + .unwrap(), + ) + .unwrap(); + let interface = client.prepare_interface(&session, "math").unwrap(); + client + .complete( + interface.id, + encode_fetch_interface_response( + &FetchInterfaceRequest { + interface: "math".into(), + }, + &FetchInterfaceResponse { + interface: "math".into(), + descriptor: descriptor(INTERFACE_FORMAT_V1), + validator: None, + }, + limits(), + ) + .unwrap(), + ) + .unwrap(); + let call = client + .prepare_call( + &session, + "/item", + "math", + "calculate", + BTreeMap::from([("value".into(), Value::Integer(2))]), + ) + .unwrap(); + client.mark_dispatched(call.id).unwrap(); + + let completion = client + .complete( + call.id, + Response::builder() + .status(StatusCode::PAYLOAD_TOO_LARGE) + .body(Vec::::new()) + .unwrap(), + ) + .unwrap(); + assert!(matches!( + completion, + Completion::Call(record) + if matches!( + &record.outcome, + CallOutcome::Unknown { + reason: OutcomeUnknownReason::ResponseDecode, + failure: Some(failure), + } if failure.kind == ClientErrorKind::Transport + ) + )); + assert_eq!( + client.object(&session, "/item").unwrap().state, + ObservationState::Stale + ); + assert_eq!(client.in_flight_len(), 0); +} diff --git a/crates/wip-fs-server/src/server.rs b/crates/wip-fs-server/src/server.rs index 7ec5419..d8ea4c4 100644 --- a/crates/wip-fs-server/src/server.rs +++ b/crates/wip-fs-server/src/server.rs @@ -5,15 +5,15 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::thread; use std::time::{Duration, Instant}; +use http::header::ALLOW; use http::{Request, Response, StatusCode}; use wip_host::{ExecutionContext, ObjectProvider}; use wip_http::{ BatchRequestKind, BatchResponse, BatchResponseItem, BatchSuccess, CodecError, EncodeHttpError, - Endpoint, RequestMetadataError, ResourceLimitKind, Route, decode_batch_request, - decode_call_operation_metadata, decode_fetch_interface_request, decode_observe_request, - encode_batch_error_response, encode_batch_resource_limit_error_response, encode_batch_response, - encode_call_operation_response, encode_fetch_interface_response, encode_observe_response, - encode_protocol_error_response, encode_resource_limit_error_response, validate_http_request, + Endpoint, RequestMetadataError, Route, decode_batch_request, decode_call_operation_metadata, + decode_fetch_interface_request, decode_observe_request, encode_batch_error_response, + encode_batch_response, encode_call_operation_response, encode_fetch_interface_response, + encode_observe_response, encode_protocol_error_response, validate_http_request, }; use wip_protocol::{ProtocolError, ProtocolErrorCode, ProtocolInteraction}; @@ -68,20 +68,25 @@ impl WipFsServer { request: Request>, deadline: Instant, ) -> Response> { - let Some(route) = self.route(request.uri().path()) else { + let Some(route) = self.route(request.uri()) else { return empty_response(StatusCode::NOT_FOUND); }; if let Err(error) = validate_http_request(&self.endpoint, route, &request) { - return self.metadata_error(route, error); + return self.metadata_error(error); } let context = ExecutionContext::new() .with_deadline(deadline) .with_resource_budget(u64::from(self.config.limits.max_observation_nodes)); - match route { + let response = match route { Route::Observe => self.observe(request.body(), &context), Route::FetchInterface => self.fetch_interface(request.body()), Route::CallOperation => self.call_operation(request.body(), &context), Route::Batch => self.batch(request.body(), &context), + }; + if Instant::now() >= deadline { + empty_response(StatusCode::GATEWAY_TIMEOUT) + } else { + response } } @@ -106,7 +111,10 @@ impl WipFsServer { }) } - fn route(&self, path: &str) -> Option { + fn route(&self, uri: &http::Uri) -> Option { + if uri.query().is_some() { + return None; + } [ Route::Observe, Route::FetchInterface, @@ -114,7 +122,7 @@ impl WipFsServer { Route::Batch, ] .into_iter() - .find(|route| self.endpoint.route(*route).path() == path) + .find(|route| self.endpoint.route(*route).path() == uri.path()) } fn observe(&self, body: &[u8], context: &ExecutionContext) -> Response> { @@ -127,7 +135,7 @@ impl WipFsServer { .unwrap_or_else(|error| { self.read_encode_error(ProtocolInteraction::Observe, error) }), - Err(error) => self.protocol_error(ProtocolInteraction::Observe, &error, None), + Err(error) => self.protocol_error(ProtocolInteraction::Observe, &error), } } @@ -143,7 +151,7 @@ impl WipFsServer { self.read_encode_error(ProtocolInteraction::FetchInterface, error) }) } - Err(error) => self.protocol_error(ProtocolInteraction::FetchInterface, &error, None), + Err(error) => self.protocol_error(ProtocolInteraction::FetchInterface, &error), } } @@ -181,7 +189,7 @@ impl WipFsServer { }, message: "operation completed without response metadata".into(), }; - return self.protocol_error(ProtocolInteraction::CallOperation, &error, None); + return self.protocol_error(ProtocolInteraction::CallOperation, &error); }; encode_call_operation_response( &request, @@ -190,6 +198,12 @@ impl WipFsServer { self.config.limits.wire, ) .unwrap_or_else(|encode_error| { + if matches!( + &encode_error, + EncodeHttpError::Codec(codec) if is_transport_limit(codec) + ) { + return empty_response(StatusCode::INTERNAL_SERVER_ERROR); + } if !may_commit { return self .read_encode_error(ProtocolInteraction::CallOperation, encode_error); @@ -200,10 +214,10 @@ impl WipFsServer { code: ProtocolErrorCode::OperationOutcomeUnknown, message: "operation completed but its response could not be encoded".into(), }; - self.protocol_error(ProtocolInteraction::CallOperation, &error, None) + self.protocol_error(ProtocolInteraction::CallOperation, &error) }) } - Err(error) => self.protocol_error(ProtocolInteraction::CallOperation, &error, None), + Err(error) => self.protocol_error(ProtocolInteraction::CallOperation, &error), } } @@ -236,22 +250,20 @@ impl WipFsServer { let response = BatchResponse { responses }; encode_batch_response(&request, &response, self.config.limits.wire).unwrap_or_else( |error| match error { - EncodeHttpError::Codec(codec) => self.batch_codec_error(codec), - EncodeHttpError::Http(_) => self.batch_top_error(&internal_error(), None), + EncodeHttpError::Codec(codec) => self.batch_encode_error(&codec), + EncodeHttpError::Http(_) => self.batch_top_error(&internal_error()), }, ) } - fn metadata_error(&self, route: Route, error: RequestMetadataError) -> Response> { + fn metadata_error(&self, error: RequestMetadataError) -> Response> { match error { - RequestMetadataError::InvalidRequest(_) => self.protocol_error( - interaction(route), - &ProtocolError { - code: ProtocolErrorCode::InvalidRequest, - message: "invalid HTTP request metadata".into(), - }, - None, - ), + RequestMetadataError::NotFound => empty_response(StatusCode::NOT_FOUND), + RequestMetadataError::MethodNotAllowed => Response::builder() + .status(StatusCode::METHOD_NOT_ALLOWED) + .header(ALLOW, "POST") + .body(Vec::new()) + .expect("static method response is valid"), RequestMetadataError::UnsupportedMediaType => { empty_response(StatusCode::UNSUPPORTED_MEDIA_TYPE) } @@ -264,8 +276,8 @@ impl WipFsServer { interaction: ProtocolInteraction, error: CodecError, ) -> Response> { - if let Some(kind) = error.resource_limit_kind() { - self.protocol_error(interaction, &resource_error(), Some(kind)) + if is_transport_limit(&error) { + empty_response(StatusCode::PAYLOAD_TOO_LARGE) } else { self.protocol_error( interaction, @@ -273,22 +285,18 @@ impl WipFsServer { code: ProtocolErrorCode::InvalidRequest, message: "malformed WIP request".into(), }, - None, ) } } fn batch_codec_error(&self, error: CodecError) -> Response> { - if let Some(kind) = error.resource_limit_kind() { - self.batch_top_error(&resource_error(), Some(kind)) + if is_transport_limit(&error) { + empty_response(StatusCode::PAYLOAD_TOO_LARGE) } else { - self.batch_top_error( - &ProtocolError { - code: ProtocolErrorCode::InvalidRequest, - message: "malformed WIP batch request".into(), - }, - None, - ) + self.batch_top_error(&ProtocolError { + code: ProtocolErrorCode::InvalidRequest, + message: "malformed WIP batch request".into(), + }) } } @@ -298,10 +306,18 @@ impl WipFsServer { error: EncodeHttpError, ) -> Response> { match error { - EncodeHttpError::Codec(codec) if codec.resource_limit_kind().is_some() => { - self.protocol_error(interaction, &resource_error(), codec.resource_limit_kind()) + EncodeHttpError::Codec(codec) if is_transport_limit(&codec) => { + empty_response(StatusCode::INTERNAL_SERVER_ERROR) } - _ => self.protocol_error(interaction, &internal_error(), None), + _ => self.protocol_error(interaction, &internal_error()), + } + } + + fn batch_encode_error(&self, error: &CodecError) -> Response> { + if is_transport_limit(error) { + empty_response(StatusCode::INTERNAL_SERVER_ERROR) + } else { + self.batch_top_error(&internal_error()) } } @@ -309,32 +325,14 @@ impl WipFsServer { &self, interaction: ProtocolInteraction, error: &ProtocolError, - resource_kind: Option, ) -> Response> { - let encoded = match resource_kind { - Some(kind) => encode_resource_limit_error_response( - interaction, - error, - kind, - self.config.limits.wire, - ), - None => encode_protocol_error_response(interaction, error, self.config.limits.wire), - }; - encoded.unwrap_or_else(|_| empty_response(StatusCode::INTERNAL_SERVER_ERROR)) + encode_protocol_error_response(interaction, error, self.config.limits.wire) + .unwrap_or_else(|_| empty_response(StatusCode::INTERNAL_SERVER_ERROR)) } - fn batch_top_error( - &self, - error: &ProtocolError, - resource_kind: Option, - ) -> Response> { - let encoded = match resource_kind { - Some(kind) => { - encode_batch_resource_limit_error_response(error, kind, self.config.limits.wire) - } - None => encode_batch_error_response(error, self.config.limits.wire), - }; - encoded.unwrap_or_else(|_| empty_response(StatusCode::INTERNAL_SERVER_ERROR)) + fn batch_top_error(&self, error: &ProtocolError) -> Response> { + encode_batch_error_response(error, self.config.limits.wire) + .unwrap_or_else(|_| empty_response(StatusCode::INTERNAL_SERVER_ERROR)) } } @@ -404,7 +402,7 @@ struct ParsedRequestHead { enum IntakeError { Timeout, HeaderTooLarge, - PayloadTooLarge { target: String }, + PayloadTooLarge, BadRequest, Socket, } @@ -425,9 +423,7 @@ fn serve_connection(service: &WipFsServer, mut stream: TcpStream) { Err(IntakeError::HeaderTooLarge) => { Some(empty_response(StatusCode::REQUEST_HEADER_FIELDS_TOO_LARGE)) } - Err(IntakeError::PayloadTooLarge { target }) => { - Some(payload_too_large_response(service, &target)) - } + Err(IntakeError::PayloadTooLarge) => Some(empty_response(StatusCode::PAYLOAD_TOO_LARGE)), Err(IntakeError::BadRequest) => Some(empty_response(StatusCode::BAD_REQUEST)), Err(IntakeError::Timeout | IntakeError::Socket) => None, }; @@ -466,9 +462,7 @@ fn read_http_request( let parsed = parse_request_head(&received[..header_end])?; if parsed.content_length > maximum_body { - return Err(IntakeError::PayloadTooLarge { - target: parsed.target, - }); + return Err(IntakeError::PayloadTooLarge); } let mut body = Vec::with_capacity(parsed.content_length); let buffered = &received[header_end..]; @@ -567,27 +561,6 @@ fn read_with_deadline( }) } -fn payload_too_large_response(service: &WipFsServer, target: &str) -> Response> { - let route = service.route(target.split('?').next().unwrap_or_default()); - route.map_or_else( - || empty_response(StatusCode::PAYLOAD_TOO_LARGE), - |route| { - if route == Route::Batch { - service.batch_top_error( - &resource_error(), - Some(ResourceLimitKind::RepresentationBytes), - ) - } else { - service.protocol_error( - interaction(route), - &resource_error(), - Some(ResourceLimitKind::RepresentationBytes), - ) - } - }, - ) -} - fn write_http_response( stream: &mut TcpStream, response: &Response>, @@ -648,20 +621,13 @@ fn write_all_with_deadline( stream.flush() } -fn interaction(route: Route) -> ProtocolInteraction { - match route { - Route::Observe => ProtocolInteraction::Observe, - Route::FetchInterface => ProtocolInteraction::FetchInterface, - Route::CallOperation => ProtocolInteraction::CallOperation, - Route::Batch => ProtocolInteraction::Observe, - } -} - -fn resource_error() -> ProtocolError { - ProtocolError { - code: ProtocolErrorCode::ResourceLimitExceeded, - message: "configured resource limit exceeded".into(), - } +fn is_transport_limit(error: &CodecError) -> bool { + matches!( + error, + CodecError::BodyTooLarge { .. } + | CodecError::NestingTooDeep { .. } + | CodecError::TooManyBatchItems { .. } + ) } fn internal_error() -> ProtocolError { diff --git a/crates/wip-fs-server/tests/end_to_end.rs b/crates/wip-fs-server/tests/end_to_end.rs index d210f2f..d95f557 100644 --- a/crates/wip-fs-server/tests/end_to_end.rs +++ b/crates/wip-fs-server/tests/end_to_end.rs @@ -9,8 +9,9 @@ use std::time::{Duration, Instant}; use tempfile::tempdir; use wip_fs_server::{AccessMode, ServerConfig, WipFsServer}; use wip_http::{ - BatchRequest, BatchRequestItem, BatchRequestKind, BatchResponse, BatchSuccess, DecodedResponse, - Endpoint, JSON_CONTENT_TYPE, Limits, Route, decode_batch_request, decode_batch_response, + BatchRequest, BatchRequestItem, BatchRequestKind, BatchResponse, BatchSuccess, + ClientResponseError, DecodedResponse, Endpoint, JSON_CONTENT_TYPE, Limits, Route, + TransportFailureKind, decode_batch_request, decode_batch_response, decode_call_operation_response, decode_fetch_interface_response, decode_observe_response, encode_batch_request, encode_call_operation_request, encode_fetch_interface_request, encode_observe_request, @@ -210,6 +211,166 @@ fn real_listener_serves_three_core_interactions_batch_and_write() { .expect("listener exits cleanly"); } +#[test] +fn real_listener_keeps_route_method_and_malformed_body_boundaries_distinct() { + let temporary = tempdir().expect("temporary root"); + let mut config = ServerConfig::for_root(temporary.path()); + config.limits.wire = Limits::new(64, 4_096, 64, 4).expect("wire limits"); + let bound = Arc::new( + WipFsServer::new(config) + .expect("build server") + .bind() + .expect("bind"), + ); + let address = bound.local_addr(); + let shutdown = Arc::new(AtomicBool::new(false)); + let worker = { + let bound = Arc::clone(&bound); + let shutdown = Arc::clone(&shutdown); + thread::spawn(move || bound.serve_until(shutdown)) + }; + + for uri in [ + "/wip/v1/unknown", + "/wip/v1/observe?unexpected=true", + "/wip/v1/observe/extra", + ] { + let response = exchange(address, json_request(http::Method::POST, uri, b"{}")); + assert_eq!(response.status(), http::StatusCode::NOT_FOUND); + assert!(response.body().is_empty()); + assert!(!response.headers().contains_key(http::header::CONTENT_TYPE)); + } + + let wrong_method = exchange( + address, + json_request(http::Method::GET, "/wip/v1/observe", b""), + ); + assert_eq!(wrong_method.status(), http::StatusCode::METHOD_NOT_ALLOWED); + assert_eq!(wrong_method.headers()[http::header::ALLOW], "POST"); + assert!(wrong_method.body().is_empty()); + assert!( + !wrong_method + .headers() + .contains_key(http::header::CONTENT_TYPE) + ); + + let mut unsupported_coding = json_request( + http::Method::POST, + "/wip/v1/observe", + br#"{"path":"/","depth":0}"#, + ); + unsupported_coding + .headers_mut() + .insert(http::header::CONTENT_ENCODING, "gzip".parse().unwrap()); + let unsupported_coding = exchange(address, unsupported_coding); + assert_eq!( + unsupported_coding.status(), + http::StatusCode::UNSUPPORTED_MEDIA_TYPE + ); + assert!(unsupported_coding.body().is_empty()); + assert!( + !unsupported_coding + .headers() + .contains_key(http::header::CONTENT_TYPE) + ); + + let malformed = exchange( + address, + json_request(http::Method::POST, "/wip/v1/observe", b"{"), + ); + assert_eq!(malformed.status(), http::StatusCode::BAD_REQUEST); + assert_eq!( + malformed.headers()[http::header::CONTENT_TYPE], + JSON_CONTENT_TYPE + ); + assert!( + std::str::from_utf8(malformed.body()) + .expect("UTF-8 error envelope") + .contains("\"invalid_request\"") + ); + + let oversized = exchange( + address, + json_request(http::Method::POST, "/wip/v1/observe", &[b'x'; 65]), + ); + assert_eq!(oversized.status(), http::StatusCode::PAYLOAD_TOO_LARGE); + assert!(oversized.body().is_empty()); + assert!(!oversized.headers().contains_key(http::header::CONTENT_TYPE)); + + let mut header_only = TcpStream::connect(address).expect("connect header-only client"); + header_only + .set_read_timeout(Some(Duration::from_secs(1))) + .expect("set header-only timeout"); + write!( + header_only, + "POST /wip/v1/observe HTTP/1.1\r\nHost: localhost\r\nContent-Type: application/json\r\nContent-Length: 65\r\n\r\n" + ) + .expect("write oversized request head"); + header_only.flush().expect("flush oversized request head"); + let mut raw = Vec::new(); + header_only + .read_to_end(&mut raw) + .expect("read immediate limit response"); + assert!(raw.starts_with(b"HTTP/1.1 413 ")); + assert!(raw.ends_with(b"\r\n\r\n")); + assert!( + !raw.windows(b"resource_limit_exceeded".len()) + .any(|window| { window == b"resource_limit_exceeded" }) + ); + + shutdown.store(true, Ordering::Release); + worker + .join() + .expect("join listener") + .expect("listener exits cleanly"); +} + +#[test] +fn raw_decoder_and_batch_packaging_limits_are_transport_failures() { + let temporary = tempdir().expect("temporary root"); + let mut config = ServerConfig::for_root(temporary.path()); + config.limits.wire = Limits::new(4_096, 4_096, 8, 1).expect("wire limits"); + let server = WipFsServer::new(config).expect("build server"); + + let nested = br#"{"path":[[[[[[[[["/"]]]]]]]]],"depth":0}"#; + let response = server.handle_http(json_request(http::Method::POST, "/wip/v1/observe", nested)); + assert_eq!(response.status(), http::StatusCode::PAYLOAD_TOO_LARGE); + assert!(response.body().is_empty()); + assert!(!response.headers().contains_key(http::header::CONTENT_TYPE)); + + let batch = br#"{"requests":[{"id":"one","method":"observe","arguments":{"path":"/","depth":0}},{"id":"two","method":"observe","arguments":{"path":"/","depth":0}}]}"#; + let response = server.handle_http(json_request(http::Method::POST, "/wip/v1/batch", batch)); + assert_eq!(response.status(), http::StatusCode::PAYLOAD_TOO_LARGE); + assert!(response.body().is_empty()); + assert!(!response.headers().contains_key(http::header::CONTENT_TYPE)); +} + +#[test] +fn logical_observation_limit_remains_protocol_resource_failure() { + let temporary = tempdir().expect("temporary root"); + let mut config = ServerConfig::for_root(temporary.path()); + config.limits.max_observation_depth = 0; + let limits = config.limits.wire; + let server = WipFsServer::new(config).expect("build server"); + let endpoint = Endpoint::parse("http://localhost/wip").expect("endpoint"); + let request = ObserveRequest { + path: "/".into(), + depth: 1, + }; + + let response = server.handle_http( + encode_observe_request(&endpoint, &request, limits).expect("encode observation"), + ); + assert_eq!(response.status(), http::StatusCode::UNPROCESSABLE_ENTITY); + assert!(matches!( + decode_observe_response(&request, &response, limits).expect("decode logical limit"), + DecodedResponse::ProtocolFailure { + error, + status_mismatch: None, + } if error.code == ProtocolErrorCode::ResourceLimitExceeded + )); +} + #[test] fn stalled_request_body_times_out_and_releases_the_only_worker() { let temporary = tempdir().expect("temporary root"); @@ -359,7 +520,7 @@ fn backend_invalid_unicode_path_is_not_found_not_internal() { } #[test] -fn read_response_size_failure_preserves_non_commit_guarantee() { +fn read_response_size_failure_is_transport_local() { let temporary = tempdir().expect("temporary root"); std::fs::write(temporary.path().join("large"), vec![b'x'; 2_000]).expect("write file"); let mut config = ServerConfig::for_root(temporary.path()); @@ -406,15 +567,16 @@ fn read_response_size_failure_preserves_non_commit_guarantee() { encode_call_operation_request(&endpoint, &call, &interface.descriptor, limits) .expect("encode read call"), ); - assert_eq!(response.status(), http::StatusCode::PAYLOAD_TOO_LARGE); - match decode_call_operation_response(&call, &interface.descriptor, &response, limits) - .expect("decode resource failure") - { - DecodedResponse::ProtocolFailure { error, .. } => { - assert_eq!(error.code, ProtocolErrorCode::ResourceLimitExceeded); - } - DecodedResponse::Success(_) => panic!("oversized read response must fail"), - } + assert_eq!(response.status(), http::StatusCode::INTERNAL_SERVER_ERROR); + assert!(response.body().is_empty()); + assert!(!response.headers().contains_key(http::header::CONTENT_TYPE)); + assert!(matches!( + decode_call_operation_response(&call, &interface.descriptor, &response, limits) + .expect_err("oversized representation is not a protocol failure"), + ClientResponseError::TransportBinding(failure) + if failure.status == http::StatusCode::INTERNAL_SERVER_ERROR + && failure.kind == TransportFailureKind::HttpStatus + )); } fn assert_batch_successes(batch: &BatchResponse) { @@ -440,6 +602,16 @@ fn success(response: DecodedResponse) -> T { } } +fn json_request(method: http::Method, uri: &str, body: &[u8]) -> http::Request> { + http::Request::builder() + .method(method) + .uri(uri) + .header(http::header::CONTENT_TYPE, JSON_CONTENT_TYPE) + .header(http::header::ACCEPT, "application/json") + .body(body.to_vec()) + .expect("build JSON request") +} + fn exchange(address: SocketAddr, request: http::Request>) -> http::Response> { let mut stream = TcpStream::connect(address).expect("connect listener"); stream diff --git a/crates/wip-http/src/batch.rs b/crates/wip-http/src/batch.rs index c6f2c86..85956f0 100644 --- a/crates/wip-http/src/batch.rs +++ b/crates/wip-http/src/batch.rs @@ -11,7 +11,8 @@ use wip_protocol::{ use crate::response::{self, DecodedResponse}; use crate::{ BodyKind, ClientResponseError, CodecError, EncodeHttpError, Endpoint, InvalidResponseKind, - JSON_CONTENT_TYPE, Limits, ResourceLimitKind, Route, is_json_content_type, json, + JSON_CONTENT_TYPE, Limits, Route, codec::has_supported_content_coding, is_json_content_type, + json, }; /// A retrieval method accepted by the standard v1 batch endpoint. @@ -335,20 +336,16 @@ 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. A -/// resource-limit failure defaults to the logical-limit status; use -/// [`encode_batch_resource_limit_error_response`] when the concrete class is -/// known. +/// Item-local failures belong in [`BatchResponseItem`]. Only `invalid_request` +/// and `internal` may reject the complete batch as protocol failures; packaging +/// and representation limits remain transport-local. pub fn encode_batch_error_response( error: &ProtocolError, limits: Limits, ) -> Result>, EncodeHttpError> { if !matches!( error.code, - ProtocolErrorCode::InvalidRequest - | ProtocolErrorCode::ResourceLimitExceeded - | ProtocolErrorCode::Internal + ProtocolErrorCode::InvalidRequest | ProtocolErrorCode::Internal ) { return Err(CodecError::InvalidField { field: "error.code".into(), @@ -363,26 +360,6 @@ 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, @@ -466,6 +443,9 @@ pub fn decode_batch_response>( value: &Response, limits: Limits, ) -> Result, ClientResponseError> { + if !has_supported_content_coding(value.headers()) { + return Err(response::unsupported_content_coding(value.status())); + } let valid_content_type = value .headers() .get(CONTENT_TYPE) @@ -496,9 +476,7 @@ pub fn decode_batch_response>( if let DecodedResponse::ProtocolFailure { error, .. } = &decoded && !matches!( error.code, - ProtocolErrorCode::InvalidRequest - | ProtocolErrorCode::ResourceLimitExceeded - | ProtocolErrorCode::Internal + ProtocolErrorCode::InvalidRequest | ProtocolErrorCode::Internal ) { return Err(ClientResponseError::invalid( diff --git a/crates/wip-http/src/codec.rs b/crates/wip-http/src/codec.rs index 00ab63d..354c220 100644 --- a/crates/wip-http/src/codec.rs +++ b/crates/wip-http/src/codec.rs @@ -1,7 +1,7 @@ use std::error::Error; use std::fmt::{self, Display, Formatter}; -use http::header::{ACCEPT, CONTENT_TYPE}; +use http::header::{ACCEPT, CONTENT_ENCODING, CONTENT_TYPE}; use http::{HeaderMap, Method, Request, Response, StatusCode}; use wip_protocol::{ CallOperationRequest, CallOperationResponse, FetchInterfaceRequest, FetchInterfaceResponse, @@ -12,7 +12,7 @@ use wip_protocol::{ use crate::response::{self, DecodedResponse}; use crate::{ BodyKind, ClientResponseError, CodecError, Endpoint, InvalidResponseKind, JSON_CONTENT_TYPE, - Limits, ResourceLimitKind, Route, json, + Limits, Route, json, }; fn request( @@ -36,6 +36,9 @@ fn response(status: StatusCode, body: Vec) -> Result>, http } fn require_response_content_type(response: &Response) -> Result<(), ClientResponseError> { + if !has_supported_content_coding(response.headers()) { + return Err(response::unsupported_content_coding(response.status())); + } let valid = response .headers() .get(CONTENT_TYPE) @@ -181,9 +184,11 @@ fn accepts_json(headers: &HeaderMap) -> bool { /// 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`. + /// The request URI does not exactly identify the selected standard route. + NotFound, + /// A known standard route was requested with a method other than `POST`. + MethodNotAllowed, + /// The request body does not use UTF-8 `application/json` or identity coding. UnsupportedMediaType, /// The request's `Accept` metadata excludes `application/json`. NotAcceptable, @@ -194,32 +199,47 @@ impl RequestMetadataError { #[must_use] pub const fn status(&self) -> StatusCode { match self { - Self::InvalidRequest(_) => StatusCode::BAD_REQUEST, + Self::NotFound => StatusCode::NOT_FOUND, + Self::MethodNotAllowed => StatusCode::METHOD_NOT_ALLOWED, Self::UnsupportedMediaType => StatusCode::UNSUPPORTED_MEDIA_TYPE, Self::NotAcceptable => StatusCode::NOT_ACCEPTABLE, } } + + /// Returns the value for an `Allow` response header, when required. + #[must_use] + pub const fn allow(&self) -> Option<&'static str> { + match self { + Self::MethodNotAllowed => Some("POST"), + Self::NotFound | Self::UnsupportedMediaType | Self::NotAcceptable => None, + } + } } impl Display for RequestMetadataError { fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result { match self { - Self::InvalidRequest(error) => error.fmt(formatter), + Self::NotFound => formatter.write_str("request URI is not a standard WIP route"), + Self::MethodNotAllowed => formatter.write_str("standard WIP routes require POST"), Self::UnsupportedMediaType => { - formatter.write_str("expected UTF-8 application/json Content-Type") + formatter.write_str("expected identity-coded 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, - } - } +impl Error for RequestMetadataError {} + +pub(crate) fn has_supported_content_coding(headers: &HeaderMap) -> bool { + headers.get_all(CONTENT_ENCODING).iter().all(|value| { + value.to_str().is_ok_and(|value| { + let mut codings = value.split(',').map(str::trim); + let first = codings.next(); + first.is_some_and(|coding| coding.eq_ignore_ascii_case("identity")) + && codings.all(|coding| coding.eq_ignore_ascii_case("identity")) + }) + }) } /// Validates mandatory HTTP metadata for an inbound WIP request. @@ -233,29 +253,19 @@ pub fn validate_http_request( route: Route, value: &Request, ) -> Result<(), RequestMetadataError> { - if value.method() != Method::POST { - 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(RequestMetadataError::InvalidRequest( - CodecError::InvalidField { - field: "route".into(), - reason: "request URI does not match the selected standard route".into(), - }, - )); + return Err(RequestMetadataError::NotFound); + } + if value.method() != Method::POST { + return Err(RequestMetadataError::MethodNotAllowed); } let valid_content_type = value .headers() .get(CONTENT_TYPE) .and_then(|value| value.to_str().ok()) .is_some_and(is_json_content_type); - if !valid_content_type { + if !valid_content_type || !has_supported_content_coding(value.headers()) { return Err(RequestMetadataError::UnsupportedMediaType); } if !accepts_json(value.headers()) { @@ -526,9 +536,8 @@ fn encode_protocol_error_with_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`]. +/// error envelopes. Logical `resource_limit_exceeded` failures use HTTP 422; +/// representation and deployment limits are not protocol errors. pub fn encode_protocol_error_response( interaction: ProtocolInteraction, error: &ProtocolError, @@ -542,28 +551,6 @@ pub fn encode_protocol_error_response( ) } -/// 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()); - } - encode_protocol_error_with_status( - interaction, - error, - response::status_for_resource_limit(kind), - limits, - ) -} - /// Failure while building an outbound framework-neutral HTTP message. #[derive(Debug)] pub enum EncodeHttpError { diff --git a/crates/wip-http/src/error.rs b/crates/wip-http/src/error.rs index 30d5b94..5b2ddff 100644 --- a/crates/wip-http/src/error.rs +++ b/crates/wip-http/src/error.rs @@ -102,15 +102,6 @@ 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 { @@ -186,20 +177,6 @@ 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-http/src/lib.rs b/crates/wip-http/src/lib.rs index 7d8827f..1e3cb70 100644 --- a/crates/wip-http/src/lib.rs +++ b/crates/wip-http/src/lib.rs @@ -40,8 +40,7 @@ mod response; pub use batch::{ 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, + encode_batch_error_response, encode_batch_request, encode_batch_response, }; pub use codec::{ CallOperationMetadata, EncodeHttpError, RequestMetadataError, decode_call_operation_metadata, @@ -49,15 +48,13 @@ pub use codec::{ decode_fetch_interface_response, decode_observe_request, decode_observe_response, encode_call_operation_request, encode_call_operation_response, encode_fetch_interface_request, encode_fetch_interface_response, encode_observe_request, encode_observe_response, - encode_protocol_error_response, encode_resource_limit_error_response, is_json_content_type, - validate_http_request, + encode_protocol_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, ResourceLimitKind}; +pub use error::{BodyKind, CodecError, LimitConfigurationError, Limits}; 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-http/src/response.rs b/crates/wip-http/src/response.rs index 8d46899..7f9263d 100644 --- a/crates/wip-http/src/response.rs +++ b/crates/wip-http/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, ResourceLimitKind, json}; +use crate::{BodyKind, CodecError, Limits, json}; /// A successfully classified client response. #[derive(Debug, Clone, PartialEq, Eq)] @@ -37,6 +37,8 @@ pub enum TransportFailureKind { AuthenticationRequired, /// HTTP 403, commonly emitted by endpoint policy or infrastructure. EndpointForbidden, + /// The response uses a content coding the client does not support. + UnsupportedContentCoding, /// Another non-success HTTP response without a WIP error envelope. HttpStatus, } @@ -119,10 +121,6 @@ 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 { @@ -145,22 +143,8 @@ pub const fn status_for_error(code: ProtocolErrorCode) -> StatusCode { } } -/// 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), - } + status == status_for_error(code) } /// Returns the stable snake-case wire spelling of a protocol code. @@ -355,6 +339,13 @@ pub(crate) fn decode( } } +pub(crate) fn unsupported_content_coding(status: StatusCode) -> ClientResponseError { + ClientResponseError::TransportBinding(TransportBindingFailure { + status, + kind: TransportFailureKind::UnsupportedContentCoding, + }) +} + pub(crate) fn transport_failure(status: StatusCode) -> ClientResponseError { let kind = match status { StatusCode::UNAUTHORIZED => TransportFailureKind::AuthenticationRequired, diff --git a/crates/wip-http/tests/batch.rs b/crates/wip-http/tests/batch.rs index 609d24c..3fab062 100644 --- a/crates/wip-http/tests/batch.rs +++ b/crates/wip-http/tests/batch.rs @@ -6,9 +6,8 @@ use wip_http::wip_protocol::{ use wip_http::{ 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, + InvalidResponseKind, Limits, TransportFailureKind, decode_batch_request, decode_batch_response, + encode_batch_error_response, encode_batch_request, encode_batch_response, }; fn limits() -> Limits { @@ -200,29 +199,93 @@ fn invalid_batch_envelopes_are_rejected_at_top_level() { } #[test] -fn batch_resource_limits_use_413_or_422_and_never_429() { +fn resource_limit_is_item_local_and_forbidden_as_a_top_level_batch_error() { let error = ProtocolError { code: ProtocolErrorCode::ResourceLimitExceeded, - message: "bounded".into(), + message: "logical observation limit exceeded".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, - } - ); - } + assert!(encode_batch_error_response(&error, limits()).is_err()); + + let request = request(); + let response = BatchResponse { + responses: vec![ + BatchResponseItem { + id: "object".into(), + outcome: Err(error.clone()), + }, + BatchResponseItem { + id: "interface".into(), + outcome: Err(ProtocolError { + code: ProtocolErrorCode::InterfaceNotFound, + message: "gone".into(), + }), + }, + ], + }; + let encoded = encode_batch_response(&request, &response, limits()).unwrap(); + assert_eq!(encoded.status(), StatusCode::OK); + assert_eq!( + decode_batch_response(&request, &encoded, limits()).unwrap(), + DecodedResponse::Success(response) + ); + + let top_level = wip_http::http::Response::builder() + .status(StatusCode::UNPROCESSABLE_ENTITY) + .header("content-type", "application/json; charset=utf-8") + .body(br#"{"error":{"code":"resource_limit_exceeded","message":"not allowed"}}"#.to_vec()) + .unwrap(); + assert!(matches!( + decode_batch_response(&request, &top_level, limits()).unwrap_err(), + ClientResponseError::InvalidResponse { + kind: InvalidResponseKind::InvalidBatch, + .. + } + )); +} + +#[test] +fn batch_transport_failures_do_not_become_protocol_errors() { + let request = request(); + let bare_413 = wip_http::http::Response::builder() + .status(StatusCode::PAYLOAD_TOO_LARGE) + .body(Vec::::new()) + .unwrap(); + assert!(matches!( + decode_batch_response(&request, &bare_413, limits()).unwrap_err(), + ClientResponseError::TransportBinding(failure) + if failure.status == StatusCode::PAYLOAD_TOO_LARGE + && failure.kind == TransportFailureKind::HttpStatus + )); + + let mut encoded = encode_batch_response( + &request, + &BatchResponse { + responses: vec![ + BatchResponseItem { + id: "object".into(), + outcome: Ok(BatchSuccess::Observe(observed_item(None))), + }, + BatchResponseItem { + id: "interface".into(), + outcome: Err(ProtocolError { + code: ProtocolErrorCode::InterfaceNotFound, + message: "gone".into(), + }), + }, + ], + }, + limits(), + ) + .unwrap(); + encoded + .headers_mut() + .insert("content-encoding", "gzip".parse().unwrap()); + assert!(matches!( + decode_batch_response(&request, &encoded, limits()).unwrap_err(), + ClientResponseError::TransportBinding(failure) + if failure.status == StatusCode::OK + && failure.kind == TransportFailureKind::UnsupportedContentCoding + )); } #[test] diff --git a/crates/wip-http/tests/codecs.rs b/crates/wip-http/tests/codecs.rs index 084a0d2..8847ce5 100644 --- a/crates/wip-http/tests/codecs.rs +++ b/crates/wip-http/tests/codecs.rs @@ -10,11 +10,11 @@ use wip_http::wip_protocol::{ }; use wip_http::{ 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_observe_request, decode_observe_response, - encode_call_operation_request, encode_call_operation_response, encode_fetch_interface_request, - encode_fetch_interface_response, encode_observe_request, encode_observe_response, + decode_call_operation_metadata, decode_call_operation_request, decode_call_operation_response, + decode_fetch_interface_request, decode_fetch_interface_response, decode_observe_request, + decode_observe_response, encode_call_operation_request, encode_call_operation_response, + encode_fetch_interface_request, encode_fetch_interface_response, encode_observe_request, + encode_observe_response, }; fn limits() -> Limits { @@ -404,23 +404,15 @@ fn malformed_unknown_null_and_noncanonical_base64_are_rejected() { } #[test] -fn caller_limits_bound_bytes_and_nesting() { +fn caller_limits_remain_local_codec_failures() { let tiny = Limits::new(8, 8, 2, 1).unwrap(); let too_large = decode_observe_request(br#"{"path":"/long","depth":0}"#, tiny).unwrap_err(); assert!(matches!(&too_large, CodecError::BodyTooLarge { .. })); - assert_eq!( - too_large.resource_limit_kind(), - Some(ResourceLimitKind::RepresentationBytes) - ); let nested = br#"{"path":[[["/"]]],"depth":0}"#; let nesting_limited = Limits::new(1024, 1024, 2, 1).unwrap(); let too_deep = decode_observe_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-http/tests/responses.rs b/crates/wip-http/tests/responses.rs index 14cd29e..fa031d8 100644 --- a/crates/wip-http/tests/responses.rs +++ b/crates/wip-http/tests/responses.rs @@ -3,10 +3,9 @@ use wip_http::wip_protocol::{ ObserveRequest, ProtocolError, ProtocolErrorCode, ProtocolInteraction, }; use wip_http::{ - ClientResponseError, DecodedResponse, InvalidResponseKind, Limits, ResourceLimitKind, - TransportFailureKind, decode_observe_response, encode_protocol_error_response, - encode_resource_limit_error_response, error_code_name, is_json_content_type, status_for_error, - status_for_resource_limit, + ClientResponseError, DecodedResponse, InvalidResponseKind, Limits, TransportFailureKind, + decode_observe_response, encode_protocol_error_response, error_code_name, is_json_content_type, + status_for_error, }; fn limits() -> Limits { @@ -120,41 +119,45 @@ fn every_protocol_code_has_stable_name_and_status() { } #[test] -fn resource_limit_statuses_encode_and_decode_by_concrete_limit_class() { +fn logical_resource_limit_uses_only_422_and_bare_413_is_transport_failure() { let error = ProtocolError { code: ProtocolErrorCode::ResourceLimitExceeded, - message: "bounded".into(), + message: "logical observation limit exceeded".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::Observe, - &error, - kind, - limits(), - ) - .unwrap(); - assert_eq!(encoded.status(), status); - assert_eq!( - decode_observe_response(&request(), &encoded, limits()).unwrap(), - DecodedResponse::ProtocolFailure { - error: error.clone(), - status_mismatch: None, - } - ); - } - - let default = + let encoded = encode_protocol_error_response(ProtocolInteraction::Observe, &error, limits()).unwrap(); - assert_eq!(default.status(), StatusCode::UNPROCESSABLE_ENTITY); - assert_ne!(default.status(), StatusCode::TOO_MANY_REQUESTS); + assert_eq!(encoded.status(), StatusCode::UNPROCESSABLE_ENTITY); + assert_eq!( + decode_observe_response(&request(), &encoded, limits()).unwrap(), + DecodedResponse::ProtocolFailure { + error: error.clone(), + status_mismatch: None, + } + ); + + let mut noncanonical = encoded; + *noncanonical.status_mut() = StatusCode::PAYLOAD_TOO_LARGE; + assert_eq!( + decode_observe_response(&request(), &noncanonical, limits()).unwrap(), + DecodedResponse::ProtocolFailure { + error, + status_mismatch: Some(wip_http::StatusMismatch { + actual: StatusCode::PAYLOAD_TOO_LARGE, + expected: StatusCode::UNPROCESSABLE_ENTITY, + }), + } + ); + + let bare_413 = Response::builder() + .status(StatusCode::PAYLOAD_TOO_LARGE) + .body(Vec::::new()) + .unwrap(); + assert!(matches!( + decode_observe_response(&request(), &bare_413, limits()).unwrap_err(), + ClientResponseError::TransportBinding(failure) + if failure.status == StatusCode::PAYLOAD_TOO_LARGE + && failure.kind == TransportFailureKind::HttpStatus + )); } #[test] @@ -171,6 +174,49 @@ fn rate_limit_status_without_protocol_envelope_stays_transport_failure() { )); } +#[test] +fn unsupported_response_content_coding_is_transport_failure() { + let mut encoded = encode_protocol_error_response( + ProtocolInteraction::Observe, + &ProtocolError { + code: ProtocolErrorCode::NotFound, + message: "missing".into(), + }, + limits(), + ) + .unwrap(); + encoded + .headers_mut() + .insert("content-encoding", "gzip".parse().unwrap()); + assert!(matches!( + decode_observe_response(&request(), &encoded, limits()).unwrap_err(), + ClientResponseError::TransportBinding(failure) + if failure.status == StatusCode::NOT_FOUND + && failure.kind == TransportFailureKind::UnsupportedContentCoding + )); + + let success = response(StatusCode::OK, br#"{"object":{"name":"","interfaces":[]}}"#); + let mut identity = success.clone(); + identity + .headers_mut() + .insert("content-encoding", "identity".parse().unwrap()); + assert!(matches!( + decode_observe_response(&request(), &identity, limits()).unwrap(), + DecodedResponse::Success(_) + )); + + let mut unsupported = success; + unsupported + .headers_mut() + .insert("content-encoding", "br".parse().unwrap()); + assert!(matches!( + decode_observe_response(&request(), &unsupported, limits()).unwrap_err(), + ClientResponseError::TransportBinding(failure) + if failure.status == StatusCode::OK + && failure.kind == TransportFailureKind::UnsupportedContentCoding + )); +} + #[test] fn valid_success_body_requires_exactly_http_200() { let success_body = br#"{"object":{"name":"","interfaces":[]}}"#; diff --git a/crates/wip-http/tests/routes.rs b/crates/wip-http/tests/routes.rs index 2e0d949..9bbe2ab 100644 --- a/crates/wip-http/tests/routes.rs +++ b/crates/wip-http/tests/routes.rs @@ -61,7 +61,10 @@ fn inbound_http_metadata_enforces_post_route_and_json() { .header("content-type", "application/json") .body(Vec::::new()) .unwrap(); - assert!(validate_http_request(&endpoint, Route::Observe, &wrong_method).is_err()); + let error = validate_http_request(&endpoint, Route::Observe, &wrong_method).unwrap_err(); + assert!(matches!(&error, RequestMetadataError::MethodNotAllowed)); + assert_eq!(error.status(), StatusCode::METHOD_NOT_ALLOWED); + assert_eq!(error.allow(), Some("POST")); let wrong_route = Request::builder() .method(Method::POST) @@ -69,7 +72,21 @@ fn inbound_http_metadata_enforces_post_route_and_json() { .header("content-type", "application/json") .body(Vec::::new()) .unwrap(); - assert!(validate_http_request(&endpoint, Route::Observe, &wrong_route).is_err()); + let error = validate_http_request(&endpoint, Route::Observe, &wrong_route).unwrap_err(); + assert!(matches!(&error, RequestMetadataError::NotFound)); + assert_eq!(error.status(), StatusCode::NOT_FOUND); + assert_eq!(error.allow(), None); + + let query_and_wrong_method = Request::builder() + .method(Method::GET) + .uri("/wip/v1/observe?unexpected=true") + .header("content-type", "application/json") + .body(Vec::::new()) + .unwrap(); + assert!(matches!( + validate_http_request(&endpoint, Route::Observe, &query_and_wrong_method).unwrap_err(), + RequestMetadataError::NotFound + )); for accept in [ "text/plain, */*;q=1, application/json;q=0", @@ -114,6 +131,29 @@ fn inbound_http_metadata_enforces_post_route_and_json() { validate_http_request(&endpoint, Route::Observe, &unsupported_media_type).unwrap_err(); assert!(matches!(&error, RequestMetadataError::UnsupportedMediaType)); assert_eq!(error.status(), StatusCode::UNSUPPORTED_MEDIA_TYPE); + + for coding in ["gzip", "identity, gzip"] { + let unsupported_coding = Request::builder() + .method(Method::POST) + .uri("/wip/v1/observe") + .header("content-type", "application/json") + .header("content-encoding", coding) + .body(Vec::::new()) + .unwrap(); + let error = + validate_http_request(&endpoint, Route::Observe, &unsupported_coding).unwrap_err(); + assert!(matches!(&error, RequestMetadataError::UnsupportedMediaType)); + assert_eq!(error.status(), StatusCode::UNSUPPORTED_MEDIA_TYPE); + } + + let identity_coding = Request::builder() + .method(Method::POST) + .uri("/wip/v1/observe") + .header("content-type", "application/json") + .header("content-encoding", "identity") + .body(Vec::::new()) + .unwrap(); + assert!(validate_http_request(&endpoint, Route::Observe, &identity_coding).is_ok()); } #[test]