Now just one is_cors_safelisted_request_header, with closer spec alignment

2 files changed, 97 insertions, 92 deletions

diff --git a/components/script/dom/headers.rs b/components/script/dom/headers.rs
index 017cde9268a..61c6fc1f98e 100644
--- a/components/script/dom/headers.rs
+++ b/components/script/dom/headers.rs
@@ -14,9 +14,8 @@ use crate::dom::bindings::str::{is_token, ByteString};
 use crate::dom::globalscope::GlobalScope;
 use dom_struct::dom_struct;
 use http::header::{self, HeaderMap as HyperHeaders, HeaderName, HeaderValue};
-use mime::{self, Mime};
+use net_traits::request::is_cors_safelisted_request_header;
 use std::cell::Cell;
-use std::result::Result;
 use std::str::{self, FromStr};
 
 #[dom_struct]
@@ -28,7 +27,7 @@ pub struct Headers {
 }
 
 // https://fetch.spec.whatwg.org/#concept-headers-guard
-#[derive(Clone, Copy, JSTraceable, MallocSizeOf, PartialEq)]
+#[derive(Clone, Copy, Debug, JSTraceable, MallocSizeOf, PartialEq)]
 pub enum Guard {
     Immutable,
     Request,
@@ -88,6 +87,9 @@ impl HeadersMethods for Headers {
             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<u8> = vec![];
         if let Some(v) = self
             .header_list
@@ -301,35 +303,6 @@ impl Iterable for Headers {
     }
 }
 
-fn is_cors_safelisted_request_content_type(value: &[u8]) -> bool {
-    let value_string = if let Ok(s) = str::from_utf8(value) {
-        s
-    } else {
-        return false;
-    };
-    let value_mime_result: Result<Mime, _> = value_string.parse();
-    match value_mime_result {
-        Err(_) => false,
-        Ok(value_mime) => match (value_mime.type_(), value_mime.subtype()) {
-            (mime::APPLICATION, mime::WWW_FORM_URLENCODED) |
-            (mime::MULTIPART, mime::FORM_DATA) |
-            (mime::TEXT, mime::PLAIN) => true,
-            _ => false,
-        },
-    }
-}
-
-// TODO: "DPR", "Downlink", "Save-Data", "Viewport-Width", "Width":
-// ... once parsed, the value should not be failure.
-// https://fetch.spec.whatwg.org/#cors-safelisted-request-header
-fn is_cors_safelisted_request_header(name: &str, value: &[u8]) -> bool {
-    match name {
-        "accept" | "accept-language" | "content-language" => true,
-        "content-type" => is_cors_safelisted_request_content_type(value),
-        _ => false,
-    }
-}
-
 // https://fetch.spec.whatwg.org/#forbidden-response-header-name
 fn is_forbidden_response_header(name: &str) -> bool {
     match name {
@@ -394,11 +367,18 @@ pub fn is_forbidden_header_name(name: &str) -> bool {
 // [2] https://tools.ietf.org/html/rfc7230#section-3.2
 // [3] https://tools.ietf.org/html/rfc7230#section-3.2.6
 // [4] https://www.rfc-editor.org/errata_search.php?rfc=7230
+//
+// As of December 2019 WHATWG, isn't even using grammar productions for value;
+// https://fetch.spec.whatg.org/#concept-header-value just says not to have
+// newlines, nulls, or leading/trailing whitespace.
 fn validate_name_and_value(name: ByteString, value: ByteString) -> Fallible<(String, Vec<u8>)> {
     let valid_name = validate_name(name)?;
+
+    // this is probably out of date
     if !is_field_content(&value) {
         return Err(Error::Type("Value is not valid".to_string()));
     }
+
     Ok((valid_name, value.into()))
 }
 
diff --git a/components/script/dom/request.rs b/components/script/dom/request.rs
index d22d4680355..03e5dee683d 100644
--- a/components/script/dom/request.rs
+++ b/components/script/dom/request.rs
@@ -90,59 +90,63 @@ impl Request {
         // Step 4
         let base_url = global.api_base_url();
 
+        // Step 5 TODO: "Let signal be null."
+
         match input {
-            // Step 5
+            // Step 6
             RequestInfo::USVString(USVString(ref usv_string)) => {
-                // Step 5.1
+                // Step 6.1
                 let parsed_url = base_url.join(&usv_string);
-                // Step 5.2
+                // Step 6.2
                 if parsed_url.is_err() {
                     return Err(Error::Type("Url could not be parsed".to_string()));
                 }
-                // Step 5.3
+                // Step 6.3
                 let url = parsed_url.unwrap();
                 if includes_credentials(&url) {
                     return Err(Error::Type("Url includes credentials".to_string()));
                 }
-                // Step 5.4
+                // Step 6.4
                 temporary_request = net_request_from_global(global, url);
-                // Step 5.5
+                // Step 6.5
                 fallback_mode = Some(NetTraitsRequestMode::CorsMode);
-                // Step 5.6
+                // Step 6.6
                 fallback_credentials = Some(NetTraitsRequestCredentials::CredentialsSameOrigin);
             },
-            // Step 6
+            // Step 7
             RequestInfo::Request(ref input_request) => {
-                // Step 6.1
+                // This looks like Step 38
+                // TODO do this in the right place to not mask other errors
                 if request_is_disturbed(input_request) || request_is_locked(input_request) {
                     return Err(Error::Type("Input is disturbed or locked".to_string()));
                 }
-                // Step 6.2
+                // Step 7.1
                 temporary_request = input_request.request.borrow().clone();
+                // Step 7.2 TODO: "Set signal to input's signal."
             },
         }
 
-        // Step 7
+        // Step 8
         // TODO: `entry settings object` is not implemented yet.
         let origin = base_url.origin();
 
-        // Step 8
+        // Step 9
         let mut window = Window::Client;
 
-        // Step 9
+        // Step 10
         // TODO: `environment settings object` is not implemented in Servo yet.
 
-        // Step 10
+        // Step 11
         if !init.window.handle().is_null_or_undefined() {
             return Err(Error::Type("Window is present and is not null".to_string()));
         }
 
-        // Step 11
+        // Step 12
         if !init.window.handle().is_undefined() {
             window = Window::NoWindow;
         }
 
-        // Step 12
+        // Step 13
         let mut request: NetTraitsRequest;
         request = net_request_from_global(global, temporary_request.current_url());
         request.method = temporary_request.method;
@@ -159,7 +163,7 @@ impl Request {
         request.redirect_mode = temporary_request.redirect_mode;
         request.integrity_metadata = temporary_request.integrity_metadata;
 
-        // Step 13
+        // Step 14
         if init.body.is_some() ||
             init.cache.is_some() ||
             init.credentials.is_some() ||
@@ -172,31 +176,33 @@ impl Request {
             init.referrerPolicy.is_some() ||
             !init.window.handle().is_undefined()
         {
-            // Step 13.1
+            // Step 14.1
             if request.mode == NetTraitsRequestMode::Navigate {
                 request.mode = NetTraitsRequestMode::SameOrigin;
             }
-            // Step 13.2
+            // Step 14.2 TODO: "Unset request's reload-navigation flag."
+            // Step 14.3 TODO: "Unset request's history-navigation flag."
+            // Step 14.4
             request.referrer = NetTraitsRequestReferrer::Client;
-            // Step 13.3
+            // Step 14.5
             request.referrer_policy = None;
         }
 
-        // Step 14
+        // Step 15
         if let Some(init_referrer) = init.referrer.as_ref() {
-            // Step 14.1
+            // Step 15.1
             let ref referrer = init_referrer.0;
-            // Step 14.2
+            // Step 15.2
             if referrer.is_empty() {
                 request.referrer = NetTraitsRequestReferrer::NoReferrer;
             } else {
-                // Step 14.3
+                // Step 15.3.1
                 let parsed_referrer = base_url.join(referrer);
-                // Step 14.4
+                // Step 15.3.2
                 if parsed_referrer.is_err() {
                     return Err(Error::Type("Failed to parse referrer url".to_string()));
                 }
-                // Step 14.5
+                // Step 15.3.3
                 if let Ok(parsed_referrer) = parsed_referrer {
                     if (parsed_referrer.cannot_be_a_base() &&
                         parsed_referrer.scheme() == "about" &&
@@ -205,55 +211,55 @@ impl Request {
                     {
                         request.referrer = NetTraitsRequestReferrer::Client;
                     } else {
-                        // Step 14.6
+                        // Step 15.3.4
                         request.referrer = NetTraitsRequestReferrer::ReferrerUrl(parsed_referrer);
                     }
                 }
             }
         }
 
-        // Step 15
+        // Step 16
         if let Some(init_referrerpolicy) = init.referrerPolicy.as_ref() {
             let init_referrer_policy = init_referrerpolicy.clone().into();
             request.referrer_policy = Some(init_referrer_policy);
         }
 
-        // Step 16
+        // Step 17
         let mode = init
             .mode
             .as_ref()
             .map(|m| m.clone().into())
             .or(fallback_mode);
 
-        // Step 17
+        // Step 18
         if let Some(NetTraitsRequestMode::Navigate) = mode {
             return Err(Error::Type("Request mode is Navigate".to_string()));
         }
 
-        // Step 18
+        // Step 19
         if let Some(m) = mode {
             request.mode = m;
         }
 
-        // Step 19
+        // Step 20
         let credentials = init
             .credentials
             .as_ref()
             .map(|m| m.clone().into())
             .or(fallback_credentials);
 
-        // Step 20
+        // Step 21
         if let Some(c) = credentials {
             request.credentials_mode = c;
         }
 
-        // Step 21
+        // Step 22
         if let Some(init_cache) = init.cache.as_ref() {
             let cache = init_cache.clone().into();
             request.cache_mode = cache;
         }
 
-        // Step 22
+        // Step 23
         if request.cache_mode == NetTraitsRequestCache::OnlyIfCached {
             if request.mode != NetTraitsRequestMode::SameOrigin {
                 return Err(Error::Type(
@@ -262,45 +268,55 @@ impl Request {
             }
         }
 
-        // Step 23
+        // Step 24
         if let Some(init_redirect) = init.redirect.as_ref() {
             let redirect = init_redirect.clone().into();
             request.redirect_mode = redirect;
         }
 
-        // Step 24
+        // Step 25
         if let Some(init_integrity) = init.integrity.as_ref() {
             let integrity = init_integrity.clone().to_string();
             request.integrity_metadata = integrity;
         }
 
-        // Step 25
+        // Step 26 TODO: "If init["keepalive"] exists..."
+
+        // Step 27.1
         if let Some(init_method) = init.method.as_ref() {
-            // Step 25.1
+            // Step 27.2
             if !is_method(&init_method) {
                 return Err(Error::Type("Method is not a method".to_string()));
             }
             if is_forbidden_method(&init_method) {
                 return Err(Error::Type("Method is forbidden".to_string()));
             }
-            // Step 25.2
+            // Step 27.3
             let method = match init_method.as_str() {
                 Some(s) => normalize_method(s)
                     .map_err(|e| Error::Type(format!("Method is not valid: {:?}", e)))?,
                 None => return Err(Error::Type("Method is not a valid UTF8".to_string())),
             };
-            // Step 25.3
+            // Step 27.4
             request.method = method;
         }
 
-        // Step 26
+        // Step 28 TODO: "If init["signal"] exists..."
+
+        // Step 29
         let r = Request::from_net_request(global, request);
+
+        // Step 30 TODO: "If signal is not null..."
+
+        // Step 31
+        // "or_init" looks unclear here
         r.headers.or_init(|| Headers::for_request(&r.global()));
 
-        // Step 27
+        // Step 32 - but spec says this should only be when non-empty init?
+        // Step 32.1
         let mut headers_copy = r.Headers();
 
-        // Step 28
+        // Step 32.2
         if let Some(possible_header) = init.headers.as_ref() {
             match possible_header {
                 &HeadersInit::Headers(ref init_headers) => {
@@ -319,7 +335,7 @@ impl Request {
             }
         }
 
-        // Step 29
+        // Step 32.3
         // We cannot empty `r.Headers().header_list` because
         // we would undo the Step 27 above.  One alternative is to set
         // `headers_copy` as a deep copy of `r.Headers()`. However,
@@ -328,21 +344,21 @@ impl Request {
         // mutable reference, we cannot mutate `r.Headers()` to be the
         // deep copied headers in Step 27.
 
-        // Step 30
+        // Step 32.4
         if r.request.borrow().mode == NetTraitsRequestMode::NoCors {
             let borrowed_request = r.request.borrow();
-            // Step 30.1
+            // Step 32.4.1
             if !is_cors_safelisted_method(&borrowed_request.method) {
                 return Err(Error::Type(
                     "The mode is 'no-cors' but the method is not a cors-safelisted method"
                         .to_string(),
                 ));
             }
-            // Step 30.2
+            // Step 32.4.2
             r.Headers().set_guard(Guard::RequestNoCors);
         }
 
-        // Step 31
+        // Step 32.5
         match init.headers {
             None => {
                 // This is equivalent to the specification's concept of
@@ -360,10 +376,11 @@ impl Request {
             _ => {},
         }
 
+        // Step 32.5-6 depending on how we got here
         // Copy the headers list onto the headers of net_traits::Request
         r.request.borrow_mut().headers = r.Headers().get_headers_list();
 
-        // Step 32
+        // Step 33
         let mut input_body = if let RequestInfo::Request(ref input_request) = input {
             let input_request_request = input_request.request.borrow();
             input_request_request.body.clone()
@@ -371,7 +388,7 @@ impl Request {
             None
         };
 
-        // Step 33
+        // Step 34
         if let Some(init_body_option) = init.body.as_ref() {
             if init_body_option.is_some() || input_body.is_some() {
                 let req = r.request.borrow();
@@ -392,14 +409,16 @@ impl Request {
             }
         }
 
-        // Step 34
+        // Step 35-36
         if let Some(Some(ref init_body)) = init.body {
-            // Step 34.2
+            // Step 36.2 TODO "If init["keepalive"] exists and is true..."
+
+            // Step 36.3
             let extracted_body_tmp = init_body.extract();
             input_body = Some(extracted_body_tmp.0);
             let content_type = extracted_body_tmp.1;
 
-            // Step 34.3
+            // Step 36.4
             if let Some(contents) = content_type {
                 if !r
                     .Headers()
@@ -414,17 +433,23 @@ impl Request {
             }
         }
 
-        // Step 35
+        // Step 37 "TODO if body is non-null and body's source is null..."
+        // This looks like where we need to set the use-preflight flag
+        // if the request has a body and nothing else has set the flag.
+
+        // Step 38 is done earlier
+
+        // Step 39
+        // TODO: `ReadableStream` object is not implemented in Servo yet.
+
+        // Step 40
         r.request.borrow_mut().body = input_body;
 
-        // Step 36
+        // Step 41
         let extracted_mime_type = r.Headers().extract_mime_type();
         *r.mime_type.borrow_mut() = extracted_mime_type;
 
-        // Step 37
-        // TODO: `ReadableStream` object is not implemented in Servo yet.
-
-        // Step 38
+        // Step 42
         Ok(r)
     }