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

feat(http1): Add support for writing Trailer Fields #3375

Merged
merged 5 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(http1): code review fixes
- remove uncessary if statement
- prefer into_iter() instead of drain()
- prefer !vec.is_empty() to vec.len() > 0
- add another test for multiple trailer headers in single response
  • Loading branch information
hjr3 committed Nov 11, 2023
commit 9cc26a59ae18ad81ff32eaf8fc1919695d7bfee4
9 changes: 2 additions & 7 deletions src/proto/h1/conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,17 +265,12 @@ where
self.state.reading = Reading::Body(Decoder::new(msg.decode));
}

if msg
self.state.allow_trailer_fields = msg
.head
.headers
.get(TE)
.map(|te_header| te_header == "trailers")
.unwrap_or(false)
{
self.state.allow_trailer_fields = true;
} else {
self.state.allow_trailer_fields = false;
}
.unwrap_or(false);

Poll::Ready(Some(Ok((msg.head, msg.decode, wants))))
}
Expand Down
57 changes: 47 additions & 10 deletions src/proto/h1/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl Encoder {

pub(crate) fn encode_trailers<B>(
hjr3 marked this conversation as resolved.
Show resolved Hide resolved
&self,
mut trailers: HeaderMap,
trailers: HeaderMap,
title_case_headers: bool,
) -> Option<EncodedBuf<B>> {
match &self.kind {
Expand All @@ -175,7 +175,7 @@ impl Encoder {
let mut cur_name = None;
let mut allowed_trailers = HeaderMap::new();

for (opt_name, value) in trailers.drain() {
for (opt_name, value) in trailers.into_iter() {
hjr3 marked this conversation as resolved.
Show resolved Hide resolved
if let Some(n) = opt_name {
cur_name = Some(n);
}
Expand Down Expand Up @@ -530,14 +530,18 @@ mod tests {
let trailers = vec![HeaderValue::from_static("chunky-trailer")];
let encoder = encoder.into_chunked_with_trailing_fields(trailers);

let mut headers = HeaderMap::new();
headers.insert(
HeaderName::from_static("chunky-trailer"),
HeaderValue::from_static("header data"),
);
headers.insert(
HeaderName::from_static("should-not-be-included"),
HeaderValue::from_static("oops"),
let headers = HeaderMap::from_iter(
vec![
(
HeaderName::from_static("chunky-trailer"),
HeaderValue::from_static("header data"),
),
(
HeaderName::from_static("should-not-be-included"),
HeaderValue::from_static("oops"),
),
]
.into_iter(),
);

let buf1 = encoder.encode_trailers::<&[u8]>(headers, false).unwrap();
Expand All @@ -547,6 +551,39 @@ mod tests {
assert_eq!(dst, b"0\r\nchunky-trailer: header data\r\n\r\n");
}

#[test]
fn chunked_with_multiple_trailer_headers() {
let encoder = Encoder::chunked();
let trailers = vec![
HeaderValue::from_static("chunky-trailer"),
HeaderValue::from_static("chunky-trailer-2"),
];
let encoder = encoder.into_chunked_with_trailing_fields(trailers);

let headers = HeaderMap::from_iter(
vec![
(
HeaderName::from_static("chunky-trailer"),
HeaderValue::from_static("header data"),
),
(
HeaderName::from_static("chunky-trailer-2"),
HeaderValue::from_static("more header data"),
),
]
.into_iter(),
);

let buf1 = encoder.encode_trailers::<&[u8]>(headers, false).unwrap();

let mut dst = Vec::new();
dst.put(buf1);
assert_eq!(
dst,
b"0\r\nchunky-trailer: header data\r\nchunky-trailer-2: more header data\r\n\r\n"
);
}

#[test]
fn chunked_with_no_trailer_header() {
let encoder = Encoder::chunked();
Expand Down
2 changes: 1 addition & 1 deletion src/proto/h1/role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,7 @@ impl Client {
let allowed_trailer_fields: Vec<HeaderValue> =
headers.get_all(header::TRAILER).iter().cloned().collect();

if allowed_trailer_fields.len() > 0 {
if !allowed_trailer_fields.is_empty() {
return enc.into_chunked_with_trailing_fields(allowed_trailer_fields);
}
}
Expand Down
Loading