Fix logic for cors cache match

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."

components/net/fetch/cors_cache.rs

@@ -20,7 +20,7 @@ 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)
@@ -43,7 +43,7 @@ impl HeaderOrMethod {
 }
 
 /// An entry in the CORS cache
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct CORSCacheEntry {
     pub origin: Origin,
     pub url: Url,
@@ -112,7 +112,7 @@ pub struct BasicCORSCache(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
+        (cors_cache.credentials || !cors_req.credentials)
 }
 
 impl BasicCORSCache {
@@ -150,7 +150,7 @@ impl CORSCache for BasicCORSCache {
         let BasicCORSCache(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);
     }

components/net/fetch/methods.rs

@@ -40,6 +40,10 @@ pub fn fetch_async(request: Request, listener: Box<AsyncFetchListener + Send>) {
 
 /// [Fetch](https://fetch.spec.whatwg.org#concept-fetch)
 pub fn fetch(request: Rc<Request>) -> Response {
+    fetch_with_cors_cache(request, &mut BasicCORSCache::new())
+}
+
+pub fn fetch_with_cors_cache(request: Rc<Request>, cache: &mut BasicCORSCache) -> Response {
 
     // Step 1
     if request.window.get() == Window::Client {
@@ -102,11 +106,11 @@ pub fn fetch(request: Rc<Request>) -> Response {
         // 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 BasicCORSCache, cors_flag: bool, recursive_flag: bool) -> Response {
     // TODO: Implement main fetch spec
 
     // Step 1
@@ -156,14 +160,14 @@ fn main_fetch(request: Rc<Request>, cors_flag: bool, recursive_flag: bool) -> Re
                 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()
@@ -175,7 +179,7 @@ fn main_fetch(request: Rc<Request>, cors_flag: bool, recursive_flag: bool) -> Re
 
                 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
                 }
@@ -183,7 +187,7 @@ fn main_fetch(request: Rc<Request>, cors_flag: bool, recursive_flag: bool) -> Re
 
             } else {
                 request.response_tainting.set(ResponseTainting::CORSTainting);
-                http_fetch(request.clone(), BasicCORSCache::new(), true, false, false)
+                http_fetch(request.clone(), cache, true, false, false)
             }
         }
     };
@@ -280,7 +284,7 @@ fn main_fetch(request: Rc<Request>, cors_flag: bool, recursive_flag: bool) -> Re
 }
 
 /// [Basic fetch](https://fetch.spec.whatwg.org#basic-fetch)
-fn basic_fetch(request: Rc<Request>) -> Response {
+fn basic_fetch(request: Rc<Request>, cache: &mut BasicCORSCache) -> Response {
 
     let url = request.current_url();
 
@@ -294,7 +298,7 @@ fn basic_fetch(request: Rc<Request>) -> Response {
         },
 
         "http" | "https" => {
-            http_fetch(request.clone(), BasicCORSCache::new(), false, false, false)
+            http_fetch(request.clone(), cache, false, false, false)
         },
 
         "data" => {
@@ -324,7 +328,7 @@ fn basic_fetch(request: Rc<Request>) -> Response {
 
 /// [HTTP fetch](https://fetch.spec.whatwg.org#http-fetch)
 fn http_fetch(request: Rc<Request>,
-              mut cache: BasicCORSCache,
+              cache: &mut BasicCORSCache,
               cors_flag: bool,
               cors_preflight_flag: bool,
               authentication_fetch_flag: bool) -> Response {
@@ -394,7 +398,7 @@ fn http_fetch(request: Rc<Request>,
 
             // 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();
@@ -443,7 +447,7 @@ fn http_fetch(request: Rc<Request>,
                 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)
                 }
             }
         },
@@ -466,7 +470,7 @@ fn http_fetch(request: Rc<Request>,
             }
 
             // 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
@@ -482,7 +486,7 @@ fn http_fetch(request: Rc<Request>,
             // 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);
         }
@@ -503,6 +507,7 @@ fn http_fetch(request: Rc<Request>,
 
 /// [HTTP redirect fetch](https://fetch.spec.whatwg.org#http-redirect-fetch)
 fn http_redirect_fetch(request: Rc<Request>,
+                       cache: &mut BasicCORSCache,
                        response: Rc<Response>,
                        cors_flag: bool) -> Response {
 
@@ -580,7 +585,7 @@ fn http_redirect_fetch(request: Rc<Request>,
     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)
@@ -917,7 +922,7 @@ fn http_network_fetch(request: Rc<Request>,
 }
 
 /// [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 BasicCORSCache) -> Response {
     // Step 1
     let mut preflight = Request::new(request.current_url(), Some(request.origin.borrow().clone()), false);
     *preflight.method.borrow_mut() = Method::Options;
@@ -995,12 +1000,6 @@ fn cors_preflight_fetch(request: Rc<Request>, cache: Option<BasicCORSCache>) ->
 
         // 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 {

components/net_traits/request.rs

@@ -33,7 +33,7 @@ pub enum Destination {
 }
 
 /// A request [origin](https://fetch.spec.whatwg.org/#concept-request-origin)
-#[derive(Clone, PartialEq)]
+#[derive(Clone, PartialEq, Debug)]
 pub enum Origin {
     Client,
     Origin(UrlOrigin)

tests/unit/net/fetch.rs

@@ -3,7 +3,8 @@
  * 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;
@@ -12,7 +13,8 @@ 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::{BasicCORSCache, 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};
@@ -174,6 +176,61 @@ fn test_cors_preflight_fetch() {
     };
 }
 
+#[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 = BasicCORSCache::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";