diff --git a/README.md b/README.md index ee9948e..6323277 100644 --- a/README.md +++ b/README.md @@ -82,21 +82,6 @@ tree and an obvious API. It is inspired by libraries like [superagent](http://visionmedia.github.io/superagent/) and [fetch API](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API). -This library does not try to enforce web standards correctness. It uses HTTP/1.1, -but whether the request is _perfect_ HTTP/1.1 compatible is up to the user of the -library. For example: - -```rust - let resp = ureq::post("https://myapi.acme.com/blah") - .set("Jättegött", "Vegankörv") - .call(); -``` - -The header name and value would be encoded in utf-8 and sent, but that is actually not -correct according to spec cause an HTTP header name should be ascii. The absolutely -correct way would be to have `.set(header, value)` return a `Result`. This library opts -for convenience over correctness, so the decision is left to the user. - ### Sync forever This library uses blocking socket reads and writes. When it was diff --git a/src/header.rs b/src/header.rs index bf3dfed..2e6ac3c 100644 --- a/src/header.rs +++ b/src/header.rs @@ -63,6 +63,13 @@ impl Header { pub fn is_name(&self, other: &str) -> bool { self.name().eq_ignore_ascii_case(other) } + + pub(crate) fn validate(&self) -> Result<(), Error> { + if !valid_name(self.name()) || !valid_value(&self.line.as_str()[self.index + 1..]) { + return Err(Error::BadHeader); + } + Ok(()) + } } pub fn get_header<'a, 'b>(headers: &'b [Header], name: &'a str) -> Option<&'b str> { @@ -89,7 +96,7 @@ pub fn add_header(headers: &mut Vec
, header: Header) { headers.push(header); } -// https://tools.ietf.org/html/rfc7230#section-3.2.3 +// https://tools.ietf.org/html/rfc7230#section-3.2 // Each header field consists of a case-insensitive field name followed // by a colon (":"), optional leading whitespace, the field value, and // optional trailing whitespace. @@ -112,6 +119,32 @@ fn is_tchar(b: u8) -> bool { } } +// https://tools.ietf.org/html/rfc7230#section-3.2 +// Note that field-content has an errata: +// https://www.rfc-editor.org/errata/eid4189 +// field-value = *( field-content / obs-fold ) +// field-content = field-vchar [ 1*( SP / HTAB ) field-vchar ] +// field-vchar = VCHAR / obs-text +// +// obs-fold = CRLF 1*( SP / HTAB ) +// ; obsolete line folding +// ; see Section 3.2.4 +// https://tools.ietf.org/html/rfc5234#appendix-B.1 +// VCHAR = %x21-7E +// ; visible (printing) characters +fn valid_value(value: &str) -> bool { + value.bytes().all(is_field_vchar_or_obs_fold) +} + +#[inline] +fn is_field_vchar_or_obs_fold(b: u8) -> bool { + match b { + b' ' | b'\t' => true, + 0x21..=0x7E => true, + _ => false, + } +} + impl FromStr for Header { type Err = Error; fn from_str(s: &str) -> Result { @@ -124,11 +157,9 @@ impl FromStr for Header { return Err(Error::BadHeader); } - if !valid_name(&line[0..index]) { - return Err(Error::BadHeader); - } - - Ok(Header { line, index }) + let header = Header { line, index }; + header.validate()?; + Ok(header) } } @@ -144,12 +175,39 @@ fn test_valid_name() { assert!(!valid_name("Gödel")); } +#[test] +fn test_valid_value() { + assert!(valid_value("example")); + assert!(valid_value("foo bar")); + assert!(valid_value(" foobar ")); + assert!(valid_value(" foo\tbar ")); + assert!(valid_value(" foo~")); + assert!(valid_value(" !bar")); + assert!(valid_value(" ")); + assert!(!valid_value(" \nfoo")); + assert!(!valid_value("foo\x7F")); +} + #[test] fn test_parse_invalid_name() { - let h = "Content-Type :".parse::
(); - match h { - Err(Error::BadHeader) => {} - h => assert!(false, "expected BadHeader error, got {:?}", h), + let cases = vec![ + "Content-Type :", + " Content-Type: foo", + "Content-Type foo", + "\"some-header\": foo", + "Gödel: Escher, Bach", + "Foo: \n", + "Foo: \nbar", + "Foo: \x7F bar", + ]; + for c in cases { + let result = c.parse::
(); + assert!( + matches!(result, Err(Error::BadHeader)), + "'{}'.parse(): expected BadHeader, got {:?}", + c, + result + ); } } diff --git a/src/lib.rs b/src/lib.rs index 4be61cc..07d2e8b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,7 +7,6 @@ //! * Minimal dependency tree //! * Obvious API //! * Blocking API -//! * Convenience over correctness //! * No use of unsafe //! //! ``` diff --git a/src/request.rs b/src/request.rs index ee5f200..1423eef 100644 --- a/src/request.rs +++ b/src/request.rs @@ -112,6 +112,9 @@ impl Request { } fn do_call(&self, payload: Payload) -> Result { + for h in &self.headers { + h.validate()?; + } let response = self.to_url().and_then(|url| { let reader = payload.into_read(); let unit = Unit::new(&self, &url, true, &reader); diff --git a/src/test/simple.rs b/src/test/simple.rs index d49d67d..7be9d1b 100644 --- a/src/test/simple.rs +++ b/src/test/simple.rs @@ -10,7 +10,10 @@ fn header_passing() { assert_eq!(unit.header("X-Foo").unwrap(), "bar"); test::make_response(200, "OK", vec!["X-Bar: foo"], vec![]) }); - let resp = get("test://host/header_passing").set("X-Foo", "bar").call().unwrap(); + let resp = get("test://host/header_passing") + .set("X-Foo", "bar") + .call() + .unwrap(); assert_eq!(resp.status(), 200); assert!(resp.has("X-Bar")); assert_eq!(resp.header("X-Bar").unwrap(), "foo"); @@ -26,7 +29,8 @@ fn repeat_non_x_header() { let resp = get("test://host/repeat_non_x_header") .set("Accept", "bar") .set("Accept", "baz") - .call().unwrap(); + .call() + .unwrap(); assert_eq!(resp.status(), 200); } @@ -44,7 +48,8 @@ fn repeat_x_header() { let resp = get("test://host/repeat_x_header") .set("X-Forwarded-For", "130.240.19.2") .set("X-Forwarded-For", "130.240.19.3") - .call().unwrap(); + .call() + .unwrap(); assert_eq!(resp.status(), 200); } @@ -156,11 +161,12 @@ fn non_ascii_header() { }); let resp = get("test://host/non_ascii_header") .set("Bäd", "Headör") - .call().unwrap(); - // surprisingly, this is ok, because this lib is not about enforcing standards. - assert!(resp.ok()); - assert_eq!(resp.status(), 200); - assert_eq!(resp.status_text(), "OK"); + .call(); + assert!( + matches!(resp, Err(Error::BadHeader)), + "expected Some(&BadHeader), got {:?}", + resp + ); } #[test] @@ -184,7 +190,8 @@ pub fn header_with_spaces_before_value() { }); let resp = get("test://host/space_before_value") .set("X-Test", " value") - .call().unwrap(); + .call() + .unwrap(); assert_eq!(resp.status(), 200); }