From aeb42486afead1e7ae23b142855e63661355000d Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Sat, 23 Aug 2025 12:23:57 +0900 Subject: [PATCH] fix(actix-web): improve streaming response behavior for Content-Length and Content-Type (#3737) - Set default Content-Type to application/octet-stream for streaming responses - Respect Content-Length header by automatically calling `no_chunking()` when set - Resolves issue where Content-Length was ignored in streaming responses Fixes #2306 --------- Co-authored-by: Yinuo Deng --- actix-web/CHANGES.md | 3 + actix-web/src/response/builder.rs | 21 ++++ actix-web/tests/test_streaming_response.rs | 115 +++++++++++++++++++++ 3 files changed, 139 insertions(+) create mode 100644 actix-web/tests/test_streaming_response.rs diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index ab40eea98..85b9c6063 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -2,6 +2,9 @@ ## Unreleased +- `actix_web::response::builder::HttpResponseBuilder::streaming()` now sets `Content-Type` to `application/octet-stream` if `Content-Type` does not exist. +- `actix_web::response::builder::HttpResponseBuilder::streaming()` now calls `actix_web::response::builder::HttpResponseBuilder::no_chunking()` if `Content-Length` is set by user. + ## 4.11.0 - Add `Logger::log_level()` method. diff --git a/actix-web/src/response/builder.rs b/actix-web/src/response/builder.rs index c23de8e36..b81ce3568 100644 --- a/actix-web/src/response/builder.rs +++ b/actix-web/src/response/builder.rs @@ -318,12 +318,33 @@ impl HttpResponseBuilder { /// Set a streaming body and build the `HttpResponse`. /// /// `HttpResponseBuilder` can not be used after this call. + /// + /// If `Content-Type` is not set, then it is automatically set to `application/octet-stream`. + /// + /// If `Content-Length` is set, then [`no_chunking()`](Self::no_chunking) is automatically called. #[inline] pub fn streaming(&mut self, stream: S) -> HttpResponse where S: Stream> + 'static, E: Into + 'static, { + // Set mime type to application/octet-stream if it is not set + if let Some(parts) = self.inner() { + if !parts.headers.contains_key(header::CONTENT_TYPE) { + self.insert_header((header::CONTENT_TYPE, mime::APPLICATION_OCTET_STREAM)); + } + } + + if let Some(parts) = self.inner() { + if let Some(length) = parts.headers.get(header::CONTENT_LENGTH) { + if let Ok(length) = length.to_str() { + if let Ok(length) = length.parse::() { + self.no_chunking(length); + } + } + } + } + self.body(BodyStream::new(stream)) } diff --git a/actix-web/tests/test_streaming_response.rs b/actix-web/tests/test_streaming_response.rs new file mode 100644 index 000000000..1c22da38b --- /dev/null +++ b/actix-web/tests/test_streaming_response.rs @@ -0,0 +1,115 @@ +use std::{ + pin::Pin, + task::{Context, Poll}, +}; + +use actix_web::{ + http::header::{self, HeaderValue}, + HttpResponse, +}; +use bytes::Bytes; +use futures_core::Stream; + +struct FixedSizeStream { + data: Vec, + yielded: bool, +} + +impl FixedSizeStream { + fn new(size: usize) -> Self { + Self { + data: vec![0u8; size], + yielded: false, + } + } +} + +impl Stream for FixedSizeStream { + type Item = Result; + + fn poll_next(mut self: Pin<&mut Self>, _: &mut Context<'_>) -> Poll> { + if self.yielded { + Poll::Ready(None) + } else { + self.yielded = true; + let data = std::mem::take(&mut self.data); + Poll::Ready(Some(Ok(Bytes::from(data)))) + } + } +} + +#[actix_rt::test] +async fn test_streaming_response_with_content_length() { + let stream = FixedSizeStream::new(100); + + let resp = HttpResponse::Ok() + .append_header((header::CONTENT_LENGTH, "100")) + .streaming(stream); + + assert_eq!( + resp.headers().get(header::CONTENT_LENGTH), + Some(&HeaderValue::from_static("100")), + "Content-Length should be preserved when explicitly set" + ); + + let has_chunked = resp + .headers() + .get(header::TRANSFER_ENCODING) + .map(|v| v.to_str().unwrap_or("")) + .unwrap_or("") + .contains("chunked"); + + assert!( + !has_chunked, + "chunked should not be used when Content-Length is provided" + ); + + assert_eq!( + resp.headers().get(header::CONTENT_TYPE), + Some(&HeaderValue::from_static("application/octet-stream")), + "Content-Type should default to application/octet-stream" + ); +} + +#[actix_rt::test] +async fn test_streaming_response_default_content_type() { + let stream = FixedSizeStream::new(50); + + let resp = HttpResponse::Ok().streaming(stream); + + assert_eq!( + resp.headers().get(header::CONTENT_TYPE), + Some(&HeaderValue::from_static("application/octet-stream")), + "Content-Type should default to application/octet-stream" + ); +} + +#[actix_rt::test] +async fn test_streaming_response_user_defined_content_type() { + let stream = FixedSizeStream::new(25); + + let resp = HttpResponse::Ok() + .insert_header((header::CONTENT_TYPE, "text/plain")) + .streaming(stream); + + assert_eq!( + resp.headers().get(header::CONTENT_TYPE), + Some(&HeaderValue::from_static("text/plain")), + "User-defined Content-Type should be preserved" + ); +} + +#[actix_rt::test] +async fn test_streaming_response_empty_stream() { + let stream = FixedSizeStream::new(0); + + let resp = HttpResponse::Ok() + .append_header((header::CONTENT_LENGTH, "0")) + .streaming(stream); + + assert_eq!( + resp.headers().get(header::CONTENT_LENGTH), + Some(&HeaderValue::from_static("0")), + "Content-Length 0 should be preserved for empty streams" + ); +}