From 4ee3f575f6fc5fe0e2b6573e8a8d9c94c863d49e Mon Sep 17 00:00:00 2001 From: 2shiori17 <98276492+2shiori17@users.noreply.github.com> Date: Sun, 5 Mar 2023 00:52:19 +0900 Subject: [PATCH 01/11] Implement Headers.prototype.getSetCookie --- components/script/dom/headers.rs | 10 ++++++ components/script/dom/webidls/Headers.webidl | 1 + .../api/headers/header-setcookie.any.js.ini | 36 ------------------- 3 files changed, 11 insertions(+), 36 deletions(-) diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs index 03fecd79c5a..c2ae32e4079 100644 --- a/components/script/dom/headers.rs +++ b/components/script/dom/headers.rs @@ -155,6 +155,16 @@ impl HeadersMethods for Headers { .map(|v| ByteString::new(v.as_bytes().to_vec()))) } + // https://fetch.spec.whatwg.org/#dom-headers-getsetcookie + fn GetSetCookie(&self) -> Vec { + self.header_list + .borrow() + .get_all("Set-Cookie") + .iter() + .map(|v| ByteString::new(v.as_bytes().to_vec())) + .collect() + } + // https://fetch.spec.whatwg.org/#dom-headers-has fn Has(&self, name: ByteString) -> Fallible { // Step 1 diff --git a/components/script/dom/webidls/Headers.webidl b/components/script/dom/webidls/Headers.webidl index 16e35060b90..5def3e7011e 100644 --- a/components/script/dom/webidls/Headers.webidl +++ b/components/script/dom/webidls/Headers.webidl @@ -15,6 +15,7 @@ interface Headers { undefined delete(ByteString name); [Throws] ByteString? get(ByteString name); + sequence getSetCookie(); [Throws] boolean has(ByteString name); [Throws] diff --git a/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini b/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini index 73ccab628cd..bf12e514657 100644 --- a/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini +++ b/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini @@ -23,33 +23,15 @@ [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 @@ -79,33 +61,15 @@ [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 From ee1f24123101b822ad18ccd7bf4a6b0c20c45a04 Mon Sep 17 00:00:00 2001 From: 2shiori17 <98276492+2shiori17@users.noreply.github.com> Date: Mon, 6 Mar 2023 23:34:04 +0900 Subject: [PATCH 02/11] Implement concept-header-list-sort-and-combine --- Cargo.lock | 1 + components/net/fetch/headers.rs | 2 +- components/script/Cargo.toml | 1 + components/script/dom/headers.rs | 32 +++++++++++++++++++++----------- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d001700cf6f..c4c15ecc754 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5304,6 +5304,7 @@ dependencies = [ "mozangle", "mozjs", "msg", + "net", "net_traits", "num-traits", "parking_lot 0.11.2", diff --git a/components/net/fetch/headers.rs b/components/net/fetch/headers.rs index 83e515a2305..652f91810c1 100644 --- a/components/net/fetch/headers.rs +++ b/components/net/fetch/headers.rs @@ -157,7 +157,7 @@ fn collect_http_quoted_string(position: &mut Peekable, extract_value: boo } /// -fn get_value_from_header_list(name: &str, headers: &HeaderMap) -> Option { +pub fn get_value_from_header_list(name: &str, headers: &HeaderMap) -> Option { let values = headers .get_all(name) .iter() diff --git a/components/script/Cargo.toml b/components/script/Cargo.toml index 95b7a2ff86c..b5c72795a34 100644 --- a/components/script/Cargo.toml +++ b/components/script/Cargo.toml @@ -76,6 +76,7 @@ mime = "0.3.13" mime_guess = "2.0.0" mitochondria = "1.1.2" msg = { path = "../msg" } +net = { path = "../net" } net_traits = { path = "../net_traits" } num-traits = "0.2" parking_lot = "0.11" diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs index c2ae32e4079..ef38f475877 100644 --- a/components/script/dom/headers.rs +++ b/components/script/dom/headers.rs @@ -13,6 +13,7 @@ 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::fetch::headers::get_value_from_header_list; use net_traits::request::is_cors_safelisted_request_header; use std::cell::Cell; use std::str::{self, FromStr}; @@ -159,7 +160,7 @@ impl HeadersMethods for Headers { fn GetSetCookie(&self) -> Vec { self.header_list .borrow() - .get_all("Set-Cookie") + .get_all("set-cookie") .iter() .map(|v| ByteString::new(v.as_bytes().to_vec())) .collect() @@ -283,16 +284,24 @@ impl Headers { extract_mime_type(&*self.header_list.borrow()).unwrap_or(vec![]) } - pub fn sort_header_list(&self) -> Vec<(String, Vec)> { + // https://fetch.spec.whatwg.org/#concept-header-list-sort-and-combine + pub fn sort_and_combine(&self) -> Vec<(String, Vec)> { 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(); - let value = value.as_bytes().to_vec(); - let name_value = (name, value); - header_vec.push(name_value); + + for name in borrowed_header_list.keys() { + let name = name.as_str(); + if name == "set-cookie" { + for value in borrowed_header_list.get_all(name).iter() { + header_vec.push((name.to_owned(), value.as_bytes().to_vec())); + } + } else { + if let Some(value) = get_value_from_header_list(name, &borrowed_header_list) { + header_vec.push((name.to_owned(), value.as_bytes().to_vec())); + } + } } + header_vec.sort(); header_vec } @@ -303,17 +312,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()) } From b5b6225850174545830e5b8647d1ecb6b370ba6a Mon Sep 17 00:00:00 2001 From: 2shiori17 <98276492+2shiori17@users.noreply.github.com> Date: Sat, 11 Mar 2023 17:32:38 +0900 Subject: [PATCH 03/11] Fix concept-headers-append --- components/script/dom/headers.rs | 60 +++++++------------ .../api/headers/header-setcookie.any.js.ini | 40 +------------ 2 files changed, 24 insertions(+), 76 deletions(-) diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs index ef38f475877..f29c059fc05 100644 --- a/components/script/dom/headers.rs +++ b/components/script/dom/headers.rs @@ -64,57 +64,41 @@ impl Headers { impl HeadersMethods for Headers { // https://fetch.spec.whatwg.org/#concept-headers-append fn Append(&self, name: ByteString, value: ByteString) -> ErrorResult { - // Step 1 let value = normalize_value(value); - // Step 2 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 - // another copy of an existing header. HyperHeaders - // might not expose the information we need to do it right. - let mut combined_value: Vec = vec![]; - if let Some(v) = self - .header_list - .borrow() - .get(HeaderName::from_str(&valid_name).unwrap()) - { - combined_value = v.as_bytes().to_vec(); - combined_value.extend(b", "); + + if self.guard.get() == Guard::RequestNoCors { + let tmp_value = if let Some(value) = get_value_from_header_list(&valid_name, &self.header_list.borrow()) { + let mut l = value.as_bytes().to_vec(); + l.extend(b", "); + l.extend(valid_value.clone()); + l + } 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) { - Ok(value) => { - self.header_list - .borrow_mut() - .insert(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 - ); - }, - }; + + if self.guard.get() != Guard::RequestNoCors || valid_name != "range" { + self.header_list + .borrow_mut() + .append(HeaderName::from_str(&valid_name).unwrap(), HeaderValue::from_bytes(&valid_value).unwrap()); + } + Ok(()) } diff --git a/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini b/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini index bf12e514657..16ef1d0aa0f 100644 --- a/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini +++ b/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini @@ -11,31 +11,13 @@ [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 multiple headers] - expected: FAIL - - [Headers.prototype.getSetCookie with two equal headers] - expected: FAIL - - [Headers.prototype.getSetCookie preserves header ordering] - expected: FAIL - - [Headers iterator is correctly updated with set-cookie changes #2] - expected: FAIL - - [Adding Set-Cookie headers normalizes their value] + [Headers.prototype.get combines set-cookie headers in order] expected: FAIL @@ -49,31 +31,13 @@ [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 multiple headers] - expected: FAIL - - [Headers.prototype.getSetCookie with two equal headers] - expected: FAIL - - [Headers.prototype.getSetCookie preserves header ordering] - expected: FAIL - - [Headers iterator is correctly updated with set-cookie changes #2] - expected: FAIL - - [Adding Set-Cookie headers normalizes their value] + [Headers.prototype.get combines set-cookie headers in order] expected: FAIL From 7d1d387f3d634f6c1688c020bdb75c9f89158551 Mon Sep 17 00:00:00 2001 From: 2shiori17 <98276492+2shiori17@users.noreply.github.com> Date: Sat, 11 Mar 2023 20:05:32 +0900 Subject: [PATCH 04/11] Fix header ordering --- components/script/dom/headers.rs | 13 ++++---- .../api/headers/header-setcookie.any.js.ini | 30 ------------------- 2 files changed, 8 insertions(+), 35 deletions(-) diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs index f29c059fc05..cfec92f06d1 100644 --- a/components/script/dom/headers.rs +++ b/components/script/dom/headers.rs @@ -79,7 +79,9 @@ impl HeadersMethods for Headers { } if self.guard.get() == Guard::RequestNoCors { - let tmp_value = if let Some(value) = get_value_from_header_list(&valid_name, &self.header_list.borrow()) { + let tmp_value = if let Some(value) = + get_value_from_header_list(&valid_name, &self.header_list.borrow()) + { let mut l = value.as_bytes().to_vec(); l.extend(b", "); l.extend(valid_value.clone()); @@ -94,9 +96,10 @@ impl HeadersMethods for Headers { } if self.guard.get() != Guard::RequestNoCors || valid_name != "range" { - self.header_list - .borrow_mut() - .append(HeaderName::from_str(&valid_name).unwrap(), HeaderValue::from_bytes(&valid_value).unwrap()); + self.header_list.borrow_mut().append( + HeaderName::from_str(&valid_name).unwrap(), + HeaderValue::from_bytes(&valid_value).unwrap(), + ); } Ok(()) @@ -286,7 +289,7 @@ impl Headers { } } - header_vec.sort(); + header_vec.sort_by(|a, b| a.0.cmp(&b.0)); header_vec } } diff --git a/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini b/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini index 16ef1d0aa0f..6a5a9e3a3a4 100644 --- a/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini +++ b/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini @@ -2,41 +2,11 @@ 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 (and ignores value ordering)] - expected: FAIL - - [Headers.prototype.append works for set-cookie] - expected: FAIL - [Headers.prototype.get combines set-cookie headers in order] 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 (and ignores value ordering)] - expected: FAIL - - [Headers.prototype.append works for set-cookie] - expected: FAIL - [Headers.prototype.get combines set-cookie headers in order] expected: FAIL From 68472fabf8d949e85ced3235822033ca4654ee35 Mon Sep 17 00:00:00 2001 From: 2shiori17 <98276492+2shiori17@users.noreply.github.com> Date: Sat, 11 Mar 2023 20:15:14 +0900 Subject: [PATCH 05/11] Fix dom-headers-get --- components/script/dom/headers.rs | 9 ++++----- .../fetch/api/headers/header-setcookie.any.js.ini | 6 ------ 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs index cfec92f06d1..cd70edb96f1 100644 --- a/components/script/dom/headers.rs +++ b/components/script/dom/headers.rs @@ -136,11 +136,10 @@ impl HeadersMethods for Headers { fn Get(&self, name: ByteString) -> Fallible> { // Step 1 let valid_name = validate_name(name)?; - Ok(self - .header_list - .borrow() - .get(HeaderName::from_str(&valid_name).unwrap()) - .map(|v| ByteString::new(v.as_bytes().to_vec()))) + Ok( + get_value_from_header_list(&valid_name, &self.header_list.borrow()) + .map(|v| ByteString::new(v.as_bytes().to_vec())), + ) } // https://fetch.spec.whatwg.org/#dom-headers-getsetcookie diff --git a/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini b/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini index 6a5a9e3a3a4..cadd1784f46 100644 --- a/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini +++ b/tests/wpt/metadata/fetch/api/headers/header-setcookie.any.js.ini @@ -2,14 +2,8 @@ expected: ERROR [header-setcookie.any.worker.html] - [Headers.prototype.get combines set-cookie headers in order] - expected: FAIL - [header-setcookie.any.html] - [Headers.prototype.get combines set-cookie headers in order] - expected: FAIL - [header-setcookie.any.sharedworker.html] expected: ERROR From 74ecc2bd64a8f5a95e6a88318aa01b3fb3033681 Mon Sep 17 00:00:00 2001 From: 2shiori17 <98276492+2shiori17@users.noreply.github.com> Date: Mon, 20 Mar 2023 22:12:50 +0900 Subject: [PATCH 06/11] Remove net from dependencies --- Cargo.lock | 1 - components/net/fetch/headers.rs | 17 +---------------- components/net_traits/fetch/headers.rs | 21 +++++++++++++++++++++ components/net_traits/lib.rs | 5 +++++ components/script/Cargo.toml | 1 - components/script/dom/headers.rs | 5 +++-- 6 files changed, 30 insertions(+), 20 deletions(-) create mode 100644 components/net_traits/fetch/headers.rs diff --git a/Cargo.lock b/Cargo.lock index c4c15ecc754..d001700cf6f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5304,7 +5304,6 @@ dependencies = [ "mozangle", "mozjs", "msg", - "net", "net_traits", "num-traits", "parking_lot 0.11.2", diff --git a/components/net/fetch/headers.rs b/components/net/fetch/headers.rs index 652f91810c1..8a31d68738b 100644 --- a/components/net/fetch/headers.rs +++ b/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; @@ -155,19 +156,3 @@ fn collect_http_quoted_string(position: &mut Peekable, extract_value: boo // Step 6, 7 return value; } - -/// -pub fn get_value_from_header_list(name: &str, headers: &HeaderMap) -> Option { - 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::>().join(", ")); -} diff --git a/components/net_traits/fetch/headers.rs b/components/net_traits/fetch/headers.rs new file mode 100644 index 00000000000..4b293b13c18 --- /dev/null +++ b/components/net_traits/fetch/headers.rs @@ -0,0 +1,21 @@ +/* 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; + +/// +pub fn get_value_from_header_list(name: &str, headers: &HeaderMap) -> Option { + 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::>().join(", ")); +} diff --git a/components/net_traits/lib.rs b/components/net_traits/lib.rs index ac6ef9e3ad3..323fc9a59cb 100644 --- a/components/net_traits/lib.rs +++ b/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 /// #[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)] diff --git a/components/script/Cargo.toml b/components/script/Cargo.toml index b5c72795a34..95b7a2ff86c 100644 --- a/components/script/Cargo.toml +++ b/components/script/Cargo.toml @@ -76,7 +76,6 @@ mime = "0.3.13" mime_guess = "2.0.0" mitochondria = "1.1.2" msg = { path = "../msg" } -net = { path = "../net" } net_traits = { path = "../net_traits" } num-traits = "0.2" parking_lot = "0.11" diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs index cd70edb96f1..aa81743fb57 100644 --- a/components/script/dom/headers.rs +++ b/components/script/dom/headers.rs @@ -13,8 +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::fetch::headers::get_value_from_header_list; -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}; From 80a140e82fd1d5d94ce21399ace6de8c1c36dec0 Mon Sep 17 00:00:00 2001 From: 2shiori17 <98276492+2shiori17@users.noreply.github.com> Date: Tue, 21 Mar 2023 17:15:57 +0900 Subject: [PATCH 07/11] Add comments for concept-headers-append --- components/script/dom/headers.rs | 41 +++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs index aa81743fb57..6b545c28ad3 100644 --- a/components/script/dom/headers.rs +++ b/components/script/dom/headers.rs @@ -65,7 +65,11 @@ impl Headers { impl HeadersMethods for Headers { // https://fetch.spec.whatwg.org/#concept-headers-append 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(); @@ -79,6 +83,7 @@ impl HeadersMethods for Headers { return Ok(()); } + // Step 3 if self.guard.get() == Guard::RequestNoCors { let tmp_value = if let Some(value) = get_value_from_header_list(&valid_name, &self.header_list.borrow()) @@ -96,11 +101,25 @@ impl HeadersMethods for Headers { } } - if self.guard.get() != Guard::RequestNoCors || valid_name != "range" { - self.header_list.borrow_mut().append( - HeaderName::from_str(&valid_name).unwrap(), - HeaderValue::from_bytes(&valid_value).unwrap(), - ); + // Step 4 + match HeaderValue::from_bytes(&valid_value) { + Ok(value) => { + self.header_list + .borrow_mut() + .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.", + valid_value + ); + }, + }; + + // Step 5 + if self.guard.get() != Guard::RequestNoCors { + self.remove_privileged_no_cors_request_headers(); } Ok(()) @@ -282,16 +301,20 @@ impl Headers { for value in borrowed_header_list.get_all(name).iter() { header_vec.push((name.to_owned(), value.as_bytes().to_vec())); } - } else { - if let Some(value) = get_value_from_header_list(name, &borrowed_header_list) { - header_vec.push((name.to_owned(), value.as_bytes().to_vec())); - } + } else if let Some(value) = get_value_from_header_list(name, &borrowed_header_list) { + header_vec.push((name.to_owned(), value.as_bytes().to_vec())); } } 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 { From e4a4f60c53a64533f2343a2fa46b262a8802eff1 Mon Sep 17 00:00:00 2001 From: 2shiori17 <98276492+2shiori17@users.noreply.github.com> Date: Sun, 26 Mar 2023 00:54:04 +0900 Subject: [PATCH 08/11] Fix concept-headers-append --- components/script/dom/headers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs index 6b545c28ad3..a71a816c620 100644 --- a/components/script/dom/headers.rs +++ b/components/script/dom/headers.rs @@ -118,7 +118,7 @@ impl HeadersMethods for Headers { }; // Step 5 - if self.guard.get() != Guard::RequestNoCors { + if self.guard.get() == Guard::RequestNoCors { self.remove_privileged_no_cors_request_headers(); } From d6d48510d9ede472f789a83b68ba8c5437f0de49 Mon Sep 17 00:00:00 2001 From: 2shiori17 <98276492+2shiori17@users.noreply.github.com> Date: Sun, 26 Mar 2023 12:32:54 +0900 Subject: [PATCH 09/11] Update wpt results --- .../api/headers/headers-no-cors.any.js.ini | 36 ------------------- 1 file changed, 36 deletions(-) diff --git a/tests/wpt/metadata/fetch/api/headers/headers-no-cors.any.js.ini b/tests/wpt/metadata/fetch/api/headers/headers-no-cors.any.js.ini index b02ed0ec7a9..38082045af2 100644 --- a/tests/wpt/metadata/fetch/api/headers/headers-no-cors.any.js.ini +++ b/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 From b6e4e44ccf4c963e03cc47e5a623ac95b8290970 Mon Sep 17 00:00:00 2001 From: 2shiori17 <98276492+2shiori17@users.noreply.github.com> Date: Sun, 26 Mar 2023 19:57:39 +0900 Subject: [PATCH 10/11] Fix return value of concept-header-list-get to Option> --- components/net/fetch/headers.rs | 30 +++++++++++++++----------- components/net_traits/fetch/headers.rs | 9 +++----- components/script/dom/headers.rs | 13 ++++++----- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/components/net/fetch/headers.rs b/components/net/fetch/headers.rs index 8a31d68738b..e88fe641866 100644 --- a/components/net/fetch/headers.rs +++ b/components/net/fetch/headers.rs @@ -26,49 +26,53 @@ fn get_header_value_as_list(name: &str, headers: &HeaderMap) -> Option(); - // Step 5 + // Step 2 + let mut position = s.chars().peekable(); + + // Step 3 let mut values: Vec = 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(); } diff --git a/components/net_traits/fetch/headers.rs b/components/net_traits/fetch/headers.rs index 4b293b13c18..ae95066bcf5 100644 --- a/components/net_traits/fetch/headers.rs +++ b/components/net_traits/fetch/headers.rs @@ -5,11 +5,8 @@ use headers::HeaderMap; /// -pub fn get_value_from_header_list(name: &str, headers: &HeaderMap) -> Option { - let values = headers - .get_all(name) - .iter() - .map(|val| val.to_str().unwrap()); +pub fn get_value_from_header_list(name: &str, headers: &HeaderMap) -> Option> { + let values = headers.get_all(name).iter().map(|val| val.as_bytes()); // Step 1 if values.size_hint() == (0, Some(0)) { @@ -17,5 +14,5 @@ pub fn get_value_from_header_list(name: &str, headers: &HeaderMap) -> Option>().join(", ")); + return Some(values.collect::>().join(&[0x2C, 0x20][..])); } diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs index a71a816c620..de81bd7dc4c 100644 --- a/components/script/dom/headers.rs +++ b/components/script/dom/headers.rs @@ -85,13 +85,12 @@ impl HeadersMethods for Headers { // Step 3 if self.guard.get() == Guard::RequestNoCors { - let tmp_value = if let Some(value) = + let tmp_value = if let Some(mut value) = get_value_from_header_list(&valid_name, &self.header_list.borrow()) { - let mut l = value.as_bytes().to_vec(); - l.extend(b", "); - l.extend(valid_value.clone()); - l + value.extend(b", "); + value.extend(valid_value.clone()); + value } else { valid_value.clone() }; @@ -158,7 +157,7 @@ impl HeadersMethods for Headers { let valid_name = validate_name(name)?; Ok( get_value_from_header_list(&valid_name, &self.header_list.borrow()) - .map(|v| ByteString::new(v.as_bytes().to_vec())), + .map(|v| ByteString::new(v)), ) } @@ -302,7 +301,7 @@ impl Headers { header_vec.push((name.to_owned(), value.as_bytes().to_vec())); } } else if let Some(value) = get_value_from_header_list(name, &borrowed_header_list) { - header_vec.push((name.to_owned(), value.as_bytes().to_vec())); + header_vec.push((name.to_owned(), value)); } } From b7d0451d562300cba781b164033d77cf4444749a Mon Sep 17 00:00:00 2001 From: 2shiori17 <98276492+2shiori17@users.noreply.github.com> Date: Sun, 26 Mar 2023 20:07:12 +0900 Subject: [PATCH 11/11] Fix concept-header-list-get-decode-split --- components/net/fetch/headers.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/net/fetch/headers.rs b/components/net/fetch/headers.rs index e88fe641866..54fbe636f6f 100644 --- a/components/net/fetch/headers.rs +++ b/components/net/fetch/headers.rs @@ -26,16 +26,16 @@ fn get_header_value_as_list(name: &str, headers: &HeaderMap) -> Option(); // Step 2 - let mut position = s.chars().peekable(); + let mut position = input.chars().peekable(); // Step 3 let mut values: Vec = vec![];