Remove same-origin-data-url flag from fetch implementation

The spec removed it. Check the scheme instead, data is always same origin now,
except for workers.
This also updates the comments to make step numbers match the spec.
Closes #13362

components/net/fetch/methods.rs

@@ -194,8 +194,8 @@ fn main_fetch(request: Rc<Request>, cache: &mut CORSCache, cors_flag: bool,
             };
 
             if (same_origin && !cors_flag ) ||
-                (current_url.scheme() == "data" && request.same_origin_data.get()) ||
-                (current_url.scheme() == "file" && request.same_origin_data.get()) ||
+                current_url.scheme() == "data" ||
+                current_url.scheme() == "file" ||
                 current_url.scheme() == "about" ||
                 request.mode == RequestMode::Navigate {
                 basic_fetch(request.clone(), cache, target, done_chan, context)
@@ -640,41 +640,35 @@ fn http_redirect_fetch(request: Rc<Request>,
     // Step 1
     assert_eq!(response.return_internal.get(), true);
 
-    // Step 3
-    // this step is done early, because querying if Location exists says
-    // if it is None or Some, making it easy to seperate from the retrieval failure case
+    // Step 2
     if !response.actual_response().headers.has::<Location>() {
         return Rc::try_unwrap(response).ok().unwrap();
     }
 
-    // Step 2
+    // Step 3
     let location = match response.actual_response().headers.get::<Location>() {
         Some(&Location(ref location)) => location.clone(),
-        // Step 4
         _ => return Response::network_error()
     };
-
-    // Step 5
     let response_url = response.actual_response().url.as_ref().unwrap();
     let location_url = response_url.join(&*location);
-
-    // Step 6
     let location_url = match location_url {
         Ok(url) => url,
         _ => return Response::network_error()
     };
 
-    // Step 7
+    // Step 4
+    // TODO implement return network_error if not HTTP(S)
+
+    // Step 5
     if request.redirect_count.get() >= 20 {
         return Response::network_error();
     }
 
-    // Step 8
+    // Step 6
     request.redirect_count.set(request.redirect_count.get() + 1);
 
-    // Step 9
-    request.same_origin_data.set(false);
-
+    // Step 7
     let same_origin = if let Origin::Origin(ref origin) = *request.origin.borrow() {
         *origin == request.current_url().origin()
     } else {
@@ -682,22 +676,21 @@ fn http_redirect_fetch(request: Rc<Request>,
     };
     let has_credentials = has_credentials(&location_url);
 
-    // Step 10
     if request.mode == RequestMode::CORSMode && !same_origin && has_credentials {
         return Response::network_error();
     }
 
-    // Step 11
+    // Step 8
     if cors_flag && has_credentials {
         return Response::network_error();
     }
 
-    // Step 12
+    // Step 9
     if cors_flag && !same_origin {
         *request.origin.borrow_mut() = Origin::Origin(UrlOrigin::new_opaque());
     }
 
-    // Step 13
+    // Step 10
     let status_code = response.actual_response().status.unwrap();
     if ((status_code == StatusCode::MovedPermanently || status_code == StatusCode::Found) &&
         *request.method.borrow() == Method::Post) ||
@@ -706,10 +699,13 @@ fn http_redirect_fetch(request: Rc<Request>,
         *request.body.borrow_mut() = None;
     }
 
-    // Step 14
+    // Step 11
     request.url_list.borrow_mut().push(location_url);
 
-    // Step 15
+    // Step 12
+    // TODO implement referrer policy
+
+    // Step 13
     main_fetch(request, cache, cors_flag, true, target, done_chan, context)
 }
 

components/net_traits/request.rs

@@ -120,7 +120,6 @@ pub struct RequestInit {
             serialize_with = "::hyper_serde::serialize")]
     pub headers: Headers,
     pub unsafe_request: bool,
-    pub same_origin_data: bool,
     pub body: Option<Vec<u8>>,
     // TODO: client object
     pub type_: Type,
@@ -146,7 +145,6 @@ impl Default for RequestInit {
             url: Url::parse("about:blank").unwrap(),
             headers: Headers::new(),
             unsafe_request: false,
-            same_origin_data: false,
             body: None,
             type_: Type::None,
             destination: Destination::None,
@@ -188,7 +186,6 @@ pub struct Request {
     // TODO: priority object
     pub origin: RefCell<Origin>,
     pub omit_origin_header: Cell<bool>,
-    pub same_origin_data: Cell<bool>,
     /// https://fetch.spec.whatwg.org/#concept-request-referrer
     pub referrer: RefCell<Referrer>,
     pub referrer_policy: Cell<Option<ReferrerPolicy>>,
@@ -230,7 +227,6 @@ impl Request {
             destination: Destination::None,
             origin: RefCell::new(origin.unwrap_or(Origin::Client)),
             omit_origin_header: Cell::new(false),
-            same_origin_data: Cell::new(false),
             referrer: RefCell::new(Referrer::Client),
             referrer_policy: Cell::new(None),
             pipeline_id: Cell::new(pipeline_id),
@@ -256,7 +252,6 @@ impl Request {
         *req.method.borrow_mut() = init.method;
         *req.headers.borrow_mut() = init.headers;
         req.unsafe_request = init.unsafe_request;
-        req.same_origin_data.set(init.same_origin_data);
         *req.body.borrow_mut() = init.body;
         req.type_ = init.type_;
         req.destination = init.destination;

components/script/dom/htmlscriptelement.rs

@@ -244,8 +244,6 @@ fn fetch_a_classic_script(script: &HTMLScriptElement,
         },
         origin: doc.url().clone(),
         pipeline_id: Some(script.global().r().pipeline_id()),
-        // FIXME: Set to true for now, discussion in https://github.com/whatwg/fetch/issues/381
-        same_origin_data: true,
         referrer_url: Some(doc.url().clone()),
         referrer_policy: doc.get_referrer_policy(),
         .. RequestInit::default()

components/script/dom/request.rs

@@ -159,7 +159,6 @@ impl Request {
         // TODO: `entry settings object` is not implemented in Servo yet.
         *request.origin.borrow_mut() = Origin::Client;
         request.omit_origin_header = temporary_request.omit_origin_header;
-        request.same_origin_data.set(true);
         request.referrer = temporary_request.referrer;
         request.referrer_policy = temporary_request.referrer_policy;
         request.mode = temporary_request.mode;

components/script/dom/xmlhttprequest.rs

@@ -587,7 +587,6 @@ impl XMLHttpRequestMethods for XMLHttpRequest {
             url: self.request_url.borrow().clone().unwrap(),
             headers: (*self.request_headers.borrow()).clone(),
             unsafe_request: true,
-            same_origin_data: true,
             // XXXManishearth figure out how to avoid this clone
             body: extracted.as_ref().map(|e| e.0.clone()),
             // XXXManishearth actually "subresource", but it doesn't exist

components/script/fetch.rs

@@ -44,7 +44,6 @@ fn request_init_from_request(request: NetTraitsRequest) -> NetTraitsRequestInit
         url: request.url(),
         headers: request.headers.borrow().clone(),
         unsafe_request: request.unsafe_request,
-        same_origin_data: request.same_origin_data.get(),
         body: request.body.borrow().clone(),
         type_: request.type_,
         destination: request.destination,

tests/unit/net/fetch.rs

@@ -144,7 +144,6 @@ fn test_fetch_data() {
     let url = Url::parse("data:text/html,<p>Servo</p>").unwrap();
     let origin = Origin::Origin(url.origin());
     let request = Request::new(url, Some(origin), false, None);
-    request.same_origin_data.set(true);
     let expected_resp_body = "<p>Servo</p>".to_owned();
     let fetch_response = fetch_sync(request, None);
 
@@ -173,7 +172,6 @@ fn test_fetch_file() {
     let url = Url::from_file_path(path.clone()).unwrap();
     let origin = Origin::Origin(url.origin());
     let request = Request::new(url, Some(origin), false, None);
-    request.same_origin_data.set(true);
 
     let fetch_response = fetch_sync(request, None);
     assert!(!fetch_response.is_network_error());