servo: Merge #10867 - Fix logic for cors cache match (from danlrobertson:sandbox); r=KiChjang
authorDaniel Robertson <dan.robertson@anidata.org>
Thu, 28 Apr 2016 13:36:05 -0700
changeset 338654 6bac9f9d8656ee19607a1a24d10c8bf872354844
parent 338653 9b4c283f5d48e778205955cb281a72376e8ac009
child 338655 5c96e151fb9e73a2beb42dd396c8942ec0a80772
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersKiChjang
servo: Merge #10867 - Fix logic for cors cache match (from danlrobertson:sandbox); r=KiChjang The current logic for a cors cache match does not consider "credentials is false and request's credentials mode is not "include" or credentials is true." I could have missed something, but `CacheRequestDetails::credentials` is set to true if credentials mode is "include", and false otherwise. So `(!cors_cache.credentials && !cors_req.credentials) || cors_cache.credentials` would be directly following the spec, but unless I'm mistaken `cors_cache.credentials || !cors_req.credentials` is logically the same. Fixes: #10525 Source-Repo: https://github.com/servo/servo Source-Revision: 3d38a60cee8a2e19ae8f04df7c2374fc9d97999c
servo/components/net/fetch/cors_cache.rs
servo/components/net/fetch/methods.rs
servo/components/net_traits/request.rs
servo/tests/unit/net/fetch.rs
--- a/servo/components/net/fetch/cors_cache.rs
+++ b/servo/components/net/fetch/cors_cache.rs
@@ -7,25 +7,24 @@
 //! For stuff involving `<img>`, `<iframe>`, `<form>`, etc please check what
 //! the request mode should be and compare with the fetch spec
 //! This library will eventually become the core of the Fetch crate
 //! with CORSRequest being expanded into FetchRequest (etc)
 
 use hyper::method::Method;
 use net_traits::request::Origin;
 use std::ascii::AsciiExt;
-use std::sync::mpsc::{Sender, Receiver, channel};
 use time;
 use time::{now, Timespec};
 use url::Url;
 
 /// Union type for CORS cache entries
 ///
 /// Each entry might pertain to a header or method
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub enum HeaderOrMethod {
     HeaderData(String),
     MethodData(Method)
 }
 
 impl HeaderOrMethod {
     fn match_header(&self, header_name: &str) -> bool {
         match *self {
@@ -38,17 +37,17 @@ impl HeaderOrMethod {
         match *self {
             HeaderOrMethod::MethodData(ref m) => m == method,
             _ => false
         }
     }
 }
 
 /// An entry in the CORS cache
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct CORSCacheEntry {
     pub origin: Origin,
     pub url: Url,
     pub max_age: u32,
     pub credentials: bool,
     pub header_or_method: HeaderOrMethod,
     created: Timespec
 }
@@ -69,255 +68,105 @@ impl CORSCacheEntry {
 
 /// Properties of Request required to cache match.
 pub struct CacheRequestDetails {
     pub origin: Origin,
     pub destination: Url,
     pub credentials: bool
 }
 
-/// Trait for a generic CORS Cache
-pub trait CORSCache {
-    /// [Clear the cache](https://fetch.spec.whatwg.org/#concept-cache-clear)
-    fn clear (&mut self, request: CacheRequestDetails);
-
-    /// Remove old entries
-    fn cleanup(&mut self);
-
-    /// Returns true if an entry with a
-    /// [matching header](https://fetch.spec.whatwg.org/#concept-cache-match-header) is found
-    fn match_header(&mut self, request: CacheRequestDetails, header_name: &str) -> bool;
-
-    /// Updates max age if an entry for a
-    /// [matching header](https://fetch.spec.whatwg.org/#concept-cache-match-header) is found.
-    ///
-    /// If not, it will insert an equivalent entry
-    fn match_header_and_update(&mut self, request: CacheRequestDetails, header_name: &str, new_max_age: u32) -> bool;
-
-    /// Returns true if an entry with a
-    /// [matching method](https://fetch.spec.whatwg.org/#concept-cache-match-method) is found
-    fn match_method(&mut self, request: CacheRequestDetails, method: Method) -> bool;
-
-    /// Updates max age if an entry for
-    /// [a matching method](https://fetch.spec.whatwg.org/#concept-cache-match-method) is found.
-    ///
-    /// If not, it will insert an equivalent entry
-    fn match_method_and_update(&mut self, request: CacheRequestDetails, method: Method, new_max_age: u32) -> bool;
-    /// Insert an entry
-    fn insert(&mut self, entry: CORSCacheEntry);
+fn match_headers(cors_cache: &CORSCacheEntry, cors_req: &CacheRequestDetails) -> bool {
+    cors_cache.origin == cors_req.origin &&
+        cors_cache.url == cors_req.destination &&
+        (cors_cache.credentials || !cors_req.credentials)
 }
 
 /// A simple, vector-based CORS Cache
 #[derive(Clone)]
-pub struct BasicCORSCache(Vec<CORSCacheEntry>);
+pub struct CORSCache(Vec<CORSCacheEntry>);
 
-fn match_headers(cors_cache: &CORSCacheEntry, cors_req: &CacheRequestDetails) -> bool {
-    cors_cache.origin == cors_req.origin &&
-        cors_cache.url == cors_req.destination &&
-        cors_cache.credentials == cors_req.credentials
-}
+impl CORSCache {
 
-impl BasicCORSCache {
-
-    pub fn new() -> BasicCORSCache {
-        BasicCORSCache(vec![])
+    pub fn new() -> CORSCache {
+        CORSCache(vec![])
     }
 
     fn find_entry_by_header<'a>(&'a mut self, request: &CacheRequestDetails,
                                 header_name: &str) -> Option<&'a mut CORSCacheEntry> {
         self.cleanup();
         self.0.iter_mut().find(|e| match_headers(e, request) && e.header_or_method.match_header(header_name))
     }
 
     fn find_entry_by_method<'a>(&'a mut self, request: &CacheRequestDetails,
                                 method: Method) -> Option<&'a mut CORSCacheEntry> {
         // we can take the method from CORSRequest itself
         self.cleanup();
         self.0.iter_mut().find(|e| match_headers(e, request) && e.header_or_method.match_method(&method))
     }
-}
 
-impl CORSCache for BasicCORSCache {
-    /// https://fetch.spec.whatwg.org/#concept-cache-clear
-    #[allow(dead_code)]
-    fn clear (&mut self, request: CacheRequestDetails) {
-        let BasicCORSCache(buf) = self.clone();
+    /// [Clear the cache](https://fetch.spec.whatwg.org/#concept-cache-clear)
+    pub fn clear (&mut self, request: CacheRequestDetails) {
+        let CORSCache(buf) = self.clone();
         let new_buf: Vec<CORSCacheEntry> =
             buf.into_iter().filter(|e| e.origin == request.origin && request.destination == e.url).collect();
-        *self = BasicCORSCache(new_buf);
+        *self = CORSCache(new_buf);
     }
 
-    // Remove old entries
-    fn cleanup(&mut self) {
-        let BasicCORSCache(buf) = self.clone();
+    /// Remove old entries
+    pub fn cleanup(&mut self) {
+        let CORSCache(buf) = self.clone();
         let now = time::now().to_timespec();
         let new_buf: Vec<CORSCacheEntry> = buf.into_iter()
-                                              .filter(|e| now.sec > e.created.sec + e.max_age as i64)
+                                              .filter(|e| now.sec < e.created.sec + e.max_age as i64)
                                               .collect();
-        *self = BasicCORSCache(new_buf);
+        *self = CORSCache(new_buf);
     }
 
-    fn match_header(&mut self, request: CacheRequestDetails, header_name: &str) -> bool {
+    /// Returns true if an entry with a
+    /// [matching header](https://fetch.spec.whatwg.org/#concept-cache-match-header) is found
+    pub fn match_header(&mut self, request: CacheRequestDetails, header_name: &str) -> bool {
         self.find_entry_by_header(&request, header_name).is_some()
     }
 
-    fn match_header_and_update(&mut self, request: CacheRequestDetails, header_name: &str, new_max_age: u32) -> bool {
+    /// Updates max age if an entry for a
+    /// [matching header](https://fetch.spec.whatwg.org/#concept-cache-match-header) is found.
+    ///
+    /// If not, it will insert an equivalent entry
+    pub fn match_header_and_update(&mut self, request: CacheRequestDetails,
+                                   header_name: &str, new_max_age: u32) -> bool {
         match self.find_entry_by_header(&request, header_name).map(|e| e.max_age = new_max_age) {
             Some(_) => true,
             None => {
                 self.insert(CORSCacheEntry::new(request.origin, request.destination, new_max_age,
                                                 request.credentials,
                                                 HeaderOrMethod::HeaderData(header_name.to_owned())));
                 false
             }
         }
     }
 
-    fn match_method(&mut self, request: CacheRequestDetails, method: Method) -> bool {
+    /// Returns true if an entry with a
+    /// [matching method](https://fetch.spec.whatwg.org/#concept-cache-match-method) is found
+    pub fn match_method(&mut self, request: CacheRequestDetails, method: Method) -> bool {
         self.find_entry_by_method(&request, method).is_some()
     }
 
-    fn match_method_and_update(&mut self, request: CacheRequestDetails, method: Method, new_max_age: u32) -> bool {
+    /// Updates max age if an entry for
+    /// [a matching method](https://fetch.spec.whatwg.org/#concept-cache-match-method) is found.
+    ///
+    /// If not, it will insert an equivalent entry
+    pub fn match_method_and_update(&mut self, request: CacheRequestDetails, method: Method, new_max_age: u32) -> bool {
         match self.find_entry_by_method(&request, method.clone()).map(|e| e.max_age = new_max_age) {
             Some(_) => true,
             None => {
                 self.insert(CORSCacheEntry::new(request.origin, request.destination, new_max_age,
                                                 request.credentials, HeaderOrMethod::MethodData(method)));
                 false
             }
         }
     }
 
-    fn insert(&mut self, entry: CORSCacheEntry) {
+    /// Insert an entry
+    pub fn insert(&mut self, entry: CORSCacheEntry) {
         self.cleanup();
         self.0.push(entry);
     }
 }
-
-/// Various messages that can be sent to a CORSCacheThread
-pub enum CORSCacheThreadMsg {
-    Clear(CacheRequestDetails, Sender<()>),
-    Cleanup(Sender<()>),
-    MatchHeader(CacheRequestDetails, String, Sender<bool>),
-    MatchHeaderUpdate(CacheRequestDetails, String, u32, Sender<bool>),
-    MatchMethod(CacheRequestDetails, Method, Sender<bool>),
-    MatchMethodUpdate(CacheRequestDetails, Method, u32, Sender<bool>),
-    Insert(CORSCacheEntry, Sender<()>),
-    ExitMsg
-}
-
-/// A Sender to a CORSCacheThread
-///
-/// This can be used as a CORS Cache.
-/// The methods on this type block until they can run, and it behaves similar to a mutex
-pub type CORSCacheSender = Sender<CORSCacheThreadMsg>;
-
-impl CORSCache for CORSCacheSender {
-    fn clear (&mut self, request: CacheRequestDetails) {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::Clear(request, tx));
-        let _ = rx.recv();
-    }
-
-    fn cleanup(&mut self) {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::Cleanup(tx));
-        let _ = rx.recv();
-    }
-
-    fn match_header(&mut self, request: CacheRequestDetails, header_name: &str) -> bool {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::MatchHeader(request, header_name.to_owned(), tx));
-        rx.recv().unwrap_or(false)
-    }
-
-    fn match_header_and_update(&mut self, request: CacheRequestDetails, header_name: &str, new_max_age: u32) -> bool {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::MatchHeaderUpdate(request, header_name.to_owned(), new_max_age, tx));
-        rx.recv().unwrap_or(false)
-    }
-
-    fn match_method(&mut self, request: CacheRequestDetails, method: Method) -> bool {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::MatchMethod(request, method, tx));
-        rx.recv().unwrap_or(false)
-    }
-
-    fn match_method_and_update(&mut self, request: CacheRequestDetails, method: Method, new_max_age: u32) -> bool {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::MatchMethodUpdate(request, method, new_max_age, tx));
-        rx.recv().unwrap_or(false)
-    }
-
-    fn insert(&mut self, entry: CORSCacheEntry) {
-        let (tx, rx) = channel();
-        let _ = self.send(CORSCacheThreadMsg::Insert(entry, tx));
-        let _ = rx.recv();
-    }
-}
-
-/// A simple thread-based CORS Cache that can be sent messages
-///
-/// #Example
-/// ```ignore
-/// let thread = CORSCacheThread::new();
-/// let builder = ThreadBuilder::new().named("XHRThread");
-/// let mut sender = thread.sender();
-/// builder.spawn(move || { thread.run() });
-/// sender.insert(CORSCacheEntry::new(/* parameters here */));
-/// ```
-pub struct CORSCacheThread {
-    receiver: Receiver<CORSCacheThreadMsg>,
-    cache: BasicCORSCache,
-    sender: CORSCacheSender
-}
-
-impl CORSCacheThread {
-    pub fn new() -> CORSCacheThread {
-        let (tx, rx) = channel();
-        CORSCacheThread {
-            receiver: rx,
-            cache: BasicCORSCache(vec![]),
-            sender: tx
-        }
-    }
-
-    /// Provides a sender to the cache thread
-    pub fn sender(&self) -> CORSCacheSender {
-        self.sender.clone()
-    }
-
-    /// Runs the cache thread
-    /// This blocks the current thread, so it is advised
-    /// to spawn a new thread for this
-    /// Send ExitMsg to the associated Sender to exit
-    pub fn run(&mut self) {
-        loop {
-            match self.receiver.recv().unwrap() {
-                CORSCacheThreadMsg::Clear(request, tx) => {
-                    self.cache.clear(request);
-                    let _ = tx.send(());
-                },
-                CORSCacheThreadMsg::Cleanup(tx) => {
-                    self.cache.cleanup();
-                    let _ = tx.send(());
-                },
-                CORSCacheThreadMsg::MatchHeader(request, header, tx) => {
-                    let _ = tx.send(self.cache.match_header(request, &header));
-                },
-                CORSCacheThreadMsg::MatchHeaderUpdate(request, header, new_max_age, tx) => {
-                    let _ = tx.send(self.cache.match_header_and_update(request, &header, new_max_age));
-                },
-                CORSCacheThreadMsg::MatchMethod(request, method, tx) => {
-                    let _ = tx.send(self.cache.match_method(request, method));
-                },
-                CORSCacheThreadMsg::MatchMethodUpdate(request, method, new_max_age, tx) => {
-                    let _ = tx.send(self.cache.match_method_and_update(request, method, new_max_age));
-                },
-                CORSCacheThreadMsg::Insert(entry, tx) => {
-                    self.cache.insert(entry);
-                    let _ = tx.send(());
-                },
-                CORSCacheThreadMsg::ExitMsg => break
-            }
-        }
-    }
-}
--- a/servo/components/net/fetch/methods.rs
+++ b/servo/components/net/fetch/methods.rs
@@ -1,14 +1,14 @@
 /* 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 http://mozilla.org/MPL/2.0/. */
 
 use data_loader::decode;
-use fetch::cors_cache::{BasicCORSCache, CORSCache, CacheRequestDetails};
+use fetch::cors_cache::{CORSCache, CacheRequestDetails};
 use http_loader::{NetworkHttpRequestFactory, create_http_connector, obtain_response};
 use hyper::header::{Accept, AcceptLanguage, Authorization, AccessControlAllowCredentials};
 use hyper::header::{AccessControlAllowOrigin, AccessControlAllowHeaders, AccessControlAllowMethods};
 use hyper::header::{AccessControlRequestHeaders, AccessControlMaxAge, AccessControlRequestMethod, Basic};
 use hyper::header::{CacheControl, CacheDirective, ContentEncoding, ContentLength, ContentLanguage, ContentType};
 use hyper::header::{Encoding, HeaderView, Headers, IfMatch, IfRange, IfUnmodifiedSince, IfModifiedSince};
 use hyper::header::{IfNoneMatch, Pragma, Location, QualityItem, Referer as RefererHeader, UserAgent, q, qitem};
 use hyper::method::Method;
@@ -35,16 +35,20 @@ pub fn fetch_async(request: Request, lis
         let fetch_response = fetch(request);
         fetch_response.wait_until_done();
         listener.response_available(fetch_response);
     })
 }
 
 /// [Fetch](https://fetch.spec.whatwg.org#concept-fetch)
 pub fn fetch(request: Rc<Request>) -> Response {
+    fetch_with_cors_cache(request, &mut CORSCache::new())
+}
+
+pub fn fetch_with_cors_cache(request: Rc<Request>, cache: &mut CORSCache) -> Response {
 
     // Step 1
     if request.window.get() == Window::Client {
         // TODO: Set window to request's client object if client is a Window object
     } else {
         request.window.set(Window::NoWindow);
     }
 
@@ -97,21 +101,21 @@ pub fn fetch(request: Rc<Request>) -> Re
     // Step 5
     // TODO: Figure out what a Priority object is
 
     // Step 6
     if request.is_subresource_request() {
         // TODO: create a fetch record and append it to request's client's fetch group list
     }
     // Step 7
-    main_fetch(request, false, false)
+    main_fetch(request, cache, false, false)
 }
 
 /// [Main fetch](https://fetch.spec.whatwg.org/#concept-main-fetch)
-fn main_fetch(request: Rc<Request>, cors_flag: bool, recursive_flag: bool) -> Response {
+fn main_fetch(request: Rc<Request>, cache: &mut CORSCache, cors_flag: bool, recursive_flag: bool) -> Response {
     // TODO: Implement main fetch spec
 
     // Step 1
     let mut response = None;
 
     // Step 2
     if request.local_urls_only {
         match request.current_url().scheme() {
@@ -151,44 +155,44 @@ fn main_fetch(request: Rc<Request>, cors
                 false
             };
 
             if (same_origin && !cors_flag ) ||
                 (current_url.scheme() == "data" && request.same_origin_data.get()) ||
                 current_url.scheme() == "about" ||
                 request.mode == RequestMode::Navigate {
 
-                basic_fetch(request.clone())
+                basic_fetch(request.clone(), cache)
 
             } else if request.mode == RequestMode::SameOrigin {
                 Response::network_error()
 
             } else if request.mode == RequestMode::NoCORS {
                 request.response_tainting.set(ResponseTainting::Opaque);
-                basic_fetch(request.clone())
+                basic_fetch(request.clone(), cache)
 
             } else if !matches!(current_url.scheme(), "http" | "https") {
                 Response::network_error()
 
             } else if request.use_cors_preflight ||
                 (request.unsafe_request &&
                  (!is_simple_method(&request.method.borrow()) ||
                   request.headers.borrow().iter().any(|h| !is_simple_header(&h)))) {
 
                 request.response_tainting.set(ResponseTainting::CORSTainting);
                 request.redirect_mode.set(RedirectMode::Error);
-                let response = http_fetch(request.clone(), BasicCORSCache::new(), true, true, false);
+                let response = http_fetch(request.clone(), cache, true, true, false);
                 if response.is_network_error() {
                     // TODO clear cache entries using request
                 }
                 response
 
             } else {
                 request.response_tainting.set(ResponseTainting::CORSTainting);
-                http_fetch(request.clone(), BasicCORSCache::new(), true, false, false)
+                http_fetch(request.clone(), cache, true, false, false)
             }
         }
     };
 
     // Step 10
     if recursive_flag {
         return response;
     }
@@ -275,31 +279,31 @@ fn main_fetch(request: Rc<Request>, cors
         // TODO this step
     }
 
     // TODO remove this line when only asynchronous fetches are used
     return response;
 }
 
 /// [Basic fetch](https://fetch.spec.whatwg.org#basic-fetch)
-fn basic_fetch(request: Rc<Request>) -> Response {
+fn basic_fetch(request: Rc<Request>, cache: &mut CORSCache) -> Response {
 
     let url = request.current_url();
 
     match url.scheme() {
 
         "about" if url.path() == "blank" => {
             let mut response = Response::new();
             response.headers.set(ContentType(mime!(Text / Html; Charset = Utf8)));
             *response.body.lock().unwrap() = ResponseBody::Done(vec![]);
             response
         },
 
         "http" | "https" => {
-            http_fetch(request.clone(), BasicCORSCache::new(), false, false, false)
+            http_fetch(request.clone(), cache, false, false, false)
         },
 
         "data" => {
             if *request.method.borrow() == Method::Get {
                 match decode(&url) {
                     Ok((mime, bytes)) => {
                         let mut response = Response::new();
                         *response.body.lock().unwrap() = ResponseBody::Done(bytes);
@@ -319,17 +323,17 @@ fn basic_fetch(request: Rc<Request>) -> 
         },
 
         _ => Response::network_error()
     }
 }
 
 /// [HTTP fetch](https://fetch.spec.whatwg.org#http-fetch)
 fn http_fetch(request: Rc<Request>,
-              mut cache: BasicCORSCache,
+              cache: &mut CORSCache,
               cors_flag: bool,
               cors_preflight_flag: bool,
               authentication_fetch_flag: bool) -> Response {
 
     // Step 1
     let mut response: Option<Response> = None;
 
     // Step 2
@@ -389,17 +393,17 @@ fn http_fetch(request: Rc<Request>,
                     origin: origin.clone(),
                     destination: url.clone(),
                     credentials: credentials
                 }, view.name()) && !is_simple_header(&view)
             );
 
             // Sub-substep 1
             if method_mismatch || header_mismatch {
-                let preflight_result = cors_preflight_fetch(request.clone(), Some(cache));
+                let preflight_result = cors_preflight_fetch(request.clone(), cache);
                 // Sub-substep 2
                 if preflight_result.response_type == ResponseType::Error {
                     return Response::network_error();
                 }
             }
         }
 
         // Substep 2
@@ -438,17 +442,17 @@ fn http_fetch(request: Rc<Request>,
             response = match request.redirect_mode.get() {
                 RedirectMode::Error => Response::network_error(),
                 RedirectMode::Manual => {
                     response.to_filtered(ResponseType::OpaqueRedirect)
                 },
                 RedirectMode::Follow => {
                     // set back to default
                     response.return_internal.set(true);
-                    http_redirect_fetch(request, Rc::new(response), cors_flag)
+                    http_redirect_fetch(request, cache, Rc::new(response), cors_flag)
                 }
             }
         },
 
         // Code 401
         StatusCode::Unauthorized => {
 
             // Step 1
@@ -461,33 +465,33 @@ fn http_fetch(request: Rc<Request>,
             // TODO: Spec says requires testing on multiple WWW-Authenticate headers
 
             // Step 3
             if !request.use_url_credentials || authentication_fetch_flag {
                 // TODO: Prompt the user for username and password from the window
             }
 
             // Step 4
-            return http_fetch(request, BasicCORSCache::new(), cors_flag, cors_preflight_flag, true);
+            return http_fetch(request, cache, cors_flag, cors_preflight_flag, true);
         }
 
         // Code 407
         StatusCode::ProxyAuthenticationRequired => {
 
             // Step 1
             // TODO: Figure out what to do with request window objects
 
             // Step 2
             // TODO: Spec says requires testing on Proxy-Authenticate headers
 
             // Step 3
             // TODO: Prompt the user for proxy authentication credentials
 
             // Step 4
-            return http_fetch(request, BasicCORSCache::new(),
+            return http_fetch(request, cache,
                               cors_flag, cors_preflight_flag,
                               authentication_fetch_flag);
         }
 
         _ => { }
     }
 
     // Step 6
@@ -498,16 +502,17 @@ fn http_fetch(request: Rc<Request>,
     // set back to default
     response.return_internal.set(true);
     // Step 7
     response
 }
 
 /// [HTTP redirect fetch](https://fetch.spec.whatwg.org#http-redirect-fetch)
 fn http_redirect_fetch(request: Rc<Request>,
+                       cache: &mut CORSCache,
                        response: Rc<Response>,
                        cors_flag: bool) -> Response {
 
     // Step 1
     assert_eq!(response.return_internal.get(), true);
 
     // Step 3
     // this step is done early, because querying if Location exists says
@@ -575,17 +580,17 @@ fn http_redirect_fetch(request: Rc<Reque
         *request.method.borrow_mut() = Method::Get;
         *request.body.borrow_mut() = None;
     }
 
     // Step 14
     request.url_list.borrow_mut().push(location_url);
 
     // Step 15
-    main_fetch(request, cors_flag, true)
+    main_fetch(request, cache, cors_flag, true)
 }
 
 /// [HTTP network or cache fetch](https://fetch.spec.whatwg.org#http-network-or-cache-fetch)
 fn http_network_or_cache_fetch(request: Rc<Request>,
                                credentials_flag: bool,
                                authentication_fetch_flag: bool) -> Response {
 
     // TODO: Implement Window enum for Request
@@ -912,17 +917,17 @@ fn http_network_fetch(request: Rc<Reques
             // Sub-substep 4
         // Substep 3
 
     // Step 11
     response
 }
 
 /// [CORS preflight fetch](https://fetch.spec.whatwg.org#cors-preflight-fetch)
-fn cors_preflight_fetch(request: Rc<Request>, cache: Option<BasicCORSCache>) -> Response {
+fn cors_preflight_fetch(request: Rc<Request>, cache: &mut CORSCache) -> Response {
     // Step 1
     let mut preflight = Request::new(request.current_url(), Some(request.origin.borrow().clone()), false);
     *preflight.method.borrow_mut() = Method::Options;
     preflight.initiator = request.initiator.clone();
     preflight.type_ = request.type_.clone();
     preflight.destination = request.destination.clone();
     preflight.referer = request.referer.clone();
 
@@ -990,22 +995,16 @@ fn cors_preflight_fetch(request: Rc<Requ
             return Response::network_error();
         }
 
         // Substep 7, 8
         let max_age = response.headers.get::<AccessControlMaxAge>().map(|acma| acma.0).unwrap_or(0);
 
         // TODO: Substep 9 - Need to define what an imposed limit on max-age is
 
-        // Substep 10
-        let mut cache = match cache {
-            Some(c) => c,
-            None => return response
-        };
-
         // Substep 11, 12
         for method in &methods {
             cache.match_method_and_update(CacheRequestDetails {
                 origin: request.origin.borrow().clone(),
                 destination: request.current_url(),
                 credentials: request.credentials_mode == CredentialsMode::Include
             }, method.clone(), max_age);
         }
--- a/servo/components/net_traits/request.rs
+++ b/servo/components/net_traits/request.rs
@@ -28,17 +28,17 @@ pub enum Type {
 #[derive(Copy, Clone, PartialEq)]
 pub enum Destination {
     None, Document, Embed, Font, Image, Manifest,
     Media, Object, Report, Script, ServiceWorker,
     SharedWorker, Style, Worker, XSLT
 }
 
 /// A request [origin](https://fetch.spec.whatwg.org/#concept-request-origin)
-#[derive(Clone, PartialEq)]
+#[derive(Clone, PartialEq, Debug)]
 pub enum Origin {
     Client,
     Origin(UrlOrigin)
 }
 
 /// A [referer](https://fetch.spec.whatwg.org/#concept-request-referrer)
 #[derive(Clone, PartialEq)]
 pub enum Referer {
--- a/servo/tests/unit/net/fetch.rs
+++ b/servo/tests/unit/net/fetch.rs
@@ -1,23 +1,25 @@
 /* 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 http://mozilla.org/MPL/2.0/. */
 
 use hyper::header::{AccessControlAllowCredentials, AccessControlAllowHeaders, AccessControlAllowOrigin};
-use hyper::header::{AccessControlAllowMethods, AccessControlRequestHeaders, AccessControlRequestMethod};
+use hyper::header::{AccessControlAllowMethods, AccessControlMaxAge};
+use hyper::header::{AccessControlRequestHeaders, AccessControlRequestMethod};
 use hyper::header::{CacheControl, ContentLanguage, ContentType, Expires, LastModified};
 use hyper::header::{Headers, HttpDate, Location, SetCookie, Pragma};
 use hyper::method::Method;
 use hyper::mime::{Mime, TopLevel, SubLevel};
 use hyper::server::{Handler, Listening, Server};
 use hyper::server::{Request as HyperRequest, Response as HyperResponse};
 use hyper::status::StatusCode;
 use hyper::uri::RequestUri;
-use net::fetch::methods::{fetch, fetch_async};
+use net::fetch::cors_cache::{CacheRequestDetails, CORSCache};
+use net::fetch::methods::{fetch, fetch_async, fetch_with_cors_cache};
 use net_traits::AsyncFetchListener;
 use net_traits::request::{Origin, RedirectMode, Referer, Request, RequestMode};
 use net_traits::response::{CacheState, Response, ResponseBody, ResponseType};
 use std::rc::Rc;
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::mpsc::{Sender, channel};
 use std::sync::{Arc, Mutex};
 use time::{self, Duration};
@@ -170,16 +172,71 @@ fn test_cors_preflight_fetch() {
 
     match *fetch_response.body.lock().unwrap() {
         ResponseBody::Done(ref body) => assert_eq!(&**body, ACK),
         _ => panic!()
     };
 }
 
 #[test]
+fn test_cors_preflight_cache_fetch() {
+    static ACK: &'static [u8] = b"ACK";
+    let state = Arc::new(AtomicUsize::new(0));
+    let counter = state.clone();
+    let mut cache = CORSCache::new();
+    let handler = move |request: HyperRequest, mut response: HyperResponse| {
+        if request.method == Method::Options && state.clone().fetch_add(1, Ordering::SeqCst) == 0 {
+            assert!(request.headers.has::<AccessControlRequestMethod>());
+            assert!(request.headers.has::<AccessControlRequestHeaders>());
+            response.headers_mut().set(AccessControlAllowOrigin::Any);
+            response.headers_mut().set(AccessControlAllowCredentials);
+            response.headers_mut().set(AccessControlAllowMethods(vec![Method::Get]));
+            response.headers_mut().set(AccessControlMaxAge(6000));
+        } else {
+            response.headers_mut().set(AccessControlAllowOrigin::Any);
+            response.send(ACK).unwrap();
+        }
+    };
+    let (mut server, url) = make_server(handler);
+
+    let origin = Origin::Origin(UrlOrigin::new_opaque());
+    let mut request = Request::new(url.clone(), Some(origin.clone()), false);
+    request.referer = Referer::NoReferer;
+    request.use_cors_preflight = true;
+    request.mode = RequestMode::CORSMode;
+    let wrapped_request0 = Rc::new(request.clone());
+    let wrapped_request1 = Rc::new(request);
+
+    let fetch_response0 = fetch_with_cors_cache(wrapped_request0, &mut cache);
+    let fetch_response1 = fetch_with_cors_cache(wrapped_request1, &mut cache);
+    let _ = server.close();
+
+    assert!(!fetch_response0.is_network_error() && !fetch_response1.is_network_error());
+
+    // The response from the CORS-preflight cache was used
+    assert_eq!(1, counter.load(Ordering::SeqCst));
+
+    // The entry exists in the CORS-preflight cache
+    assert_eq!(true, cache.match_method(CacheRequestDetails {
+        origin: origin,
+        destination: url,
+        credentials: false
+    }, Method::Get));
+
+    match *fetch_response0.body.lock().unwrap() {
+        ResponseBody::Done(ref body) => assert_eq!(&**body, ACK),
+        _ => panic!()
+    };
+    match *fetch_response1.body.lock().unwrap() {
+        ResponseBody::Done(ref body) => assert_eq!(&**body, ACK),
+        _ => panic!()
+    };
+}
+
+#[test]
 fn test_cors_preflight_fetch_network_error() {
     static ACK: &'static [u8] = b"ACK";
     let state = Arc::new(AtomicUsize::new(0));
     let handler = move |request: HyperRequest, mut response: HyperResponse| {
         if request.method == Method::Options && state.clone().fetch_add(1, Ordering::SeqCst) == 0 {
             assert!(request.headers.has::<AccessControlRequestMethod>());
             assert!(request.headers.has::<AccessControlRequestHeaders>());
             response.headers_mut().set(AccessControlAllowOrigin::Any);