From 7325f77060d5102172f423882a3b3f07ebf1703e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Augusto=20C=C3=A9sar?= Date: Mon, 30 Mar 2026 09:24:35 +0200 Subject: [PATCH 1/2] fix: only use manual conversion for DELETE with content ### Description On #263 we changed the conversion of the request from the internal try_into from workers-rs for a manual one to be able to proxy DELETE requests with body. This caused the performance issues reported and with a proposed fix on #268. The changes on #268 however re-introduced the original issue that #263 was attempting to fix, as in it's core, it is what the `try_into` does. As a middle ground, this PR proposes a workaround for the DELETE with body without compromising the performance of other requests. --- worker/src/lib.rs | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/worker/src/lib.rs b/worker/src/lib.rs index 8d12e55..979b451 100644 --- a/worker/src/lib.rs +++ b/worker/src/lib.rs @@ -331,9 +331,28 @@ async fn linkup_request_handler( req.headers_mut().remove(http::header::HOST); linkup::normalize_cookie_header(req.headers_mut()); - let upstream_request = match convert_request(req).await { - Ok(worker_request) => worker_request, - Err(error) => return error.into_response(), + let upstream_request = if req.headers().contains_key(http::header::CONTENT_LENGTH) + && req.method() == http::Method::DELETE + { + // NOTE(augustoccesar)[2026-03-30]: This exists only to handle some non-compliant + // DELETE requests and should be removed as soon as possible. + // Ideally only the `else` branch of this should exist where we delegate the conversion + // to workers-rs. + match convert_request(req).await { + Ok(worker_request) => worker_request, + Err(error) => return error.into_response(), + } + } else { + match req.try_into() { + Ok(req) => req, + Err(e) => { + return HttpError::new( + format!("Failed to parse request: {}", e), + StatusCode::BAD_REQUEST, + ) + .into_response() + } + } }; let cacheable_req = is_cacheable_request(&upstream_request, &config); @@ -391,15 +410,11 @@ async fn linkup_request_handler( } } +// TODO(augustoccesar)[2025-03-30]: Deprecate this once non-compliant DELETE endpoints are removed. // NOTE(augustoccesar)[2026-03-19]: The reason to build this manually instead of using the TryFrom for // the worker::Request is because of how body is constructed. // We were seeing body for DELETE requests not being proxied correctly. Because of that we are now // doing the reconstruct manually to ensure the body is present. -// -// This is not ideal and would be great to keep trusting the `to_wasm` from the workers-rs, -// but for now this solves our issue. -// Main concern here is if we might be missing some of the other internal operations that are -// done during the conversion. But for our usecases, it seems to be working. async fn convert_request( req: http::Request, ) -> Result { From 3377740409351e6e679acc081f1932b9b5760c34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Augusto=20C=C3=A9sar?= Date: Mon, 30 Mar 2026 09:27:24 +0200 Subject: [PATCH 2/2] refactor: change order of checks --- worker/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/worker/src/lib.rs b/worker/src/lib.rs index 979b451..9b22922 100644 --- a/worker/src/lib.rs +++ b/worker/src/lib.rs @@ -331,8 +331,8 @@ async fn linkup_request_handler( req.headers_mut().remove(http::header::HOST); linkup::normalize_cookie_header(req.headers_mut()); - let upstream_request = if req.headers().contains_key(http::header::CONTENT_LENGTH) - && req.method() == http::Method::DELETE + let upstream_request = if req.method() == http::Method::DELETE + && req.headers().contains_key(http::header::CONTENT_LENGTH) { // NOTE(augustoccesar)[2026-03-30]: This exists only to handle some non-compliant // DELETE requests and should be removed as soon as possible.