Skip to content

Commit

Permalink
fix(headers): correctly handle repeated headers
Browse files Browse the repository at this point in the history
    HeaderX: a
    HeaderX: b

MUST be interpreted as

    HeaderX: a, b

See: https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2

Fixes #683
  • Loading branch information
Stebalien committed Mar 24, 2016
1 parent 028f586 commit 70c6914
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 27 deletions.
7 changes: 2 additions & 5 deletions src/header/common/cache_control.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::fmt;
use std::str::FromStr;
use header::{Header, HeaderFormat};
use header::parsing::{from_one_comma_delimited, fmt_comma_delimited};
use header::parsing::{from_comma_delimited, fmt_comma_delimited};

/// `Cache-Control` header, defined in [RFC7234](https://tools.ietf.org/html/rfc7234#section-5.2)
///
Expand Down Expand Up @@ -55,10 +55,7 @@ impl Header for CacheControl {
}

fn parse_header(raw: &[Vec<u8>]) -> ::Result<CacheControl> {
let directives = raw.iter()
.filter_map(|line| from_one_comma_delimited(&line[..]).ok())
.collect::<Vec<Vec<CacheDirective>>>()
.concat();
let directives = try!(from_comma_delimited(raw));
if !directives.is_empty() {
Ok(CacheControl(directives))
} else {
Expand Down
11 changes: 10 additions & 1 deletion src/header/common/content_length.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,16 @@ __hyper__tm!(ContentLength, tests {
test_header!(test1, vec![b"3495"], Some(HeaderField(3495)));

test_header!(test_invalid, vec![b"34v95"], None);
test_header!(test_duplicates, vec![b"5", b"5"], Some(HeaderField(5)));

// Can't use the test_header macro because "5, 5" gets cleaned to "5".
#[test]
fn test_duplicates() {
let parsed = HeaderField::parse_header(&[b"5"[..].into(),
b"5"[..].into()]).unwrap();
assert_eq!(parsed, HeaderField(5));
assert_eq!(format!("{}", parsed), "5");
}

test_header!(test_duplicates_vary, vec![b"5", b"6", b"5"], None);
});

Expand Down
11 changes: 9 additions & 2 deletions src/header/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,15 @@ macro_rules! test_header {
assert_eq!(val.ok(), typed);
// Test formatting
if typed.is_some() {
let res: &str = str::from_utf8($raw[0]).unwrap();
assert_eq!(format!("{}", typed.unwrap()), res);
let raw = &($raw)[..];
let mut iter = raw.iter().map(|b|str::from_utf8(&b[..]).unwrap());
let mut joined = String::new();
joined.push_str(iter.next().unwrap());
for s in iter {
joined.push_str(", ");
joined.push_str(s);
}
assert_eq!(format!("{}", typed.unwrap()), joined);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/header/common/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::fmt::{self, Display};
use std::str::FromStr;

use header::{Header, HeaderFormat};
use header::parsing::{from_one_raw_str, from_one_comma_delimited};
use header::parsing::{from_one_raw_str, from_comma_delimited};

/// `Range` header, defined in [RFC7233](https://tools.ietf.org/html/rfc7233#section-3.1)
///
Expand Down Expand Up @@ -130,7 +130,7 @@ impl FromStr for Range {

match (iter.next(), iter.next()) {
(Some("bytes"), Some(ranges)) => {
match from_one_comma_delimited(ranges.as_bytes()) {
match from_comma_delimited(&[ranges]) {
Ok(ranges) => {
if ranges.is_empty() {
return Err(::Error::Header);
Expand Down
7 changes: 7 additions & 0 deletions src/header/common/transfer_encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ header! {
Some(HeaderField(
vec![Encoding::Gzip, Encoding::Chunked]
)));
// Issue: #683
test_header!(
test2,
vec![b"chunked", b"chunked"],
Some(HeaderField(
vec![Encoding::Chunked, Encoding::Chunked]
)));

}
}
Expand Down
28 changes: 11 additions & 17 deletions src/header/parsing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,18 @@ pub fn from_raw_str<T: str::FromStr>(raw: &[u8]) -> ::Result<T> {

/// Reads a comma-delimited raw header into a Vec.
#[inline]
pub fn from_comma_delimited<T: str::FromStr>(raw: &[Vec<u8>]) -> ::Result<Vec<T>> {
if raw.len() != 1 {
return Err(::Error::Header);
pub fn from_comma_delimited<T: str::FromStr, S: AsRef<[u8]>>(raw: &[S]) -> ::Result<Vec<T>> {
let mut result = Vec::new();
for s in raw {
let s = try!(str::from_utf8(s.as_ref()));
result.extend(s.split(',')
.filter_map(|x| match x.trim() {
"" => None,
y => Some(y)
})
.filter_map(|x| x.parse().ok()))
}
// we JUST checked that raw.len() == 1, so raw[0] WILL exist.
from_one_comma_delimited(& unsafe { raw.get_unchecked(0) }[..])
}

/// Reads a comma-delimited raw string into a Vec.
pub fn from_one_comma_delimited<T: str::FromStr>(raw: &[u8]) -> ::Result<Vec<T>> {
let s = try!(str::from_utf8(raw));
Ok(s.split(',')
.filter_map(|x| match x.trim() {
"" => None,
y => Some(y)
})
.filter_map(|x| x.parse().ok())
.collect())
Ok(result)
}

/// Format an array into a comma-delimited string.
Expand Down

0 comments on commit 70c6914

Please sign in to comment.