From 1f0c177b35b14054eb1e5108e75f8bd3ff52813e Mon Sep 17 00:00:00 2001
From: Anthony Ramine <123095+nox@users.noreply.github.com>
Date: Wed, 9 Feb 2022 21:59:23 +0100
Subject: [PATCH] feat(http1): implement obsolete line folding (#2734)

The client now has an option to allow parsing responses with obsolete line folding in headers. The option is off by default, since the spec recommends to reject such things if you can.
---
 Cargo.toml           |  2 +-
 src/client/client.rs | 40 +++++++++++++++++++++++++++++++
 src/client/conn.rs   | 43 +++++++++++++++++++++++++++++++++
 src/proto/h1/role.rs | 16 ++++++++++++-
 tests/client.rs      | 57 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 862c20f901..6e5d9a7284 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -30,7 +30,7 @@ futures-util = { version = "0.3", default-features = false }
 http = "0.2"
 http-body = "0.4"
 httpdate = "1.0"
-httparse = "1.5.1"
+httparse = "1.6"
 h2 = { version = "0.3.9", optional = true }
 itoa = "1"
 tracing = { version = "0.1", default-features = false, features = ["std"] }
diff --git a/src/client/client.rs b/src/client/client.rs
index 3c1a843090..112a5af806 100644
--- a/src/client/client.rs
+++ b/src/client/client.rs
@@ -1000,6 +1000,9 @@ impl Builder {
     /// Set whether HTTP/1 connections will accept spaces between header names
     /// and the colon that follow them in responses.
     ///
+    /// Newline codepoints (`\r` and `\n`) will be transformed to spaces when
+    /// parsing.
+    ///
     /// You probably don't need this, here is what [RFC 7230 Section 3.2.4.] has
     /// to say about it:
     ///
@@ -1022,6 +1025,43 @@ impl Builder {
         self
     }
 
+    /// Set whether HTTP/1 connections will accept obsolete line folding for
+    /// header values.
+    ///
+    /// You probably don't need this, here is what [RFC 7230 Section 3.2.4.] has
+    /// to say about it:
+    ///
+    /// > A server that receives an obs-fold in a request message that is not
+    /// > within a message/http container MUST either reject the message by
+    /// > sending a 400 (Bad Request), preferably with a representation
+    /// > explaining that obsolete line folding is unacceptable, or replace
+    /// > each received obs-fold with one or more SP octets prior to
+    /// > interpreting the field value or forwarding the message downstream.
+    ///
+    /// > A proxy or gateway that receives an obs-fold in a response message
+    /// > that is not within a message/http container MUST either discard the
+    /// > message and replace it with a 502 (Bad Gateway) response, preferably
+    /// > with a representation explaining that unacceptable line folding was
+    /// > received, or replace each received obs-fold with one or more SP
+    /// > octets prior to interpreting the field value or forwarding the
+    /// > message downstream.
+    ///
+    /// > A user agent that receives an obs-fold in a response message that is
+    /// > not within a message/http container MUST replace each received
+    /// > obs-fold with one or more SP octets prior to interpreting the field
+    /// > value.
+    ///
+    /// Note that this setting does not affect HTTP/2.
+    ///
+    /// Default is false.
+    ///
+    /// [RFC 7230 Section 3.2.4.]: https://tools.ietf.org/html/rfc7230#section-3.2.4
+    pub fn http1_allow_obsolete_multiline_headers_in_responses(&mut self, val: bool) -> &mut Self {
+        self.conn_builder
+            .http1_allow_obsolete_multiline_headers_in_responses(val);
+        self
+    }
+
     /// Set whether HTTP/1 connections should try to use vectored writes,
     /// or always flatten into a single buffer.
     ///
diff --git a/src/client/conn.rs b/src/client/conn.rs
index 2418c9fa83..85bc366be9 100644
--- a/src/client/conn.rs
+++ b/src/client/conn.rs
@@ -615,6 +615,49 @@ impl Builder {
         self
     }
 
+    /// Set whether HTTP/1 connections will accept obsolete line folding for
+    /// header values.
+    ///
+    /// Newline codepoints (`\r` and `\n`) will be transformed to spaces when
+    /// parsing.
+    ///
+    /// You probably don't need this, here is what [RFC 7230 Section 3.2.4.] has
+    /// to say about it:
+    ///
+    /// > A server that receives an obs-fold in a request message that is not
+    /// > within a message/http container MUST either reject the message by
+    /// > sending a 400 (Bad Request), preferably with a representation
+    /// > explaining that obsolete line folding is unacceptable, or replace
+    /// > each received obs-fold with one or more SP octets prior to
+    /// > interpreting the field value or forwarding the message downstream.
+    ///
+    /// > A proxy or gateway that receives an obs-fold in a response message
+    /// > that is not within a message/http container MUST either discard the
+    /// > message and replace it with a 502 (Bad Gateway) response, preferably
+    /// > with a representation explaining that unacceptable line folding was
+    /// > received, or replace each received obs-fold with one or more SP
+    /// > octets prior to interpreting the field value or forwarding the
+    /// > message downstream.
+    ///
+    /// > A user agent that receives an obs-fold in a response message that is
+    /// > not within a message/http container MUST replace each received
+    /// > obs-fold with one or more SP octets prior to interpreting the field
+    /// > value.
+    ///
+    /// Note that this setting does not affect HTTP/2.
+    ///
+    /// Default is false.
+    ///
+    /// [RFC 7230 Section 3.2.4.]: https://tools.ietf.org/html/rfc7230#section-3.2.4
+    pub fn http1_allow_obsolete_multiline_headers_in_responses(
+        &mut self,
+        enabled: bool,
+    ) -> &mut Builder {
+        self.h1_parser_config
+            .allow_obsolete_multiline_headers_in_responses(enabled);
+        self
+    }
+
     /// Set whether HTTP/1 connections should try to use vectored writes,
     /// or always flatten into a single buffer.
     ///
diff --git a/src/proto/h1/role.rs b/src/proto/h1/role.rs
index 4b6b447608..968b63cb8e 100644
--- a/src/proto/h1/role.rs
+++ b/src/proto/h1/role.rs
@@ -955,7 +955,21 @@ impl Http1Transaction for Client {
                 }
             };
 
-            let slice = buf.split_to(len).freeze();
+            let mut slice = buf.split_to(len);
+
+            if ctx.h1_parser_config.obsolete_multiline_headers_in_responses_are_allowed() {
+                for header in &headers_indices[..headers_len] {
+                    // SAFETY: array is valid up to `headers_len`
+                    let header = unsafe { &*header.as_ptr() };
+                    for b in &mut slice[header.value.0..header.value.1] {
+                        if *b == b'\r' || *b == b'\n' {
+                            *b = b' ';
+                        }
+                    }
+                }
+            }
+
+            let slice = slice.freeze();
 
             let mut headers = ctx.cached_headers.take().unwrap_or_else(HeaderMap::new);
 
diff --git a/tests/client.rs b/tests/client.rs
index 47e827f4bd..417e9bf2d9 100644
--- a/tests/client.rs
+++ b/tests/client.rs
@@ -2214,6 +2214,63 @@ mod conn {
         future::join(server, client).await;
     }
 
+    #[tokio::test]
+    async fn get_obsolete_line_folding() {
+        let _ = ::pretty_env_logger::try_init();
+        let listener = TkTcpListener::bind(SocketAddr::from(([127, 0, 0, 1], 0)))
+            .await
+            .unwrap();
+        let addr = listener.local_addr().unwrap();
+
+        let server = async move {
+            let mut sock = listener.accept().await.unwrap().0;
+            let mut buf = [0; 4096];
+            let n = sock.read(&mut buf).await.expect("read 1");
+
+            // Notably:
+            // - Just a path, since just a path was set
+            // - No host, since no host was set
+            let expected = "GET /a HTTP/1.1\r\n\r\n";
+            assert_eq!(s(&buf[..n]), expected);
+
+            sock.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: \r\n 0\r\nLine-Folded-Header: hello\r\n world \r\n \r\n\r\n")
+                .await
+                .unwrap();
+        };
+
+        let client = async move {
+            let tcp = tcp_connect(&addr).await.expect("connect");
+            let (mut client, conn) = conn::Builder::new()
+                .http1_allow_obsolete_multiline_headers_in_responses(true)
+                .handshake::<_, Body>(tcp)
+                .await
+                .expect("handshake");
+
+            tokio::task::spawn(async move {
+                conn.await.expect("http conn");
+            });
+
+            let req = Request::builder()
+                .uri("/a")
+                .body(Default::default())
+                .unwrap();
+            let mut res = client.send_request(req).await.expect("send_request");
+            assert_eq!(res.status(), hyper::StatusCode::OK);
+            assert_eq!(res.headers().len(), 2);
+            assert_eq!(
+                res.headers().get(http::header::CONTENT_LENGTH).unwrap(),
+                "0"
+            );
+            assert_eq!(
+                res.headers().get("line-folded-header").unwrap(),
+                "hello   world"
+            );
+            assert!(res.body_mut().next().await.is_none());
+        };
+
+        future::join(server, client).await;
+    }
+
     #[test]
     fn incoming_content_length() {
         use hyper::body::HttpBody;