From c99b4c9adc67bf46402962618acb4d961ba9db83 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 9 Dec 2016 13:13:27 -0800 Subject: [PATCH] servo: Merge #14508 - Rewrite determine_request_referrer() to explicitly limit it to the checks it can do (from servo:determine_request_referrer); r=jdm,frewsxcv Checks for the Client value should reside in the script thread. I also noted some other issues in this code. Source-Repo: https://github.com/servo/servo Source-Revision: 882d5512bb9aa7263864fb18d702c1efb6401914 --- servo/components/net/fetch/methods.rs | 31 ++++++++++++++++------- servo/components/net/http_loader.rs | 35 +++++++++++++------------- servo/components/net_traits/request.rs | 16 ------------ 3 files changed, 40 insertions(+), 42 deletions(-) diff --git a/servo/components/net/fetch/methods.rs b/servo/components/net/fetch/methods.rs index 994f3d367024..ebff48a80ba5 100644 --- a/servo/components/net/fetch/methods.rs +++ b/servo/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.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::(); - 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::(); + None + }, + Referrer::ReferrerUrl(url) => { + request.headers.borrow_mut().remove::(); + 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/servo/components/net/http_loader.rs b/servo/components/net/http_loader.rs index cc381bfe8e8f..6199c45209f3 100644 --- a/servo/components/net/http_loader.rs +++ b/servo/components/net/http_loader.rs @@ -238,27 +238,28 @@ fn strip_url(mut referrer_url: ServoUrl, origin_only: bool) -> Option } /// 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, - url: ServoUrl) -> Option { - //TODO - algorithm step 2 not addressed + referrer_source: ServoUrl, + current_url: ServoUrl) + -> Option { assert!(!headers.has::()); - 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>) { diff --git a/servo/components/net_traits/request.rs b/servo/components/net_traits/request.rs index 795a8556f6f5..3c780a2c5d53 100644 --- a/servo/components/net_traits/request.rs +++ b/servo/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) -> Self { - if let Some(url) = url { - Referrer::ReferrerUrl(url) - } else { - Referrer::NoReferrer - } - } - pub fn take(&mut self) -> Option { - let mut new = Referrer::Client; - swap(self, &mut new); - match new { - Referrer::NoReferrer | Referrer::Client => None, - Referrer::ReferrerUrl(url) => Some(url) - } - } }