Skip to content

Commit 0ff3b20

Browse files
committed
Implemented a few PR Suggestions:
1. Added a few more #[cfg(feature = "ffi")]'s all around 2. Replaced a few temp descriptions 3. Removed printlns. 4. Removed a ` + std::fmt::Display,` 5. Removed unused param for OriginalHeaderOrder::append and OriginalHeaderOrder::insert 6. Made the new connection options more like the current boolean options
1 parent a2d8e5d commit 0ff3b20

File tree

6 files changed

+118
-34
lines changed

6 files changed

+118
-34
lines changed

capi/include/hyper.h

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -356,14 +356,20 @@ void hyper_clientconn_free(struct hyper_clientconn *conn);
356356
struct hyper_clientconn_options *hyper_clientconn_options_new(void);
357357

358358
/*
359-
temp
359+
Set the whether or not header case is preserved.
360+
361+
Pass `0` to allow lowercase normalization (default), `1` to retain original case.
360362
*/
361-
void hyper_clientconn_options_set_preserve_header_case(struct hyper_clientconn_options *opts);
363+
void hyper_clientconn_options_set_preserve_header_case(struct hyper_clientconn_options *opts,
364+
int enabled);
362365

363366
/*
364-
temp
367+
Set the whether or not header order is preserved.
368+
369+
Pass `0` to allow reordering (default), `1` to retain original ordering.
365370
*/
366-
void hyper_clientconn_options_set_preserve_header_order(struct hyper_clientconn_options *opts);
371+
void hyper_clientconn_options_set_preserve_header_order(struct hyper_clientconn_options *opts,
372+
int enabled);
367373

368374
/*
369375
Free a `hyper_clientconn_options *`.
@@ -606,7 +612,7 @@ void hyper_headers_foreach(const struct hyper_headers *headers,
606612
void *userdata);
607613

608614
/*
609-
Iterates the headers passing each name and value pair to the callback.
615+
Iterates the headers in the order the were recieved, passing each name and value pair to the callback.
610616
611617
The `userdata` pointer is also passed to the callback.
612618

src/client/conn.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,6 +716,7 @@ impl Builder {
716716
/// Note that this setting does not affect HTTP/2.
717717
///
718718
/// Default is false.
719+
#[cfg(feature = "ffi")]
719720
pub fn http1_preserve_header_order(&mut self, enabled: bool) -> &mut Builder {
720721
self.h1_preserve_header_order = enabled;
721722
self

src/ext.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use http::header::HeaderName;
55
#[cfg(feature = "http1")]
66
use http::header::{IntoHeaderName, ValueIter};
77
use http::HeaderMap;
8-
use std::collections::{HashSet, HashMap};
8+
use std::collections::HashMap;
99
#[cfg(feature = "http2")]
1010
use std::fmt;
1111

@@ -117,7 +117,7 @@ impl HeaderCaseMap {
117117

118118
pub(crate) fn append<N>(&mut self, name: N, orig: Bytes)
119119
where
120-
N: IntoHeaderName + std::fmt::Display,
120+
N: IntoHeaderName,
121121
{
122122
self.0.append(name, orig);
123123
}
@@ -127,14 +127,14 @@ impl HeaderCaseMap {
127127
/// Hashmap<Headername, numheaders with that name>
128128
pub(crate) struct OriginalHeaderOrder(HashMap<HeaderName, usize>, Vec<(HeaderName, usize)>);
129129

130-
#[cfg(feature = "http1")]
130+
#[cfg(all(feature = "http1", feature = "ffi"))]
131131
impl OriginalHeaderOrder {
132132
pub(crate) fn default() -> Self {
133133
Self(Default::default(), Default::default())
134134
}
135135

136136
#[cfg(any(test, feature = "ffi"))]
137-
pub(crate) fn insert(&mut self, name: HeaderName, orig: Bytes) {
137+
pub(crate) fn insert(&mut self, name: HeaderName) {
138138
if !self.0.contains_key(&name) {
139139
let idx = 0;
140140
self.0.insert(name.clone(), 1);
@@ -145,7 +145,7 @@ impl OriginalHeaderOrder {
145145
// header name encountered
146146
}
147147

148-
pub(crate) fn append<N>(&mut self, name: N, orig: Bytes)
148+
pub(crate) fn append<N>(&mut self, name: N)
149149
where
150150
N: IntoHeaderName + Into<HeaderName> + Clone,
151151
{

src/ffi/client.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,18 +105,22 @@ ffi_fn! {
105105
}
106106

107107
ffi_fn! {
108-
/// temp
109-
fn hyper_clientconn_options_set_preserve_header_case(opts: *mut hyper_clientconn_options) {
108+
/// Set the whether or not header case is preserved.
109+
///
110+
/// Pass `0` to allow lowercase normalization (default), `1` to retain original case.
111+
fn hyper_clientconn_options_set_preserve_header_case(opts: *mut hyper_clientconn_options, enabled: c_int) {
110112
let opts = non_null! { &mut *opts ?= () };
111-
opts.builder.http1_preserve_header_case(true);
113+
opts.builder.http1_preserve_header_case(enabled != 0);
112114
}
113115
}
114116

115117
ffi_fn! {
116-
/// temp
117-
fn hyper_clientconn_options_set_preserve_header_order(opts: *mut hyper_clientconn_options) {
118+
/// Set the whether or not header order is preserved.
119+
///
120+
/// Pass `0` to allow reordering (default), `1` to retain original ordering.
121+
fn hyper_clientconn_options_set_preserve_header_order(opts: *mut hyper_clientconn_options, enabled: c_int) {
118122
let opts = non_null! { &mut *opts ?= () };
119-
opts.builder.http1_preserve_header_order(true);
123+
opts.builder.http1_preserve_header_order(enabled != 0);
120124
}
121125
}
122126

src/ffi/http_types.rs

Lines changed: 83 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -411,26 +411,32 @@ ffi_fn! {
411411
// and for each one, try to pair the originally cased name with the value.
412412
//
413413
// TODO: consider adding http::HeaderMap::entries() iterator
414-
for (name, idx) in headers.orig_order.get_in_order() {
415-
//println!("OHEA {:?}", (name, idx));
416-
let orig_name = headers.orig_casing.get_all(&name).nth(*idx).unwrap();
417-
let value = headers.headers.get_all(name).iter().nth(*idx).unwrap();
418-
419-
let name_ptr = orig_name.as_ref().as_ptr();
420-
let name_len = orig_name.as_ref().len();
421-
422-
let val_ptr = value.as_bytes().as_ptr();
423-
let val_len = value.as_bytes().len();
424-
425-
if HYPER_ITER_CONTINUE != func(userdata, name_ptr, name_len, val_ptr, val_len) {
426-
return;
414+
for name in headers.headers.keys() {
415+
let mut names = headers.orig_casing.get_all(name);
416+
417+
for value in headers.headers.get_all(name) {
418+
let (name_ptr, name_len) = if let Some(orig_name) = names.next() {
419+
(orig_name.as_ref().as_ptr(), orig_name.as_ref().len())
420+
} else {
421+
(
422+
name.as_str().as_bytes().as_ptr(),
423+
name.as_str().as_bytes().len(),
424+
)
425+
};
426+
427+
let val_ptr = value.as_bytes().as_ptr();
428+
let val_len = value.as_bytes().len();
429+
430+
if HYPER_ITER_CONTINUE != func(userdata, name_ptr, name_len, val_ptr, val_len) {
431+
return;
432+
}
427433
}
428434
}
429435
}
430436
}
431437

432438
ffi_fn! {
433-
/// Iterates the headers passing each name and value pair to the callback.
439+
/// Iterates the headers in the order the were recieved, passing each name and value pair to the callback.
434440
///
435441
/// The `userdata` pointer is also passed to the callback.
436442
///
@@ -470,7 +476,7 @@ ffi_fn! {
470476
Ok((name, value, orig_name)) => {
471477
headers.headers.insert(&name, value);
472478
headers.orig_casing.insert(name.clone(), orig_name.clone());
473-
headers.orig_order.insert(name, orig_name);
479+
headers.orig_order.insert(name);
474480
hyper_code::HYPERE_OK
475481
}
476482
Err(code) => code,
@@ -490,7 +496,7 @@ ffi_fn! {
490496
Ok((name, value, orig_name)) => {
491497
headers.headers.append(&name, value);
492498
headers.orig_casing.append(&name, orig_name.clone());
493-
headers.orig_order.append(name, orig_name);
499+
headers.orig_order.append(name);
494500
hyper_code::HYPERE_OK
495501
}
496502
Err(code) => code,
@@ -590,4 +596,65 @@ mod tests {
590596
HYPER_ITER_CONTINUE
591597
}
592598
}
599+
600+
#[cfg(all(feature = "http1", feature = "ffi"))]
601+
#[test]
602+
fn test_headers_foreach_order_preserved() {
603+
let mut headers = hyper_headers::default();
604+
605+
let name1 = b"Set-CookiE";
606+
let value1 = b"a=b";
607+
hyper_headers_add(
608+
&mut headers,
609+
name1.as_ptr(),
610+
name1.len(),
611+
value1.as_ptr(),
612+
value1.len(),
613+
);
614+
615+
let name2 = b"Content-Encoding";
616+
let value2 = b"gzip";
617+
hyper_headers_add(
618+
&mut headers,
619+
name2.as_ptr(),
620+
name2.len(),
621+
value2.as_ptr(),
622+
value2.len(),
623+
);
624+
625+
let name3 = b"SET-COOKIE";
626+
let value3 = b"c=d";
627+
hyper_headers_add(
628+
&mut headers,
629+
name3.as_ptr(),
630+
name3.len(),
631+
value3.as_ptr(),
632+
value3.len(),
633+
);
634+
635+
let mut vec = Vec::<u8>::new();
636+
hyper_headers_foreach_ordered(&headers, concat, &mut vec as *mut _ as *mut c_void);
637+
638+
println!("{}", std::str::from_utf8(&vec).unwrap());
639+
assert_eq!(vec, b"Set-CookiE: a=b\r\nContent-Encoding: gzip\r\nSET-COOKIE: c=d\r\n");
640+
641+
extern "C" fn concat(
642+
vec: *mut c_void,
643+
name: *const u8,
644+
name_len: usize,
645+
value: *const u8,
646+
value_len: usize,
647+
) -> c_int {
648+
unsafe {
649+
let vec = &mut *(vec as *mut Vec<u8>);
650+
let name = std::slice::from_raw_parts(name, name_len);
651+
let value = std::slice::from_raw_parts(value, value_len);
652+
vec.extend(name);
653+
vec.extend(b": ");
654+
vec.extend(value);
655+
vec.extend(b"\r\n");
656+
}
657+
HYPER_ITER_CONTINUE
658+
}
659+
}
593660
}

src/proto/h1/role.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use std::fmt::{self, Write};
22
use std::mem::MaybeUninit;
33

4+
#[cfg(all(feature = "server", feature = "runtime"))]
5+
use tokio::time::Instant;
46
#[cfg(any(test, feature = "server", feature = "ffi"))]
57
use bytes::Bytes;
68
use bytes::BytesMut;
@@ -212,6 +214,7 @@ impl Http1Transaction for Server {
212214
None
213215
};
214216

217+
#[cfg(feature = "ffi")]
215218
let mut header_order = if ctx.preserve_header_order {
216219
Some(OriginalHeaderOrder::default())
217220
} else {
@@ -294,8 +297,9 @@ impl Http1Transaction for Server {
294297
header_case_map.append(&name, slice.slice(header.name.0..header.name.1));
295298
}
296299

300+
#[cfg(feature = "ffi")]
297301
if let Some(ref mut header_order) = header_order {
298-
header_order.append(&name, slice.slice(header.name.0..header.name.1));
302+
header_order.append(&name);
299303
}
300304

301305
headers.append(name, value);
@@ -996,6 +1000,7 @@ impl Http1Transaction for Client {
9961000
None
9971001
};
9981002

1003+
#[cfg(feature = "ffi")]
9991004
let mut header_order = if ctx.preserve_header_order {
10001005
Some(OriginalHeaderOrder::default())
10011006
} else {
@@ -1024,8 +1029,9 @@ impl Http1Transaction for Client {
10241029
header_case_map.append(&name, slice.slice(header.name.0..header.name.1));
10251030
}
10261031

1032+
#[cfg(feature = "ffi")]
10271033
if let Some(ref mut header_order) = header_order {
1028-
header_order.append(&name, slice.slice(header.name.0..header.name.1));
1034+
header_order.append(&name);
10291035
}
10301036

10311037
headers.append(name, value);

0 commit comments

Comments
 (0)