Fix pipelining with default answers

- Fix non idempotent connect_to_backend (probably suboptimal) - Explicitely exclude body when no length info is received, add a `Content-Length: 0` to avoid message smuggling (may break some apps?) Signed-off-by: Eloi DEMOLIS <[email protected]>
sozu-proxy · Jan 14, 2025 · 309c091 · 309c091
1 parent 0471a4c
commit 309c091
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 16 deletions.
diff --git a/lib/src/protocol/kawa_h1/editor.rs b/lib/src/protocol/kawa_h1/editor.rs
@@ -83,6 +83,14 @@ impl HttpContext {
     ///   - sticky cookie
     ///   - user-agent
     fn on_request_headers(&mut self, request: &mut GenericHttpStream) {
+        if request.body_size == kawa::BodySize::Empty {
+            request.parsing_phase = kawa::ParsingPhase::Terminated;
+            request.push_block(kawa::Block::Header(kawa::Pair {
+                key: kawa::Store::Static(b"Content-Length"),
+                val: kawa::Store::Static(b"0"),
+            }));
+        };
+
         let buf = &mut request.storage.mut_buffer();
 
         // Captures the request line
@@ -104,10 +112,6 @@ impl HttpContext {
                 .map(ToOwned::to_owned);
         }
 
-        // if self.method == Some(Method::Get) && request.body_size == kawa::BodySize::Empty {
-        //     request.parsing_phase = kawa::ParsingPhase::Terminated;
-        // }
-
         let public_ip = self.public_address.ip();
         let public_port = self.public_address.port();
         let proto = match self.protocol {

diff --git a/lib/src/protocol/kawa_h1/mod.rs b/lib/src/protocol/kawa_h1/mod.rs
@@ -1461,24 +1461,28 @@ impl<Front: SocketHandler, L: ListenerHandler + L7ListenerHandler> Http<Front, L
     ) -> Result<BackendConnectAction, BackendConnectionError> {
         self.check_circuit_breaker()?;
 
-        // TODO: this is called on every connect attempt, not just the first
-        // cluster_id is determined from the triplet (method, authority, path)
-        // which doesn't ever change for a request
-        // this is wasteful and potentially dangerous with the rewrite logic
-        let cluster_id = self
-            .cluster_id_from_request(proxy.clone())
-            .map_err(BackendConnectionError::RetrieveClusterError)?;
+        let cluster_id = if self.connection_attempts == 0 {
+            // cluster_id is determined from the triplet (method, authority, path)
+            // which doesn't ever change for a request
+            // WARNING: cluster_id_from_request is NOT idempotent, but connect_to_backend SHOULD be
+            self
+                .cluster_id_from_request(proxy.clone())
+                .map_err(BackendConnectionError::RetrieveClusterError)?
+        } else if let Some(cluster_id) = self.context.cluster_id.take() {
+            cluster_id
+        } else {
+            unreachable!();
+        };
+
+        // update response cluster producer
+        self.context.cluster_id = Some(cluster_id.clone());
 
         trace!(
-            "{} {:?} Connect_to_backend: {:?}",
+            "{} Connect_to_backend: {:?}",
             log_context!(self),
-            self.origin,
             cluster_id,
         );
 
-        // update response cluster producer
-        self.context.cluster_id = Some(cluster_id.clone());
-
         // check if we can reuse the backend connection
         if let Some(origin) = &self.origin {
             if origin.is_connected_to(&cluster_id) {