Fix manual framing incorrectly falling back to automatic mode

When using manual framing with only a content-length or transfer encoding header set, viceroy was falling back to auto mode

Author: tyler-goodwin

cli/tests/integration/http_semantics.rs

@@ -39,6 +39,32 @@ viceroy_test!(framing_headers_are_overridden, |is_component| {
     Ok(())
 });
 
+// Regression test: ManuallyFromHeaders mode should preserve Content-Length header.
+// Previously, the header was stripped because absent Transfer-Encoding was treated as invalid.
+viceroy_test!(manual_framing_preserves_content_length, |is_component| {
+    let test = Test::using_fixture("manual-framing-headers.wasm")
+        .adapt_component(is_component)
+        .backend("TheOrigin", "/", None, |req| {
+            // Manual mode should preserve Content-Length, not strip it
+            assert!(
+                !req.headers().contains_key(header::CONTENT_LENGTH),
+                "Content-Length was incorrectly added"
+            );
+            // Transfer-Encoding should be preserved
+            assert_eq!(
+                req.headers().get(header::TRANSFER_ENCODING),
+                Some(&hyper::header::HeaderValue::from_static("chunked")),
+                "Transfer-Encoding should have been preserved"
+            );
+            Response::new(Vec::new())
+        })
+        .await;
+
+    let resp = test.via_hyper().against_empty().await?;
+    assert_eq!(resp.status(), StatusCode::OK);
+    Ok(())
+});
+
 viceroy_test!(content_length_is_computed_correctly, |is_component| {
     // Set up the test harness
     let test = Test::using_fixture("content-length.wasm")

src/framing.rs

@@ -5,26 +5,22 @@ use http::{header, HeaderMap};
 pub fn content_length_is_valid(headers: &HeaderMap) -> bool {
     let mut values = headers.get_all(header::CONTENT_LENGTH).iter();
 
-    if let Some(val) = values.next() {
-        if val.as_bytes().iter().all(|b| b.is_ascii_digit()) && values.next().is_none() {
-            return true;
-        }
+    match values.next() {
+        None => true,
+        Some(val) => val.as_bytes().iter().all(|b| b.is_ascii_digit()) && values.next().is_none(),
     }
-    false
 }
 
 pub fn transfer_encoding_is_supported(headers: &HeaderMap) -> bool {
     let mut values = headers.get_all(header::TRANSFER_ENCODING).iter();
 
-    if let Some(val) = values.next() {
-        if val
-            .to_str()
-            .map(|s| s.eq_ignore_ascii_case("chunked"))
-            .unwrap_or(false)
-            && values.next().is_none()
-        {
-            return true;
+    match values.next() {
+        None => true,
+        Some(val) => {
+            val.to_str()
+                .map(|s| s.eq_ignore_ascii_case("chunked"))
+                .unwrap_or(false)
+                && values.next().is_none()
         }
     }
-    false
 }

src/session/downstream.rs

@@ -67,12 +67,20 @@ impl DownstreamResponseState {
 
         if framing_headers_mode == FramingHeadersMode::ManuallyFromHeaders {
             if !content_length_is_valid(response.headers()) {
-                tracing::warn!("Downstream response has invalid Content-Length header, falling back to automatic framing.");
+                tracing::warn!("Downstream response has malformed Content-Length header, falling back to automatic framing.");
                 framing_headers_mode = FramingHeadersMode::Automatic;
-            }
-            if !transfer_encoding_is_supported(response.headers()) {
+            } else if !transfer_encoding_is_supported(response.headers()) {
                 tracing::warn!("Downstream response has unsupported Transfer-Encoding header, falling back to automatic framing.");
                 framing_headers_mode = FramingHeadersMode::Automatic;
+            } else if !response
+                .headers()
+                .contains_key(hyper::header::CONTENT_LENGTH)
+                && !response
+                    .headers()
+                    .contains_key(hyper::header::TRANSFER_ENCODING)
+            {
+                tracing::warn!("Downstream response has neither Content-Length nor Transfer-Encoding header, falling back to automatic framing.");
+                framing_headers_mode = FramingHeadersMode::Automatic;
             }
         }
         if framing_headers_mode != FramingHeadersMode::ManuallyFromHeaders {

src/upstream.rs

@@ -316,12 +316,16 @@ pub fn send_request(
 
     if framing_headers_mode == FramingHeadersMode::ManuallyFromHeaders {
         if !content_length_is_valid(req.headers()) {
-            warn!("Backend request has invalid Content-Length header, falling back to automatic framing.");
+            warn!("Backend request has malformed Content-Length header, falling back to automatic framing.");
             framing_headers_mode = FramingHeadersMode::Automatic;
-        }
-        if !transfer_encoding_is_supported(req.headers()) {
+        } else if !transfer_encoding_is_supported(req.headers()) {
             warn!("Backend request has unsupported Transfer-Encoding header, falling back to automatic framing.");
             framing_headers_mode = FramingHeadersMode::Automatic;
+        } else if !req.headers().contains_key(header::CONTENT_LENGTH)
+            && !req.headers().contains_key(header::TRANSFER_ENCODING)
+        {
+            warn!("Backend request has neither Content-Length nor Transfer-Encoding header, falling back to automatic framing.");
+            framing_headers_mode = FramingHeadersMode::Automatic;
         }
     }
     if framing_headers_mode != FramingHeadersMode::ManuallyFromHeaders {

test-fixtures/src/bin/manual-framing-headers.rs

@@ -0,0 +1,24 @@
+//! Test that ManuallyFromHeaders mode preserves Transfer-Encoding header.
+//!
+//! With manual framing mode, the guest explicitly sets framing headers (Content-Length
+//! or Transfer-Encoding) and they should be preserved rather than stripped.
+//!
+//! We use a streaming body so hyper doesn't know the length upfront.
+
+use fastly::http::{header, FramingHeadersMode, HeaderValue};
+use fastly::{Error, Request, Response};
+use std::io::Write;
+
+fn main() -> Result<(), Error> {
+    let (mut stream, pending) = Request::post("http://example.org/")
+        .with_header(header::TRANSFER_ENCODING, HeaderValue::from_static("chunked"))
+        .with_framing_headers_mode(FramingHeadersMode::ManuallyFromHeaders)
+        .send_async_streaming("TheOrigin")?;
+
+    write!(stream, "test")?;
+    stream.finish()?;
+    pending.wait()?;
+
+    Response::new().send_to_client();
+    Ok(())
+}