feat(ext): support non-canonical HTTP/1 reason phrases

Add a new extension type `hyper::ext::ReasonPhrase` gated by a new feature flag
`http1_reason_phrase`. When enabled, store any non-canonical reason phrases in this extension when
parsing responses, and write this reason phrase instead of the canonical reason phrase when emitting
responses.

Reason phrases are a disused corner of the spec that implementations ought to treat as opaque blobs
of bytes. Unfortunately, real-world traffic sometimes does depend on being able to inspect and
manipulate them.

My main outstanding question is: should this new feature flag be included in `full`? Excluding it
means this patch also has to modify the CI configuration in order to run the `client` and `server`
tests.

.github/workflows/CI.yml

@@ -61,11 +61,11 @@ jobs:
 
         include:
           - rust: stable
-            features: "--features full"
+            features: "--features full,http1_reason_phrase"
           - rust: beta
-            features: "--features full"
+            features: "--features full,http1_reason_phrase"
           - rust: nightly
-            features: "--features full,nightly"
+            features: "--features full,http1_reason_phrase,nightly"
             benches: true
 
     runs-on: ${{ matrix.os }}

Cargo.toml

@@ -86,6 +86,9 @@ full = [
 http1 = []
 http2 = ["h2"]
 
+# Support for non-canonical HTTP/1 reason phrases
+http1_reason_phrase = ["http1"]
+
 # Client/Server
 client = []
 server = []
@@ -239,7 +242,7 @@ required-features = ["full"]
 [[test]]
 name = "client"
 path = "tests/client.rs"
-required-features = ["full"]
+required-features = ["full", "http1_reason_phrase"]
 
 [[test]]
 name = "integration"
@@ -249,4 +252,4 @@ required-features = ["full"]
 [[test]]
 name = "server"
 path = "tests/server.rs"
-required-features = ["full"]
+required-features = ["full", "http1_reason_phrase"]

src/ext.rs

@@ -7,6 +7,11 @@ use http::HeaderMap;
 #[cfg(feature = "http2")]
 use std::fmt;
 
+#[cfg(any(feature = "http1_reason_phrase", feature = "ffi"))]
+mod h1_reason_phrase;
+#[cfg(any(feature = "http1_reason_phrase", feature = "ffi"))]
+pub use h1_reason_phrase::ReasonPhrase;
+
 #[cfg(feature = "http2")]
 /// Represents the `:protocol` pseudo-header used by
 /// the [Extended CONNECT Protocol].

src/ext/h1_reason_phrase.rs

@@ -0,0 +1,101 @@
+use std::str::Utf8Error;
+
+use bytes::Bytes;
+
+/// A reason phrase in an HTTP/1 response.
+///
+/// # Clients
+///
+/// For clients, a `ReasonPhrase` will be present in the extensions of the `http::Response` returned
+/// for a request if the reason phrase is different from the canonical reason phrase for the
+/// response's status code. For example, if a server returns `HTTP/1.1 200 Awesome`, the
+/// `ReasonPhrase` will be present and contain `Awesome`, but if a server returns `HTTP/1.1 200 OK`,
+/// the response will not contain a `ReasonPhrase`.
+///
+/// ```no_run
+/// # #[cfg(all(feature = "tcp", feature = "client", feature = "http1_reason_phrase"))]
+/// # async fn fake_fetch() -> hyper::Result<()> {
+/// use hyper::{Client, Uri};
+/// use hyper::ext::ReasonPhrase;
+///
+/// let res = Client::new().get(Uri::from_static("http://example.com/non_canonical_reason")).await?;
+///
+/// // Print out the non-canonical reason phrase, if it has one...
+/// if let Some(reason) = res.extensions().get::<ReasonPhrase>() {
+///     println!("non-canonical reason: {}", reason.to_str().unwrap());
+/// }
+/// # Ok(())
+/// # }
+/// ```
+///
+/// # Servers
+///
+/// When a `ReasonPhrase` is present in the extensions of the `http::Response` written by a server,
+/// its contents will be written in place of the canonical reason phrase when responding via HTTP/1.
+#[cfg(any(feature = "http1_reason_phrase", feature = "ffi"))]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct ReasonPhrase(pub(crate) Bytes);
+
+impl ReasonPhrase {
+    /// Gets the reason phrase as bytes.
+    pub fn as_bytes(&self) -> &[u8] {
+        &self.0
+    }
+
+    /// Gets the reason phrase as a `&str` if the phrase is valid UTF-8.
+    pub fn to_str(&self) -> Result<&str, Utf8Error> {
+        std::str::from_utf8(self.as_bytes())
+    }
+
+    /// Converts a static byte slice to a reason phrase.
+    pub const fn from_static(reason: &'static [u8]) -> Self {
+        Self(Bytes::from_static(reason))
+    }
+
+    /// Converts a static string to a reason phrase.
+    pub const fn from_static_str(reason: &'static str) -> Self {
+        Self(Bytes::from_static(reason.as_bytes()))
+    }
+}
+
+impl From<&'static [u8]> for ReasonPhrase {
+    fn from(reason: &'static [u8]) -> Self {
+        Self(reason.into())
+    }
+}
+
+impl From<&'static str> for ReasonPhrase {
+    fn from(reason: &'static str) -> Self {
+        Self(reason.into())
+    }
+}
+
+impl From<Vec<u8>> for ReasonPhrase {
+    fn from(reason: Vec<u8>) -> Self {
+        Self(reason.into())
+    }
+}
+
+impl From<String> for ReasonPhrase {
+    fn from(reason: String) -> Self {
+        Self(reason.into())
+    }
+}
+
+impl From<Bytes> for ReasonPhrase {
+    fn from(reason: Bytes) -> Self {
+        Self(reason)
+    }
+}
+
+impl Into<Bytes> for ReasonPhrase {
+    fn into(self) -> Bytes {
+        self.0
+    }
+}
+
+impl AsRef<[u8]> for ReasonPhrase {
+    fn as_ref(&self) -> &[u8] {
+        &self.0
+    }
+}

src/ffi/http_types.rs

@@ -6,7 +6,7 @@ use super::body::{hyper_body, hyper_buf};
 use super::error::hyper_code;
 use super::task::{hyper_task_return_type, AsTaskType};
 use super::{UserDataPointer, HYPER_ITER_CONTINUE};
-use crate::ext::HeaderCaseMap;
+use crate::ext::{HeaderCaseMap, ReasonPhrase};
 use crate::header::{HeaderName, HeaderValue};
 use crate::{Body, HeaderMap, Method, Request, Response, Uri};
 
@@ -24,9 +24,6 @@ pub struct hyper_headers {
     orig_casing: HeaderCaseMap,
 }
 
-#[derive(Debug)]
-pub(crate) struct ReasonPhrase(pub(crate) Bytes);
-
 pub(crate) struct RawHeaders(pub(crate) hyper_buf);
 
 pub(crate) struct OnInformational {

src/proto/h1/role.rs

@@ -358,7 +358,17 @@ impl Http1Transaction for Server {
 
         let init_cap = 30 + msg.head.headers.len() * AVERAGE_HEADER_SIZE;
         dst.reserve(init_cap);
-        if msg.head.version == Version::HTTP_11 && msg.head.subject == StatusCode::OK {
+
+        #[cfg(feature = "http1_reason_phrase")]
+        let custom_reason_phrase = msg.head.extensions.get::<crate::ext::ReasonPhrase>();
+        #[cfg(not(feature = "http1_reason_phrase"))]
+        #[allow(non_upper_case_globals)]
+        const custom_reason_phrase: Option<()> = None;
+
+        if msg.head.version == Version::HTTP_11
+            && msg.head.subject == StatusCode::OK
+            && custom_reason_phrase.is_none()
+        {
             extend(dst, b"HTTP/1.1 200 OK\r\n");
         } else {
             match msg.head.version {
@@ -373,15 +383,26 @@ impl Http1Transaction for Server {
 
             extend(dst, msg.head.subject.as_str().as_bytes());
             extend(dst, b" ");
-            // a reason MUST be written, as many parsers will expect it.
-            extend(
-                dst,
-                msg.head
-                    .subject
-                    .canonical_reason()
-                    .unwrap_or("<none>")
-                    .as_bytes(),
-            );
+
+            if custom_reason_phrase.is_none() {
+                // a reason MUST be written, as many parsers will expect it.
+                extend(
+                    dst,
+                    msg.head
+                        .subject
+                        .canonical_reason()
+                        .unwrap_or("<none>")
+                        .as_bytes(),
+                );
+            }
+            // This is mutually exclusive with the canonical reason phrase logic above, but due to
+            // having to work around `cfg` attributes it is in a distinct conditional rather than an
+            // `else`.
+            #[cfg(feature = "http1_reason_phrase")]
+            if let Some(crate::ext::ReasonPhrase(reason)) = custom_reason_phrase {
+                extend(dst, reason);
+            }
+
             extend(dst, b"\r\n");
         }
 
@@ -918,9 +939,9 @@ impl Http1Transaction for Client {
                         trace!("Response.parse Complete({})", len);
                         let status = StatusCode::from_u16(res.code.unwrap())?;
 
-                        #[cfg(not(feature = "ffi"))]
+                        #[cfg(not(any(feature = "http1_reason_phrase", feature = "ffi")))]
                         let reason = ();
-                        #[cfg(feature = "ffi")]
+                        #[cfg(any(feature = "http1_reason_phrase", feature = "ffi"))]
                         let reason = {
                             let reason = res.reason.unwrap();
                             // Only save the reason phrase if it isnt the canonical reason
@@ -944,9 +965,9 @@ impl Http1Transaction for Client {
                     Err(httparse::Error::Version) if ctx.h09_responses => {
                         trace!("Response.parse accepted HTTP/0.9 response");
 
-                        #[cfg(not(feature = "ffi"))]
+                        #[cfg(not(any(feature = "http1_reason_phrase", feature = "ffi")))]
                         let reason = ();
-                        #[cfg(feature = "ffi")]
+                        #[cfg(any(feature = "http1_reason_phrase", feature = "ffi"))]
                         let reason = None;
 
                         (0, StatusCode::OK, reason, Version::HTTP_09, 0)
@@ -1012,11 +1033,11 @@ impl Http1Transaction for Client {
                 extensions.insert(header_case_map);
             }
 
-            #[cfg(feature = "ffi")]
+            #[cfg(any(feature = "http1_reason_phrase", feature = "ffi"))]
             if let Some(reason) = reason {
-                extensions.insert(crate::ffi::ReasonPhrase(reason));
+                extensions.insert(crate::ext::ReasonPhrase(reason));
             }
-            #[cfg(not(feature = "ffi"))]
+            #[cfg(not(any(feature = "http1_reason_phrase", feature = "ffi")))]
             drop(reason);
 
             #[cfg(feature = "ffi")]

tests/client.rs

@@ -2271,6 +2271,63 @@ mod conn {
         future::join(server, client).await;
     }
 
+    #[tokio::test]
+    async fn get_custom_reason_phrase() {
+        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 Alright\r\nContent-Length: 0\r\n\r\n")
+                .await
+                .unwrap();
+        };
+
+        let client = async move {
+            let tcp = tcp_connect(&addr).await.expect("connect");
+            let (mut client, conn) = conn::handshake(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.extensions()
+                    .get::<hyper::ext::ReasonPhrase>()
+                    .expect("custom reason phrase is present")
+                    .to_str()
+                    .expect("reason phrase is utf-8"),
+                "Alright"
+            );
+            assert_eq!(res.headers().len(), 1);
+            assert_eq!(
+                res.headers().get(http::header::CONTENT_LENGTH).unwrap(),
+                "0"
+            );
+            assert!(res.body_mut().next().await.is_none());
+        };
+
+        future::join(server, client).await;
+    }
+
     #[test]
     fn incoming_content_length() {
         use hyper::body::HttpBody;

tests/server.rs

@@ -383,6 +383,33 @@ mod response_body_lengths {
     }
 }
 
+#[test]
+fn get_response_custom_reason_phrase() {
+    let _ = pretty_env_logger::try_init();
+    let server = serve();
+    server.reply().reason_phrase("Cool");
+    let mut req = connect(server.addr());
+    req.write_all(
+        b"\
+        GET / HTTP/1.1\r\n\
+        Host: example.domain\r\n\
+        Connection: close\r\n\
+        \r\n\
+    ",
+    )
+    .unwrap();
+
+    let mut response = String::new();
+    req.read_to_string(&mut response).unwrap();
+
+    let mut lines = response.lines();
+    assert_eq!(lines.next(), Some("HTTP/1.1 200 Cool"));
+
+    let mut lines = lines.skip_while(|line| !line.is_empty());
+    assert_eq!(lines.next(), Some(""));
+    assert_eq!(lines.next(), None);
+}
+
 #[test]
 fn get_chunked_response_with_ka() {
     let foo_bar = b"foo bar baz";
@@ -2651,6 +2678,15 @@ impl<'a> ReplyBuilder<'a> {
         self
     }
 
+    fn reason_phrase(self, reason: &str) -> Self {
+        self.tx
+            .lock()
+            .unwrap()
+            .send(Reply::ReasonPhrase(Bytes::from(reason.to_owned())))
+            .unwrap();
+        self
+    }
+
     fn version(self, version: hyper::Version) -> Self {
         self.tx
             .lock()
@@ -2724,6 +2760,7 @@ struct TestService {
 #[derive(Debug)]
 enum Reply {
     Status(hyper::StatusCode),
+    ReasonPhrase(Bytes),
     Version(hyper::Version),
     Header(HeaderName, HeaderValue),
     Body(hyper::Body),
@@ -2779,6 +2816,10 @@ impl TestService {
                 Reply::Status(s) => {
                     *res.status_mut() = s;
                 }
+                Reply::ReasonPhrase(reason) => {
+                    res.extensions_mut()
+                        .insert(hyper::ext::ReasonPhrase::from(reason));
+                }
                 Reply::Version(v) => {
                     *res.version_mut() = v;
                 }