Fix Connection and Expect header parsing

actix-http/CHANGES.md

@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+- Parse all HTTP/1 `Connection` header options consistently. [#2692]
+- Only treat `Expect: 100-continue` as a continue expectation in HTTP/1.1 requests.
+
+[#2692]: https://github.com/actix/actix-web/issues/2692
+
 ## 3.13.5
 
 - Reject invalid WebSocket close-frame status codes, malformed payloads, and invalid UTF-8 close reasons.

actix-http/src/builder.rs

@@ -206,7 +206,7 @@ where
 
     /// Provide service for `EXPECT: 100-Continue` support.
     ///
-    /// Service get called with request that contains `EXPECT` header.
+    /// Service get called with request that contains `EXPECT: 100-Continue` header.
     /// Service must return request in case of success, in that case
     /// request will be forwarded to main service.
     pub fn expect<F, X1>(self, expect: F) -> HttpServiceBuilder<T, S, X1, U>

actix-http/src/encoding/encoder.rs

@@ -366,7 +366,7 @@ impl ContentEncoder {
             ContentEncoder::Brotli(ref mut encoder) => match encoder.write_all(data) {
                 Ok(_) => Ok(()),
                 Err(err) => {
-                    trace!("Error decoding br encoding: {}", err);
+                    trace!("Error encoding br data: {}", err);
                     Err(err)
                 }
             },
@@ -375,7 +375,7 @@ impl ContentEncoder {
             ContentEncoder::Gzip(ref mut encoder) => match encoder.write_all(data) {
                 Ok(_) => Ok(()),
                 Err(err) => {
-                    trace!("Error decoding gzip encoding: {}", err);
+                    trace!("Error encoding gzip data: {}", err);
                     Err(err)
                 }
             },
@@ -384,7 +384,7 @@ impl ContentEncoder {
             ContentEncoder::Deflate(ref mut encoder) => match encoder.write_all(data) {
                 Ok(_) => Ok(()),
                 Err(err) => {
-                    trace!("Error decoding deflate encoding: {}", err);
+                    trace!("Error encoding deflate data: {}", err);
                     Err(err)
                 }
             },
@@ -393,7 +393,7 @@ impl ContentEncoder {
             ContentEncoder::Zstd(ref mut encoder) => match encoder.write_all(data) {
                 Ok(_) => Ok(()),
                 Err(err) => {
-                    trace!("Error decoding ztsd encoding: {}", err);
+                    trace!("Error encoding zstd data: {}", err);
                     Err(err)
                 }
             },

actix-http/src/h1/decoder.rs

@@ -151,19 +151,21 @@ pub(crate) trait MessageType: Sized {
 
                     // connection keep-alive state
                     header::CONNECTION => {
-                        ka = if let Ok(conn) = value.to_str().map(str::trim) {
-                            if conn.eq_ignore_ascii_case("keep-alive") {
-                                Some(ConnectionType::KeepAlive)
-                            } else if conn.eq_ignore_ascii_case("close") {
-                                Some(ConnectionType::Close)
-                            } else if conn.eq_ignore_ascii_case("upgrade") {
-                                Some(ConnectionType::Upgrade)
-                            } else {
-                                None
+                        if let Ok(conn) = value.to_str() {
+                            for option in conn.split(',').map(str::trim) {
+                                if option.eq_ignore_ascii_case("close") {
+                                    ka = Some(ConnectionType::Close);
+                                    break;
+                                } else if option.eq_ignore_ascii_case("upgrade")
+                                    && ka != Some(ConnectionType::Close)
+                                {
+                                    ka = Some(ConnectionType::Upgrade);
+                                } else if option.eq_ignore_ascii_case("keep-alive") && ka.is_none()
+                                {
+                                    ka = Some(ConnectionType::KeepAlive);
+                                }
                             }
-                        } else {
-                            None
-                        };
+                        }
                     }
 
                     header::UPGRADE => {
@@ -174,11 +176,27 @@ pub(crate) trait MessageType: Sized {
                         }
                     }
 
-                    header::EXPECT => {
-                        let bytes = value.as_bytes();
-                        if bytes.len() >= 4 && &bytes[0..4] == b"100-" {
-                            expect = true;
-                        }
+                    header::EXPECT if version == Version::HTTP_11 => {
+                        let mut quoted = false;
+                        let mut escaped = false;
+                        expect = expect
+                            || value
+                                .as_bytes()
+                                .split(|&byte| {
+                                    if escaped {
+                                        escaped = false;
+                                    } else if quoted && byte == b'\\' {
+                                        escaped = true;
+                                    } else if byte == b'"' {
+                                        quoted = !quoted;
+                                    } else {
+                                        return byte == b',' && !quoted;
+                                    }
+                                    false
+                                })
+                                .any(|item| {
+                                    item.trim_ascii().eq_ignore_ascii_case(b"100-continue")
+                                });
                     }
 
                     _ => {}
@@ -848,6 +866,30 @@ mod tests {
         assert_eq!(val[1], "c2=cookie2");
     }
 
+    #[test]
+    fn test_expect_100_continue() {
+        for (value, expected) in [
+            ("100-custom, 100-Continue", true),
+            ("100-custom", false),
+            (r#"custom="foo, 100-continue, bar""#, false),
+            (r#"custom="foo\", 100-continue, bar""#, false),
+            (r#"custom="foo\\", 100-Continue"#, true),
+            (r#"custom="é, bar", 100-Continue"#, true),
+        ] {
+            let raw =
+                format!("POST /test HTTP/1.1\r\ncontent-length: 1\r\nexpect: {value}\r\n\r\n");
+            let req = parse_ready!(&mut BytesMut::from(raw.as_str()));
+            assert_eq!(req.head().expect(), expected, "{value:?}");
+        }
+
+        let req = parse_ready!(&mut BytesMut::from(
+            "POST /test HTTP/1.0\r\n\
+             content-length: 1\r\n\
+             expect: 100-continue\r\n\r\n",
+        ));
+        assert!(!req.head().expect());
+    }
+
     #[test]
     fn test_conn_default_1_0() {
         let req = parse_ready!(&mut BytesMut::from("GET /test HTTP/1.0\r\n\r\n"));
@@ -947,6 +989,29 @@ mod tests {
         assert_eq!(req.head().connection_type(), ConnectionType::Upgrade);
     }
 
+    #[test]
+    fn test_conn_multi_value() {
+        for (connection, expected) in [
+            ("keep-alive, Upgrade", ConnectionType::Upgrade),
+            ("keep-alive\r\nconnection: Upgrade", ConnectionType::Upgrade),
+            ("close, upgrade", ConnectionType::Close),
+            ("upgrade, close", ConnectionType::Close),
+            ("close\r\nconnection: upgrade", ConnectionType::Close),
+            ("upgrade\r\nconnection: close", ConnectionType::Close),
+            ("not-upgrade", ConnectionType::KeepAlive),
+        ] {
+            let raw = format!("GET /test HTTP/1.1\r\nconnection: {connection}\r\n\r\n");
+            let req = parse_ready!(&mut BytesMut::from(raw.as_str()));
+            assert_eq!(req.head().connection_type(), expected, "{connection:?}");
+            assert_eq!(
+                req.upgrade(),
+                expected == ConnectionType::Upgrade,
+                "{connection:?}"
+            );
+            assert_eq!(req.head().upgrade(), req.upgrade(), "{connection:?}");
+        }
+    }
+
     #[test]
     fn test_conn_upgrade_connect_method() {
         let req = parse_ready!(&mut BytesMut::from(
@@ -1032,12 +1097,7 @@ mod tests {
         );
         let mut reader = MessageDecoder::<Request>::default();
         let (req, pl) = reader.decode(&mut buf).unwrap().unwrap();
-        // `connection: upgrade, http2-settings` doesn't work properly..
-        // see MessageType::set_headers().
-        //
-        // The line below should be:
-        // assert_eq!(req.head().connection_type(), ConnectionType::Upgrade);
-        assert_eq!(req.head().connection_type(), ConnectionType::KeepAlive);
+        assert_eq!(req.head().connection_type(), ConnectionType::Upgrade);
         assert!(req.upgrade());
         assert!(!pl.is_unhandled());
     }

actix-http/src/requests/head.rs

@@ -107,16 +107,21 @@ impl RequestHead {
 
     /// Connection upgrade status
     pub fn upgrade(&self) -> bool {
-        self.headers()
-            .get(header::CONNECTION)
-            .map(|hdr| {
-                if let Ok(s) = hdr.to_str() {
-                    s.to_ascii_lowercase().contains("upgrade")
-                } else {
-                    false
+        let mut upgrade = false;
+
+        for conn in self.headers().get_all(header::CONNECTION) {
+            if let Ok(conn) = conn.to_str() {
+                for option in conn.split(',').map(str::trim) {
+                    if option.eq_ignore_ascii_case("close") {
+                        return false;
+                    } else if option.eq_ignore_ascii_case("upgrade") {
+                        upgrade = true;
+                    }
                 }
-            })
-            .unwrap_or(false)
+            }
+        }
+
+        upgrade
     }
 
     #[inline]

actix-http/src/requests/request.rs

@@ -160,9 +160,9 @@ impl<P> Request<P> {
     /// Check if request requires connection upgrade
     #[inline]
     pub fn upgrade(&self) -> bool {
-        if let Some(conn) = self.head().headers.get(header::CONNECTION) {
-            if let Ok(s) = conn.to_str() {
-                return s.to_lowercase().contains("upgrade");
+        for conn in self.head().headers.get_all(header::CONNECTION) {
+            if conn.to_str().is_ok() {
+                return self.head().upgrade();
             }
         }
         self.head().method == Method::CONNECT

actix-http/src/service.rs

@@ -139,8 +139,8 @@ where
 {
     /// Sets service for `Expect: 100-Continue` handling.
     ///
-    /// An expect service is called with requests that contain an `Expect` header. A successful
-    /// response type is also a request which will be forwarded to the main service.
+    /// An expect service is called with requests that contain an `Expect: 100-Continue` header. A
+    /// successful response type is also a request which will be forwarded to the main service.
     pub fn expect<X1>(self, expect: X1) -> HttpService<T, S, B, X1, U>
     where
         X1: ServiceFactory<Request, Config = (), Response = Request>,

actix-http/src/ws/mod.rs

@@ -331,8 +331,9 @@ mod tests {
             ))
             .insert_header((
                 header::CONNECTION,
-                header::HeaderValue::from_static("upgrade"),
+                header::HeaderValue::from_static("keep-alive"),
             ))
+            .append_header((header::CONNECTION, "Upgrade"))
             .insert_header((
                 header::SEC_WEBSOCKET_VERSION,
                 header::HeaderValue::from_static("13"),
@@ -344,7 +345,7 @@ mod tests {
             .finish();
         assert_eq!(
             StatusCode::SWITCHING_PROTOCOLS,
-            handshake_response(req.head()).finish().status()
+            handshake(req.head()).unwrap().finish().status()
         );
     }
 

actix-web/src/http/header/last_modified.rs

@@ -11,7 +11,7 @@ crate::http::header::common_header! {
     ///
     /// # ABNF
     /// ```plain
-    /// Expires = HTTP-date
+    /// Last-Modified = HTTP-date
     /// ```
     ///
     /// # Example Values

awc/CHANGES.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Parse all `Connection` header options when validating WebSocket handshakes.
 - Add camel-case header controls to `WebsocketsRequest` via `camel_case_headers()` and `set_camel_case_headers()`. [#3953]
 - Update `hickory-resolver` dependency to `0.26.1`.
 - Update `rand` dependency to `0.10`.

awc/src/ws.rs

@@ -385,19 +385,27 @@ impl WebsocketsRequest {
         }
 
         // Check for "CONNECTION" header
-        if let Some(conn) = head.headers.get(&header::CONNECTION) {
-            if let Ok(s) = conn.to_str() {
-                if !s.to_ascii_lowercase().contains("upgrade") {
-                    log::trace!("Invalid connection header: {}", s);
-                    return Err(WsClientError::InvalidConnectionHeader(conn.clone()));
+        let mut upgrade = false;
+        for conn in head.headers.get_all(header::CONNECTION) {
+            if let Ok(value) = conn.to_str() {
+                for option in value.split(',').map(str::trim) {
+                    if option.eq_ignore_ascii_case("close") {
+                        log::trace!("Invalid connection header: {:?}", conn);
+                        return Err(WsClientError::InvalidConnectionHeader(conn.clone()));
+                    } else if option.eq_ignore_ascii_case("upgrade") {
+                        upgrade = true;
+                    }
                 }
-            } else {
+            }
+        }
+        if !upgrade {
+            if let Some(conn) = head.headers.get(header::CONNECTION) {
                 log::trace!("Invalid connection header: {:?}", conn);
                 return Err(WsClientError::InvalidConnectionHeader(conn.clone()));
+            } else {
+                log::trace!("Missing connection header");
+                return Err(WsClientError::MissingConnectionHeader);
             }
-        } else {
-            log::trace!("Missing connection header");
-            return Err(WsClientError::MissingConnectionHeader);
         }
 
         if let Some(hdr_key) = head.headers.get(&header::SEC_WEBSOCKET_ACCEPT) {