Skip to content

Commit

Permalink
feat(ext): support non-canonical HTTP/1 reason phrases
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acfoltzer committed Mar 23, 2022
1 parent 53f15e5 commit e7be2e1
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 27 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
7 changes: 5 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ full = [
http1 = []
http2 = ["h2"]

# Support for non-canonical HTTP/1 reason phrases
http1_reason_phrase = ["http1"]

# Client/Server
client = []
server = []
Expand Down Expand Up @@ -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"
Expand All @@ -249,4 +252,4 @@ required-features = ["full"]
[[test]]
name = "server"
path = "tests/server.rs"
required-features = ["full"]
required-features = ["full", "http1_reason_phrase"]
5 changes: 5 additions & 0 deletions src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down
101 changes: 101 additions & 0 deletions src/ext/h1_reason_phrase.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
5 changes: 1 addition & 4 deletions src/ffi/http_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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 {
Expand Down
62 changes: 44 additions & 18 deletions src/proto/h1/role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ use std::mem::MaybeUninit;

#[cfg(all(feature = "server", feature = "runtime"))]
use tokio::time::Instant;
#[cfg(any(test, feature = "server", feature = "ffi"))]
#[cfg(any(
test,
feature = "server",
feature = "ffi",
feature = "http1_reason_phrase"
))]
use bytes::Bytes;
use bytes::BytesMut;
#[cfg(feature = "server")]
Expand Down Expand Up @@ -358,7 +363,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 {
Expand All @@ -373,15 +388,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");
}

Expand Down Expand Up @@ -918,9 +944,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
Expand All @@ -944,9 +970,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)
Expand Down Expand Up @@ -1012,11 +1038,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")]
Expand Down
57 changes: 57 additions & 0 deletions tests/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit e7be2e1

Please sign in to comment.