Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ffi): Change hyper_headers_foreach to iterate in recieved order #2798

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions capi/include/hyper.h
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
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 {
liamwarfield marked this conversation as resolved.
Show resolved Hide resolved
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")]
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
@@ -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
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this removal makes a lint trigger about no longer needing to be let mut builder.

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