Skip to content

Commit

Permalink
feat(http1): decouple preserving header case from FFI (fixes hyperium…
Browse files Browse the repository at this point in the history
  • Loading branch information
nox committed Mar 26, 2021
1 parent 51ed71b commit 39c33ee
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 86 deletions.
15 changes: 14 additions & 1 deletion src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -964,14 +964,27 @@ impl Builder {
/// Set whether HTTP/1 connections will write header names as title case at
/// the socket level.
///
/// Note that this setting does not affect HTTP/2.
/// Note that this setting does not affect HTTP/2 and supersedes the
/// `http1_preserve_header_case` setting.
///
/// Default is false.
pub fn http1_title_case_headers(&mut self, val: bool) -> &mut Self {
self.conn_builder.h1_title_case_headers(val);
self
}

/// Set whether HTTP/1 connections will write header names as provided
/// at the socket level.
///
/// Note that this setting does not affect HTTP/2 and is superseded
/// by the `http1_title_case_headers` setting.
///
/// Default is false.
pub fn http1_preserve_header_case(&mut self, val: bool) -> &mut Self {
self.conn_builder.h1_preserve_header_case(val);
self
}

/// Set whether the connection **must** use HTTP/2.
///
/// The destination must either allow HTTP2 Prior Knowledge, or the
Expand Down
10 changes: 10 additions & 0 deletions src/client/conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ where
pub struct Builder {
pub(super) exec: Exec,
h1_title_case_headers: bool,
h1_preserve_header_case: bool,
h1_read_buf_exact_size: Option<usize>,
h1_max_buf_size: Option<usize>,
#[cfg(feature = "http2")]
Expand Down Expand Up @@ -495,6 +496,7 @@ impl Builder {
exec: Exec::Default,
h1_read_buf_exact_size: None,
h1_title_case_headers: false,
h1_preserve_header_case: false,
h1_max_buf_size: None,
#[cfg(feature = "http2")]
h2_builder: Default::default(),
Expand All @@ -519,6 +521,11 @@ impl Builder {
self
}

pub(crate) fn h1_preserve_header_case(&mut self, enabled: bool) -> &mut Builder {
self.h1_preserve_header_case = enabled;
self
}

pub(super) fn h1_read_buf_exact_size(&mut self, sz: Option<usize>) -> &mut Builder {
self.h1_read_buf_exact_size = sz;
self.h1_max_buf_size = None;
Expand Down Expand Up @@ -700,6 +707,9 @@ impl Builder {
if opts.h1_title_case_headers {
conn.set_title_case_headers();
}
if opts.h1_preserve_header_case {
conn.set_preserve_header_case();
}
if let Some(sz) = opts.h1_read_buf_exact_size {
conn.set_read_buf_exact_size(sz);
}
Expand Down
5 changes: 4 additions & 1 deletion src/ffi/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,11 @@ unsafe impl AsTaskType for hyper_clientconn {
ffi_fn! {
/// Creates a new set of HTTP clientconn options to be used in a handshake.
fn hyper_clientconn_options_new() -> *mut hyper_clientconn_options {
let mut builder = conn::Builder::new();
builder.h1_preserve_header_case(true);

Box::into_raw(Box::new(hyper_clientconn_options {
builder: conn::Builder::new(),
builder,
exec: WeakExec::new(),
}))
}
Expand Down
24 changes: 1 addition & 23 deletions src/ffi/http_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use super::error::hyper_code;
use super::task::{hyper_task_return_type, AsTaskType};
use super::HYPER_ITER_CONTINUE;
use crate::header::{HeaderName, HeaderValue};
use crate::proto::h1::HeaderCaseMap;
use crate::{Body, HeaderMap, Method, Request, Response, Uri};

/// An HTTP request.
Expand All @@ -24,10 +25,6 @@ pub struct hyper_headers {
orig_casing: HeaderCaseMap,
}

// Will probably be moved to `hyper::ext::http1`
#[derive(Debug, Default)]
pub(crate) struct HeaderCaseMap(HeaderMap<Bytes>);

#[derive(Debug)]
pub(crate) struct ReasonPhrase(pub(crate) Bytes);

Expand Down Expand Up @@ -370,25 +367,6 @@ unsafe fn raw_name_value(
Ok((name, value, orig_name))
}

// ===== impl HeaderCaseMap =====

impl HeaderCaseMap {
pub(crate) fn get_all(&self, name: &HeaderName) -> http::header::GetAll<'_, Bytes> {
self.0.get_all(name)
}

pub(crate) fn insert(&mut self, name: HeaderName, orig: Bytes) {
self.0.insert(name, orig);
}

pub(crate) fn append<N>(&mut self, name: N, orig: Bytes)
where
N: http::header::IntoHeaderName,
{
self.0.append(name, orig);
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
2 changes: 1 addition & 1 deletion src/ffi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub use self::io::*;
pub use self::task::*;

pub(crate) use self::body::UserBody;
pub(crate) use self::http_types::{HeaderCaseMap, ReasonPhrase};
pub(crate) use self::http_types::ReasonPhrase;

/// Return in iter functions to continue iterating.
pub const HYPER_ITER_CONTINUE: libc::c_int = 0;
Expand Down
18 changes: 5 additions & 13 deletions src/proto/h1/conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ where
error: None,
keep_alive: KA::Busy,
method: None,
#[cfg(feature = "ffi")]
preserve_header_case: false,
title_case_headers: false,
notify_read: false,
Expand Down Expand Up @@ -78,6 +77,11 @@ where
self.state.title_case_headers = true;
}

#[cfg(feature = "client")]
pub(crate) fn set_preserve_header_case(&mut self) {
self.state.preserve_header_case = true;
}

#[cfg(feature = "server")]
pub(crate) fn set_allow_half_close(&mut self) {
self.state.allow_half_close = true;
Expand Down Expand Up @@ -144,7 +148,6 @@ where
ParseContext {
cached_headers: &mut self.state.cached_headers,
req_method: &mut self.state.method,
#[cfg(feature = "ffi")]
preserve_header_case: self.state.preserve_header_case,
}
)) {
Expand Down Expand Up @@ -478,16 +481,6 @@ where

self.enforce_version(&mut head);

// Maybe check if we should preserve header casing on received
// message headers...
#[cfg(feature = "ffi")]
{
if T::is_client() && !self.state.preserve_header_case {
self.state.preserve_header_case =
head.extensions.get::<crate::ffi::HeaderCaseMap>().is_some();
}
}

let buf = self.io.headers_buf();
match super::role::encode_headers::<T>(
Encode {
Expand Down Expand Up @@ -750,7 +743,6 @@ struct State {
/// This is used to know things such as if the message can include
/// a body or not.
method: Option<Method>,
#[cfg(feature = "ffi")]
preserve_header_case: bool,
title_case_headers: bool,
/// Set to true when the Dispatcher should poll read operations
Expand Down
2 changes: 0 additions & 2 deletions src/proto/h1/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ where
ParseContext {
cached_headers: parse_ctx.cached_headers,
req_method: parse_ctx.req_method,
#[cfg(feature = "ffi")]
preserve_header_case: parse_ctx.preserve_header_case,
},
)? {
Expand Down Expand Up @@ -638,7 +637,6 @@ mod tests {
let parse_ctx = ParseContext {
cached_headers: &mut None,
req_method: &mut None,
#[cfg(feature = "ffi")]
preserve_header_case: false,
};
assert!(buffered
Expand Down
25 changes: 23 additions & 2 deletions src/proto/h1/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use bytes::BytesMut;
use bytes::{Bytes, BytesMut};
use http::header::{GetAll, HeaderName, IntoHeaderName};
use http::{HeaderMap, Method};

use crate::body::DecodedLength;
Expand Down Expand Up @@ -70,7 +71,6 @@ pub(crate) struct ParsedMessage<T> {
pub(crate) struct ParseContext<'a> {
cached_headers: &'a mut Option<HeaderMap>,
req_method: &'a mut Option<Method>,
#[cfg(feature = "ffi")]
preserve_header_case: bool,
}

Expand Down Expand Up @@ -102,3 +102,24 @@ impl Wants {
(self.0 & other.0) == other.0
}
}

#[derive(Debug, Default)]
pub(crate) struct HeaderCaseMap(HeaderMap<Bytes>);

impl HeaderCaseMap {
pub(crate) fn get_all(&self, name: &HeaderName) -> GetAll<'_, Bytes> {
self.0.get_all(name)
}

#[cfg(any(test, feature = "ffi"))]
pub(crate) fn insert(&mut self, name: HeaderName, orig: Bytes) {
self.0.insert(name, orig);
}

pub(crate) fn append<N>(&mut self, name: N, orig: Bytes)
where
N: IntoHeaderName,
{
self.0.append(name, orig);
}
}
Loading

0 comments on commit 39c33ee

Please sign in to comment.