Skip to content

Commit

Permalink
feature(ffi): add connection option to preserve header order (#2798)
Browse files Browse the repository at this point in the history
Libcurl expects that headers are iterated in the same order that they
are recieved. Previously this caused curl tests 580 and 581 to fail.
This necessitated exposing a way to preserve the original ordering of
http headers.

SUMMARY OF CHANGES: Add a new data structure called OriginalHeaderOrder that
represents the order in which headers originally appear in a HTTP
message. This datastructure is `Vec<(Headername, multimap-index)>`.
This vector is ordered by the order which headers were recieved.
Add the following ffi functions:
- ffi::client::hyper_clientconn_options_set_preserve_header_order : An
     ffi interface to configure a connection to preserve header order.
- ffi::client::hyper_clientconn_options_set_preserve_header_case : An
     ffi interface to configure a connection to preserve header case.
- Add a new option to ParseContext, and Conn::State called `preserve_header_order`.
  This option, and all the code paths it creates are behind the `ffi`
  feature flag. This should not change performance of response parsing for
  non-ffi users.

Closes #2780

BREAKING CHANGE: hyper_clientconn_options_new no longer
  sets the http1_preserve_header_case connection option by default.
  Users should now call
  hyper_clientconn_options_set_preserve_header_case
  if they desire that functionality.
  • Loading branch information
liamwarfield authored Apr 23, 2022
1 parent e1138d7 commit 78de891
Show file tree
Hide file tree
Showing 9 changed files with 381 additions and 30 deletions.
16 changes: 16 additions & 0 deletions capi/include/hyper.h
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,22 @@ void hyper_clientconn_free(struct hyper_clientconn *conn);
*/
struct hyper_clientconn_options *hyper_clientconn_options_new(void);

/*
Set the whether or not header case is preserved.
Pass `0` to allow lowercase normalization (default), `1` to retain original case.
*/
void hyper_clientconn_options_set_preserve_header_case(struct hyper_clientconn_options *opts,
int enabled);

/*
Set the whether or not header order is preserved.
Pass `0` to allow reordering (default), `1` to retain original ordering.
*/
void hyper_clientconn_options_set_preserve_header_order(struct hyper_clientconn_options *opts,
int enabled);

/*
Free a `hyper_clientconn_options *`.
*/
Expand Down
24 changes: 24 additions & 0 deletions src/client/conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ pub struct Builder {
h1_writev: Option<bool>,
h1_title_case_headers: bool,
h1_preserve_header_case: bool,
#[cfg(feature = "ffi")]
h1_preserve_header_order: bool,
h1_read_buf_exact_size: Option<usize>,
h1_max_buf_size: Option<usize>,
#[cfg(feature = "ffi")]
Expand Down Expand Up @@ -558,6 +560,8 @@ impl Builder {
h1_parser_config: Default::default(),
h1_title_case_headers: false,
h1_preserve_header_case: false,
#[cfg(feature = "ffi")]
h1_preserve_header_order: false,
h1_max_buf_size: None,
#[cfg(feature = "ffi")]
h1_headers_raw: false,
Expand Down Expand Up @@ -704,6 +708,21 @@ impl Builder {
self
}

/// Set whether to support preserving original header order.
///
/// Currently, this will record the order in which headers are received, and store this
/// ordering in a private extension on the `Response`. It will also look for and use
/// such an extension in any provided `Request`.
///
/// Note that this setting does not affect HTTP/2.
///
/// Default is false.
#[cfg(feature = "ffi")]
pub fn http1_preserve_header_order(&mut self, enabled: bool) -> &mut Builder {
self.h1_preserve_header_order = enabled;
self
}

/// Sets the exact size of the read buffer to *always* use.
///
/// Note that setting this option unsets the `http1_max_buf_size` option.
Expand Down Expand Up @@ -948,9 +967,14 @@ impl Builder {
if opts.h1_title_case_headers {
conn.set_title_case_headers();
}
#[cfg(feature = "ffi")]

This comment has been minimized.

Copy link
@nox

nox Apr 25, 2022

Contributor

This broke using http1_preserve_header_case without ffi.

if opts.h1_preserve_header_case {
conn.set_preserve_header_case();
}
#[cfg(feature = "ffi")]
if opts.h1_preserve_header_order {
conn.set_preserve_header_order();
}
if opts.h09_responses {
conn.set_h09_responses();
}
Expand Down
101 changes: 100 additions & 1 deletion src/ext.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
//! HTTP extensions.

use bytes::Bytes;
use http::header::HeaderName;
#[cfg(feature = "http1")]
use http::header::{HeaderName, IntoHeaderName, ValueIter};
use http::header::{IntoHeaderName, ValueIter};
use http::HeaderMap;
#[cfg(feature = "ffi")]
use std::collections::HashMap;
#[cfg(feature = "http2")]
use std::fmt;

Expand Down Expand Up @@ -120,3 +123,99 @@ impl HeaderCaseMap {
self.0.append(name, orig);
}
}

#[cfg(feature = "ffi")]
#[derive(Clone, Debug)]
/// Hashmap<Headername, numheaders with that name>
pub(crate) struct OriginalHeaderOrder {
/// Stores how many entries a Headername maps to. This is used
/// for accounting.
num_entries: HashMap<HeaderName, usize>,
/// Stores the ordering of the headers. ex: `vec[i] = (headerName, idx)`,
/// The vector is ordered such that the ith element
/// represents the ith header that came in off the line.
/// The `HeaderName` and `idx` are then used elsewhere to index into
/// the multi map that stores the header values.
entry_order: Vec<(HeaderName, usize)>,
}

#[cfg(all(feature = "http1", feature = "ffi"))]
impl OriginalHeaderOrder {
pub(crate) fn default() -> Self {
OriginalHeaderOrder {
num_entries: HashMap::new(),
entry_order: Vec::new(),
}
}

pub(crate) fn insert(&mut self, name: HeaderName) {
if !self.num_entries.contains_key(&name) {
let idx = 0;
self.num_entries.insert(name.clone(), 1);
self.entry_order.push((name, idx));
}
// Replacing an already existing element does not
// change ordering, so we only care if its the first
// header name encountered
}

pub(crate) fn append<N>(&mut self, name: N)
where
N: IntoHeaderName + Into<HeaderName> + Clone,
{
let name: HeaderName = name.into();
let idx;
if self.num_entries.contains_key(&name) {
idx = self.num_entries[&name];
*self.num_entries.get_mut(&name).unwrap() += 1;
} else {
idx = 0;
self.num_entries.insert(name.clone(), 1);
}
self.entry_order.push((name, idx));
}

// No doc test is run here because `RUSTFLAGS='--cfg hyper_unstable_ffi'`
// is needed to compile. Once ffi is stablized `no_run` should be removed
// here.
/// This returns an iterator that provides header names and indexes
/// in the original order recieved.
///
/// # Examples
/// ```no_run
/// use hyper::ext::OriginalHeaderOrder;
/// use hyper::header::{HeaderName, HeaderValue, HeaderMap};
///
/// let mut h_order = OriginalHeaderOrder::default();
/// let mut h_map = Headermap::new();
///
/// let name1 = b"Set-CookiE";
/// let value1 = b"a=b";
/// h_map.append(name1);
/// h_order.append(name1);
///
/// let name2 = b"Content-Encoding";
/// let value2 = b"gzip";
/// h_map.append(name2, value2);
/// h_order.append(name2);
///
/// let name3 = b"SET-COOKIE";
/// let value3 = b"c=d";
/// h_map.append(name3, value3);
/// h_order.append(name3)
///
/// let mut iter = h_order.get_in_order()
///
/// let (name, idx) = iter.next();
/// assert_eq!(b"a=b", h_map.get_all(name).nth(idx).unwrap());
///
/// let (name, idx) = iter.next();
/// assert_eq!(b"gzip", h_map.get_all(name).nth(idx).unwrap());
///
/// let (name, idx) = iter.next();
/// assert_eq!(b"c=d", h_map.get_all(name).nth(idx).unwrap());
/// ```
pub(crate) fn get_in_order(&self) -> impl Iterator<Item = &(HeaderName, usize)> {
self.entry_order.iter()
}
}
23 changes: 21 additions & 2 deletions src/ffi/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,7 @@ 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.http1_preserve_header_case(true);
let builder = conn::Builder::new();

Box::into_raw(Box::new(hyper_clientconn_options {
builder,
Expand All @@ -103,6 +102,26 @@ ffi_fn! {
} ?= std::ptr::null_mut()
}

ffi_fn! {
/// Set the whether or not header case is preserved.
///
/// Pass `0` to allow lowercase normalization (default), `1` to retain original case.
fn hyper_clientconn_options_set_preserve_header_case(opts: *mut hyper_clientconn_options, enabled: c_int) {
let opts = non_null! { &mut *opts ?= () };
opts.builder.http1_preserve_header_case(enabled != 0);
}
}

ffi_fn! {
/// Set the whether or not header order is preserved.
///
/// Pass `0` to allow reordering (default), `1` to retain original ordering.
fn hyper_clientconn_options_set_preserve_header_order(opts: *mut hyper_clientconn_options, enabled: c_int) {
let opts = non_null! { &mut *opts ?= () };
opts.builder.http1_preserve_header_order(enabled != 0);
}
}

ffi_fn! {
/// Free a `hyper_clientconn_options *`.
fn hyper_clientconn_options_free(opts: *mut hyper_clientconn_options) {
Expand Down
Loading

0 comments on commit 78de891

Please sign in to comment.