Skip to content

Commit

Permalink
feature(ffi): Added connection option to preserve header order.
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.
- ffi::http_types::hyper_headers_foreach_ordered : Iterates the headers in
     the order the were recieved, passing each name and value pair to the callback.
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 ISSUE: #2780
  • Loading branch information
liamwarfield authored and Liam Warfield committed Apr 11, 2022
1 parent 740654e commit f4fec5c
Show file tree
Hide file tree
Showing 9 changed files with 327 additions and 20 deletions.
28 changes: 28 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 Expand Up @@ -595,6 +611,18 @@ void hyper_headers_foreach(const struct hyper_headers *headers,
hyper_headers_foreach_callback func,
void *userdata);

/*
Iterates the headers in the order the were recieved, passing each name and value pair to the callback.
The `userdata` pointer is also passed to the callback.
The callback should return `HYPER_ITER_CONTINUE` to keep iterating, or
`HYPER_ITER_BREAK` to stop.
*/
void hyper_headers_foreach_ordered(const struct hyper_headers *headers,
hyper_headers_foreach_callback func,
void *userdata);

/*
Sets the header with the provided name to the provided value.
Expand Down
21 changes: 21 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,7 @@ impl Builder {
h1_parser_config: Default::default(),
h1_title_case_headers: false,
h1_preserve_header_case: false,
h1_preserve_header_order: false,
h1_max_buf_size: None,
#[cfg(feature = "ffi")]
h1_headers_raw: false,
Expand Down Expand Up @@ -704,6 +707,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 @@ -951,6 +969,9 @@ impl Builder {
if opts.h1_preserve_header_case {
conn.set_preserve_header_case();
}
if opts.h1_preserve_header_order {
conn.set_preserve_header_order();
}
if opts.h09_responses {
conn.set_h09_responses();
}
Expand Down
50 changes: 49 additions & 1 deletion src/ext.rs
@@ -1,9 +1,11 @@
//! 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;
use std::collections::HashMap;
#[cfg(feature = "http2")]
use std::fmt;

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

#[derive(Clone, Debug)]
/// Hashmap<Headername, numheaders with that name>
pub(crate) struct OriginalHeaderOrder(HashMap<HeaderName, usize>, Vec<(HeaderName, usize)>);

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

#[cfg(any(test, feature = "ffi"))]
pub(crate) fn insert(&mut self, name: HeaderName) {
if !self.0.contains_key(&name) {
let idx = 0;
self.0.insert(name.clone(), 1);
self.1.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.0.contains_key(&name) {
idx = self.0[&name];
*self.0.get_mut(&name).unwrap() += 1;
} else {
idx = 0;
self.0.insert(name.clone(), 1);
}
self.1.push((name, idx));
}

/// This returns an iterator that provides header names and indexes
/// in the original order recieved.
#[cfg(any(test, feature = "ffi"))]
pub(crate) fn get_in_order(&self) -> impl Iterator<Item = &(HeaderName, usize)> {
self.1.iter()
}
}
21 changes: 21 additions & 0 deletions src/ffi/client.rs
Expand Up @@ -95,6 +95,7 @@ ffi_fn! {
fn hyper_clientconn_options_new() -> *mut hyper_clientconn_options {
let mut builder = conn::Builder::new();
builder.http1_preserve_header_case(true);
builder.http1_preserve_header_order(true);

Box::into_raw(Box::new(hyper_clientconn_options {
builder,
Expand All @@ -103,6 +104,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
108 changes: 105 additions & 3 deletions src/ffi/http_types.rs
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, OriginalHeaderOrder};
use crate::header::{HeaderName, HeaderValue};
use crate::{Body, HeaderMap, Method, Request, Response, Uri};

Expand All @@ -22,6 +22,7 @@ pub struct hyper_response(pub(super) Response<Body>);
pub struct hyper_headers {
pub(super) headers: HeaderMap,
orig_casing: HeaderCaseMap,
orig_order: OriginalHeaderOrder,
}

#[derive(Debug)]
Expand Down Expand Up @@ -233,6 +234,7 @@ impl hyper_request {
if let Some(headers) = self.0.extensions_mut().remove::<hyper_headers>() {
*self.0.headers_mut() = headers.headers;
self.0.extensions_mut().insert(headers.orig_casing);
self.0.extensions_mut().insert(headers.orig_order);
}
}
}
Expand Down Expand Up @@ -348,9 +350,14 @@ impl hyper_response {
.extensions_mut()
.remove::<HeaderCaseMap>()
.unwrap_or_else(HeaderCaseMap::default);
let orig_order = resp
.extensions_mut()
.remove::<OriginalHeaderOrder>()
.unwrap_or_else(OriginalHeaderOrder::default);
resp.extensions_mut().insert(hyper_headers {
headers,
orig_casing,
orig_order,
});

hyper_response(resp)
Expand Down Expand Up @@ -428,6 +435,37 @@ ffi_fn! {
}
}

ffi_fn! {
/// Iterates the headers in the order the were recieved, passing each name and value pair to the callback.
///
/// The `userdata` pointer is also passed to the callback.
///
/// The callback should return `HYPER_ITER_CONTINUE` to keep iterating, or
/// `HYPER_ITER_BREAK` to stop.
fn hyper_headers_foreach_ordered(headers: *const hyper_headers, func: hyper_headers_foreach_callback, userdata: *mut c_void) {
let headers = non_null!(&*headers ?= ());
// For each header name/value pair, there may be a value in the casemap
// that corresponds to the HeaderValue. So, we iterator all the keys,
// and for each one, try to pair the originally cased name with the value.
//
// TODO: consider adding http::HeaderMap::entries() iterator
for (name, idx) in headers.orig_order.get_in_order() {
let orig_name = headers.orig_casing.get_all(&name).nth(*idx).unwrap();
let value = headers.headers.get_all(name).iter().nth(*idx).unwrap();

let name_ptr = orig_name.as_ref().as_ptr();
let name_len = orig_name.as_ref().len();

let val_ptr = value.as_bytes().as_ptr();
let val_len = value.as_bytes().len();

if HYPER_ITER_CONTINUE != func(userdata, name_ptr, name_len, val_ptr, val_len) {
return;
}
}
}
}

ffi_fn! {
/// Sets the header with the provided name to the provided value.
///
Expand All @@ -437,7 +475,8 @@ ffi_fn! {
match unsafe { raw_name_value(name, name_len, value, value_len) } {
Ok((name, value, orig_name)) => {
headers.headers.insert(&name, value);
headers.orig_casing.insert(name, orig_name);
headers.orig_casing.insert(name.clone(), orig_name.clone());
headers.orig_order.insert(name);
hyper_code::HYPERE_OK
}
Err(code) => code,
Expand All @@ -456,7 +495,8 @@ ffi_fn! {
match unsafe { raw_name_value(name, name_len, value, value_len) } {
Ok((name, value, orig_name)) => {
headers.headers.append(&name, value);
headers.orig_casing.append(name, orig_name);
headers.orig_casing.append(&name, orig_name.clone());
headers.orig_order.append(name);
hyper_code::HYPERE_OK
}
Err(code) => code,
Expand All @@ -469,6 +509,7 @@ impl Default for hyper_headers {
Self {
headers: Default::default(),
orig_casing: HeaderCaseMap::default(),
orig_order: OriginalHeaderOrder::default(),
}
}
}
Expand Down Expand Up @@ -555,4 +596,65 @@ mod tests {
HYPER_ITER_CONTINUE
}
}

#[cfg(all(feature = "http1", feature = "ffi"))]
#[test]
fn test_headers_foreach_order_preserved() {
let mut headers = hyper_headers::default();

let name1 = b"Set-CookiE";
let value1 = b"a=b";
hyper_headers_add(
&mut headers,
name1.as_ptr(),
name1.len(),
value1.as_ptr(),
value1.len(),
);

let name2 = b"Content-Encoding";
let value2 = b"gzip";
hyper_headers_add(
&mut headers,
name2.as_ptr(),
name2.len(),
value2.as_ptr(),
value2.len(),
);

let name3 = b"SET-COOKIE";
let value3 = b"c=d";
hyper_headers_add(
&mut headers,
name3.as_ptr(),
name3.len(),
value3.as_ptr(),
value3.len(),
);

let mut vec = Vec::<u8>::new();
hyper_headers_foreach_ordered(&headers, concat, &mut vec as *mut _ as *mut c_void);

println!("{}", std::str::from_utf8(&vec).unwrap());
assert_eq!(vec, b"Set-CookiE: a=b\r\nContent-Encoding: gzip\r\nSET-COOKIE: c=d\r\n");

extern "C" fn concat(
vec: *mut c_void,
name: *const u8,
name_len: usize,
value: *const u8,
value_len: usize,
) -> c_int {
unsafe {
let vec = &mut *(vec as *mut Vec<u8>);
let name = std::slice::from_raw_parts(name, name_len);
let value = std::slice::from_raw_parts(value, value_len);
vec.extend(name);
vec.extend(b": ");
vec.extend(value);
vec.extend(b"\r\n");
}
HYPER_ITER_CONTINUE
}
}
}
11 changes: 11 additions & 0 deletions src/proto/h1/conn.rs
Expand Up @@ -58,6 +58,8 @@ where
#[cfg(all(feature = "server", feature = "runtime"))]
h1_header_read_timeout_running: false,
preserve_header_case: false,
#[cfg(feature = "ffi")]
preserve_header_order: false,
title_case_headers: false,
h09_responses: false,
#[cfg(feature = "ffi")]
Expand Down Expand Up @@ -111,6 +113,11 @@ where
self.state.preserve_header_case = true;
}

#[cfg(feature = "ffi")]
pub(crate) fn set_preserve_header_order(&mut self) {
self.state.preserve_header_order = true;
}

#[cfg(feature = "client")]
pub(crate) fn set_h09_responses(&mut self) {
self.state.h09_responses = true;
Expand Down Expand Up @@ -200,6 +207,8 @@ where
#[cfg(all(feature = "server", feature = "runtime"))]
h1_header_read_timeout_running: &mut self.state.h1_header_read_timeout_running,
preserve_header_case: self.state.preserve_header_case,
#[cfg(feature = "ffi")]
preserve_header_order: self.state.preserve_header_order,
h09_responses: self.state.h09_responses,
#[cfg(feature = "ffi")]
on_informational: &mut self.state.on_informational,
Expand Down Expand Up @@ -824,6 +833,8 @@ struct State {
#[cfg(all(feature = "server", feature = "runtime"))]
h1_header_read_timeout_running: bool,
preserve_header_case: bool,
#[cfg(feature = "ffi")]
preserve_header_order: bool,
title_case_headers: bool,
h09_responses: bool,
/// If set, called with each 1xx informational response received for
Expand Down

0 comments on commit f4fec5c

Please sign in to comment.