From 7d4d228236b58ee2a5a893456cd261f5e47c5e97 Mon Sep 17 00:00:00 2001 From: Gregory Schier Date: Wed, 11 Feb 2026 17:11:35 -0800 Subject: [PATCH] Fix HTTP/2 requests failing with duplicate Content-Length (#391) Co-authored-by: Claude Opus 4.6 --- Cargo.lock | 1 + crates-tauri/yaak-app/src/http_request.rs | 4 +- crates/yaak-http/Cargo.toml | 1 + crates/yaak-http/src/sender.rs | 61 +++++++++++++++- crates/yaak-http/src/types.rs | 89 ++++++++++------------- 5 files changed, 98 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e86ab19..6897229a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8167,6 +8167,7 @@ dependencies = [ "cookie", "flate2", "futures-util", + "http-body", "hyper-util", "log", "mime_guess", diff --git a/crates-tauri/yaak-app/src/http_request.rs b/crates-tauri/yaak-app/src/http_request.rs index d2e060aa..4618bc87 100644 --- a/crates-tauri/yaak-app/src/http_request.rs +++ b/crates-tauri/yaak-app/src/http_request.rs @@ -414,7 +414,7 @@ async fn execute_transaction( sendable_request.body = Some(SendableBody::Bytes(bytes)); None } - Some(SendableBody::Stream(stream)) => { + Some(SendableBody::Stream { data: stream, content_length }) => { // Wrap stream with TeeReader to capture data as it's read // Use unbounded channel to ensure all data is captured without blocking the HTTP request let (body_chunk_tx, body_chunk_rx) = tokio::sync::mpsc::unbounded_channel::>(); @@ -448,7 +448,7 @@ async fn execute_transaction( None }; - sendable_request.body = Some(SendableBody::Stream(pinned)); + sendable_request.body = Some(SendableBody::Stream { data: pinned, content_length }); handle } None => { diff --git a/crates/yaak-http/Cargo.toml b/crates/yaak-http/Cargo.toml index 1df3b31e..20183503 100644 --- a/crates/yaak-http/Cargo.toml +++ b/crates/yaak-http/Cargo.toml @@ -12,6 +12,7 @@ bytes = "1.11.1" cookie = "0.18.1" flate2 = "1" futures-util = "0.3" +http-body = "1" url = "2" zstd = "0.13" hyper-util = { version = "0.1.17", default-features = false, features = ["client-legacy"] } diff --git a/crates/yaak-http/src/sender.rs b/crates/yaak-http/src/sender.rs index 940d0f31..911f2338 100644 --- a/crates/yaak-http/src/sender.rs +++ b/crates/yaak-http/src/sender.rs @@ -2,7 +2,9 @@ use crate::decompress::{ContentEncoding, streaming_decoder}; use crate::error::{Error, Result}; use crate::types::{SendableBody, SendableHttpRequest}; use async_trait::async_trait; +use bytes::Bytes; use futures_util::StreamExt; +use http_body::{Body as HttpBody, Frame, SizeHint}; use reqwest::{Client, Method, Version}; use std::fmt::Display; use std::pin::Pin; @@ -413,10 +415,16 @@ impl HttpSender for ReqwestSender { Some(SendableBody::Bytes(bytes)) => { req_builder = req_builder.body(bytes); } - Some(SendableBody::Stream(stream)) => { - // Convert AsyncRead stream to reqwest Body - let stream = tokio_util::io::ReaderStream::new(stream); - let body = reqwest::Body::wrap_stream(stream); + Some(SendableBody::Stream { data, content_length }) => { + // Convert AsyncRead stream to reqwest Body. If content length is + // known, wrap with a SizedBody so hyper can set Content-Length + // automatically (for both HTTP/1.1 and HTTP/2). + let stream = tokio_util::io::ReaderStream::new(data); + let body = if let Some(len) = content_length { + reqwest::Body::wrap(SizedBody::new(stream, len)) + } else { + reqwest::Body::wrap_stream(stream) + }; req_builder = req_builder.body(body); } } @@ -520,6 +528,51 @@ impl HttpSender for ReqwestSender { } } +/// A wrapper around a byte stream that reports a known content length via +/// `size_hint()`. This lets hyper set the `Content-Length` header +/// automatically based on the body size, without us having to add it as an +/// explicit header — which can cause duplicate `Content-Length` headers and +/// break HTTP/2. +struct SizedBody { + stream: std::sync::Mutex, + remaining: u64, +} + +impl SizedBody { + fn new(stream: S, content_length: u64) -> Self { + Self { stream: std::sync::Mutex::new(stream), remaining: content_length } + } +} + +impl HttpBody for SizedBody +where + S: futures_util::Stream> + Send + Unpin + 'static, +{ + type Data = Bytes; + type Error = std::io::Error; + + fn poll_frame( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + ) -> Poll, Self::Error>>> { + let this = self.get_mut(); + let mut stream = this.stream.lock().unwrap(); + match stream.poll_next_unpin(cx) { + Poll::Ready(Some(Ok(chunk))) => { + this.remaining = this.remaining.saturating_sub(chunk.len() as u64); + Poll::Ready(Some(Ok(Frame::data(chunk)))) + } + Poll::Ready(Some(Err(e))) => Poll::Ready(Some(Err(e))), + Poll::Ready(None) => Poll::Ready(None), + Poll::Pending => Poll::Pending, + } + } + + fn size_hint(&self) -> SizeHint { + SizeHint::with_exact(self.remaining) + } +} + fn version_to_str(version: &Version) -> String { match *version { Version::HTTP_09 => "HTTP/0.9".to_string(), diff --git a/crates/yaak-http/src/types.rs b/crates/yaak-http/src/types.rs index 135ce8bb..aee41313 100644 --- a/crates/yaak-http/src/types.rs +++ b/crates/yaak-http/src/types.rs @@ -16,7 +16,13 @@ pub(crate) const MULTIPART_BOUNDARY: &str = "------YaakFormBoundary"; pub enum SendableBody { Bytes(Bytes), - Stream(Pin>), + Stream { + data: Pin>, + /// Known content length for the stream, if available. This is used by + /// the sender to set the body size hint so that hyper can set + /// Content-Length automatically for both HTTP/1.1 and HTTP/2. + content_length: Option, + }, } enum SendableBodyWithMeta { @@ -31,7 +37,10 @@ impl From for SendableBody { fn from(value: SendableBodyWithMeta) -> Self { match value { SendableBodyWithMeta::Bytes(b) => SendableBody::Bytes(b), - SendableBodyWithMeta::Stream { data, .. } => SendableBody::Stream(data), + SendableBodyWithMeta::Stream { data, content_length } => SendableBody::Stream { + data, + content_length: content_length.map(|l| l as u64), + }, } } } @@ -186,23 +195,11 @@ async fn build_body( } } - // Check if Transfer-Encoding: chunked is already set - let has_chunked_encoding = headers.iter().any(|h| { - h.0.to_lowercase() == "transfer-encoding" && h.1.to_lowercase().contains("chunked") - }); - - // Add a Content-Length header only if chunked encoding is not being used - if !has_chunked_encoding { - let content_length = match body { - Some(SendableBodyWithMeta::Bytes(ref bytes)) => Some(bytes.len()), - Some(SendableBodyWithMeta::Stream { content_length, .. }) => content_length, - None => None, - }; - - if let Some(cl) = content_length { - headers.push(("Content-Length".to_string(), cl.to_string())); - } - } + // NOTE: Content-Length is NOT set as an explicit header here. Instead, the + // body's content length is carried via SendableBody::Stream { content_length } + // and used by the sender to set the body size hint. This lets hyper handle + // Content-Length automatically for both HTTP/1.1 and HTTP/2, avoiding the + // duplicate Content-Length that breaks HTTP/2 servers. Ok((body.map(|b| b.into()), headers)) } @@ -928,7 +925,27 @@ mod tests { } #[tokio::test] - async fn test_no_content_length_with_chunked_encoding() -> Result<()> { + async fn test_no_content_length_header_added_by_build_body() -> Result<()> { + let mut body = BTreeMap::new(); + body.insert("text".to_string(), json!("Hello, World!")); + + let headers = vec![]; + + let (_, result_headers) = + build_body("POST", &Some("text/plain".to_string()), &body, headers).await?; + + // Content-Length should NOT be set as an explicit header. Instead, the + // sender uses the body's size_hint to let hyper set it automatically, + // which works correctly for both HTTP/1.1 and HTTP/2. + let has_content_length = + result_headers.iter().any(|h| h.0.to_lowercase() == "content-length"); + assert!(!has_content_length, "Content-Length should not be set as an explicit header"); + + Ok(()) + } + + #[tokio::test] + async fn test_chunked_encoding_header_preserved() -> Result<()> { let mut body = BTreeMap::new(); body.insert("text".to_string(), json!("Hello, World!")); @@ -938,11 +955,6 @@ mod tests { let (_, result_headers) = build_body("POST", &Some("text/plain".to_string()), &body, headers).await?; - // Verify that Content-Length is NOT present when Transfer-Encoding: chunked is set - let has_content_length = - result_headers.iter().any(|h| h.0.to_lowercase() == "content-length"); - assert!(!has_content_length, "Content-Length should not be present with chunked encoding"); - // Verify that the Transfer-Encoding header is still present let has_chunked = result_headers.iter().any(|h| { h.0.to_lowercase() == "transfer-encoding" && h.1.to_lowercase().contains("chunked") @@ -951,31 +963,4 @@ mod tests { Ok(()) } - - #[tokio::test] - async fn test_content_length_without_chunked_encoding() -> Result<()> { - let mut body = BTreeMap::new(); - body.insert("text".to_string(), json!("Hello, World!")); - - // Headers without Transfer-Encoding: chunked - let headers = vec![]; - - let (_, result_headers) = - build_body("POST", &Some("text/plain".to_string()), &body, headers).await?; - - // Verify that Content-Length IS present when Transfer-Encoding: chunked is NOT set - let content_length_header = - result_headers.iter().find(|h| h.0.to_lowercase() == "content-length"); - assert!( - content_length_header.is_some(), - "Content-Length should be present without chunked encoding" - ); - assert_eq!( - content_length_header.unwrap().1, - "13", - "Content-Length should match the body size" - ); - - Ok(()) - } }