diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index eadc3ea05976..801c4f53e4d5 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -5,6 +5,7 @@ use crate::data_loader::decode; use crate::fetch::cors_cache::CorsCache; use crate::filemanager_thread::{fetch_file_in_chunks, FileManager, FILE_CHUNK_SIZE}; +use crate::http_cache::HttpCacheEntry; use crate::http_loader::{determine_request_referrer, http_fetch, HttpState}; use crate::http_loader::{set_default_accept, set_default_accept_language}; use crate::subresource_integrity::is_response_integrity_valid; @@ -40,6 +41,7 @@ lazy_static! { pub type Target<'a> = &'a mut (dyn FetchTaskTarget + Send); +#[derive(Clone)] pub enum Data { Payload(Vec), Done, @@ -224,6 +226,13 @@ pub fn main_fetch( // Step 11. // Not applicable: see fetch_async. + // Get the cache entry corresponding to the url, after potential hsts switch. + let cache_entry = if let Ok(mut http_cache) = context.state.http_cache.write() { + http_cache.get_entry(&request) + } else { + None + }; + // Step 12. let mut response = response.unwrap_or_else(|| { let current_url = request.current_url(); @@ -245,7 +254,7 @@ pub fn main_fetch( request.response_tainting = ResponseTainting::Basic; // Substep 2. - scheme_fetch(request, cache, target, done_chan, context) + scheme_fetch(request, cache, target, done_chan, context, &cache_entry) } else if request.mode == RequestMode::SameOrigin { Response::network_error(NetworkError::Internal("Cross-origin response".into())) } else if request.mode == RequestMode::NoCors { @@ -253,7 +262,7 @@ pub fn main_fetch( request.response_tainting = ResponseTainting::Opaque; // Substep 2. - scheme_fetch(request, cache, target, done_chan, context) + scheme_fetch(request, cache, target, done_chan, context, &cache_entry) } else if !matches!(current_url.scheme(), "http" | "https") { Response::network_error(NetworkError::Internal("Non-http scheme".into())) } else if request.use_cors_preflight || @@ -267,7 +276,15 @@ pub fn main_fetch( request.response_tainting = ResponseTainting::CorsTainting; // Substep 2. let response = http_fetch( - request, cache, true, true, false, target, done_chan, context, + request, + cache, + true, + true, + false, + target, + done_chan, + context, + &cache_entry, ); // Substep 3. if response.is_network_error() { @@ -280,7 +297,15 @@ pub fn main_fetch( request.response_tainting = ResponseTainting::CorsTainting; // Substep 2. http_fetch( - request, cache, true, false, false, target, done_chan, context, + request, + cache, + true, + false, + false, + target, + done_chan, + context, + &cache_entry, ) } }); @@ -455,8 +480,14 @@ pub fn main_fetch( // Step 24. target.process_response_eof(&response); - if let Ok(mut http_cache) = context.state.http_cache.write() { - http_cache.update_awaiting_consumers(&request, &response); + // Note: we need to get an entry again, to take potential re-directs into account. + let cache_entry = if let Ok(mut http_cache) = context.state.http_cache.write() { + http_cache.get_entry(&request) + } else { + None + }; + if let Some(entry) = cache_entry { + entry.update_awaiting_consumers(&request, &response); } // Steps 25-27. @@ -477,7 +508,7 @@ fn wait_for_response(response: &mut Response, target: Target, done_chan: &mut Do }, Data::Done => break, Data::Cancelled => { - response.aborted.store(true, Ordering::Relaxed); + response.aborted.store(true, Ordering::Release); break; }, } @@ -570,6 +601,7 @@ fn scheme_fetch( target: Target, done_chan: &mut DoneChannel, context: &FetchContext, + cache_entry: &Option, ) -> Response { let url = request.current_url(); @@ -586,7 +618,15 @@ fn scheme_fetch( }, "http" | "https" => http_fetch( - request, cache, false, false, false, target, done_chan, context, + request, + cache, + false, + false, + false, + target, + done_chan, + context, + cache_entry, ), "data" => match decode(&url) { diff --git a/components/net/http_cache.rs b/components/net/http_cache.rs index 4f751405d751..ba2b43f8a5bf 100644 --- a/components/net/http_cache.rs +++ b/components/net/http_cache.rs @@ -27,12 +27,12 @@ use servo_url::ServoUrl; use std::collections::HashMap; use std::ops::Bound; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::Mutex; -use std::time::SystemTime; +use std::sync::{Condvar, Mutex, RwLock}; +use std::time::{Duration as StdDuration, SystemTime}; use time::{Duration, Timespec, Tm}; /// The key used to differentiate requests in the cache. -#[derive(Clone, Eq, Hash, MallocSizeOf, PartialEq)] +#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq)] pub struct CacheKey { url: ServoUrl, } @@ -53,7 +53,7 @@ impl CacheKey { /// A complete cached resource. #[derive(Clone)] -struct CachedResource { +pub(crate) struct CachedResource { request_headers: Arc>, body: Arc>, aborted: Arc, @@ -124,10 +124,18 @@ pub struct CachedResponse { #[derive(MallocSizeOf)] pub struct HttpCache { /// cached responses. - entries: HashMap>, + entries: HashMap, } -/// Determine if a given response is cacheable based on the initial metadata received. +/// Determine if a response is cacheable by default +fn is_cacheable_by_default(status_code: u16) -> bool { + match status_code { + 200 | 203 | 204 | 206 | 300 | 301 | 404 | 405 | 410 | 414 | 501 => true, + _ => false, + } +} + +/// Determine if a given response is cacheable. /// Based on fn response_is_cacheable(metadata: &Metadata) -> bool { // TODO: if we determine that this cache should be considered shared: @@ -239,19 +247,16 @@ fn get_response_expiry(response: &Response) -> Duration { } else { max_heuristic }; - match *code { - 200 | 203 | 204 | 206 | 300 | 301 | 404 | 405 | 410 | 414 | 501 => { - // Status codes that are cacheable by default - return heuristic_freshness; - }, - _ => { - // Other status codes can only use heuristic freshness if the public cache directive is present. - if let Some(ref directives) = response.headers.typed_get::() { - if directives.public() { - return heuristic_freshness; - } + if is_cacheable_by_default(*code) { + // Status codes that are cacheable by default can use heuristics to determine freshness. + return heuristic_freshness; + } else { + // Other status codes can only use heuristic freshness if the public cache directive is present. + if let Some(ref directives) = response.headers.typed_get::() { + if directives.public() { + return heuristic_freshness; } - }, + } } } // Requires validation upon first use as default. @@ -296,7 +301,10 @@ fn create_cached_response( cached_resource: &CachedResource, cached_headers: &HeaderMap, done_chan: &mut DoneChannel, -) -> CachedResponse { +) -> Option { + if cached_resource.aborted.load(Ordering::Acquire) { + return None; + } let resource_timing = ResourceFetchTiming::new(request.timing_type()); let mut response = Response::new( cached_resource.data.metadata.data.final_url.clone(), @@ -331,10 +339,11 @@ fn create_cached_response( // let has_expired = (adjusted_expires < time_since_validated) || (adjusted_expires == time_since_validated); - CachedResponse { + let cached_response = CachedResponse { response: response, needs_validation: has_expired, - } + }; + Some(cached_response) } /// Create a new resource, based on the bytes requested, and an existing resource, @@ -364,7 +373,7 @@ fn create_resource_with_bytes_from_resource( /// Support for range requests . fn handle_range_request( request: &Request, - candidates: Vec<&CachedResource>, + candidates: &[&CachedResource], range_spec: Vec<(Bound, Bound)>, done_chan: &mut DoneChannel, ) -> Option { @@ -409,9 +418,11 @@ fn handle_range_request( let cached_headers = new_resource.data.metadata.headers.lock().unwrap(); let cached_response = create_cached_response(request, &new_resource, &*cached_headers, done_chan); - return Some(cached_response); + if let Some(cached_response) = cached_response { + return Some(cached_response); + } } - } + }; }, (&(Bound::Included(beginning), Bound::Included(end)), None) => { for partial_resource in partial_cached_resources { @@ -442,7 +453,9 @@ fn handle_range_request( create_resource_with_bytes_from_resource(&bytes, partial_resource); let cached_response = create_cached_response(request, &new_resource, &*headers, done_chan); - return Some(cached_response); + if let Some(cached_response) = cached_response { + return Some(cached_response); + } } } } @@ -457,9 +470,11 @@ fn handle_range_request( let cached_headers = new_resource.data.metadata.headers.lock().unwrap(); let cached_response = create_cached_response(request, &new_resource, &*cached_headers, done_chan); - return Some(cached_response); + if let Some(cached_response) = cached_response { + return Some(cached_response); + } } - } + }; }, (&(Bound::Included(beginning), Bound::Unbounded), None) => { for partial_resource in partial_cached_resources { @@ -491,7 +506,9 @@ fn handle_range_request( create_resource_with_bytes_from_resource(&bytes, partial_resource); let cached_response = create_cached_response(request, &new_resource, &*headers, done_chan); - return Some(cached_response); + if let Some(cached_response) = cached_response { + return Some(cached_response); + } } } } @@ -506,9 +523,11 @@ fn handle_range_request( let cached_headers = new_resource.data.metadata.headers.lock().unwrap(); let cached_response = create_cached_response(request, &new_resource, &*cached_headers, done_chan); - return Some(cached_response); + if let Some(cached_response) = cached_response { + return Some(cached_response); + } } - } + }; }, (&(Bound::Unbounded, Bound::Included(offset)), None) => { for partial_resource in partial_cached_resources { @@ -544,7 +563,10 @@ fn handle_range_request( create_resource_with_bytes_from_resource(&bytes, partial_resource); let cached_response = create_cached_response(request, &new_resource, &*headers, done_chan); - return Some(cached_response); + if let Some(cached_response) = cached_response { + return Some(cached_response); + } + continue; } } } @@ -563,6 +585,151 @@ impl HttpCache { } } + /// Get the cache entry corresponding to this request + pub fn get_entry(&mut self, request: &Request) -> Option { + if pref!(network.http_cache.disabled) { + return None; + } + let entry_key = CacheKey::new(request.clone()); + let entry = self + .entries + .entry(entry_key.clone()) + .or_insert(HttpCacheEntry::new(entry_key)); + Some(entry.clone()) + } + + fn invalidate_for_url(&mut self, url: &ServoUrl) { + let entry_key = CacheKey::from_servo_url(url); + if let Some(entry) = self.entries.get(&entry_key.clone()) { + entry.invalidate(); + } else { + warn!("Http-cache: invalidate_for_url called for unknown entry."); + } + } + + /// Invalidation. + /// + pub fn invalidate(&mut self, request: &Request, response: &Response) { + if let Some(Ok(location)) = response + .headers + .get(header::LOCATION) + .map(HeaderValue::to_str) + { + if let Ok(url) = request.current_url().join(location) { + self.invalidate_for_url(&url); + } + } + if let Some(Ok(ref content_location)) = response + .headers + .get(header::CONTENT_LOCATION) + .map(HeaderValue::to_str) + { + if let Ok(url) = request.current_url().join(&content_location) { + self.invalidate_for_url(&url); + } + } + self.invalidate_for_url(&request.url()); + } +} + +fn check_vary_headers(request: &Request, cached_resource: &CachedResource) -> bool { + let mut can_be_constructed = true; + let cached_headers = cached_resource.data.metadata.headers.lock().unwrap(); + let original_request_headers = cached_resource.request_headers.lock().unwrap(); + if let Some(vary_value) = cached_headers.typed_get::() { + if vary_value.is_any() { + can_be_constructed = false + } else { + // For every header name found in the Vary header of the stored response. + // Calculating Secondary Keys with Vary + for vary_val in vary_value.iter_strs() { + match request.headers.get(vary_val) { + Some(header_data) => { + // If the header is present in the request. + if let Some(original_header_data) = original_request_headers.get(vary_val) { + // Check that the value of the nominated header field, + // in the original request, matches the value in the current request. + if original_header_data != header_data { + can_be_constructed = false; + break; + } + } + }, + None => { + // If a header field is absent from a request, + // it can only match a stored response if those headers, + // were also absent in the original request. + can_be_constructed = original_request_headers.get(vary_val).is_none(); + }, + } + if !can_be_constructed { + break; + } + } + } + } + can_be_constructed +} + +/// The various states a HttpCacheEntry can be in. +#[derive(Debug, Eq, PartialEq)] +enum CacheEntryState { + /// The entry is fully up-to-date, + /// there are no pending concurrent stores, + /// and it is ready to construct cached responses. + ReadyToConstruct, + /// The entry is pending a concurrent store. + PendingStore, +} + +#[derive(Clone)] +/// A cache entry, corresponding to a cache-key, +/// and containing a list of cached resources. +pub struct HttpCacheEntry { + /// Resources corresponding to the entry. + resources: Arc>>, + /// The state of the entry. + /// + /// A state of `PendingStore` + /// will see any concurrent client to `construct_response` block on the condvar, + /// untile the state is set to `ReadyToConstruct`. + state: Arc<(Mutex, Condvar)>, + /// The request key of this entry. + key: CacheKey, +} + +/// Only count the size of the cached resources, leaving aside the key an and state for now. +impl MallocSizeOf for HttpCacheEntry { + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + let mut size = 0; + for entry in self.resources.read().unwrap().iter() { + size += entry.size_of(ops); + } + size + } +} + +/// An entry is used by the fetch algorithm as the "interface" to the Http-cache. +/// +/// The benefit is that concurrent fetches will not contend on an entry, unless they share the same key, +/// which only happens if those fetches are fetching the same resource. +/// +/// Another benefit is that we can block fetches using the same entry if necessary, +/// see the usage around the condvar in `state`, +/// withouth affecting fetches using other entries. +impl HttpCacheEntry { + /// Create a new cache-entry instance. + pub fn new(key: CacheKey) -> HttpCacheEntry { + HttpCacheEntry { + resources: Arc::new(RwLock::new(vec![])), + state: Arc::new(( + Mutex::new(CacheEntryState::ReadyToConstruct), + Condvar::new(), + )), + key, + } + } + /// Constructing Responses from Caches. /// pub fn construct_response( @@ -571,70 +738,53 @@ impl HttpCache { done_chan: &mut DoneChannel, ) -> Option { // TODO: generate warning headers as appropriate + if request.method != Method::GET { - // Only Get requests are cached, avoid a url based match for others. + // Only responses to GET requests are cached. return None; } + let entry_key = CacheKey::new(request.clone()); - let resources = self - .entries - .get(&entry_key)? - .into_iter() - .filter(|r| !r.aborted.load(Ordering::Relaxed)); - let mut candidates = vec![]; - for cached_resource in resources { - let mut can_be_constructed = true; - let cached_headers = cached_resource.data.metadata.headers.lock().unwrap(); - let original_request_headers = cached_resource.request_headers.lock().unwrap(); - if let Some(vary_value) = cached_headers.typed_get::() { - if vary_value.is_any() { - can_be_constructed = false - } else { - // For every header name found in the Vary header of the stored response. - // Calculating Secondary Keys with Vary - for vary_val in vary_value.iter_strs() { - match request.headers.get(vary_val) { - Some(header_data) => { - // If the header is present in the request. - if let Some(original_header_data) = - original_request_headers.get(vary_val) - { - // Check that the value of the nominated header field, - // in the original request, matches the value in the current request. - if original_header_data != header_data { - can_be_constructed = false; - break; - } - } - }, - None => { - // If a header field is absent from a request, - // it can only match a stored response if those headers, - // were also absent in the original request. - can_be_constructed = - original_request_headers.get(vary_val).is_none(); - }, - } - if !can_be_constructed { - break; - } - } - } - } - if can_be_constructed { - candidates.push(cached_resource); + assert_eq!(entry_key, self.key); + + // If the entry is not ready to construct a response, wait. + // + // The entry is not ready if a previous fetch checked the cache, found nothing, + // and moved on to a network fetch, and hasn't updated the cache yet with a pending resource. + // + // Note that this is a different workflow from the one involving `wait_for_cached_response`. + // That one happens when a fetch gets a cache hit, and the resource is pending completion from the network. + let (lock, cvar) = &*self.state; + let mut state = lock.lock().unwrap(); + while *state == CacheEntryState::PendingStore { + let (current_state, time_out) = cvar + .wait_timeout(state, StdDuration::from_millis(500)) + .unwrap(); + state = current_state; + if time_out.timed_out() { + // After a timeout, ignore the pending store. + break; } } + + let cached_resources = self.resources.read().unwrap(); + let mut candidates: Vec<&CachedResource> = cached_resources + .iter() + .filter(|r| check_vary_headers(request, r)) + .collect(); + // Support for range requests if let Some(range_spec) = request.headers.typed_get::() { - return handle_range_request( + if let Some(res) = handle_range_request( request, - candidates, + candidates.as_slice(), range_spec.iter().collect(), done_chan, - ); + ) { + return Some(res); + } } else { - while let Some(cached_resource) = candidates.pop() { + while let Some(ref cached_resource) = candidates.pop() { // Not a Range request. // Do not allow 206 responses to be constructed. // @@ -655,142 +805,141 @@ impl HttpCache { None => continue, } // Returning a response that can be constructed - // TODO: select the most appropriate one, using a known mechanism from a selecting header field, - // or using the Date header to return the most recent one. let cached_headers = cached_resource.data.metadata.headers.lock().unwrap(); let cached_response = create_cached_response(request, cached_resource, &*cached_headers, done_chan); - return Some(cached_response); + if let Some(cached_response) = cached_response { + return Some(cached_response); + } + continue; } } // The cache wasn't able to construct anything. + // Update its state and fetch the response from the network. + *state = CacheEntryState::PendingStore; None } /// Updating consumers who received a response constructed with a ResponseBody::Receiving. - pub fn update_awaiting_consumers(&mut self, request: &Request, response: &Response) { - if let ResponseBody::Done(ref completed_body) = *response.body.lock().unwrap() { - let entry_key = CacheKey::new(request.clone()); - if let Some(cached_resources) = self.entries.get(&entry_key) { - // Ensure we only wake-up consumers of relevant resources, - // ie we don't want to wake-up 200 awaiting consumers with a 206. - let relevant_cached_resources = cached_resources - .iter() - .filter(|resource| resource.data.raw_status == response.raw_status); - for cached_resource in relevant_cached_resources { - let mut awaiting_consumers = cached_resource.awaiting_body.lock().unwrap(); - for done_sender in awaiting_consumers.drain(..) { - if cached_resource.aborted.load(Ordering::Relaxed) || - response.is_network_error() - { - // In the case of an aborted fetch or a network errror, - // wake-up all awaiting consumers. - // Each will then start a new network request. - // TODO: Wake-up only one consumer, and make it the producer on which others wait. - let _ = done_sender.send(Data::Cancelled); - } else { - let _ = done_sender.send(Data::Payload(completed_body.clone())); - let _ = done_sender.send(Data::Done); - } - } + pub fn update_awaiting_consumers(&self, request: &Request, response: &Response) { + let entry_key = CacheKey::new(request.clone()); + assert_eq!(entry_key, self.key); + + let cached_resources = self.resources.read().unwrap(); + + // Ensure we only wake-up consumers of relevant resources, + // ie we don't want to wake-up 200 awaiting consumers with a 206. + let relevant_cached_resources = cached_resources.iter().filter(|resource| { + if response.is_network_error() { + return *resource.body.lock().unwrap() == ResponseBody::Empty; + } + resource.data.raw_status == response.raw_status + }); + + for cached_resource in relevant_cached_resources { + let mut awaiting_consumers = cached_resource.awaiting_body.lock().unwrap(); + if awaiting_consumers.is_empty() { + continue; + } + let to_send = if cached_resource.aborted.load(Ordering::Acquire) { + // In the case of an aborted fetch, + // wake-up all awaiting consumers. + // Each will then start a new network request. + // TODO: Wake-up only one consumer, and make it the producer on which others wait. + Data::Cancelled + } else { + match *cached_resource.body.lock().unwrap() { + ResponseBody::Done(_) | ResponseBody::Empty => Data::Done, + ResponseBody::Receiving(_) => { + continue; + }, } + }; + for done_sender in awaiting_consumers.drain(..) { + let _ = done_sender.send(to_send.clone()); } } } + /// Invalidation. + /// + pub fn invalidate(&self) { + for cached_resource in self.resources.write().unwrap().iter_mut() { + cached_resource.data.expires = Duration::seconds(0i64); + } + } + /// Freshening Stored Responses upon Validation. /// pub fn refresh( - &mut self, + &self, request: &Request, response: Response, done_chan: &mut DoneChannel, ) -> Option { - assert_eq!(response.status.map(|s| s.0), Some(StatusCode::NOT_MODIFIED)); let entry_key = CacheKey::new(request.clone()); - if let Some(cached_resources) = self.entries.get_mut(&entry_key) { - for cached_resource in cached_resources.iter_mut() { - // done_chan will have been set to Some(..) by http_network_fetch. - // If the body is not receiving data, set the done_chan back to None. - // Otherwise, create a new dedicated channel to update the consumer. - // The response constructed here will replace the 304 one from the network. - let in_progress_channel = match *cached_resource.body.lock().unwrap() { - ResponseBody::Receiving(..) => Some(unbounded()), - ResponseBody::Empty | ResponseBody::Done(..) => None, - }; - match in_progress_channel { - Some((done_sender, done_receiver)) => { - *done_chan = Some((done_sender.clone(), done_receiver)); - cached_resource - .awaiting_body - .lock() - .unwrap() - .push(done_sender); - }, - None => *done_chan = None, - } - // Received a response with 304 status code, in response to a request that matches a cached resource. - // 1. update the headers of the cached resource. - // 2. return a response, constructed from the cached resource. - let resource_timing = ResourceFetchTiming::new(request.timing_type()); - let mut constructed_response = Response::new( - cached_resource.data.metadata.data.final_url.clone(), - resource_timing, - ); - constructed_response.body = cached_resource.body.clone(); - constructed_response.status = cached_resource.data.status.clone(); - constructed_response.https_state = cached_resource.data.https_state.clone(); - constructed_response.referrer = request.referrer.to_url().cloned(); - constructed_response.referrer_policy = request.referrer_policy.clone(); - constructed_response.raw_status = cached_resource.data.raw_status.clone(); - constructed_response.url_list = cached_resource.data.url_list.clone(); - cached_resource.data.expires = get_response_expiry(&constructed_response); - let mut stored_headers = cached_resource.data.metadata.headers.lock().unwrap(); - stored_headers.extend(response.headers); - constructed_response.headers = stored_headers.clone(); - return Some(constructed_response); + assert_eq!(entry_key, self.key); + assert_eq!(response.status.map(|s| s.0), Some(StatusCode::NOT_MODIFIED)); + for cached_resource in self.resources.write().unwrap().iter_mut() { + // done_chan will have been set to Some(..) by http_network_fetch. + // If the body is not receiving data, set the done_chan back to None. + // Otherwise, create a new dedicated channel to update the consumer. + // The response constructed here will replace the 304 one from the network. + let in_progress_channel = match *cached_resource.body.lock().unwrap() { + ResponseBody::Receiving(..) => Some(unbounded()), + ResponseBody::Empty | ResponseBody::Done(..) => None, + }; + match in_progress_channel { + Some((done_sender, done_receiver)) => { + *done_chan = Some((done_sender.clone(), done_receiver)); + cached_resource + .awaiting_body + .lock() + .unwrap() + .push(done_sender); + }, + None => *done_chan = None, } - } - None - } + // Received a response with 304 status code, in response to a request that matches a cached resource. + // 1. update the headers of the cached resource. + // 2. return a response, constructed from the cached resource. + let resource_timing = ResourceFetchTiming::new(request.timing_type()); + let mut constructed_response = Response::new( + cached_resource.data.metadata.data.final_url.clone(), + resource_timing, + ); - fn invalidate_for_url(&mut self, url: &ServoUrl) { - let entry_key = CacheKey::from_servo_url(url); - if let Some(cached_resources) = self.entries.get_mut(&entry_key) { - for cached_resource in cached_resources.iter_mut() { - cached_resource.data.expires = Duration::seconds(0i64); - } + constructed_response.body = cached_resource.body.clone(); + constructed_response.status = cached_resource.data.status.clone(); + constructed_response.https_state = cached_resource.data.https_state.clone(); + constructed_response.referrer = request.referrer.to_url().cloned(); + constructed_response.referrer_policy = request.referrer_policy.clone(); + constructed_response.raw_status = cached_resource.data.raw_status.clone(); + constructed_response.url_list = cached_resource.data.url_list.clone(); + + cached_resource.data.expires = get_response_expiry(&constructed_response); + + let mut stored_headers = cached_resource.data.metadata.headers.lock().unwrap(); + stored_headers.extend(response.headers); + constructed_response.headers = stored_headers.clone(); + return Some(constructed_response); } + None } - /// Invalidation. - /// - pub fn invalidate(&mut self, request: &Request, response: &Response) { - // TODO(eijebong): Once headers support typed_get, update this to use them - if let Some(Ok(location)) = response - .headers - .get(header::LOCATION) - .map(HeaderValue::to_str) - { - if let Ok(url) = request.current_url().join(location) { - self.invalidate_for_url(&url); - } - } - if let Some(Ok(ref content_location)) = response - .headers - .get(header::CONTENT_LOCATION) - .map(HeaderValue::to_str) - { - if let Ok(url) = request.current_url().join(&content_location) { - self.invalidate_for_url(&url); - } - } - self.invalidate_for_url(&request.url()); + /// Set the state to ready to construct, and wake-up any concurrent client. + pub fn set_state_to_ready(&self) { + let (lock, cvar) = &*self.state; + let mut state = lock.lock().unwrap(); + *state = CacheEntryState::ReadyToConstruct; + cvar.notify_all(); } /// Storing Responses in Caches. /// - pub fn store(&mut self, request: &Request, response: &Response) { + pub fn store(&self, request: &Request, response: &Response) { + let entry_key = CacheKey::new(request.clone()); + assert_eq!(entry_key, self.key); if pref!(network.http_cache.disabled) { return; } @@ -806,8 +955,7 @@ impl HttpCache { // TODO: unless a cache directive that allows such // responses to be stored is present in the response. return; - }; - let entry_key = CacheKey::new(request.clone()); + } let metadata = match response.metadata() { Ok(FetchMetadata::Filtered { filtered: _, @@ -845,10 +993,9 @@ impl HttpCache { last_validated: time::now(), }), }; - let entry = self.entries.entry(entry_key).or_insert(vec![]); - entry.push(entry_resource); // TODO: Complete incomplete responses, including 206 response, when stored here. // See A cache MAY complete a stored incomplete response by making a subsequent range request // https://tools.ietf.org/html/rfc7234#section-3.1 + self.resources.write().unwrap().push(entry_resource); } } diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 225751e2cd52..f335585ff34d 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -12,7 +12,7 @@ use crate::fetch::methods::{ }; use crate::fetch::methods::{Data, DoneChannel, FetchContext, Target}; use crate::hsts::HstsList; -use crate::http_cache::HttpCache; +use crate::http_cache::{HttpCache, HttpCacheEntry}; use crate::resource_thread::AuthCache; use crossbeam_channel::{unbounded, Sender}; use devtools_traits::{ @@ -475,6 +475,7 @@ pub fn http_fetch( target: Target, done_chan: &mut DoneChannel, context: &FetchContext, + cache_entry: &Option, ) -> Response { // This is a new async fetch, reset the channel we are waiting on *done_chan = None; @@ -527,7 +528,7 @@ pub fn http_fetch( // Sub-substep 1 if method_mismatch || header_mismatch { - let preflight_result = cors_preflight_fetch(&request, cache, context); + let preflight_result = cors_preflight_fetch(&request, cache, context, cache_entry); // Sub-substep 2 if let Some(e) = preflight_result.get_network_error() { return Response::network_error(e.clone()); @@ -557,6 +558,7 @@ pub fn http_fetch( cors_flag, done_chan, context, + cache_entry, ); // Substep 4 @@ -825,6 +827,7 @@ fn http_network_or_cache_fetch( cors_flag: bool, done_chan: &mut DoneChannel, context: &FetchContext, + cache_entry: &Option, ) -> Response { // Step 2 let mut response: Option = None; @@ -1020,8 +1023,8 @@ fn http_network_or_cache_fetch( // TODO If there’s a proxy-authentication entry, use it as appropriate. // Step 5.19 - if let Ok(http_cache) = context.state.http_cache.read() { - if let Some(response_from_cache) = http_cache.construct_response(&http_request, done_chan) { + if let Some(entry) = cache_entry { + if let Some(response_from_cache) = entry.construct_response(&http_request, done_chan) { let response_headers = response_from_cache.response.headers.clone(); // Substep 1, 2, 3, 4 let (cached_response, needs_revalidation) = @@ -1038,6 +1041,11 @@ fn http_network_or_cache_fetch( response_from_cache.needs_validation, ), }; + if cached_response.is_none() { + // Ensure the done chan is not set if we're not using the cached response, + // as the cache might have set it to Some if it constructed a pending response. + *done_chan = None; + } if needs_revalidation { revalidating_flag = true; // Substep 5 @@ -1061,6 +1069,7 @@ fn http_network_or_cache_fetch( fn wait_for_cached_response(done_chan: &mut DoneChannel, response: &mut Option) { if let Some(ref ch) = *done_chan { + assert!(response.is_some()); // The cache constructed a response with a body of ResponseBody::Receiving. // We wait for the response in the cache to "finish", // with a body of either Done or Cancelled. @@ -1094,6 +1103,11 @@ fn http_network_or_cache_fetch( if response.is_none() { // Substep 1 if http_request.cache_mode == CacheMode::OnlyIfCached { + // The cache will not be updated, + // set its state to ready to construct. + if let Some(entry) = cache_entry { + entry.set_state_to_ready(); + } return Response::network_error(NetworkError::Internal( "Couldn't find response in cache".into(), )); @@ -1119,8 +1133,8 @@ fn http_network_or_cache_fetch( .as_ref() .map_or(false, |s| s.0 == StatusCode::NOT_MODIFIED) { - if let Ok(mut http_cache) = context.state.http_cache.write() { - response = http_cache.refresh(&http_request, forward_response.clone(), done_chan); + if let Some(entry) = cache_entry { + response = entry.refresh(&http_request, forward_response.clone(), done_chan); wait_for_cached_response(done_chan, &mut response); } } @@ -1129,8 +1143,8 @@ fn http_network_or_cache_fetch( if response.is_none() { if http_request.cache_mode != CacheMode::NoStore { // Subsubstep 2, doing it first to avoid a clone of forward_response. - if let Ok(mut http_cache) = context.state.http_cache.write() { - http_cache.store(&http_request, &forward_response); + if let Some(entry) = cache_entry { + entry.store(&http_request, &forward_response); } } // Subsubstep 1 @@ -1140,6 +1154,11 @@ fn http_network_or_cache_fetch( let mut response = response.unwrap(); + if let Some(entry) = cache_entry { + // The entry has been updated, set its state to ready to construct. + entry.set_state_to_ready(); + } + // Step 8 // TODO: if necessary set response's range-requested flag @@ -1169,6 +1188,10 @@ fn http_network_or_cache_fetch( return response; } + // Make sure this is set to None, + // since we're about to start a new `http_network_or_cache_fetch`. + *done_chan = None; + // Substep 4 response = http_network_or_cache_fetch( http_request, @@ -1176,6 +1199,7 @@ fn http_network_or_cache_fetch( cors_flag, done_chan, context, + cache_entry, ); } @@ -1465,6 +1489,7 @@ fn cors_preflight_fetch( request: &Request, cache: &mut CorsCache, context: &FetchContext, + cache_entry: &Option, ) -> Response { // Step 1 let mut preflight = Request::new( @@ -1507,7 +1532,14 @@ fn cors_preflight_fetch( } // Step 5 - let response = http_network_or_cache_fetch(&mut preflight, false, false, &mut None, context); + let response = http_network_or_cache_fetch( + &mut preflight, + false, + false, + &mut None, + context, + cache_entry, + ); // Step 6 if cors_check(&request, &response).is_ok() && diff --git a/components/net/tests/http_cache.rs b/components/net/tests/http_cache.rs index 743e6f431889..f4609f157488 100644 --- a/components/net/tests/http_cache.rs +++ b/components/net/tests/http_cache.rs @@ -32,14 +32,15 @@ fn test_refreshing_resource_sets_done_chan_the_appropriate_value() { .headers .insert(EXPIRES, HeaderValue::from_str("-10").unwrap()); let mut cache = HttpCache::new(); + let entry = cache.get_entry(&request).unwrap(); response_bodies.iter().for_each(|body| { *response.body.lock().unwrap() = body.clone(); // First, store the 'normal' response. - cache.store(&request, &response); + entry.store(&request, &response); // Second, mutate the response into a 304 response, and refresh the stored one. response.status = Some((StatusCode::NOT_MODIFIED, String::from("304"))); let mut done_chan = Some(unbounded()); - let refreshed_response = cache.refresh(&request, response.clone(), &mut done_chan); + let refreshed_response = entry.refresh(&request, response.clone(), &mut done_chan); // Ensure a resource was found, and refreshed. assert!(refreshed_response.is_some()); match body {