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.

Sign up for GitHub

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

return a 4xx status code on non-ascii headers #172

Closed
wants to merge 1 commit into from
Closed

return a 4xx status code on non-ascii headers #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions src/server/decode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ where
req.set_version(Some(http_types::Version::Http1_1));

for header in httparse_req.headers.iter() {
// https://tools.ietf.org/html/rfc822#section-3.1
http_types::ensure_status!(header.value.is_ascii(), 400, "None ascii header");
Copy link
Member

@jbr jbr Jan 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This requires two iterations over each byte — one for the is_ascii() check and the other for the from_utf8. I believe we should just map the error value for the result of from_utf8, like std::str::from_utf8(header.value).map_err(|_| Error::from_str(400, "non-ascii header"))? or using Status, std::str::from_utf8(header.value).status(400)?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I commented below already.
There is a panic that prevent mapping the error unfortunately.
Unwrap occurs here: https://github.com/http-rs/http-types/blob/41655460b0229064535308e329417f8de014c2a8/src/headers/headers.rs#L66
Error comes from here:
https://github.com/http-rs/http-types/blob/41655460b0229064535308e329417f8de014c2a8/src/headers/header_value.rs#L80

req.append_header(header.name, std::str::from_utf8(header.value)?);
}

Expand Down
33 changes: 32 additions & 1 deletion tests/server_decode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ mod test_utils;
mod server_decode {
use super::test_utils::TestIO;
use async_std::io::prelude::*;
use http_types::headers::TRANSFER_ENCODING;
use http_types::Request;
use http_types::Result;
use http_types::Url;
use http_types::{headers::TRANSFER_ENCODING, StatusCode};
use pretty_assertions::assert_eq;

async fn decode_lines(lines: Vec<&str>) -> Result<Option<Request>> {
Expand Down Expand Up @@ -125,4 +125,35 @@ mod server_decode {

Ok(())
}

#[async_std::test]
async fn none_utf8_header() {
let s = vec![
b"GET / HTTP/1.1" as &[u8],
b"host: localhost:8080",
b"none-utf8-header: \xc3\x28",
b"",
b"",
]
.join(b"\r\n" as &[u8]);
let (mut client, server) = TestIO::new();
client.write_all(&s).await.unwrap();
client.close();
let err = async_h1::server::decode(server).await.unwrap_err();
assert_eq!(err.status(), StatusCode::BadRequest);
}

#[async_std::test]
async fn none_ascii_header() {
let err = decode_lines(vec![
"GET / HTTP/1.1",
"host: localhost:8080",
"none-ascii-header: élo",
"",
"",
])
.await
.unwrap_err();
assert_eq!(err.status(), StatusCode::BadRequest);
}
}