Auto merge of #12634 - malisas:malisa-headersAPI, r=jdm

Headers API constructor and methods

<!-- Please describe your changes on the following line: -->
This PR fills out the constructor and the delete, get, has, and set methods for the Headers API. Addresses issue #11897 .

The PR also rewrites the append method to support `hyper::header::Headers`'s HashMap `insert` method, which overwrites entries instead of appending.
As a result of this, for a given header name there is at most one value in the inner "header list"/HashMap. Multiple values for the same name are comma-delimited.

There are still a few TODOs:
- Support `OpenEndedDictionary<ByteString>`  as a possible `HeadersInit` value. [OpenEndedDictionary<T> is a future IDL construct.](https://fetch.spec.whatwg.org/#headers-class)
- Support `iterable<ByteString, ByteString>`. Related issue: #12628
- Values are comma-delimited, except for values with the name `set-cookie`, which are newline-delimited. This is because values for `set-cookie` are [allowed to contain](https://tools.ietf.org/html/rfc7230#section-3.2.2) inner commas. This violates the [spec](https://fetch.spec.whatwg.org/#concept-header-value-combined).
- The TODOs from PR #12467 regarding value parsing also still need to be resolved.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because tests for the Headers API already exists, but this commit does not implement the interface fully. The tests will fail.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12634)
<!-- Reviewable:end -->

6 files changed, 155 insertions, 75 deletions

diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs
index e97edc00572..4ae212aee78 100644
--- a/components/script/dom/headers.rs
+++ b/components/script/dom/headers.rs
@@ -5,12 +5,13 @@
 use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::HeadersBinding;
 use dom::bindings::codegen::Bindings::HeadersBinding::HeadersMethods;
-use dom::bindings::error::Error;
+use dom::bindings::codegen::UnionTypes::HeadersOrByteStringSequenceSequence;
+use dom::bindings::error::{Error, ErrorResult, Fallible};
 use dom::bindings::global::GlobalRef;
 use dom::bindings::js::Root;
 use dom::bindings::reflector::{Reflector, reflect_dom_object};
 use dom::bindings::str::{ByteString, is_token};
-use hyper;
+use hyper::header::Headers as HyperHeaders;
 use std::result::Result;
 
 #[dom_struct]
@@ -18,7 +19,7 @@ pub struct Headers {
     reflector_: Reflector,
     guard: Guard,
     #[ignore_heap_size_of = "Defined in hyper"]
-    header_list: DOMRefCell<hyper::header::Headers>
+    header_list: DOMRefCell<HyperHeaders>
 }
 
 // https://fetch.spec.whatwg.org/#concept-headers-guard
@@ -36,12 +37,50 @@ impl Headers {
         Headers {
             reflector_: Reflector::new(),
             guard: Guard::None,
-            header_list: DOMRefCell::new(hyper::header::Headers::new()),
+            header_list: DOMRefCell::new(HyperHeaders::new()),
         }
     }
 
-    pub fn new(global: GlobalRef) -> Root<Headers> {
-        reflect_dom_object(box Headers::new_inherited(), global, HeadersBinding::Wrap)
+    // https://fetch.spec.whatwg.org/#concept-headers-fill
+    pub fn new(global: GlobalRef, init: Option<HeadersBinding::HeadersInit>)
+               -> Fallible<Root<Headers>> {
+        let dom_headers_new = reflect_dom_object(box Headers::new_inherited(), global, HeadersBinding::Wrap);
+        match init {
+            // Step 1
+            Some(HeadersOrByteStringSequenceSequence::Headers(h)) => {
+                // header_list_copy has type hyper::header::Headers
+                let header_list_copy = h.header_list.clone();
+                for header in header_list_copy.borrow().iter() {
+                    try!(dom_headers_new.Append(
+                        ByteString::new(Vec::from(header.name())),
+                        ByteString::new(Vec::from(header.value_string().into_bytes()))
+                    ));
+                }
+                Ok(dom_headers_new)
+            },
+            // Step 2
+            Some(HeadersOrByteStringSequenceSequence::ByteStringSequenceSequence(v)) => {
+                for mut seq in v {
+                    if seq.len() == 2 {
+                        let val = seq.pop().unwrap();
+                        let name = seq.pop().unwrap();
+                        try!(dom_headers_new.Append(name, val));
+                    } else {
+                        return Err(Error::Type(
+                            format!("Each header object must be a sequence of length 2 - found one with length {}",
+                                    seq.len())));
+                    }
+                }
+                Ok(dom_headers_new)
+            },
+            // Step 3 TODO constructor for when init is an open-ended dictionary
+            None => Ok(dom_headers_new),
+        }
+    }
+
+    pub fn Constructor(global: GlobalRef, init: Option<HeadersBinding::HeadersInit>)
+                       -> Fallible<Root<Headers>> {
+        Headers::new(global, init)
     }
 }
 
@@ -50,32 +89,102 @@ impl HeadersMethods for Headers {
     fn Append(&self, name: ByteString, value: ByteString) -> Result<(), Error> {
         // Step 1
         let value = normalize_value(value);
-
         // Step 2
-        let (valid_name, valid_value) = try!(validate_name_and_value(name, value));
+        let (mut valid_name, valid_value) = try!(validate_name_and_value(name, value));
+        valid_name = valid_name.to_lowercase();
         // Step 3
         if self.guard == Guard::Immutable {
             return Err(Error::Type("Guard is immutable".to_string()));
         }
-
         // Step 4
         if self.guard == Guard::Request && is_forbidden_header_name(&valid_name) {
             return Ok(());
         }
-
         // Step 5
         if self.guard == Guard::RequestNoCors && !is_cors_safelisted_request_header(&valid_name) {
             return Ok(());
         }
-
         // Step 6
         if self.guard == Guard::Response && is_forbidden_response_header(&valid_name) {
             return Ok(());
         }
+        // Step 7
+        let mut combined_value = self.header_list.borrow_mut().get_raw(&valid_name).unwrap()[0].clone();
+        combined_value.push(b","[0]);
+        combined_value.extend(valid_value.iter().cloned());
+        self.header_list.borrow_mut().set_raw(valid_name, vec![combined_value]);
+        Ok(())
+    }
+
+    // https://fetch.spec.whatwg.org/#dom-headers-delete
+    fn Delete(&self, name: ByteString) -> ErrorResult {
+        // Step 1
+        let valid_name = try!(validate_name(name));
+        // Step 2
+        if self.guard == Guard::Immutable {
+            return Err(Error::Type("Guard is immutable".to_string()));
+        }
+        // Step 3
+        if self.guard == Guard::Request && is_forbidden_header_name(&valid_name) {
+            return Ok(());
+        }
+        // Step 4
+        if self.guard == Guard::RequestNoCors && !is_cors_safelisted_request_header(&valid_name) {
+            return Ok(());
+        }
+        // Step 5
+        if self.guard == Guard::Response && is_forbidden_response_header(&valid_name) {
+            return Ok(());
+        }
+        // Step 6
+        self.header_list.borrow_mut().remove_raw(&valid_name);
+        Ok(())
+    }
+
+    // https://fetch.spec.whatwg.org/#dom-headers-get
+    fn Get(&self, name: ByteString) -> Fallible<Option<ByteString>> {
+        // Step 1
+        let valid_name = &try!(validate_name(name));
+        Ok(self.header_list.borrow().get_raw(&valid_name).map(|v| {
+            ByteString::new(v[0].clone())
+        }))
+    }
 
+    // https://fetch.spec.whatwg.org/#dom-headers-has
+    fn Has(&self, name: ByteString) -> Fallible<bool> {
+        // Step 1
+        let valid_name = try!(validate_name(name));
+        // Step 2
+        Ok(self.header_list.borrow_mut().get_raw(&valid_name).is_some())
+    }
+
+    // https://fetch.spec.whatwg.org/#dom-headers-set
+    fn Set(&self, name: ByteString, value: ByteString) -> Fallible<()> {
+        // Step 1
+        let value = normalize_value(value);
+        // Step 2
+        let (mut valid_name, valid_value) = try!(validate_name_and_value(name, value));
+        valid_name = valid_name.to_lowercase();
+        // Step 3
+        if self.guard == Guard::Immutable {
+            return Err(Error::Type("Guard is immutable".to_string()));
+        }
+        // Step 4
+        if self.guard == Guard::Request && is_forbidden_header_name(&valid_name) {
+            return Ok(());
+        }
+        // Step 5
+        if self.guard == Guard::RequestNoCors && !is_cors_safelisted_request_header(&valid_name) {
+            return Ok(());
+        }
+        // Step 6
+        if self.guard == Guard::Response && is_forbidden_response_header(&valid_name) {
+            return Ok(());
+        }
         // Step 7
+        // https://fetch.spec.whatwg.org/#concept-header-list-set
         self.header_list.borrow_mut().set_raw(valid_name, vec![valid_value]);
-        return Ok(());
+        Ok(())
     }
 }
 
@@ -130,7 +239,7 @@ pub fn is_forbidden_header_name(name: &str) -> bool {
 // ISSUE 1:
 // It defines a value as "a byte sequence that matches the field-content token production."
 // To note, there is a difference between field-content and
-// field-value (which is made up of fied-content and obs-fold). The
+// field-value (which is made up of field-content and obs-fold). The
 // current definition does not allow for obs-fold (which are white
 // space and newlines) in values. So perhaps a value should be defined
 // as "a byte sequence that matches the field-value token production."
@@ -147,14 +256,19 @@ pub fn is_forbidden_header_name(name: &str) -> bool {
 // [4] https://www.rfc-editor.org/errata_search.php?rfc=7230
 fn validate_name_and_value(name: ByteString, value: ByteString)
                            -> Result<(String, Vec<u8>), Error> {
-    if !is_field_name(&name) {
-        return Err(Error::Type("Name is not valid".to_string()));
-    }
+    let valid_name = try!(validate_name(name));
     if !is_field_content(&value) {
         return Err(Error::Type("Value is not valid".to_string()));
     }
+    Ok((valid_name, value.into()))
+}
+
+fn validate_name(name: ByteString) -> Result<String, Error> {
+    if !is_field_name(&name) {
+        return Err(Error::Type("Name is not valid".to_string()));
+    }
     match String::from_utf8(name.into()) {
-        Ok(ns) => Ok((ns, value.into())),
+        Ok(ns) => Ok(ns),
         _ => Err(Error::Type("Non-UTF8 header name found".to_string())),
     }
 }
diff --git a/components/script/dom/webidls/Headers.webidl b/components/script/dom/webidls/Headers.webidl
index 038dbe46f74..6696bf64731 100644
--- a/components/script/dom/webidls/Headers.webidl
+++ b/components/script/dom/webidls/Headers.webidl
@@ -4,19 +4,21 @@
 
 // https://fetch.spec.whatwg.org/#headers-class
 
-/* typedef (Headers or sequence<sequence<ByteString>>) HeadersInit; */
-
-/* [Constructor(optional HeadersInit init),*/
- [Exposed=(Window,Worker)]
+// TODO support OpenEndedDictionary<ByteString>
+typedef (Headers or sequence<sequence<ByteString>>) HeadersInit;
 
+[Constructor(optional HeadersInit init),
+ Exposed=(Window,Worker)]
 interface Headers {
   [Throws]
   void append(ByteString name, ByteString value);
+  [Throws]
+  void delete(ByteString name);
+  [Throws]
+  ByteString? get(ByteString name);
+  [Throws]
+  boolean has(ByteString name);
+  [Throws]
+  void set(ByteString name, ByteString value);
+  // iterable<ByteString, ByteString>; // TODO see issue #12628
 };
-
-/* void delete(ByteString name);
- * ByteString? get(ByteString name);
- * boolean has(ByteString name);
- * void set(ByteString name, ByteString value);
- * iterable<ByteString, ByteString>;
- * }; */
diff --git a/tests/wpt/metadata/fetch/api/headers/headers-basic.html.ini b/tests/wpt/metadata/fetch/api/headers/headers-basic.html.ini
index 317a5ee2729..c4943c982c1 100644
--- a/tests/wpt/metadata/fetch/api/headers/headers-basic.html.ini
+++ b/tests/wpt/metadata/fetch/api/headers/headers-basic.html.ini
@@ -1,11 +1,5 @@
 [headers-basic.html]
   type: testharness
-  [Create headers from no parameter]
-    expected: FAIL
-
-  [Create headers from undefined parameter]
-    expected: FAIL
-
   [Create headers from empty object]
     expected: FAIL
 
@@ -15,7 +9,7 @@
   [Create headers with OpenEndedDictionary]
     expected: FAIL
 
-  [Create headers whith existing headers]
+  [Create headers with existing headers]
     expected: FAIL
 
   [Check append method]
@@ -47,4 +41,3 @@
 
   [Check forEach method]
     expected: FAIL
-
diff --git a/tests/wpt/metadata/fetch/api/headers/headers-errors.html.ini b/tests/wpt/metadata/fetch/api/headers/headers-errors.html.ini
index c8480f1ac83..121fea81b51 100644
--- a/tests/wpt/metadata/fetch/api/headers/headers-errors.html.ini
+++ b/tests/wpt/metadata/fetch/api/headers/headers-errors.html.ini
@@ -1,44 +1,7 @@
 [headers-errors.html]
   type: testharness
-  [Check headers get with an invalid name invalidĀ]
-    expected: FAIL
-
-  [Check headers get with an invalid name [object Object\]]
-    expected: FAIL
-
-  [Check headers delete with an invalid name invalidĀ]
-    expected: FAIL
-
-  [Check headers delete with an invalid name [object Object\]]
-    expected: FAIL
-
-  [Check headers has with an invalid name invalidĀ]
-    expected: FAIL
-
-  [Check headers has with an invalid name [object Object\]]
-    expected: FAIL
-
-  [Check headers set with an invalid name invalidĀ]
-    expected: FAIL
-
-  [Check headers set with an invalid name [object Object\]]
-    expected: FAIL
-
-  [Check headers set with an invalid value invalidĀ]
-    expected: FAIL
-
-  [Check headers append with an invalid name invalidĀ]
-    expected: FAIL
-
-  [Check headers append with an invalid name [object Object\]]
-    expected: FAIL
-
-  [Check headers append with an invalid value invalidĀ]
-    expected: FAIL
-
   [Headers forEach throws if argument is not callable]
     expected: FAIL
 
   [Headers forEach loop should stop if callback is throwing exception]
     expected: FAIL
-
diff --git a/tests/wpt/metadata/fetch/api/headers/headers-structure.html.ini b/tests/wpt/metadata/fetch/api/headers/headers-structure.html.ini
index db946a8f8fd..967cb42ecfd 100644
--- a/tests/wpt/metadata/fetch/api/headers/headers-structure.html.ini
+++ b/tests/wpt/metadata/fetch/api/headers/headers-structure.html.ini
@@ -1,3 +1,11 @@
 [headers-structure.html]
   type: testharness
-  expected: TIMEOUT
+  expected: OK
+  [Headers has entries method]
+    expected: FAIL
+
+  [Headers has keys method]
+    expected: FAIL
+
+  [Headers has values method]
+    expected: FAIL
diff --git a/tests/wpt/web-platform-tests/fetch/api/headers/headers-basic.html b/tests/wpt/web-platform-tests/fetch/api/headers/headers-basic.html
index 5c374ed1faf..26bbfe35935 100644
--- a/tests/wpt/web-platform-tests/fetch/api/headers/headers-basic.html
+++ b/tests/wpt/web-platform-tests/fetch/api/headers/headers-basic.html
@@ -66,7 +66,7 @@
           assert_equals(headers2.get(name), String(headerDict[name]),
             "name: " + name + " has value: " + headerDict[name]);
         }
-      }, "Create headers whith existing headers");
+      }, "Create headers with existing headers");
 
       test(function() {
         var headers = new Headers();