Rewrite determine_request_referrer() to explicitly limit it to the checks it can do.

Checks for the Client value should reside in the script thread.

I also noted some other issues in this code.

3 files changed, 40 insertions, 42 deletions

diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs
index 994f3d36702..ebff48a80ba 100644
--- a/components/net/fetch/methods.rs
+++ b/components/net/fetch/methods.rs
@@ -21,6 +21,7 @@ use net_traits::response::{Response, ResponseBody, ResponseType};
 use std::borrow::Cow;
 use std::fs::File;
 use std::io::Read;
+use std::mem;
 use std::rc::Rc;
 use std::sync::mpsc::{Sender, Receiver};
 
@@ -154,15 +155,27 @@ pub fn main_fetch(request: Rc<Request>,
     request.referrer_policy.set(Some(referrer_policy));
 
     // Step 8
-    if *request.referrer.borrow() != Referrer::NoReferrer {
-        // remove Referrer headers set in past redirects/preflights
-        // this stops the assertion in determine_request_referrer from failing
-        request.headers.borrow_mut().remove::<RefererHeader>();
-        let referrer_url = determine_request_referrer(&mut *request.headers.borrow_mut(),
-                                                      referrer_policy,
-                                                      request.referrer.borrow_mut().take(),
-                                                      request.current_url().clone());
-        *request.referrer.borrow_mut() = Referrer::from_url(referrer_url);
+    {
+        let mut referrer = request.referrer.borrow_mut();
+        let referrer_url = match mem::replace(&mut *referrer, Referrer::NoReferrer) {
+            Referrer::NoReferrer => None,
+            Referrer::Client => {
+                // FIXME(#14507): We should never get this value here; it should
+                //                already have been handled in the script thread.
+                request.headers.borrow_mut().remove::<RefererHeader>();
+                None
+            },
+            Referrer::ReferrerUrl(url) => {
+                request.headers.borrow_mut().remove::<RefererHeader>();
+                determine_request_referrer(&mut *request.headers.borrow_mut(),
+                                           referrer_policy,
+                                           url,
+                                           request.current_url().clone())
+            }
+        };
+        if let Some(referrer_url) = referrer_url {
+            *referrer = Referrer::ReferrerUrl(referrer_url);
+        }
     }
 
     // Step 9
diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs
index ddcdef9a439..a3969e8845a 100644
--- a/components/net/http_loader.rs
+++ b/components/net/http_loader.rs
@@ -276,27 +276,28 @@ fn strip_url(mut referrer_url: ServoUrl, origin_only: bool) -> Option<ServoUrl>
 }
 
 /// https://w3c.github.io/webappsec-referrer-policy/#determine-requests-referrer
+/// Steps 4-6.
 pub fn determine_request_referrer(headers: &mut Headers,
                                   referrer_policy: ReferrerPolicy,
-                                  referrer_url: Option<ServoUrl>,
-                                  url: ServoUrl) -> Option<ServoUrl> {
-    //TODO - algorithm step 2 not addressed
+                                  referrer_source: ServoUrl,
+                                  current_url: ServoUrl)
+                                  -> Option<ServoUrl> {
     assert!(!headers.has::<Referer>());
-    if let Some(ref_url) = referrer_url {
-        let cross_origin = ref_url.origin() != url.origin();
-        return match referrer_policy {
-            ReferrerPolicy::NoReferrer => None,
-            ReferrerPolicy::Origin => strip_url(ref_url, true),
-            ReferrerPolicy::SameOrigin => if cross_origin { None } else { strip_url(ref_url, false) },
-            ReferrerPolicy::UnsafeUrl => strip_url(ref_url, false),
-            ReferrerPolicy::OriginWhenCrossOrigin => strip_url(ref_url, cross_origin),
-            ReferrerPolicy::StrictOrigin => strict_origin(ref_url, url),
-            ReferrerPolicy::StrictOriginWhenCrossOrigin => strict_origin_when_cross_origin(ref_url, url),
-            ReferrerPolicy::NoReferrerWhenDowngrade =>
-                no_referrer_when_downgrade_header(ref_url, url),
-        };
+    // FIXME(#14505): this does not seem to be the correct way of checking for
+    //                same-origin requests.
+    let cross_origin = referrer_source.origin() != current_url.origin();
+    // FIXME(#14506): some of these cases are expected to consider whether the
+    //                request's client is "TLS-protected", whatever that means.
+    match referrer_policy {
+        ReferrerPolicy::NoReferrer => None,
+        ReferrerPolicy::Origin => strip_url(referrer_source, true),
+        ReferrerPolicy::SameOrigin => if cross_origin { None } else { strip_url(referrer_source, false) },
+        ReferrerPolicy::UnsafeUrl => strip_url(referrer_source, false),
+        ReferrerPolicy::OriginWhenCrossOrigin => strip_url(referrer_source, cross_origin),
+        ReferrerPolicy::StrictOrigin => strict_origin(referrer_source, current_url),
+        ReferrerPolicy::StrictOriginWhenCrossOrigin => strict_origin_when_cross_origin(referrer_source, current_url),
+        ReferrerPolicy::NoReferrerWhenDowngrade => no_referrer_when_downgrade_header(referrer_source, current_url),
     }
-    return None;
 }
 
 pub fn set_request_cookies(url: &ServoUrl, headers: &mut Headers, cookie_jar: &Arc<RwLock<CookieStorage>>) {
diff --git a/components/net_traits/request.rs b/components/net_traits/request.rs
index 795a8556f6f..3c780a2c5d5 100644
--- a/components/net_traits/request.rs
+++ b/components/net_traits/request.rs
@@ -9,7 +9,6 @@ use msg::constellation_msg::PipelineId;
 use servo_url::ServoUrl;
 use std::cell::{Cell, RefCell};
 use std::default::Default;
-use std::mem::swap;
 use url::{Origin as UrlOrigin};
 
 /// An [initiator](https://fetch.spec.whatwg.org/#concept-request-initiator)
@@ -308,19 +307,4 @@ impl Referrer {
             Referrer::ReferrerUrl(ref url) => Some(url)
         }
     }
-    pub fn from_url(url: Option<ServoUrl>) -> Self {
-        if let Some(url) = url {
-            Referrer::ReferrerUrl(url)
-        } else {
-            Referrer::NoReferrer
-        }
-    }
-    pub fn take(&mut self) -> Option<ServoUrl> {
-        let mut new = Referrer::Client;
-        swap(self, &mut new);
-        match new {
-            Referrer::NoReferrer | Referrer::Client => None,
-            Referrer::ReferrerUrl(url) => Some(url)
-        }
-    }
 }