From 3af3926d632cf913bb55e944df54e744e0b52534 Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Tue, 16 Feb 2016 20:50:03 +0100 Subject: [PATCH 1/4] remove rarely used `is_token` method from ByteString --- components/script/dom/bindings/str.rs | 6 ------ components/script/dom/xmlhttprequest.rs | 6 +++--- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/components/script/dom/bindings/str.rs b/components/script/dom/bindings/str.rs index 850277f596e..021d154191a 100644 --- a/components/script/dom/bindings/str.rs +++ b/components/script/dom/bindings/str.rs @@ -49,12 +49,6 @@ impl ByteString { ByteString::new(self.0.to_ascii_lowercase()) } - /// Returns whether `self` is a `token`, as defined by - /// [RFC 2616](http://tools.ietf.org/html/rfc2616#page-17). - pub fn is_token(&self) -> bool { - is_token(&self.0) - } - /// Returns whether `self` is a `field-value`, as defined by /// [RFC 2616](http://tools.ietf.org/html/rfc2616#page-32). pub fn is_field_value(&self) -> bool { diff --git a/components/script/dom/xmlhttprequest.rs b/components/script/dom/xmlhttprequest.rs index 749de9ac29b..947835f11b9 100644 --- a/components/script/dom/xmlhttprequest.rs +++ b/components/script/dom/xmlhttprequest.rs @@ -21,7 +21,7 @@ use dom::bindings::js::{JS, MutHeapJSVal, MutNullableHeap}; use dom::bindings::js::{Root, RootedReference}; use dom::bindings::refcounted::Trusted; use dom::bindings::reflector::{Reflectable, reflect_dom_object}; -use dom::bindings::str::{ByteString, USVString}; +use dom::bindings::str::{ByteString, USVString, is_token}; use dom::blob::Blob; use dom::document::DocumentSource; use dom::document::{Document, IsHTMLDocument}; @@ -345,7 +345,7 @@ impl XMLHttpRequestMethods for XMLHttpRequest { Some(Method::Extension(ref t)) if &**t == "TRACK" => Err(Error::Security), Some(parsed_method) => { // Step 3 - if !method.is_token() { + if !is_token(&method) { return Err(Error::Syntax) } @@ -413,7 +413,7 @@ impl XMLHttpRequestMethods for XMLHttpRequest { } // FIXME(#9548): Step 3. Normalize value // Step 4 - if !name.is_token() || !value.is_field_value() { + if !is_token(&name) || !value.is_field_value() { return Err(Error::Syntax); } let name_lower = name.to_lower(); From 746f7dd29eb5d71b211a48b3b5a2b70d7a7f340e Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Tue, 16 Feb 2016 20:57:56 +0100 Subject: [PATCH 2/4] move method from ByteString to module where it was used --- components/script/dom/bindings/str.rs | 64 ------------------------ components/script/dom/xmlhttprequest.rs | 66 ++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 65 deletions(-) diff --git a/components/script/dom/bindings/str.rs b/components/script/dom/bindings/str.rs index 021d154191a..6276c2c19bd 100644 --- a/components/script/dom/bindings/str.rs +++ b/components/script/dom/bindings/str.rs @@ -48,70 +48,6 @@ impl ByteString { pub fn to_lower(&self) -> ByteString { ByteString::new(self.0.to_ascii_lowercase()) } - - /// Returns whether `self` is a `field-value`, as defined by - /// [RFC 2616](http://tools.ietf.org/html/rfc2616#page-32). - pub fn is_field_value(&self) -> bool { - // Classifications of characters necessary for the [CRLF] (SP|HT) rule - #[derive(PartialEq)] - enum PreviousCharacter { - Other, - CR, - LF, - SPHT, // SP or HT - } - let mut prev = PreviousCharacter::Other; // The previous character - self.0.iter().all(|&x| { - // http://tools.ietf.org/html/rfc2616#section-2.2 - match x { - 13 => { // CR - if prev == PreviousCharacter::Other || prev == PreviousCharacter::SPHT { - prev = PreviousCharacter::CR; - true - } else { - false - } - }, - 10 => { // LF - if prev == PreviousCharacter::CR { - prev = PreviousCharacter::LF; - true - } else { - false - } - }, - 32 => { // SP - if prev == PreviousCharacter::LF || prev == PreviousCharacter::SPHT { - prev = PreviousCharacter::SPHT; - true - } else if prev == PreviousCharacter::Other { - // Counts as an Other here, since it's not preceded by a CRLF - // SP is not a CTL, so it can be used anywhere - // though if used immediately after a CR the CR is invalid - // We don't change prev since it's already Other - true - } else { - false - } - }, - 9 => { // HT - if prev == PreviousCharacter::LF || prev == PreviousCharacter::SPHT { - prev = PreviousCharacter::SPHT; - true - } else { - false - } - }, - 0...31 | 127 => false, // CTLs - x if x > 127 => false, // non ASCII - _ if prev == PreviousCharacter::Other || prev == PreviousCharacter::SPHT => { - prev = PreviousCharacter::Other; - true - }, - _ => false // Previous character was a CR/LF but not part of the [CRLF] (SP|HT) rule - } - }) - } } impl Into> for ByteString { diff --git a/components/script/dom/xmlhttprequest.rs b/components/script/dom/xmlhttprequest.rs index 947835f11b9..45fa9e1f58f 100644 --- a/components/script/dom/xmlhttprequest.rs +++ b/components/script/dom/xmlhttprequest.rs @@ -413,7 +413,7 @@ impl XMLHttpRequestMethods for XMLHttpRequest { } // FIXME(#9548): Step 3. Normalize value // Step 4 - if !is_token(&name) || !value.is_field_value() { + if !is_token(&name) || !is_field_value(&value) { return Err(Error::Syntax); } let name_lower = name.to_lower(); @@ -1388,3 +1388,67 @@ impl Extractable for SendParam { } } } + +/// Returns whether `bs` is a `field-value`, as defined by +/// [RFC 2616](http://tools.ietf.org/html/rfc2616#page-32). +pub fn is_field_value(slice: &[u8]) -> bool { + // Classifications of characters necessary for the [CRLF] (SP|HT) rule + #[derive(PartialEq)] + enum PreviousCharacter { + Other, + CR, + LF, + SPHT, // SP or HT + } + let mut prev = PreviousCharacter::Other; // The previous character + slice.iter().all(|&x| { + // http://tools.ietf.org/html/rfc2616#section-2.2 + match x { + 13 => { // CR + if prev == PreviousCharacter::Other || prev == PreviousCharacter::SPHT { + prev = PreviousCharacter::CR; + true + } else { + false + } + }, + 10 => { // LF + if prev == PreviousCharacter::CR { + prev = PreviousCharacter::LF; + true + } else { + false + } + }, + 32 => { // SP + if prev == PreviousCharacter::LF || prev == PreviousCharacter::SPHT { + prev = PreviousCharacter::SPHT; + true + } else if prev == PreviousCharacter::Other { + // Counts as an Other here, since it's not preceded by a CRLF + // SP is not a CTL, so it can be used anywhere + // though if used immediately after a CR the CR is invalid + // We don't change prev since it's already Other + true + } else { + false + } + }, + 9 => { // HT + if prev == PreviousCharacter::LF || prev == PreviousCharacter::SPHT { + prev = PreviousCharacter::SPHT; + true + } else { + false + } + }, + 0...31 | 127 => false, // CTLs + x if x > 127 => false, // non ASCII + _ if prev == PreviousCharacter::Other || prev == PreviousCharacter::SPHT => { + prev = PreviousCharacter::Other; + true + }, + _ => false // Previous character was a CR/LF but not part of the [CRLF] (SP|HT) rule + } + }) +} From fac4c31b4243a2e51b0154825cd3944b72a2bd97 Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Tue, 16 Feb 2016 21:06:45 +0100 Subject: [PATCH 3/4] implement XHR::SetRequestHeader Step 3 --- components/script/dom/xmlhttprequest.rs | 46 ++++++++++++++++++++----- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/components/script/dom/xmlhttprequest.rs b/components/script/dom/xmlhttprequest.rs index 45fa9e1f58f..df2cb8810d8 100644 --- a/components/script/dom/xmlhttprequest.rs +++ b/components/script/dom/xmlhttprequest.rs @@ -55,6 +55,7 @@ use std::ascii::AsciiExt; use std::borrow::ToOwned; use std::cell::{Cell, RefCell}; use std::default::Default; +use std::str; use std::sync::{Arc, Mutex}; use string_cache::Atom; use time; @@ -406,12 +407,15 @@ impl XMLHttpRequestMethods for XMLHttpRequest { } // https://xhr.spec.whatwg.org/#the-setrequestheader()-method - fn SetRequestHeader(&self, name: ByteString, mut value: ByteString) -> ErrorResult { + fn SetRequestHeader(&self, name: ByteString, value: ByteString) -> ErrorResult { // Step 1, 2 if self.ready_state.get() != XMLHttpRequestState::Opened || self.send_flag.get() { return Err(Error::InvalidState); } - // FIXME(#9548): Step 3. Normalize value + + // Step 3 + let value = trim_http_whitespace(&value); + // Step 4 if !is_token(&name) || !is_field_value(&value) { return Err(Error::Syntax); @@ -438,24 +442,24 @@ impl XMLHttpRequestMethods for XMLHttpRequest { None => unreachable!() }; - debug!("SetRequestHeader: name={:?}, value={:?}", name.as_str(), value.as_str()); + debug!("SetRequestHeader: name={:?}, value={:?}", name.as_str(), str::from_utf8(value).ok()); let mut headers = self.request_headers.borrow_mut(); // Step 6 - match headers.get_raw(name_str) { + let value = match headers.get_raw(name_str) { Some(raw) => { debug!("SetRequestHeader: old value = {:?}", raw[0]); let mut buf = raw[0].clone(); buf.extend_from_slice(b", "); - buf.extend_from_slice(&value); + buf.extend_from_slice(value); debug!("SetRequestHeader: new value = {:?}", buf); - value = ByteString::new(buf); + buf }, - None => {} - } + None => value.into(), + }; - headers.set_raw(name_str.to_owned(), vec![value.into()]); + headers.set_raw(name_str.to_owned(), vec![value]); Ok(()) } @@ -1452,3 +1456,27 @@ pub fn is_field_value(slice: &[u8]) -> bool { } }) } + +/// Normalize `self`, as defined by +/// [the Fetch Spec](https://fetch.spec.whatwg.org/#concept-header-value-normalize). +pub fn trim_http_whitespace(mut slice: &[u8]) -> &[u8] { + const HTTP_WS_BYTES: &'static [u8] = b"\x09\x0A\x0D\x20"; + + loop { + match slice.split_first() { + Some((first, remainder)) if HTTP_WS_BYTES.contains(first) => + slice = remainder, + _ => break, + } + } + + loop { + match slice.split_last() { + Some((last, remainder)) if HTTP_WS_BYTES.contains(last) => + slice = remainder, + _ => break, + } + } + + slice +} From 43c46090cf294788fb25e4426798f4170eca62f4 Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Tue, 16 Feb 2016 21:10:32 +0100 Subject: [PATCH 4/4] add XHR::SetRequestHeader Step 3 unit test --- tests/unit/script/dom/xmlhttprequest.rs | 27 +++++++++++++++++++++++++ tests/unit/script/lib.rs | 1 + 2 files changed, 28 insertions(+) create mode 100644 tests/unit/script/dom/xmlhttprequest.rs diff --git a/tests/unit/script/dom/xmlhttprequest.rs b/tests/unit/script/dom/xmlhttprequest.rs new file mode 100644 index 00000000000..f9d4ebc25e8 --- /dev/null +++ b/tests/unit/script/dom/xmlhttprequest.rs @@ -0,0 +1,27 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +use script::dom::xmlhttprequest::trim_http_whitespace; + +#[test] +fn test_trim_http_whitespace() { + fn test_trim(in_: &[u8], out: &[u8]) { + let b = trim_http_whitespace(in_); + assert_eq!(b, out); + } + + test_trim(b"", b""); + + test_trim(b" ", b""); + test_trim(b"a", b"a"); + test_trim(b" a", b"a"); + test_trim(b"a ", b"a"); + test_trim(b" a ", b"a"); + + test_trim(b"\t", b""); + test_trim(b"a", b"a"); + test_trim(b"\ta", b"a"); + test_trim(b"a\t", b"a"); + test_trim(b"\ta\t", b"a"); +} diff --git a/tests/unit/script/lib.rs b/tests/unit/script/lib.rs index fd5bea0f718..8270d8542d7 100644 --- a/tests/unit/script/lib.rs +++ b/tests/unit/script/lib.rs @@ -11,4 +11,5 @@ extern crate util; #[cfg(test)] mod dom { mod bindings; mod blob; + mod xmlhttprequest; }