Reject if the HTTP header contains duplicated Content-Length values.

Author: eaufavor

.bleep

@@ -1 +1 @@
-c21de982c54c255ff1893d71795ec08cf1928ade
+595e312ee6c5ab80b34cae937ff818ae880eccd2

pingora-core/src/protocols/http/v1/client.rs

@@ -166,6 +166,20 @@ impl HttpSession {
         Ok(res)
     }
 
+    // Validate the response header read. This function must be called after the response header
+    // read.
+    fn validate_response(&self) -> Result<()> {
+        let resp_header = self
+            .response_header
+            .as_ref()
+            .expect("response header must be read");
+
+        // ad-hoc checks
+        super::common::check_dup_content_length(&resp_header.headers)?;
+
+        Ok(())
+    }
+
     /// Read the response header from the server
     /// This function can be called multiple times, if the headers received are just informational
     /// headers.
@@ -287,6 +301,7 @@ impl HttpSession {
                     self.buf = buf;
                     self.upgraded = self.is_upgrade(&response_header).unwrap_or(false);
                     self.response_header = Some(response_header);
+                    self.validate_response()?;
                     return Ok(s);
                 }
                 HeaderParseState::Partial => { /* continue the loop */ }

pingora-core/src/protocols/http/v1/common.rs

@@ -16,6 +16,7 @@
 
 use http::{header, HeaderValue};
 use log::warn;
+use pingora_error::Result;
 use pingora_http::{HMap, RequestHeader, ResponseHeader};
 use std::str;
 use std::time::Duration;
@@ -243,3 +244,51 @@ pub(super) fn populate_headers(
     }
     used_header_index
 }
+
+// RFC 7230:
+// If a message is received without Transfer-Encoding and with
+// either multiple Content-Length header fields having differing
+// field-values or a single Content-Length header field having an
+// invalid value, then the message framing is invalid and the
+// recipient MUST treat it as an unrecoverable error.
+pub(super) fn check_dup_content_length(headers: &HMap) -> Result<()> {
+    if headers.get(header::TRANSFER_ENCODING).is_some() {
+        // If TE header, ignore CL
+        return Ok(());
+    }
+    let mut cls = headers.get_all(header::CONTENT_LENGTH).into_iter();
+    if cls.next().is_none() {
+        // no CL header is fine.
+        return Ok(());
+    }
+    if cls.next().is_some() {
+        // duplicated CL is bad
+        return crate::Error::e_explain(
+            crate::ErrorType::InvalidHTTPHeader,
+            "duplicated Content-Length header",
+        );
+    }
+    Ok(())
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use http::header::{CONTENT_LENGTH, TRANSFER_ENCODING};
+
+    #[test]
+    fn test_check_dup_content_length() {
+        let mut headers = HMap::new();
+
+        assert!(check_dup_content_length(&headers).is_ok());
+
+        headers.append(CONTENT_LENGTH, "1".try_into().unwrap());
+        assert!(check_dup_content_length(&headers).is_ok());
+
+        headers.append(CONTENT_LENGTH, "2".try_into().unwrap());
+        assert!(check_dup_content_length(&headers).is_err());
+
+        headers.append(TRANSFER_ENCODING, "chunkeds".try_into().unwrap());
+        assert!(check_dup_content_length(&headers).is_ok());
+    }
+}

pingora-core/src/protocols/http/v1/server.rs

@@ -247,6 +247,7 @@ impl HttpSession {
                         self.body_reader.reinit();
                         self.response_written = None;
                         self.respect_keepalive();
+                        self.validate_request()?;
 
                         return Ok(Some(s));
                     }
@@ -287,6 +288,17 @@ impl HttpSession {
         }
     }
 
+    // Validate the request header read. This function must be called after the request header
+    // read.
+    fn validate_request(&self) -> Result<()> {
+        let req_header = self.req_header();
+
+        // ad-hoc checks
+        super::common::check_dup_content_length(&req_header.headers)?;
+
+        Ok(())
+    }
+
     /// Return a reference of the `RequestHeader` this session read
     /// # Panics
     /// this function and most other functions will panic if called before [`Self::read_request()`]