From 1120e2e87a290e1641cfcb165ee8f9ac20c36ddf Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 24 Feb 2026 11:01:57 -0600 Subject: [PATCH 01/13] Pre-allocate the buffer based on the expected `Content-Length` with the Rust HTTP client --- rust/src/http_client.rs | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/rust/src/http_client.rs b/rust/src/http_client.rs index 9bbdff8b454..44d328ac73f 100644 --- a/rust/src/http_client.rs +++ b/rust/src/http_client.rs @@ -16,6 +16,7 @@ use std::{collections::HashMap, future::Future, sync::OnceLock}; use anyhow::Context; use futures::TryStreamExt; +use http::header; use once_cell::sync::OnceCell; use pyo3::{create_exception, exceptions::PyException, prelude::*}; use reqwest::RequestBuilder; @@ -235,8 +236,37 @@ impl HttpClient { let status = response.status(); + // Find the expected `Content-Length` so we can pre-allocate the buffer + // necessary to read the response. + let content_length = { + let content_length = response + .headers() + .get(reqwest::header::CONTENT_LENGTH) + .and_then(|value| value.to_str().ok()) + .and_then(|s| s.parse::().ok()); + + // Sanity check that the request isn't too large from the information + // they told us (may be inaccurate so we also check below as we actually + // read the bytes) + if let Some(content_length_bytes) = content_length { + if content_length_bytes > response_limit { + Err(anyhow::anyhow!( + "Response size (defined by `Content-Length`) too large" + ))?; + } + } + + content_length + }; + let mut stream = response.bytes_stream(); - let mut buffer = Vec::new(); + // Pre-allocate the buffer based on the expected `Content-Length` + let mut buffer = Vec::with_capacity( + content_length + // Default to pre-allocating nothing when the request doesn't have a + // `Content-Length` header + .unwrap_or(0), + ); while let Some(chunk) = stream.try_next().await.context("reading body")? { if buffer.len() + chunk.len() > response_limit { Err(anyhow::anyhow!("Response size too large"))?; From d61e217be7588ba2e9b02806b3695bd44c3746c4 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 24 Feb 2026 11:08:45 -0600 Subject: [PATCH 02/13] Add changelog --- changelog.d/19498.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/19498.misc diff --git a/changelog.d/19498.misc b/changelog.d/19498.misc new file mode 100644 index 00000000000..7d048c283d6 --- /dev/null +++ b/changelog.d/19498.misc @@ -0,0 +1 @@ +Pre-allocate the buffer based on the expected `Content-Length` with the Rust HTTP client. From 2ca3ca3ed3635e738efe1d60a90250a3bc5124c9 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 24 Feb 2026 11:11:03 -0600 Subject: [PATCH 03/13] Fix lints --- rust/src/http_client.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rust/src/http_client.rs b/rust/src/http_client.rs index 44d328ac73f..092c9455d5e 100644 --- a/rust/src/http_client.rs +++ b/rust/src/http_client.rs @@ -16,7 +16,6 @@ use std::{collections::HashMap, future::Future, sync::OnceLock}; use anyhow::Context; use futures::TryStreamExt; -use http::header; use once_cell::sync::OnceCell; use pyo3::{create_exception, exceptions::PyException, prelude::*}; use reqwest::RequestBuilder; From 9ccc7fc2f6b6d9c9f1d3a4c8fbc564636614ef22 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 Feb 2026 14:30:54 -0600 Subject: [PATCH 04/13] Better comments --- rust/src/http_client.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/rust/src/http_client.rs b/rust/src/http_client.rs index 092c9455d5e..97708f1e199 100644 --- a/rust/src/http_client.rs +++ b/rust/src/http_client.rs @@ -236,7 +236,15 @@ impl HttpClient { let status = response.status(); // Find the expected `Content-Length` so we can pre-allocate the buffer - // necessary to read the response. + // necessary to read the response. It's expected that not every request will + // have a `Content-Length` header. + // + // `response.content_length()` does exist but the "value does not directly + // represents the value of the `Content-Length` header, but rather the size + // of the response’s body" + // (https://docs.rs/reqwest/latest/reqwest/struct.Response.html#method.content_length) + // and we want to avoid reading the entire body at this point because we + // purposely stream it below until the `response_limit`. let content_length = { let content_length = response .headers() @@ -258,6 +266,8 @@ impl HttpClient { content_length }; + // Stream the response to avoid allocating a giant object on the server + // above our expected `response_limit`. let mut stream = response.bytes_stream(); // Pre-allocate the buffer based on the expected `Content-Length` let mut buffer = Vec::with_capacity( From e9e75ba0a212e7e923b271b41372142150ab63ca Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 Feb 2026 14:39:59 -0600 Subject: [PATCH 05/13] Use `response.headers().typed_get::()` See https://github.com/element-hq/synapse/pull/19498#discussion_r2863569606 --- rust/src/http_client.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rust/src/http_client.rs b/rust/src/http_client.rs index 97708f1e199..dd37a10426b 100644 --- a/rust/src/http_client.rs +++ b/rust/src/http_client.rs @@ -16,6 +16,7 @@ use std::{collections::HashMap, future::Future, sync::OnceLock}; use anyhow::Context; use futures::TryStreamExt; +use headers::HeaderMapExt; use once_cell::sync::OnceCell; use pyo3::{create_exception, exceptions::PyException, prelude::*}; use reqwest::RequestBuilder; @@ -248,9 +249,9 @@ impl HttpClient { let content_length = { let content_length = response .headers() - .get(reqwest::header::CONTENT_LENGTH) - .and_then(|value| value.to_str().ok()) - .and_then(|s| s.parse::().ok()); + .typed_get::() + // We need a `usize` for the `Vec::with_capacity(...)` usage below + .and_then(|content_length| content_length.0.try_into().ok()); // Sanity check that the request isn't too large from the information // they told us (may be inaccurate so we also check below as we actually From dac4ff027e773b7e40bd616cc47a462524180df7 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 Feb 2026 15:27:40 -0600 Subject: [PATCH 06/13] Simplify Rust HTTP client body limiting See https://github.com/element-hq/synapse/pull/19498#discussion_r2865607737 --- Cargo.lock | 1 + rust/Cargo.toml | 3 ++ rust/src/errors.rs | 2 +- rust/src/http_client.rs | 67 ++++++++--------------------------------- 4 files changed, 18 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 15342020c90..5b2cce9dadb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -818,6 +818,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab53c047fcd1a1d2a8820fe84f05d6be69e9526be40cb03b73f86b6b03e6d87d" dependencies = [ "anyhow", + "bytes", "indoc", "libc", "memoffset", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 350701d3279..8199a4e02ba 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -35,6 +35,9 @@ pyo3 = { version = "0.27.2", features = [ "anyhow", "abi3", "abi3-py310", + # So we can pass `bytes::Bytes` directly back to Python efficiently, + # https://docs.rs/pyo3/latest/pyo3/bytes/index.html + "bytes", ] } pyo3-log = "0.13.1" pythonize = "0.27.0" diff --git a/rust/src/errors.rs b/rust/src/errors.rs index 149019ff4b4..012f9a79904 100644 --- a/rust/src/errors.rs +++ b/rust/src/errors.rs @@ -62,7 +62,7 @@ impl NotFoundError { import_exception!(synapse.api.errors, HttpResponseException); impl HttpResponseException { - pub fn new(status: StatusCode, bytes: Vec) -> pyo3::PyErr { + pub fn new(status: StatusCode, bytes: bytes::Bytes) -> pyo3::PyErr { HttpResponseException::new_err(( status.as_u16(), status.canonical_reason().unwrap_or_default(), diff --git a/rust/src/http_client.rs b/rust/src/http_client.rs index dd37a10426b..57880d6fb99 100644 --- a/rust/src/http_client.rs +++ b/rust/src/http_client.rs @@ -15,8 +15,7 @@ use std::{collections::HashMap, future::Future, sync::OnceLock}; use anyhow::Context; -use futures::TryStreamExt; -use headers::HeaderMapExt; +use http_body_util::BodyExt; use once_cell::sync::OnceCell; use pyo3::{create_exception, exceptions::PyException, prelude::*}; use reqwest::RequestBuilder; @@ -236,62 +235,22 @@ impl HttpClient { let status = response.status(); - // Find the expected `Content-Length` so we can pre-allocate the buffer - // necessary to read the response. It's expected that not every request will - // have a `Content-Length` header. - // - // `response.content_length()` does exist but the "value does not directly - // represents the value of the `Content-Length` header, but rather the size - // of the response’s body" - // (https://docs.rs/reqwest/latest/reqwest/struct.Response.html#method.content_length) - // and we want to avoid reading the entire body at this point because we - // purposely stream it below until the `response_limit`. - let content_length = { - let content_length = response - .headers() - .typed_get::() - // We need a `usize` for the `Vec::with_capacity(...)` usage below - .and_then(|content_length| content_length.0.try_into().ok()); - - // Sanity check that the request isn't too large from the information - // they told us (may be inaccurate so we also check below as we actually - // read the bytes) - if let Some(content_length_bytes) = content_length { - if content_length_bytes > response_limit { - Err(anyhow::anyhow!( - "Response size (defined by `Content-Length`) too large" - ))?; - } - } - - content_length - }; - - // Stream the response to avoid allocating a giant object on the server - // above our expected `response_limit`. - let mut stream = response.bytes_stream(); - // Pre-allocate the buffer based on the expected `Content-Length` - let mut buffer = Vec::with_capacity( - content_length - // Default to pre-allocating nothing when the request doesn't have a - // `Content-Length` header - .unwrap_or(0), - ); - while let Some(chunk) = stream.try_next().await.context("reading body")? { - if buffer.len() + chunk.len() > response_limit { - Err(anyhow::anyhow!("Response size too large"))?; - } - - buffer.extend_from_slice(&chunk); - } + let body = reqwest::Body::from(response); + let limited_body = http_body_util::Limited::new(body, response_limit); + let collected = limited_body + .collect() + .await + .map_err(anyhow::Error::from_boxed) + .with_context(|| { + format!("Response exceeded response limit ({})", response_limit) + })?; + let bytes: bytes::Bytes = collected.to_bytes(); if !status.is_success() { - return Err(HttpResponseException::new(status, buffer)); + return Err(HttpResponseException::new(status, bytes)); } - let r = Python::attach(|py| buffer.into_pyobject(py).map(|o| o.unbind()))?; - - Ok(r) + Ok(bytes) }) } } From 585fabdd08de94de7aa9c296beca983b958ae2f7 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 Feb 2026 15:34:26 -0600 Subject: [PATCH 07/13] Add test --- tests/synapse_rust/test_http_client.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/synapse_rust/test_http_client.py b/tests/synapse_rust/test_http_client.py index 032eab77e80..880527f97a0 100644 --- a/tests/synapse_rust/test_http_client.py +++ b/tests/synapse_rust/test_http_client.py @@ -171,6 +171,24 @@ async def do_request() -> None: self.get_success(self.till_deferred_has_result(do_request())) self.assertEqual(self.server.calls, 1) + def test_request_response_limit_exceeded(self) -> None: + """ + Test to make sure we handle the response limit being exceeded + """ + + async def do_request() -> None: + self.assertFailure( + self._rust_http_client.get( + url=self.server.endpoint, + # Small limit so we hit the limit + response_limit=1, + ), + RuntimeError, + ) + + self.get_success(self.till_deferred_has_result(do_request())) + self.assertEqual(self.server.calls, 1) + async def test_logging_context(self) -> None: """ Test to make sure the `LoggingContext` (logcontext) is handled correctly From aaa40864972f2d359b07cdd37572a22b7f25cc67 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 Feb 2026 15:35:07 -0600 Subject: [PATCH 08/13] More specific error message --- rust/src/http_client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/src/http_client.rs b/rust/src/http_client.rs index 57880d6fb99..841ad8cb2ed 100644 --- a/rust/src/http_client.rs +++ b/rust/src/http_client.rs @@ -242,7 +242,7 @@ impl HttpClient { .await .map_err(anyhow::Error::from_boxed) .with_context(|| { - format!("Response exceeded response limit ({})", response_limit) + format!("Response body exceeded response limit ({})", response_limit) })?; let bytes: bytes::Bytes = collected.to_bytes(); From 41ab6f6d42ee940ef4956ca2991a76632528e68b Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 Feb 2026 15:44:21 -0600 Subject: [PATCH 09/13] Better test --- tests/synapse_rust/test_http_client.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/synapse_rust/test_http_client.py b/tests/synapse_rust/test_http_client.py index 880527f97a0..56fab3a0e1d 100644 --- a/tests/synapse_rust/test_http_client.py +++ b/tests/synapse_rust/test_http_client.py @@ -177,16 +177,16 @@ def test_request_response_limit_exceeded(self) -> None: """ async def do_request() -> None: - self.assertFailure( - self._rust_http_client.get( - url=self.server.endpoint, - # Small limit so we hit the limit - response_limit=1, - ), - RuntimeError, + await self._rust_http_client.get( + url=self.server.endpoint, + # Small limit so we hit the limit + response_limit=1, ) - self.get_success(self.till_deferred_has_result(do_request())) + self.assertFailure( + self.till_deferred_has_result(do_request()), + RuntimeError, + ) self.assertEqual(self.server.calls, 1) async def test_logging_context(self) -> None: From c0d344accc1da20704cf0f23bf826f0e646233dc Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 Feb 2026 15:46:31 -0600 Subject: [PATCH 10/13] Add unit --- rust/src/http_client.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rust/src/http_client.rs b/rust/src/http_client.rs index 841ad8cb2ed..203d75fb68f 100644 --- a/rust/src/http_client.rs +++ b/rust/src/http_client.rs @@ -242,7 +242,10 @@ impl HttpClient { .await .map_err(anyhow::Error::from_boxed) .with_context(|| { - format!("Response body exceeded response limit ({})", response_limit) + format!( + "Response body exceeded response limit ({} bytes)", + response_limit + ) })?; let bytes: bytes::Bytes = collected.to_bytes(); From bb3aa399fe8fd4bfd6c14bf813fcae7d55b5e5a4 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 Feb 2026 15:49:26 -0600 Subject: [PATCH 11/13] Add changelog --- changelog.d/19510.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/19510.misc diff --git a/changelog.d/19510.misc b/changelog.d/19510.misc new file mode 100644 index 00000000000..cafc26601f9 --- /dev/null +++ b/changelog.d/19510.misc @@ -0,0 +1 @@ +Simplify Rust HTTP client response streaming and limiting. From e14ea953eb6732077242f36518ba918bfbac042d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 Feb 2026 16:18:26 -0600 Subject: [PATCH 12/13] Add some comments on why --- rust/src/http_client.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rust/src/http_client.rs b/rust/src/http_client.rs index 203d75fb68f..398ba9041f2 100644 --- a/rust/src/http_client.rs +++ b/rust/src/http_client.rs @@ -235,6 +235,9 @@ impl HttpClient { let status = response.status(); + // A light-weight way to read the response up until the `response_limit`. We + // want to avoid allocating a giant response object on the server above our + // expected `response_limit` to avoid out-of-memory DOS problems. let body = reqwest::Body::from(response); let limited_body = http_body_util::Limited::new(body, response_limit); let collected = limited_body @@ -253,6 +256,8 @@ impl HttpClient { return Err(HttpResponseException::new(status, bytes)); } + // Because of the `pyo3` `bytes` feature, we can pass this back to Python + // land efficiently Ok(bytes) }) } From 521f3e75aabbc9d72ccc738da0724eaaa23898ff Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 27 Feb 2026 16:27:46 -0600 Subject: [PATCH 13/13] Restore comments --- rust/src/http_client.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rust/src/http_client.rs b/rust/src/http_client.rs index 203d75fb68f..398ba9041f2 100644 --- a/rust/src/http_client.rs +++ b/rust/src/http_client.rs @@ -235,6 +235,9 @@ impl HttpClient { let status = response.status(); + // A light-weight way to read the response up until the `response_limit`. We + // want to avoid allocating a giant response object on the server above our + // expected `response_limit` to avoid out-of-memory DOS problems. let body = reqwest::Body::from(response); let limited_body = http_body_util::Limited::new(body, response_limit); let collected = limited_body @@ -253,6 +256,8 @@ impl HttpClient { return Err(HttpResponseException::new(status, bytes)); } + // Because of the `pyo3` `bytes` feature, we can pass this back to Python + // land efficiently Ok(bytes) }) }