Auto merge of #29465 - 2shiori17:issue-29388, r=jdm

Implement Headers.prototype.getSetCookie

<!-- Please describe your changes on the following line: -->

---
<!-- 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
- [x] These changes fix #29388

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

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

components/net/fetch/headers.rs

@@ -3,6 +3,7 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use headers::HeaderMap;
+use net_traits::fetch::headers::get_value_from_header_list;
 use std::iter::Peekable;
 use std::str::Chars;
 
@@ -29,45 +30,49 @@ fn get_header_value_as_list(name: &str, headers: &HeaderMap) -> Option<Vec<Strin
     let initial_value = get_value_from_header_list(name, headers);
 
     if let Some(input) = initial_value {
-        // Step 4
+        // https://fetch.spec.whatwg.org/#header-value-get-decode-and-split
+        // Step 1
+        let input = input.into_iter().map(|u| char::from(u)).collect::<String>();
+
+        // Step 2
         let mut position = input.chars().peekable();
 
-        // Step 5
+        // Step 3
         let mut values: Vec<String> = vec![];
 
-        // Step 6
+        // Step 4
         let mut value = String::new();
 
-        // Step 7
+        // Step 5
         while position.peek().is_some() {
-            // Step 7.1
+            // Step 5.1
             value += &*collect_sequence(&mut position, char_is_not_quote_or_comma);
 
-            // Step 7.2
+            // Step 5.2
             if let Some(&ch) = position.peek() {
                 if ch == '\u{0022}' {
-                    // Step 7.2.1.1
+                    // Step 5.2.1.1
                     value += &*collect_http_quoted_string(&mut position, false);
 
-                    // Step 7.2.1.2
+                    // Step 5.2.1.2
                     if position.peek().is_some() {
                         continue;
                     }
                 } else {
                     // ch == '\u{002C}'
 
-                    // Step 7.2.2.2
+                    // Step 5.2.2.2
                     position.next();
                 }
             }
 
-            // Step 7.3
+            // Step 5.3
             value = value.trim_matches(HTTP_TAB_OR_SPACE).to_string();
 
-            // Step 7.4
+            // Step 5.4
             values.push(value);
 
-            // Step 7.5
+            // Step 5.5
             value = String::new();
         }
 
@@ -155,19 +160,3 @@ fn collect_http_quoted_string(position: &mut Peekable<Chars>, extract_value: boo
     // Step 6, 7
     return value;
 }
-
-/// <https://fetch.spec.whatwg.org/#concept-header-list-get>
-fn get_value_from_header_list(name: &str, headers: &HeaderMap) -> Option<String> {
-    let values = headers
-        .get_all(name)
-        .iter()
-        .map(|val| val.to_str().unwrap());
-
-    // Step 1
-    if values.size_hint() == (0, Some(0)) {
-        return None;
-    }
-
-    // Step 2
-    return Some(values.collect::<Vec<&str>>().join(", "));
-}

components/net_traits/fetch/headers.rs

@@ -0,0 +1,18 @@
+/* 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 https://mozilla.org/MPL/2.0/. */
+
+use headers::HeaderMap;
+
+/// <https://fetch.spec.whatwg.org/#concept-header-list-get>
+pub fn get_value_from_header_list(name: &str, headers: &HeaderMap) -> Option<Vec<u8>> {
+    let values = headers.get_all(name).iter().map(|val| val.as_bytes());
+
+    // Step 1
+    if values.size_hint() == (0, Some(0)) {
+        return None;
+    }
+
+    // Step 2
+    return Some(values.collect::<Vec<&[u8]>>().join(&[0x2C, 0x20][..]));
+}

components/net_traits/lib.rs

@@ -53,6 +53,11 @@ pub mod image {
     pub mod base;
 }
 
+/// An implementation of the [Fetch specification](https://fetch.spec.whatwg.org/)
+pub mod fetch {
+    pub mod headers;
+}
+
 /// A loading context, for context-specific sniffing, as defined in
 /// <https://mimesniff.spec.whatwg.org/#context-specific-sniffing>
 #[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)]

components/script/dom/headers.rs

@@ -13,7 +13,9 @@ use crate::dom::globalscope::GlobalScope;
 use data_url::mime::Mime as DataUrlMime;
 use dom_struct::dom_struct;
 use http::header::{HeaderMap as HyperHeaders, HeaderName, HeaderValue};
-use net_traits::request::is_cors_safelisted_request_header;
+use net_traits::{
+    fetch::headers::get_value_from_header_list, request::is_cors_safelisted_request_header,
+};
 use std::cell::Cell;
 use std::str::{self, FromStr};
 
@@ -65,55 +67,60 @@ impl HeadersMethods for Headers {
     fn Append(&self, name: ByteString, value: ByteString) -> ErrorResult {
         // Step 1
         let value = normalize_value(value);
+
         // Step 2
+        // https://fetch.spec.whatwg.org/#headers-validate
         let (mut valid_name, valid_value) = validate_name_and_value(name, value)?;
         valid_name = valid_name.to_lowercase();
-        // Step 3
+
         if self.guard.get() == Guard::Immutable {
             return Err(Error::Type("Guard is immutable".to_string()));
         }
-        // Step 4
         if self.guard.get() == Guard::Request && is_forbidden_header_name(&valid_name) {
             return Ok(());
         }
-        // Step 5
-        if self.guard.get() == Guard::RequestNoCors &&
-            !is_cors_safelisted_request_header(&valid_name, &valid_value)
-        {
-            return Ok(());
-        }
-        // Step 6
         if self.guard.get() == Guard::Response && is_forbidden_response_header(&valid_name) {
             return Ok(());
         }
-        // Step 7
+
-        // FIXME: this is NOT what WHATWG says to do when appending
+        // Step 3
-        // another copy of an existing header. HyperHeaders
+        if self.guard.get() == Guard::RequestNoCors {
-        // might not expose the information we need to do it right.
+            let tmp_value = if let Some(mut value) =
-        let mut combined_value: Vec<u8> = vec![];
+                get_value_from_header_list(&valid_name, &self.header_list.borrow())
-        if let Some(v) = self
-            .header_list
-            .borrow()
-            .get(HeaderName::from_str(&valid_name).unwrap())
             {
-            combined_value = v.as_bytes().to_vec();
+                value.extend(b", ");
-            combined_value.extend(b", ");
+                value.extend(valid_value.clone());
+                value
+            } else {
+                valid_value.clone()
+            };
+
+            if !is_cors_safelisted_request_header(&valid_name, &tmp_value) {
+                return Ok(());
             }
-        combined_value.extend(valid_value.iter().cloned());
+        }
-        match HeaderValue::from_bytes(&combined_value) {
+
+        // Step 4
+        match HeaderValue::from_bytes(&valid_value) {
             Ok(value) => {
                 self.header_list
                     .borrow_mut()
-                    .insert(HeaderName::from_str(&valid_name).unwrap(), value);
+                    .append(HeaderName::from_str(&valid_name).unwrap(), value);
             },
             Err(_) => {
                 // can't add the header, but we don't need to panic the browser over it
                 warn!(
                     "Servo thinks \"{:?}\" is a valid HTTP header value but HeaderValue doesn't.",
-                    combined_value
+                    valid_value
                 );
             },
         };
+
+        // Step 5
+        if self.guard.get() == Guard::RequestNoCors {
+            self.remove_privileged_no_cors_request_headers();
+        }
+
         Ok(())
     }
 
@@ -148,11 +155,20 @@ impl HeadersMethods for Headers {
     fn Get(&self, name: ByteString) -> Fallible<Option<ByteString>> {
         // Step 1
         let valid_name = validate_name(name)?;
-        Ok(self
+        Ok(
-            .header_list
+            get_value_from_header_list(&valid_name, &self.header_list.borrow())
+                .map(|v| ByteString::new(v)),
+        )
+    }
+
+    // https://fetch.spec.whatwg.org/#dom-headers-getsetcookie
+    fn GetSetCookie(&self) -> Vec<ByteString> {
+        self.header_list
             .borrow()
-            .get(HeaderName::from_str(&valid_name).unwrap())
+            .get_all("set-cookie")
-            .map(|v| ByteString::new(v.as_bytes().to_vec())))
+            .iter()
+            .map(|v| ByteString::new(v.as_bytes().to_vec()))
+            .collect()
     }
 
     // https://fetch.spec.whatwg.org/#dom-headers-has
@@ -273,19 +289,31 @@ impl Headers {
         extract_mime_type(&*self.header_list.borrow()).unwrap_or(vec![])
     }
 
-    pub fn sort_header_list(&self) -> Vec<(String, Vec<u8>)> {
+    // https://fetch.spec.whatwg.org/#concept-header-list-sort-and-combine
+    pub fn sort_and_combine(&self) -> Vec<(String, Vec<u8>)> {
         let borrowed_header_list = self.header_list.borrow();
-        let headers_iter = borrowed_header_list.iter();
         let mut header_vec = vec![];
-        for (name, value) in headers_iter {
+
-            let name = name.as_str().to_owned();
+        for name in borrowed_header_list.keys() {
-            let value = value.as_bytes().to_vec();
+            let name = name.as_str();
-            let name_value = (name, value);
+            if name == "set-cookie" {
-            header_vec.push(name_value);
+                for value in borrowed_header_list.get_all(name).iter() {
+                    header_vec.push((name.to_owned(), value.as_bytes().to_vec()));
                 }
-        header_vec.sort();
+            } else if let Some(value) = get_value_from_header_list(name, &borrowed_header_list) {
+                header_vec.push((name.to_owned(), value));
+            }
+        }
+
+        header_vec.sort_by(|a, b| a.0.cmp(&b.0));
         header_vec
     }
+
+    // https://fetch.spec.whatwg.org/#ref-for-privileged-no-cors-request-header-name
+    pub fn remove_privileged_no_cors_request_headers(&self) {
+        // https://fetch.spec.whatwg.org/#privileged-no-cors-request-header-name
+        self.header_list.borrow_mut().remove("range");
+    }
 }
 
 impl Iterable for Headers {
@@ -293,17 +321,18 @@ impl Iterable for Headers {
     type Value = ByteString;
 
     fn get_iterable_length(&self) -> u32 {
-        self.header_list.borrow().iter().count() as u32
+        let sorted_header_vec = self.sort_and_combine();
+        sorted_header_vec.len() as u32
     }
 
     fn get_value_at_index(&self, n: u32) -> ByteString {
-        let sorted_header_vec = self.sort_header_list();
+        let sorted_header_vec = self.sort_and_combine();
         let value = sorted_header_vec[n as usize].1.clone();
         ByteString::new(value)
     }
 
     fn get_key_at_index(&self, n: u32) -> ByteString {
-        let sorted_header_vec = self.sort_header_list();
+        let sorted_header_vec = self.sort_and_combine();
         let key = sorted_header_vec[n as usize].0.clone();
         ByteString::new(key.into_bytes().to_vec())
     }

components/script/dom/webidls/Headers.webidl

@@ -15,6 +15,7 @@ interface Headers {
   undefined delete(ByteString name);
   [Throws]
   ByteString? get(ByteString name);
+  sequence<ByteString> getSetCookie();
   [Throws]
   boolean has(ByteString name);
   [Throws]

tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini

@@ -2,116 +2,8 @@
   expected: ERROR
 
 [header-setcookie.any.worker.html]
-  [Headers iterator does not combine set-cookie headers]
-    expected: FAIL
-
-  [Headers iterator does not combine set-cookie & set-cookie2 headers]
-    expected: FAIL
-
-  [Headers iterator preserves set-cookie ordering]
-    expected: FAIL
-
-  [Headers iterator preserves per header ordering, but sorts keys alphabetically]
-    expected: FAIL
-
-  [Headers iterator preserves per header ordering, but sorts keys alphabetically (and ignores value ordering)]
-    expected: FAIL
-
-  [Headers iterator is correctly updated with set-cookie changes]
-    expected: FAIL
-
-  [Headers.prototype.append works for set-cookie]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with no headers present]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with one header]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with one header created from an object]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with multiple headers]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with an empty header]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with two equal headers]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie ignores set-cookie2 headers]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie preserves header ordering]
-    expected: FAIL
-
-  [Set-Cookie is a forbidden response header]
-    expected: FAIL
-
-  [Headers iterator is correctly updated with set-cookie changes #2]
-    expected: FAIL
-
-  [Adding Set-Cookie headers normalizes their value]
-    expected: FAIL
-
 
 [header-setcookie.any.html]
-  [Headers iterator does not combine set-cookie headers]
-    expected: FAIL
-
-  [Headers iterator does not combine set-cookie & set-cookie2 headers]
-    expected: FAIL
-
-  [Headers iterator preserves set-cookie ordering]
-    expected: FAIL
-
-  [Headers iterator preserves per header ordering, but sorts keys alphabetically]
-    expected: FAIL
-
-  [Headers iterator preserves per header ordering, but sorts keys alphabetically (and ignores value ordering)]
-    expected: FAIL
-
-  [Headers iterator is correctly updated with set-cookie changes]
-    expected: FAIL
-
-  [Headers.prototype.append works for set-cookie]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with no headers present]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with one header]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with one header created from an object]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with multiple headers]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with an empty header]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie with two equal headers]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie ignores set-cookie2 headers]
-    expected: FAIL
-
-  [Headers.prototype.getSetCookie preserves header ordering]
-    expected: FAIL
-
-  [Set-Cookie is a forbidden response header]
-    expected: FAIL
-
-  [Headers iterator is correctly updated with set-cookie changes #2]
-    expected: FAIL
-
-  [Adding Set-Cookie headers normalizes their value]
-    expected: FAIL
-
 
 [header-setcookie.any.sharedworker.html]
   expected: ERROR

tests/wpt/metadata/fetch/api/headers/headers-no-cors.any.js.ini

@@ -1,48 +1,12 @@
 [headers-no-cors.any.html]
-  ["no-cors" Headers object cannot have accept set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
-  ["no-cors" Headers object cannot have content-language set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
-  ["no-cors" Headers object cannot have accept-language set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
-  ["no-cors" Headers object cannot have content-language set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
   ["no-cors" Headers object cannot have content-type set to text/plain;ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, text/plain]
     expected: FAIL
 
-  ["no-cors" Headers object cannot have accept-language set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
-  ["no-cors" Headers object cannot have accept set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
 
 [headers-no-cors.any.worker.html]
-  ["no-cors" Headers object cannot have accept set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
-  ["no-cors" Headers object cannot have content-language set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
-  ["no-cors" Headers object cannot have accept-language set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
-  ["no-cors" Headers object cannot have content-language set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
   ["no-cors" Headers object cannot have content-type set to text/plain;ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, text/plain]
     expected: FAIL
 
-  ["no-cors" Headers object cannot have accept-language set to sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss, , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
-  ["no-cors" Headers object cannot have accept set to , sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss]
-    expected: FAIL
-
 
 [headers-no-cors.any.serviceworker.html]
   expected: ERROR