diff --git a/rustfs/src/server/http.rs b/rustfs/src/server/http.rs index 1cb4a1e03..437b099f1 100644 --- a/rustfs/src/server/http.rs +++ b/rustfs/src/server/http.rs @@ -22,8 +22,8 @@ use crate::server::{ compress::{CompressionConfig, PathAwareCompressionPredicate, PathCategoryInjectionLayer}, hybrid::hybrid, layer::{ - AdminChunkedContentLengthCompatLayer, ConditionalCorsLayer, ObjectAttributesEtagFixLayer, RedirectLayer, - RequestContextLayer, S3ErrorMessageCompatLayer, + AdminChunkedContentLengthCompatLayer, BodylessStatusFixLayer, ConditionalCorsLayer, ObjectAttributesEtagFixLayer, + RedirectLayer, RequestContextLayer, S3ErrorMessageCompatLayer, }, tls_material::{TlsAcceptorHolder, TlsHandshakeFailureKind, TlsMaterialSnapshot, spawn_reload_loop}, }; @@ -593,6 +593,7 @@ fn process_connection( // 15. ObjectAttributesEtagFixLayer — ETag fix for GetObjectAttributes // 16. ConditionalCorsLayer — S3 API CORS // 17. RedirectLayer — console redirect (conditional) + // 18. BodylessStatusFixLayer — clears body for 1xx/204/205/304 responses // ───────────────────────────────────────────────────────────── let hybrid_service = ServiceBuilder::new() // NOTE: Both extension types are intentionally inserted to maintain compatibility: @@ -735,6 +736,12 @@ fn process_connection( // Bucket-level CORS takes precedence when configured (handled in router.rs for OPTIONS, and in ecfs.rs for actual requests) .layer(ConditionalCorsLayer::new()) .option_layer(if is_console { Some(RedirectLayer) } else { None }) + // Must run first on responses: clear the body and remove + // Content-Length, Content-Type, and Transfer-Encoding for statuses + // that MUST NOT carry a body (1xx/204/304). Kept innermost so all + // other response-transforming layers see the already-bodyless + // response and so no layer (e.g. CORS) re-adds body headers afterward. + .layer(BodylessStatusFixLayer) .service(service); let hybrid_service = TowerToHyperService::new(hybrid_service); diff --git a/rustfs/src/server/layer.rs b/rustfs/src/server/layer.rs index df272d190..40b8bc890 100644 --- a/rustfs/src/server/layer.rs +++ b/rustfs/src/server/layer.rs @@ -399,6 +399,91 @@ where } } +/// Tower middleware that strips the body (and body-describing headers) from +/// responses whose HTTP status code MUST NOT carry a body per RFC 9110 §6.4.1 +/// and §15 (1xx, 204, 205, 304). +/// +/// The inner s3s layer serializes every `S3Error` — including 304 `NotModified` +/// preconditions — as an XML body. Returning that body for a 304 is a protocol +/// violation: hyper's HTTP/1.1 encoder forces the body to zero length but +/// preserves the response, while the HTTP/2 path fills in `content-length` +/// from the body's size hint and writes DATA frames after a HEADERS frame that +/// should have carried END_STREAM. h2 clients (curl, browsers) and proxies see +/// the malformed response as a connection-level failure — in the wild this +/// surfaces as `GOAWAY error=0` on h2 and as an upstream-disconnect 5xx from +/// reverse proxies like ngrok (`ERR_NGROK_3004`). +#[derive(Clone)] +pub struct BodylessStatusFixLayer; + +impl Layer for BodylessStatusFixLayer { + type Service = BodylessStatusFixService; + + fn layer(&self, inner: S) -> Self::Service { + BodylessStatusFixService { inner } + } +} + +#[derive(Clone)] +pub struct BodylessStatusFixService { + inner: S, +} + +impl Service> for BodylessStatusFixService +where + S: Service, Response = Response>> + Clone + Send + 'static, + S::Future: Send + 'static, + S::Error: Send + 'static, + ReqBody: Send + 'static, + RestBody: Body + From + Send + 'static, + RestBody::Error: Into + Send + 'static, + GrpcBody: Send + 'static, +{ + type Response = Response>; + type Error = S::Error; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, req: HttpRequest) -> Self::Future { + let mut inner = self.inner.clone(); + + Box::pin(async move { + let response = inner.call(req).await?; + let (mut parts, body) = response.into_parts(); + + if !is_bodyless_status(parts.status) { + return Ok(Response::from_parts(parts, body)); + } + + let response = match body { + HybridBody::Rest { .. } => { + parts.headers.remove(http::header::CONTENT_LENGTH); + parts.headers.remove(http::header::CONTENT_TYPE); + parts.headers.remove(http::header::TRANSFER_ENCODING); + Response::from_parts( + parts, + HybridBody::Rest { + rest_body: RestBody::from(Bytes::new()), + }, + ) + } + HybridBody::Grpc { grpc_body } => Response::from_parts(parts, HybridBody::Grpc { grpc_body }), + }; + + Ok(response) + }) + } +} + +fn is_bodyless_status(status: StatusCode) -> bool { + status.is_informational() + || status == StatusCode::NO_CONTENT + || status == StatusCode::RESET_CONTENT + || status == StatusCode::NOT_MODIFIED +} + fn is_xml_response(headers: &HeaderMap) -> bool { let is_xml = headers .get(http::header::CONTENT_TYPE) @@ -1052,6 +1137,138 @@ mod tests { assert!(response_headers.get(cors::response::ACCESS_CONTROL_MAX_AGE).is_none()); } + mod bodyless_status_fix { + use super::*; + use crate::server::hybrid::HybridBody; + use http_body_util::Empty; + + // The production service takes `Request`, but `Incoming` can't be + // constructed in unit tests. `BodylessStatusFixService` doesn't inspect the + // request body, so parameterising over an arbitrary `B` is safe here. + #[derive(Clone)] + struct FixedResponse { + status: StatusCode, + body: Bytes, + content_type: Option<&'static str>, + } + + impl Service> for FixedResponse { + type Response = Response, Empty>>; + type Error = Infallible; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, _req: Request) -> Self::Future { + let this = self.clone(); + Box::pin(async move { + let body = this.body.clone(); + let len = body.len(); + let mut builder = Response::builder().status(this.status); + builder = builder.header(http::header::CONTENT_LENGTH, len.to_string()); + if let Some(ct) = this.content_type { + builder = builder.header(http::header::CONTENT_TYPE, ct); + } + builder = builder.header(http::header::ETAG, "\"abc123\""); + Ok(builder + .body(HybridBody::Rest { + rest_body: Full::from(body), + }) + .expect("build response")) + }) + } + } + + fn empty_request() -> Request<()> { + Request::builder().uri("/").body(()).expect("request") + } + + async fn collect_body>(body: B) -> Bytes + where + B::Error: std::fmt::Debug, + { + BodyExt::collect(body).await.expect("collect body").to_bytes() + } + + #[tokio::test] + async fn strips_body_and_content_headers_for_304() { + let mut svc = BodylessStatusFixLayer.layer(FixedResponse { + status: StatusCode::NOT_MODIFIED, + body: Bytes::from_static(b"NotModified"), + content_type: Some("application/xml"), + }); + + let res = svc.call(empty_request()).await.expect("service call"); + let (parts, body) = res.into_parts(); + + assert_eq!(parts.status, StatusCode::NOT_MODIFIED); + assert!(parts.headers.get(http::header::CONTENT_LENGTH).is_none()); + assert!(parts.headers.get(http::header::CONTENT_TYPE).is_none()); + assert_eq!(parts.headers.get(http::header::ETAG).unwrap(), "\"abc123\""); + + let bytes = collect_body(body).await; + assert!(bytes.is_empty(), "304 response body must be empty"); + } + + #[tokio::test] + async fn strips_body_for_204() { + let mut svc = BodylessStatusFixLayer.layer(FixedResponse { + status: StatusCode::NO_CONTENT, + body: Bytes::from_static(b"unexpected"), + content_type: None, + }); + + let res = svc.call(empty_request()).await.expect("service call"); + let (parts, body) = res.into_parts(); + + assert_eq!(parts.status, StatusCode::NO_CONTENT); + assert!(parts.headers.get(http::header::CONTENT_LENGTH).is_none()); + + let bytes = collect_body(body).await; + assert!(bytes.is_empty()); + } + + #[tokio::test] + async fn preserves_body_for_200() { + let payload = Bytes::from_static(b"hello"); + let mut svc = BodylessStatusFixLayer.layer(FixedResponse { + status: StatusCode::OK, + body: payload.clone(), + content_type: Some("text/plain"), + }); + + let res = svc.call(empty_request()).await.expect("service call"); + let (parts, body) = res.into_parts(); + + assert_eq!(parts.status, StatusCode::OK); + assert_eq!(parts.headers.get(http::header::CONTENT_TYPE).unwrap(), "text/plain"); + assert_eq!( + parts.headers.get(http::header::CONTENT_LENGTH).unwrap(), + payload.len().to_string().as_str() + ); + + let bytes = collect_body(body).await; + assert_eq!(bytes, payload); + } + + #[test] + fn is_bodyless_status_matches_rfc9110_statuses() { + assert!(is_bodyless_status(StatusCode::CONTINUE)); + assert!(is_bodyless_status(StatusCode::SWITCHING_PROTOCOLS)); + assert!(is_bodyless_status(StatusCode::NO_CONTENT)); + assert!(is_bodyless_status(StatusCode::RESET_CONTENT)); + assert!(is_bodyless_status(StatusCode::NOT_MODIFIED)); + + assert!(!is_bodyless_status(StatusCode::OK)); + assert!(!is_bodyless_status(StatusCode::PARTIAL_CONTENT)); + assert!(!is_bodyless_status(StatusCode::NOT_FOUND)); + assert!(!is_bodyless_status(StatusCode::PRECONDITION_FAILED)); + assert!(!is_bodyless_status(StatusCode::INTERNAL_SERVER_ERROR)); + } + } + #[test] fn test_apply_bucket_cors_result_replaces_existing_cors_headers() { let mut response_headers = HeaderMap::new();