From e84e1d607bd158dde576d2def8ebadfdd63630e1 Mon Sep 17 00:00:00 2001 From: ms2300 Date: Sun, 23 Sep 2018 21:12:51 -0700 Subject: [PATCH 01/13] Initial implementation of asynchronous blob url fetching --- components/net/blob_loader.rs | 87 +++------------ components/net/fetch/methods.rs | 20 ++-- components/net/filemanager_thread.rs | 154 ++++++++++++++++++++++++++- 3 files changed, 171 insertions(+), 90 deletions(-) diff --git a/components/net/blob_loader.rs b/components/net/blob_loader.rs index e6d64acc16e..50c5455808d 100644 --- a/components/net/blob_loader.rs +++ b/components/net/blob_loader.rs @@ -3,6 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use crate::filemanager_thread::FileManager; +use fetch::methods::DoneChannel; use headers_core::HeaderMapExt; use headers_ext::{ContentLength, ContentType}; use http::header::{self, HeaderValue}; @@ -11,18 +12,21 @@ use ipc_channel::ipc; use mime::{self, Mime}; use net_traits::blob_url_store::parse_blob_url; use net_traits::filemanager_thread::ReadFileProgress; +use net_traits::response::{Response, ResponseBody}; use net_traits::{http_percent_encode, NetworkError}; use servo_url::ServoUrl; +use std::sync::mpsc::channel; // TODO: Check on GET // https://w3c.github.io/FileAPI/#requestResponseModel /// https://fetch.spec.whatwg.org/#concept-basic-fetch (partial) -// TODO: make async. -pub fn load_blob_sync( +pub fn load_blob_async( url: ServoUrl, filemanager: FileManager, -) -> Result<(HeaderMap, Vec), NetworkError> { + response: &Response, + done_chan: &mut DoneChannel +) -> Result<(), NetworkError> { let (id, origin) = match parse_blob_url(&url) { Ok((id, origin)) => (id, origin), Err(()) => { @@ -31,78 +35,11 @@ pub fn load_blob_sync( }, }; - let (sender, receiver) = ipc::channel().unwrap(); + let (sender, receiver) = channel(); + *done_chan = Some((sender.clone(), receiver)); + *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]); let check_url_validity = true; - filemanager.read_file(sender, id, check_url_validity, origin); + filemanager.fetch_file(sender, id, check_url_validity, origin, response); - let blob_buf = match receiver.recv().unwrap() { - Ok(ReadFileProgress::Meta(blob_buf)) => blob_buf, - Ok(_) => { - return Err(NetworkError::Internal( - "Invalid filemanager reply".to_string(), - )); - }, - Err(e) => { - return Err(NetworkError::Internal(format!("{:?}", e))); - }, - }; - - let content_type: Mime = blob_buf.type_string.parse().unwrap_or(mime::TEXT_PLAIN); - let charset = content_type.get_param(mime::CHARSET); - - let mut headers = HeaderMap::new(); - - if let Some(name) = blob_buf.filename { - let charset = charset - .map(|c| c.as_ref().into()) - .unwrap_or("us-ascii".to_owned()); - // TODO(eijebong): Replace this once the typed header is there - headers.insert( - header::CONTENT_DISPOSITION, - HeaderValue::from_bytes( - format!( - "inline; {}", - if charset.to_lowercase() == "utf-8" { - format!( - "filename=\"{}\"", - String::from_utf8(name.as_bytes().into()).unwrap() - ) - } else { - format!( - "filename*=\"{}\"''{}", - charset, - http_percent_encode(name.as_bytes()) - ) - } - ) - .as_bytes(), - ) - .unwrap(), - ); - } - - // Basic fetch, Step 4. - headers.typed_insert(ContentLength(blob_buf.size as u64)); - // Basic fetch, Step 5. - headers.typed_insert(ContentType::from(content_type.clone())); - - let mut bytes = blob_buf.bytes; - loop { - match receiver.recv().unwrap() { - Ok(ReadFileProgress::Partial(ref mut new_bytes)) => { - bytes.append(new_bytes); - }, - Ok(ReadFileProgress::EOF) => { - return Ok((headers, bytes)); - }, - Ok(_) => { - return Err(NetworkError::Internal( - "Invalid filemanager reply".to_string(), - )); - }, - Err(e) => { - return Err(NetworkError::Internal(format!("{:?}", e))); - }, - } - } + Ok(()) } diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index 70dcf4d7983..67a02775369 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use crate::blob_loader::load_blob_sync; +use crate::blob_loader::load_blob_async; use crate::data_loader::decode; use crate::fetch::cors_cache::CorsCache; use crate::filemanager_thread::FileManager; @@ -657,19 +657,11 @@ fn scheme_fetch( )); } - match load_blob_sync(url.clone(), context.filemanager.clone()) { - Ok((headers, bytes)) => { - let mut response = - Response::new(url, ResourceFetchTiming::new(request.timing_type())); - response.headers = headers; - *response.body.lock().unwrap() = ResponseBody::Done(bytes); - response - }, - Err(e) => { - debug!("Failed to load {}: {:?}", url, e); - Response::network_error(e) - }, - } + let mut response = Response::new(url, ResourceFetchTiming::new(request.timing_type())); + + load_blob_async(url.clone(), context.filemanager.clone(), &response, done_chan); + + response }, "ftp" => { diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index 8f0fb05b229..0be8de815b4 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -4,12 +4,18 @@ use embedder_traits::{EmbedderMsg, EmbedderProxy, FilterPattern}; use ipc_channel::ipc::{self, IpcSender}; +use fetch::methods::Data; +use hyper::header::{Charset, ContentLength, ContentType, Headers}; +use hyper::header::{ContentDisposition, DispositionParam, DispositionType}; +use mime::{Attr, Mime}; use mime_guess::guess_mime_type_opt; +use net_traits::NetworkError; use net_traits::blob_url_store::{BlobBuf, BlobURLStoreError}; use net_traits::filemanager_thread::{FileManagerResult, FileManagerThreadMsg, FileOrigin}; use net_traits::filemanager_thread::{ FileManagerThreadError, ReadFileProgress, RelativePos, SelectedFile, }; +use net_traits::response::{Response, ResponseBody}; use servo_config::prefs::PREFS; use std::collections::HashMap; use std::fs::File; @@ -17,7 +23,7 @@ use std::io::{Read, Seek, SeekFrom}; use std::ops::Index; use std::path::{Path, PathBuf}; use std::sync::atomic::{self, AtomicBool, AtomicUsize, Ordering}; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, RwLock, Mutex, mpsc}; use std::thread; use url::Url; use uuid::Uuid; @@ -91,6 +97,19 @@ impl FileManager { .expect("Thread spawning failed"); } + pub fn fetch_file(&self, + sender: mpsc::Sender, + id: Uuid, + check_url_validity: bool, + origin: FileOrigin, + response: &Response) { + let store = self.store.clone(); + let mut res_body = response.body.clone(); + thread::Builder::new().name("read file".to_owned()).spawn(move || { + store.try_fetch_file(&sender, id, check_url_validity, origin, response, res_body) + }).expect("Thread spawning failed"); + } + pub fn promote_memory( &self, blob_buf: BlobBuf, @@ -489,6 +508,94 @@ impl FileManagerStore { ) } + fn fetch_blob_buf(&self, sender: &mpsc::Sender, + id: &Uuid, origin_in: &FileOrigin, rel_pos: RelativePos, + check_url_validity: bool, response: &Response, res_body: Arc>) -> Result<(), BlobURLStoreError> { + let mut bytes = vec![]; + let file_impl = self.get_impl(id, origin_in, check_url_validity)?; + match file_impl { + FileImpl::Memory(buf) => { + let range = rel_pos.to_abs_range(buf.size as usize); + let blob_buf = BlobBuf { + filename: None, + type_string: buf.type_string, + size: range.len() as u64, + bytes: buf.bytes.index(range).to_vec(), + }; + + let content_type: Mime = blob_buf.type_string.parse().unwrap_or(mime!(Text / Plain)); + let charset = content_type.get_param(Attr::Charset); + let mut headers = Headers::new(); + + if let Some(name) = blob_buf.filename { + let charset = charset.and_then(|c| c.as_str().parse().ok()); + headers.set(ContentDisposition { + disposition: DispositionType::Inline, + parameters: vec![ + DispositionParam::Filename(charset.unwrap_or(Charset::Us_Ascii), + None, name.as_bytes().to_vec()) + ] + }); + } + + headers.set(ContentLength(blob_buf.size as u64)); + headers.set(ContentType(content_type.clone())); + + bytes.extend_from_slice(&blob_buf.bytes); + + response.headers = headers; + *res_body.lock().unwrap() = ResponseBody::Done(bytes); + let _ = sender.send(Data::Done); + Ok(()) + } + FileImpl::MetaDataOnly(metadata) => { + /* XXX: Snapshot state check (optional) https://w3c.github.io/FileAPI/#snapshot-state. + Concretely, here we create another file, and this file might not + has the same underlying file state (meta-info plus content) as the time + create_entry is called. + */ + + let opt_filename = metadata.path.file_name() + .and_then(|osstr| osstr.to_str()) + .map(|s| s.to_string()); + + let mime = guess_mime_type_opt(metadata.path.clone()); + let range = rel_pos.to_abs_range(metadata.size as usize); + + let mut file = File::open(&metadata.path) + .map_err(|e| BlobURLStoreError::External(e.to_string()))?; + let seeked_start = file.seek(SeekFrom::Start(range.start as u64)) + .map_err(|e| BlobURLStoreError::External(e.to_string()))?; + + if seeked_start == (range.start as u64) { + let type_string = match mime { + Some(x) => format!("{}", x), + None => "".to_string(), + }; + + chunked_fetch(sender, &mut file, range.len(), opt_filename, + type_string, response, res_body, &mut bytes); + Ok(()) + } else { + Err(BlobURLStoreError::InvalidEntry) + } + } + FileImpl::Sliced(parent_id, inner_rel_pos) => { + // Next time we don't need to check validity since + // we have already done that for requesting URL if necessary + self.fetch_blob_buf(sender, &parent_id, origin_in, + rel_pos.slice_inner(&inner_rel_pos), false, response, res_body) + } + } + } + + fn try_fetch_file(&self, sender: &mpsc::Sender, id: Uuid, check_url_validity: bool, + origin_in: FileOrigin, response: &Response, res_body: Arc>) + -> Result<(), BlobURLStoreError> { + self.fetch_blob_buf(sender, &id, &origin_in, RelativePos::full_range(), check_url_validity, + response, res_body) + } + fn dec_ref(&self, id: &Uuid, origin_in: &FileOrigin) -> Result<(), BlobURLStoreError> { let (do_remove, opt_parent_id) = match self.entries.read().unwrap().get(id) { Some(entry) => { @@ -656,3 +763,48 @@ fn chunked_read( } } } + +fn chunked_fetch(sender: &mpsc::Sender, + file: &mut File, size: usize, opt_filename: Option, + type_string: String, response: &Response, res_body: Arc>, bytes: &mut Vec) { + // First chunk + let mut buf = vec![0; CHUNK_SIZE]; + match file.read(&mut buf) { + Ok(n) => { + buf.truncate(n); + let blob_buf = BlobBuf { + filename: opt_filename, + type_string: type_string, + size: size as u64, + bytes: buf, + }; + bytes.extend_from_slice(&blob_buf.bytes); + let _ = sender.send(Data::Payload(blob_buf.bytes)); + } + Err(_) => { + *response = Response::network_error(NetworkError::Internal("Opening file failed".into())); + return; + } + } + + // Send the remaining chunks + loop { + let mut buf = vec![0; CHUNK_SIZE]; + match file.read(&mut buf) { + Ok(0) => { + *res_body.lock().unwrap() = ResponseBody::Done(bytes.to_vec()); + let _ = sender.send(Data::Done); + return; + } + Ok(n) => { + buf.truncate(n); + bytes.extend_from_slice(&buf); + let _ = sender.send(Data::Payload(buf)); + } + Err(e) => { + *response = Response::network_error(NetworkError::Internal("Opening file failed".into())); + return; + } + } + } +} From 2f3affcfc870c8fc45b7bbe56a59d6bca2eed880 Mon Sep 17 00:00:00 2001 From: ms2300 Date: Tue, 25 Sep 2018 14:24:43 -0700 Subject: [PATCH 02/13] Blob url's changes now build and test --- components/net/blob_loader.rs | 17 ++++++------- components/net/fetch/methods.rs | 6 +---- components/net/filemanager_thread.rs | 38 +++++++++++++++------------- 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/components/net/blob_loader.rs b/components/net/blob_loader.rs index 50c5455808d..64d918224a6 100644 --- a/components/net/blob_loader.rs +++ b/components/net/blob_loader.rs @@ -13,9 +13,9 @@ use mime::{self, Mime}; use net_traits::blob_url_store::parse_blob_url; use net_traits::filemanager_thread::ReadFileProgress; use net_traits::response::{Response, ResponseBody}; -use net_traits::{http_percent_encode, NetworkError}; +use net_traits::{http_percent_encode, NetworkError, ResourceFetchTiming}; use servo_url::ServoUrl; -use std::sync::mpsc::channel; +use servo_channel::channel; // TODO: Check on GET // https://w3c.github.io/FileAPI/#requestResponseModel @@ -24,22 +24,21 @@ use std::sync::mpsc::channel; pub fn load_blob_async( url: ServoUrl, filemanager: FileManager, - response: &Response, done_chan: &mut DoneChannel -) -> Result<(), NetworkError> { +)-> Response { let (id, origin) = match parse_blob_url(&url) { Ok((id, origin)) => (id, origin), Err(()) => { - let e = format!("Invalid blob URL format {:?}", url); - return Err(NetworkError::Internal(e)); - }, + return Response::network_error(NetworkError::Internal("Invalid blob url".into())); + } }; + let mut response = Response::new(url, ResourceFetchTiming::new(request.timing_type())); let (sender, receiver) = channel(); *done_chan = Some((sender.clone(), receiver)); *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]); let check_url_validity = true; - filemanager.fetch_file(sender, id, check_url_validity, origin, response); + filemanager.fetch_file(sender, id, check_url_validity, origin, &mut response); - Ok(()) + response } diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index 67a02775369..ddd22fcda34 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -657,11 +657,7 @@ fn scheme_fetch( )); } - let mut response = Response::new(url, ResourceFetchTiming::new(request.timing_type())); - - load_blob_async(url.clone(), context.filemanager.clone(), &response, done_chan); - - response + load_blob_async(url.clone(), context.filemanager.clone(), done_chan); }, "ftp" => { diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index 0be8de815b4..6d4fdb3f2d5 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -16,6 +16,7 @@ use net_traits::filemanager_thread::{ FileManagerThreadError, ReadFileProgress, RelativePos, SelectedFile, }; use net_traits::response::{Response, ResponseBody}; +use servo_channel; use servo_config::prefs::PREFS; use std::collections::HashMap; use std::fs::File; @@ -23,7 +24,7 @@ use std::io::{Read, Seek, SeekFrom}; use std::ops::Index; use std::path::{Path, PathBuf}; use std::sync::atomic::{self, AtomicBool, AtomicUsize, Ordering}; -use std::sync::{Arc, RwLock, Mutex, mpsc}; +use std::sync::{Arc, RwLock}; use std::thread; use url::Url; use uuid::Uuid; @@ -98,15 +99,15 @@ impl FileManager { } pub fn fetch_file(&self, - sender: mpsc::Sender, + sender: servo_channel::Sender, id: Uuid, check_url_validity: bool, origin: FileOrigin, - response: &Response) { + response: &mut Response) { let store = self.store.clone(); - let mut res_body = response.body.clone(); + let mut r2 = response.clone(); thread::Builder::new().name("read file".to_owned()).spawn(move || { - store.try_fetch_file(&sender, id, check_url_validity, origin, response, res_body) + store.try_fetch_file(&sender, id, check_url_validity, origin, &mut r2) }).expect("Thread spawning failed"); } @@ -508,9 +509,9 @@ impl FileManagerStore { ) } - fn fetch_blob_buf(&self, sender: &mpsc::Sender, + fn fetch_blob_buf(&self, sender: &servo_channel::Sender, id: &Uuid, origin_in: &FileOrigin, rel_pos: RelativePos, - check_url_validity: bool, response: &Response, res_body: Arc>) -> Result<(), BlobURLStoreError> { + check_url_validity: bool, response: &mut Response) -> Result<(), BlobURLStoreError> { let mut bytes = vec![]; let file_impl = self.get_impl(id, origin_in, check_url_validity)?; match file_impl { @@ -544,7 +545,7 @@ impl FileManagerStore { bytes.extend_from_slice(&blob_buf.bytes); response.headers = headers; - *res_body.lock().unwrap() = ResponseBody::Done(bytes); + *response.body.lock().unwrap() = ResponseBody::Done(bytes); let _ = sender.send(Data::Done); Ok(()) } @@ -574,7 +575,7 @@ impl FileManagerStore { }; chunked_fetch(sender, &mut file, range.len(), opt_filename, - type_string, response, res_body, &mut bytes); + type_string, response, &mut bytes); Ok(()) } else { Err(BlobURLStoreError::InvalidEntry) @@ -584,16 +585,15 @@ impl FileManagerStore { // Next time we don't need to check validity since // we have already done that for requesting URL if necessary self.fetch_blob_buf(sender, &parent_id, origin_in, - rel_pos.slice_inner(&inner_rel_pos), false, response, res_body) + rel_pos.slice_inner(&inner_rel_pos), false, response) } } } - fn try_fetch_file(&self, sender: &mpsc::Sender, id: Uuid, check_url_validity: bool, - origin_in: FileOrigin, response: &Response, res_body: Arc>) + fn try_fetch_file(&self, sender: &servo_channel::Sender, id: Uuid, check_url_validity: bool, + origin_in: FileOrigin, response: &mut Response) -> Result<(), BlobURLStoreError> { - self.fetch_blob_buf(sender, &id, &origin_in, RelativePos::full_range(), check_url_validity, - response, res_body) + self.fetch_blob_buf(sender, &id, &origin_in, RelativePos::full_range(), check_url_validity, response) } fn dec_ref(&self, id: &Uuid, origin_in: &FileOrigin) -> Result<(), BlobURLStoreError> { @@ -764,9 +764,9 @@ fn chunked_read( } } -fn chunked_fetch(sender: &mpsc::Sender, +fn chunked_fetch(sender: &servo_channel::Sender, file: &mut File, size: usize, opt_filename: Option, - type_string: String, response: &Response, res_body: Arc>, bytes: &mut Vec) { + type_string: String, response: &mut Response, bytes: &mut Vec) { // First chunk let mut buf = vec![0; CHUNK_SIZE]; match file.read(&mut buf) { @@ -778,7 +778,9 @@ fn chunked_fetch(sender: &mpsc::Sender, size: size as u64, bytes: buf, }; + bytes.extend_from_slice(&blob_buf.bytes); + let _ = sender.send(Data::Payload(blob_buf.bytes)); } Err(_) => { @@ -792,7 +794,7 @@ fn chunked_fetch(sender: &mpsc::Sender, let mut buf = vec![0; CHUNK_SIZE]; match file.read(&mut buf) { Ok(0) => { - *res_body.lock().unwrap() = ResponseBody::Done(bytes.to_vec()); + *response.body.lock().unwrap() = ResponseBody::Done(bytes.to_vec()); let _ = sender.send(Data::Done); return; } @@ -801,7 +803,7 @@ fn chunked_fetch(sender: &mpsc::Sender, bytes.extend_from_slice(&buf); let _ = sender.send(Data::Payload(buf)); } - Err(e) => { + Err(_) => { *response = Response::network_error(NetworkError::Internal("Opening file failed".into())); return; } From 67722d194340a7f32c8bff2dc88c9698be06193c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Mon, 5 Nov 2018 18:22:04 +0100 Subject: [PATCH 03/13] Update read blob url in chunk changes to use new hyper --- components/net/filemanager_thread.rs | 42 +++++++++++++++++----------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index 6d4fdb3f2d5..9ca4f2e0ada 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -5,11 +5,13 @@ use embedder_traits::{EmbedderMsg, EmbedderProxy, FilterPattern}; use ipc_channel::ipc::{self, IpcSender}; use fetch::methods::Data; -use hyper::header::{Charset, ContentLength, ContentType, Headers}; -use hyper::header::{ContentDisposition, DispositionParam, DispositionType}; -use mime::{Attr, Mime}; +use headers_core::HeaderMapExt; +use headers_ext::{ContentLength, ContentType}; +use http::HeaderMap; +use http::header::{self, HeaderValue}; +use mime::{self, Mime}; use mime_guess::guess_mime_type_opt; -use net_traits::NetworkError; +use net_traits::{http_percent_encode, NetworkError}; use net_traits::blob_url_store::{BlobBuf, BlobURLStoreError}; use net_traits::filemanager_thread::{FileManagerResult, FileManagerThreadMsg, FileOrigin}; use net_traits::filemanager_thread::{ @@ -524,23 +526,29 @@ impl FileManagerStore { bytes: buf.bytes.index(range).to_vec(), }; - let content_type: Mime = blob_buf.type_string.parse().unwrap_or(mime!(Text / Plain)); - let charset = content_type.get_param(Attr::Charset); - let mut headers = Headers::new(); + let content_type: Mime = blob_buf.type_string.parse().unwrap_or(mime::TEXT_PLAIN); + let charset = content_type.get_param(mime::CHARSET); + let mut headers = HeaderMap::new(); if let Some(name) = blob_buf.filename { - let charset = charset.and_then(|c| c.as_str().parse().ok()); - headers.set(ContentDisposition { - disposition: DispositionType::Inline, - parameters: vec![ - DispositionParam::Filename(charset.unwrap_or(Charset::Us_Ascii), - None, name.as_bytes().to_vec()) - ] - }); + let charset = charset.map(|c| c.as_ref().into()).unwrap_or("us-ascii".to_owned()); + // TODO(eijebong): Replace this once the typed header is there + headers.insert( + header::CONTENT_DISPOSITION, + HeaderValue::from_bytes( + format!("inline; {}", + if charset.to_lowercase() == "utf-8" { + format!("filename=\"{}\"", String::from_utf8(name.as_bytes().into()).unwrap()) + } else { + format!("filename*=\"{}\"''{}", charset, http_percent_encode(name.as_bytes())) + } + ).as_bytes() + ).unwrap() + ); } - headers.set(ContentLength(blob_buf.size as u64)); - headers.set(ContentType(content_type.clone())); + headers.typed_insert(ContentLength(blob_buf.size as u64)); + headers.typed_insert(ContentType::from(content_type.clone())); bytes.extend_from_slice(&blob_buf.bytes); From 8538634210988adabd5d75c8ff28cabd59a06baa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Wed, 7 Nov 2018 15:00:49 +0100 Subject: [PATCH 04/13] Finish asynchronous blob url fetching --- components/net/blob_loader.rs | 25 ++- components/net/fetch/methods.rs | 2 +- components/net/filemanager_thread.rs | 256 ++++++++++++++------------- 3 files changed, 147 insertions(+), 136 deletions(-) diff --git a/components/net/blob_loader.rs b/components/net/blob_loader.rs index 64d918224a6..ae8f09879a4 100644 --- a/components/net/blob_loader.rs +++ b/components/net/blob_loader.rs @@ -2,20 +2,13 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use crate::fetch::methods::{Data, DoneChannel}; use crate::filemanager_thread::FileManager; -use fetch::methods::DoneChannel; -use headers_core::HeaderMapExt; -use headers_ext::{ContentLength, ContentType}; -use http::header::{self, HeaderValue}; -use http::HeaderMap; -use ipc_channel::ipc; -use mime::{self, Mime}; use net_traits::blob_url_store::parse_blob_url; -use net_traits::filemanager_thread::ReadFileProgress; use net_traits::response::{Response, ResponseBody}; -use net_traits::{http_percent_encode, NetworkError, ResourceFetchTiming}; -use servo_url::ServoUrl; +use net_traits::{NetworkError, ResourceFetchTiming}; use servo_channel::channel; +use servo_url::ServoUrl; // TODO: Check on GET // https://w3c.github.io/FileAPI/#requestResponseModel @@ -24,13 +17,13 @@ use servo_channel::channel; pub fn load_blob_async( url: ServoUrl, filemanager: FileManager, - done_chan: &mut DoneChannel -)-> Response { + done_chan: &mut DoneChannel, +) -> Response { let (id, origin) = match parse_blob_url(&url) { Ok((id, origin)) => (id, origin), Err(()) => { return Response::network_error(NetworkError::Internal("Invalid blob url".into())); - } + }, }; let mut response = Response::new(url, ResourceFetchTiming::new(request.timing_type())); @@ -38,7 +31,11 @@ pub fn load_blob_async( *done_chan = Some((sender.clone(), receiver)); *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]); let check_url_validity = true; - filemanager.fetch_file(sender, id, check_url_validity, origin, &mut response); + if let Err(err) = filemanager.fetch_file(&sender, id, check_url_validity, origin, &mut response) + { + let _ = sender.send(Data::Done); + return Response::network_error(NetworkError::Internal(err)); + }; response } diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index ddd22fcda34..21ccbad73a0 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -657,7 +657,7 @@ fn scheme_fetch( )); } - load_blob_async(url.clone(), context.filemanager.clone(), done_chan); + load_blob_async(url.clone(), context.filemanager.clone(), done_chan) }, "ftp" => { diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index 9ca4f2e0ada..660fe1d8875 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -2,22 +2,21 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use crate::fetch::methods::Data; use embedder_traits::{EmbedderMsg, EmbedderProxy, FilterPattern}; -use ipc_channel::ipc::{self, IpcSender}; -use fetch::methods::Data; -use headers_core::HeaderMapExt; -use headers_ext::{ContentLength, ContentType}; -use http::HeaderMap; +use headers_ext::{ContentLength, ContentType, HeaderMap, HeaderMapExt}; use http::header::{self, HeaderValue}; +use ipc_channel::ipc::{self, IpcSender}; use mime::{self, Mime}; use mime_guess::guess_mime_type_opt; -use net_traits::{http_percent_encode, NetworkError}; use net_traits::blob_url_store::{BlobBuf, BlobURLStoreError}; use net_traits::filemanager_thread::{FileManagerResult, FileManagerThreadMsg, FileOrigin}; use net_traits::filemanager_thread::{ FileManagerThreadError, ReadFileProgress, RelativePos, SelectedFile, }; +use net_traits::http_percent_encode; use net_traits::response::{Response, ResponseBody}; +use servo_arc::Arc as ServoArc; use servo_channel; use servo_config::prefs::PREFS; use std::collections::HashMap; @@ -26,7 +25,7 @@ use std::io::{Read, Seek, SeekFrom}; use std::ops::Index; use std::path::{Path, PathBuf}; use std::sync::atomic::{self, AtomicBool, AtomicUsize, Ordering}; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, Mutex, RwLock}; use std::thread; use url::Url; use uuid::Uuid; @@ -100,17 +99,27 @@ impl FileManager { .expect("Thread spawning failed"); } - pub fn fetch_file(&self, - sender: servo_channel::Sender, - id: Uuid, - check_url_validity: bool, - origin: FileOrigin, - response: &mut Response) { - let store = self.store.clone(); - let mut r2 = response.clone(); - thread::Builder::new().name("read file".to_owned()).spawn(move || { - store.try_fetch_file(&sender, id, check_url_validity, origin, &mut r2) - }).expect("Thread spawning failed"); + // Read a file for the Fetch implementation. + // It gets the required headers synchronously and reads the actual content + // in a separate thread. + pub fn fetch_file( + &self, + sender: &servo_channel::Sender, + id: Uuid, + check_url_validity: bool, + origin: FileOrigin, + response: &mut Response, + ) -> Result<(), String> { + self.store + .fetch_blob_buf( + sender, + &id, + &origin, + RelativePos::full_range(), + check_url_validity, + response, + ) + .map_err(|e| format!("{:?}", e)) } pub fn promote_memory( @@ -511,52 +520,36 @@ impl FileManagerStore { ) } - fn fetch_blob_buf(&self, sender: &servo_channel::Sender, - id: &Uuid, origin_in: &FileOrigin, rel_pos: RelativePos, - check_url_validity: bool, response: &mut Response) -> Result<(), BlobURLStoreError> { - let mut bytes = vec![]; + fn fetch_blob_buf( + &self, + sender: &servo_channel::Sender, + id: &Uuid, + origin_in: &FileOrigin, + rel_pos: RelativePos, + check_url_validity: bool, + response: &mut Response, + ) -> Result<(), BlobURLStoreError> { let file_impl = self.get_impl(id, origin_in, check_url_validity)?; match file_impl { FileImpl::Memory(buf) => { let range = rel_pos.to_abs_range(buf.size as usize); - let blob_buf = BlobBuf { - filename: None, - type_string: buf.type_string, - size: range.len() as u64, - bytes: buf.bytes.index(range).to_vec(), - }; + let len = range.len() as u64; - let content_type: Mime = blob_buf.type_string.parse().unwrap_or(mime::TEXT_PLAIN); - let charset = content_type.get_param(mime::CHARSET); - let mut headers = HeaderMap::new(); + set_headers( + &mut response.headers, + len, + buf.type_string.parse().unwrap_or(mime::TEXT_PLAIN), + /* filename */ None, + ); - if let Some(name) = blob_buf.filename { - let charset = charset.map(|c| c.as_ref().into()).unwrap_or("us-ascii".to_owned()); - // TODO(eijebong): Replace this once the typed header is there - headers.insert( - header::CONTENT_DISPOSITION, - HeaderValue::from_bytes( - format!("inline; {}", - if charset.to_lowercase() == "utf-8" { - format!("filename=\"{}\"", String::from_utf8(name.as_bytes().into()).unwrap()) - } else { - format!("filename*=\"{}\"''{}", charset, http_percent_encode(name.as_bytes())) - } - ).as_bytes() - ).unwrap() - ); - } + let mut bytes = vec![]; + bytes.extend_from_slice(buf.bytes.index(range)); - headers.typed_insert(ContentLength(blob_buf.size as u64)); - headers.typed_insert(ContentType::from(content_type.clone())); - - bytes.extend_from_slice(&blob_buf.bytes); - - response.headers = headers; - *response.body.lock().unwrap() = ResponseBody::Done(bytes); + let _ = sender.send(Data::Payload(bytes)); let _ = sender.send(Data::Done); + Ok(()) - } + }, FileImpl::MetaDataOnly(metadata) => { /* XXX: Snapshot state check (optional) https://w3c.github.io/FileAPI/#snapshot-state. Concretely, here we create another file, and this file might not @@ -564,46 +557,54 @@ impl FileManagerStore { create_entry is called. */ - let opt_filename = metadata.path.file_name() - .and_then(|osstr| osstr.to_str()) - .map(|s| s.to_string()); - - let mime = guess_mime_type_opt(metadata.path.clone()); - let range = rel_pos.to_abs_range(metadata.size as usize); - let mut file = File::open(&metadata.path) - .map_err(|e| BlobURLStoreError::External(e.to_string()))?; - let seeked_start = file.seek(SeekFrom::Start(range.start as u64)) - .map_err(|e| BlobURLStoreError::External(e.to_string()))?; + .map_err(|e| BlobURLStoreError::External(e.to_string()))?; - if seeked_start == (range.start as u64) { - let type_string = match mime { - Some(x) => format!("{}", x), - None => "".to_string(), - }; - - chunked_fetch(sender, &mut file, range.len(), opt_filename, - type_string, response, &mut bytes); - Ok(()) - } else { - Err(BlobURLStoreError::InvalidEntry) + let range = rel_pos.to_abs_range(metadata.size as usize); + let range_start = range.start as u64; + let seeked_start = file + .seek(SeekFrom::Start(range_start)) + .map_err(|e| BlobURLStoreError::External(e.to_string()))?; + if seeked_start != range_start { + return Err(BlobURLStoreError::InvalidEntry); } - } + + let filename = metadata + .path + .file_name() + .and_then(|osstr| osstr.to_str()) + .map(|s| s.to_string()); + + set_headers( + &mut response.headers, + metadata.size, + guess_mime_type_opt(metadata.path).unwrap_or(mime::TEXT_PLAIN), + filename, + ); + + let body = response.body.clone(); + let sender = sender.clone(); + thread::Builder::new() + .name("fetch file".to_owned()) + .spawn(move || chunked_fetch(sender, &mut file, body)) + .expect("Thread spawn failed"); + Ok(()) + }, FileImpl::Sliced(parent_id, inner_rel_pos) => { // Next time we don't need to check validity since - // we have already done that for requesting URL if necessary - self.fetch_blob_buf(sender, &parent_id, origin_in, - rel_pos.slice_inner(&inner_rel_pos), false, response) - } + // we have already done that for requesting URL if necessary. + return self.fetch_blob_buf( + sender, + &parent_id, + origin_in, + rel_pos.slice_inner(&inner_rel_pos), + false, + response, + ); + }, } } - fn try_fetch_file(&self, sender: &servo_channel::Sender, id: Uuid, check_url_validity: bool, - origin_in: FileOrigin, response: &mut Response) - -> Result<(), BlobURLStoreError> { - self.fetch_blob_buf(sender, &id, &origin_in, RelativePos::full_range(), check_url_validity, response) - } - fn dec_ref(&self, id: &Uuid, origin_in: &FileOrigin) -> Result<(), BlobURLStoreError> { let (do_remove, opt_parent_id) = match self.entries.read().unwrap().get(id) { Some(entry) => { @@ -772,49 +773,62 @@ fn chunked_read( } } -fn chunked_fetch(sender: &servo_channel::Sender, - file: &mut File, size: usize, opt_filename: Option, - type_string: String, response: &mut Response, bytes: &mut Vec) { - // First chunk - let mut buf = vec![0; CHUNK_SIZE]; - match file.read(&mut buf) { - Ok(n) => { - buf.truncate(n); - let blob_buf = BlobBuf { - filename: opt_filename, - type_string: type_string, - size: size as u64, - bytes: buf, - }; - - bytes.extend_from_slice(&blob_buf.bytes); - - let _ = sender.send(Data::Payload(blob_buf.bytes)); - } - Err(_) => { - *response = Response::network_error(NetworkError::Internal("Opening file failed".into())); - return; - } - } - - // Send the remaining chunks +fn chunked_fetch( + sender: servo_channel::Sender, + file: &mut File, + response_body: ServoArc>, +) { loop { let mut buf = vec![0; CHUNK_SIZE]; match file.read(&mut buf) { - Ok(0) => { - *response.body.lock().unwrap() = ResponseBody::Done(bytes.to_vec()); + Ok(0) | Err(_) => { + *response_body.lock().unwrap() = ResponseBody::Done(vec![]); let _ = sender.send(Data::Done); return; - } + }, Ok(n) => { buf.truncate(n); + let mut bytes = vec![]; bytes.extend_from_slice(&buf); let _ = sender.send(Data::Payload(buf)); - } - Err(_) => { - *response = Response::network_error(NetworkError::Internal("Opening file failed".into())); - return; - } + }, } } } + +fn set_headers(headers: &mut HeaderMap, content_length: u64, mime: Mime, filename: Option) { + headers.typed_insert(ContentLength(content_length)); + headers.typed_insert(ContentType::from(mime.clone())); + let name = match filename { + Some(name) => name, + None => return, + }; + let charset = mime.get_param(mime::CHARSET); + let charset = charset + .map(|c| c.as_ref().into()) + .unwrap_or("us-ascii".to_owned()); + // TODO(eijebong): Replace this once the typed header is there + // https://github.com/hyperium/headers/issues/8 + headers.insert( + header::CONTENT_DISPOSITION, + HeaderValue::from_bytes( + format!( + "inline; {}", + if charset.to_lowercase() == "utf-8" { + format!( + "filename=\"{}\"", + String::from_utf8(name.as_bytes().into()).unwrap() + ) + } else { + format!( + "filename*=\"{}\"''{}", + charset, + http_percent_encode(name.as_bytes()) + ) + } + ) + .as_bytes(), + ) + .unwrap(), + ); +} From a84442864d64242903a2b55c170b7f889ab4ab32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Sat, 10 Nov 2018 11:57:47 +0100 Subject: [PATCH 05/13] Add support fo byte range requests for blob URLs --- components/net/blob_loader.rs | 41 ------ components/net/fetch/methods.rs | 185 ++++++++++++--------------- components/net/filemanager_thread.rs | 147 +++++++++++++-------- components/net/lib.rs | 1 - 4 files changed, 179 insertions(+), 195 deletions(-) delete mode 100644 components/net/blob_loader.rs diff --git a/components/net/blob_loader.rs b/components/net/blob_loader.rs deleted file mode 100644 index ae8f09879a4..00000000000 --- a/components/net/blob_loader.rs +++ /dev/null @@ -1,41 +0,0 @@ -/* 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 https://mozilla.org/MPL/2.0/. */ - -use crate::fetch::methods::{Data, DoneChannel}; -use crate::filemanager_thread::FileManager; -use net_traits::blob_url_store::parse_blob_url; -use net_traits::response::{Response, ResponseBody}; -use net_traits::{NetworkError, ResourceFetchTiming}; -use servo_channel::channel; -use servo_url::ServoUrl; - -// TODO: Check on GET -// https://w3c.github.io/FileAPI/#requestResponseModel - -/// https://fetch.spec.whatwg.org/#concept-basic-fetch (partial) -pub fn load_blob_async( - url: ServoUrl, - filemanager: FileManager, - done_chan: &mut DoneChannel, -) -> Response { - let (id, origin) = match parse_blob_url(&url) { - Ok((id, origin)) => (id, origin), - Err(()) => { - return Response::network_error(NetworkError::Internal("Invalid blob url".into())); - }, - }; - - let mut response = Response::new(url, ResourceFetchTiming::new(request.timing_type())); - let (sender, receiver) = channel(); - *done_chan = Some((sender.clone(), receiver)); - *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]); - let check_url_validity = true; - if let Err(err) = filemanager.fetch_file(&sender, id, check_url_validity, origin, &mut response) - { - let _ = sender.send(Data::Done); - return Response::network_error(NetworkError::Internal(err)); - }; - - response -} diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index 21ccbad73a0..c5e6e45dde5 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -2,10 +2,9 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use crate::blob_loader::load_blob_async; use crate::data_loader::decode; use crate::fetch::cors_cache::CorsCache; -use crate::filemanager_thread::FileManager; +use crate::filemanager_thread::{fetch_file_in_chunks, FILE_CHUNK_SIZE, FileManager}; 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; @@ -19,28 +18,27 @@ use hyper::StatusCode; use ipc_channel::ipc::IpcReceiver; use mime::{self, Mime}; use mime_guess::guess_mime_type; +use net_traits::blob_url_store::parse_blob_url; +use net_traits::filemanager_thread::RelativePos; use net_traits::request::{CredentialsMode, Destination, Referrer, Request, RequestMode}; use net_traits::request::{Origin, ResponseTainting, Window}; use net_traits::response::{Response, ResponseBody, ResponseType}; use net_traits::{FetchTaskTarget, NetworkError, ReferrerPolicy, ResourceFetchTiming}; use servo_url::ServoUrl; use std::borrow::Cow; -use std::fs::File; -use std::io::{BufRead, BufReader, Seek, SeekFrom}; +use std::fs::{File, Metadata}; +use std::io::{BufReader, Seek, SeekFrom}; use std::mem; use std::ops::Bound; use std::str; use std::sync::atomic::Ordering; use std::sync::{Arc, Mutex}; -use std::thread; lazy_static! { static ref X_CONTENT_TYPE_OPTIONS: HeaderName = HeaderName::from_static("x-content-type-options"); } -const FILE_CHUNK_SIZE: usize = 32768; //32 KB - pub type Target<'a> = &'a mut (dyn FetchTaskTarget + Send); pub enum Data { @@ -492,6 +490,36 @@ fn wait_for_response(response: &mut Response, target: Target, done_chan: &mut Do } } +/// Get the range bounds if the `Range` header is present. +fn get_range_bounds(range: Option, metadata: Option) -> RelativePos { + if let Some(ref range) = range + { + let (start, end) = match range + .iter() + .collect::, Bound)>>() + .first() + { + Some(&(Bound::Included(start), Bound::Unbounded)) => (start, None), + Some(&(Bound::Included(start), Bound::Included(end))) => { + // `end` should be less or equal to `start`. + (start, Some(i64::max(start as i64, end as i64))) + }, + Some(&(Bound::Unbounded, Bound::Included(offset))) => { + if let Some(metadata) = metadata { + // `offset` cannot be bigger than the file size. + (metadata.len() - u64::min(metadata.len(), offset), None) + } else { + (0, None) + } + }, + _ => (0, None), + }; + RelativePos::from_opts(Some(start as i64), end) + } else { + RelativePos::from_opts(Some(0), None) + } +} + /// [Scheme fetch](https://fetch.spec.whatwg.org#scheme-fetch) fn scheme_fetch( request: &mut Request, @@ -537,106 +565,35 @@ fn scheme_fetch( } if let Ok(file_path) = url.to_file_path() { if let Ok(file) = File::open(file_path.clone()) { - let mime = guess_mime_type(file_path); + let mut response = Response::new(url, ResourceFetchTiming::new(request.timing_type())); - let mut response = - Response::new(url, ResourceFetchTiming::new(request.timing_type())); + // Get range bounds (if any) and try to seek to the requested offset. + // If seeking fails, bail out with a NetworkError. + let range = get_range_bounds(request.headers.typed_get::(), file.metadata().ok()); + let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file); + if reader.seek(SeekFrom::Start(range.start as u64)).is_err() { + *response.body.lock().unwrap() = ResponseBody::Done(vec![]); + return Response::network_error(NetworkError::Internal( + "Unexpected method for blob".into(), + )); + } + + // Set Content-Type header. + let mime = guess_mime_type(file_path); response.headers.typed_insert(ContentType::from(mime)); - let (done_sender, done_receiver) = unbounded(); + // Setup channel to receive cross-thread messages about the file fetch + // operation. + let (done_sender, done_receiver) = channel(); *done_chan = Some((done_sender.clone(), done_receiver)); *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]); - let res_body = response.body.clone(); + fetch_file_in_chunks(done_sender, + reader, + response.body.clone(), + context.cancellation_listener.clone(), + range); - let cancellation_listener = context.cancellation_listener.clone(); - - let (start, end) = if let Some(ref range) = request.headers.typed_get::() - { - match range - .iter() - .collect::, Bound)>>() - .first() - { - Some(&(Bound::Included(start), Bound::Unbounded)) => (start, None), - Some(&(Bound::Included(start), Bound::Included(end))) => { - // `end` should be less or equal to `start`. - (start, Some(u64::max(start, end))) - }, - Some(&(Bound::Unbounded, Bound::Included(offset))) => { - if let Ok(metadata) = file.metadata() { - // `offset` cannot be bigger than the file size. - (metadata.len() - u64::min(metadata.len(), offset), None) - } else { - (0, None) - } - }, - _ => (0, None), - } - } else { - (0, None) - }; - - thread::Builder::new() - .name("fetch file worker thread".to_string()) - .spawn(move || { - let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file); - if reader.seek(SeekFrom::Start(start)).is_err() { - warn!("Fetch - could not seek to {:?}", start); - } - - loop { - if cancellation_listener.lock().unwrap().cancelled() { - *res_body.lock().unwrap() = ResponseBody::Done(vec![]); - let _ = done_sender.send(Data::Cancelled); - return; - } - let length = { - let buffer = reader.fill_buf().unwrap().to_vec(); - let mut buffer_len = buffer.len(); - if let ResponseBody::Receiving(ref mut body) = - *res_body.lock().unwrap() - { - let offset = usize::min( - { - if let Some(end) = end { - let remaining_bytes = - end as usize - start as usize - body.len(); - if remaining_bytes <= FILE_CHUNK_SIZE { - // This is the last chunk so we set buffer - // len to 0 to break the reading loop. - buffer_len = 0; - remaining_bytes - } else { - FILE_CHUNK_SIZE - } - } else { - FILE_CHUNK_SIZE - } - }, - buffer.len(), - ); - body.extend_from_slice(&buffer[0..offset]); - let _ = done_sender.send(Data::Payload(buffer)); - } - buffer_len - }; - if length == 0 { - let mut body = res_body.lock().unwrap(); - let completed_body = match *body { - ResponseBody::Receiving(ref mut body) => { - mem::replace(body, vec![]) - }, - _ => vec![], - }; - *body = ResponseBody::Done(completed_body); - let _ = done_sender.send(Data::Done); - break; - } - reader.consume(length); - } - }) - .expect("Failed to create fetch file worker thread"); response } else { Response::network_error(NetworkError::Internal("Opening file failed".into())) @@ -657,7 +614,33 @@ fn scheme_fetch( )); } - load_blob_async(url.clone(), context.filemanager.clone(), done_chan) + let range = get_range_bounds(request.headers.typed_get::(), None); + + let (id, origin) = match parse_blob_url(&url) { + Ok((id, origin)) => (id, origin), + Err(()) => { + return Response::network_error(NetworkError::Internal("Invalid blob url".into())); + }, + }; + + let mut response = Response::new(url); + let (done_sender, done_receiver) = channel(); + *done_chan = Some((done_sender.clone(), done_receiver)); + *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]); + let check_url_validity = true; + if let Err(err) = context.filemanager.fetch_file(&done_sender, + context.cancellation_listener.clone(), + id, + check_url_validity, + origin, + &mut response, + range) + { + let _ = done_sender.send(Data::Done); + return Response::network_error(NetworkError::Internal(err)); + }; + + response }, "ftp" => { diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index 660fe1d8875..032199d3f43 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use crate::fetch::methods::Data; +use crate::fetch::methods::{CancellationListener, Data}; use embedder_traits::{EmbedderMsg, EmbedderProxy, FilterPattern}; use headers_ext::{ContentLength, ContentType, HeaderMap, HeaderMapExt}; use http::header::{self, HeaderValue}; @@ -21,7 +21,8 @@ use servo_channel; use servo_config::prefs::PREFS; use std::collections::HashMap; use std::fs::File; -use std::io::{Read, Seek, SeekFrom}; +use std::io::{BufRead, BufReader, Read, Seek, SeekFrom}; +use std::mem; use std::ops::Index; use std::path::{Path, PathBuf}; use std::sync::atomic::{self, AtomicBool, AtomicUsize, Ordering}; @@ -30,6 +31,8 @@ use std::thread; use url::Url; use uuid::Uuid; +pub const FILE_CHUNK_SIZE: usize = 32768; //32 KB + /// FileManagerStore's entry struct FileStoreEntry { /// Origin of the entry's "creator" @@ -104,18 +107,21 @@ impl FileManager { // in a separate thread. pub fn fetch_file( &self, - sender: &servo_channel::Sender, + done_sender: &servo_channel::Sender, + cancellation_listener: Arc>, id: Uuid, check_url_validity: bool, origin: FileOrigin, response: &mut Response, + range: RelativePos ) -> Result<(), String> { self.store .fetch_blob_buf( - sender, + done_sender, + cancellation_listener, &id, &origin, - RelativePos::full_range(), + range, check_url_validity, response, ) @@ -483,7 +489,7 @@ impl FileManagerStore { None => "".to_string(), }; - chunked_read(sender, &mut file, range.len(), opt_filename, type_string); + read_file_in_chunks(sender, &mut file, range.len(), opt_filename, type_string); Ok(()) } else { Err(BlobURLStoreError::InvalidEntry) @@ -522,17 +528,18 @@ impl FileManagerStore { fn fetch_blob_buf( &self, - sender: &servo_channel::Sender, + done_sender: &servo_channel::Sender, + cancellation_listener: Arc>, id: &Uuid, origin_in: &FileOrigin, - rel_pos: RelativePos, + range: RelativePos, check_url_validity: bool, response: &mut Response, ) -> Result<(), BlobURLStoreError> { let file_impl = self.get_impl(id, origin_in, check_url_validity)?; match file_impl { FileImpl::Memory(buf) => { - let range = rel_pos.to_abs_range(buf.size as usize); + let range = range.to_abs_range(buf.size as usize); let len = range.len() as u64; set_headers( @@ -545,8 +552,8 @@ impl FileManagerStore { let mut bytes = vec![]; bytes.extend_from_slice(buf.bytes.index(range)); - let _ = sender.send(Data::Payload(bytes)); - let _ = sender.send(Data::Done); + let _ = done_sender.send(Data::Payload(bytes)); + let _ = done_sender.send(Data::Done); Ok(()) }, @@ -557,16 +564,12 @@ impl FileManagerStore { create_entry is called. */ - let mut file = File::open(&metadata.path) + let file = File::open(&metadata.path) .map_err(|e| BlobURLStoreError::External(e.to_string()))?; - let range = rel_pos.to_abs_range(metadata.size as usize); - let range_start = range.start as u64; - let seeked_start = file - .seek(SeekFrom::Start(range_start)) - .map_err(|e| BlobURLStoreError::External(e.to_string()))?; - if seeked_start != range_start { - return Err(BlobURLStoreError::InvalidEntry); + let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file); + if reader.seek(SeekFrom::Start(range.start as u64)).is_err() { + return Err(BlobURLStoreError::External("Unexpected method for blob".into())); } let filename = metadata @@ -582,22 +585,23 @@ impl FileManagerStore { filename, ); - let body = response.body.clone(); - let sender = sender.clone(); - thread::Builder::new() - .name("fetch file".to_owned()) - .spawn(move || chunked_fetch(sender, &mut file, body)) - .expect("Thread spawn failed"); + fetch_file_in_chunks(done_sender.clone(), + reader, + response.body.clone(), + cancellation_listener, + range); + Ok(()) }, FileImpl::Sliced(parent_id, inner_rel_pos) => { // Next time we don't need to check validity since // we have already done that for requesting URL if necessary. return self.fetch_blob_buf( - sender, + done_sender, + cancellation_listener, &parent_id, origin_in, - rel_pos.slice_inner(&inner_rel_pos), + range.slice_inner(&inner_rel_pos), false, response, ); @@ -725,9 +729,7 @@ fn select_files_pref_enabled() -> bool { .unwrap_or(false) } -const CHUNK_SIZE: usize = 8192; - -fn chunked_read( +fn read_file_in_chunks( sender: &IpcSender>, file: &mut File, size: usize, @@ -735,7 +737,7 @@ fn chunked_read( type_string: String, ) { // First chunk - let mut buf = vec![0; CHUNK_SIZE]; + let mut buf = vec![0; FILE_CHUNK_SIZE]; match file.read(&mut buf) { Ok(n) => { buf.truncate(n); @@ -755,7 +757,7 @@ fn chunked_read( // Send the remaining chunks loop { - let mut buf = vec![0; CHUNK_SIZE]; + let mut buf = vec![0; FILE_CHUNK_SIZE]; match file.read(&mut buf) { Ok(0) => { let _ = sender.send(Ok(ReadFileProgress::EOF)); @@ -773,27 +775,68 @@ fn chunked_read( } } -fn chunked_fetch( - sender: servo_channel::Sender, - file: &mut File, - response_body: ServoArc>, +pub fn fetch_file_in_chunks( + done_sender: servo_channel::Sender, + mut reader: BufReader, + res_body: ServoArc>, + cancellation_listener: Arc>, + range: RelativePos, ) { - loop { - let mut buf = vec![0; CHUNK_SIZE]; - match file.read(&mut buf) { - Ok(0) | Err(_) => { - *response_body.lock().unwrap() = ResponseBody::Done(vec![]); - let _ = sender.send(Data::Done); - return; - }, - Ok(n) => { - buf.truncate(n); - let mut bytes = vec![]; - bytes.extend_from_slice(&buf); - let _ = sender.send(Data::Payload(buf)); - }, - } - } + thread::Builder::new() + .name("fetch file worker thread".to_string()) + .spawn(move || { + loop { + if cancellation_listener.lock().unwrap().cancelled() { + *res_body.lock().unwrap() = ResponseBody::Done(vec![]); + let _ = done_sender.send(Data::Cancelled); + return; + } + let length = { + let buffer = reader.fill_buf().unwrap().to_vec(); + let mut buffer_len = buffer.len(); + if let ResponseBody::Receiving(ref mut body) = + *res_body.lock().unwrap() + { + let offset = usize::min( + { + if let Some(end) = range.end { + let remaining_bytes = + end as usize - range.start as usize - body.len(); + if remaining_bytes <= FILE_CHUNK_SIZE { + // This is the last chunk so we set buffer + // len to 0 to break the reading loop. + buffer_len = 0; + remaining_bytes + } else { + FILE_CHUNK_SIZE + } + } else { + FILE_CHUNK_SIZE + } + }, + buffer.len(), + ); + body.extend_from_slice(&buffer[0..offset]); + let _ = done_sender.send(Data::Payload(buffer)); + } + buffer_len + }; + if length == 0 { + let mut body = res_body.lock().unwrap(); + let completed_body = match *body { + ResponseBody::Receiving(ref mut body) => { + mem::replace(body, vec![]) + }, + _ => vec![], + }; + *body = ResponseBody::Done(completed_body); + let _ = done_sender.send(Data::Done); + break; + } + reader.consume(length); + } + }) + .expect("Failed to create fetch file worker thread"); } fn set_headers(headers: &mut HeaderMap, content_length: u64, mime: Mime, filename: Option) { diff --git a/components/net/lib.rs b/components/net/lib.rs index 04d41dd8d30..d5943fa380c 100644 --- a/components/net/lib.rs +++ b/components/net/lib.rs @@ -17,7 +17,6 @@ extern crate profile_traits; #[macro_use] extern crate serde; -mod blob_loader; pub mod connector; pub mod cookie; pub mod cookie_storage; From 25bb740e0d55f594a2260329de0284c643c45570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Mon, 12 Nov 2018 15:07:04 +0100 Subject: [PATCH 06/13] Test range request to blob URLs --- tests/wpt/mozilla/meta/MANIFEST.json | 14 ++++- .../tests/mozilla/range_request_blob_url.html | 63 +++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 tests/wpt/mozilla/tests/mozilla/range_request_blob_url.html diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index eddddddeb97..d9fa6745d77 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -14269,7 +14269,13 @@ "/_mozilla/mozilla/range_deleteContents.html", {} ] - ], + ], + "mozilla/range_request_blob_url.html": [ + [ + "/_mozilla/mozilla/range_request_blob_url.html", + {} + ] + ], "mozilla/range_request_file_url.html": [ [ "/_mozilla/mozilla/range_request_file_url.html", @@ -27238,7 +27244,11 @@ "mozilla/range_deleteContents.html": [ "8de03455bcb0d18258f76af20f58c14868fe1c21", "testharness" - ], + ], + "mozilla/range_request_blob_url.html": [ + "4e8b55d0b836dda9de14faead2ee1d6da354e8a4", + "testharness" + ], "mozilla/range_request_file_url.html": [ "65fe13fe93d97cebc2846ff7d7deab3eb84c1787", "testharness" diff --git a/tests/wpt/mozilla/tests/mozilla/range_request_blob_url.html b/tests/wpt/mozilla/tests/mozilla/range_request_blob_url.html new file mode 100644 index 00000000000..b41f0b5382e --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/range_request_blob_url.html @@ -0,0 +1,63 @@ + + + + + + + + + From 526f6fcadddcc06cb45484bb0855aefb6560db97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Mon, 12 Nov 2018 18:08:35 +0100 Subject: [PATCH 07/13] Allow range requests to blob URLs with negative offsets --- components/net/fetch/methods.rs | 89 ++++++++++++++++++---------- components/net/filemanager_thread.rs | 34 ++++++----- tests/wpt/mozilla/meta/MANIFEST.json | 2 +- 3 files changed, 78 insertions(+), 47 deletions(-) diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index c5e6e45dde5..fe6400822ed 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -4,7 +4,7 @@ use crate::data_loader::decode; use crate::fetch::cors_cache::CorsCache; -use crate::filemanager_thread::{fetch_file_in_chunks, FILE_CHUNK_SIZE, FileManager}; +use crate::filemanager_thread::{fetch_file_in_chunks, FileManager, FILE_CHUNK_SIZE}; 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; @@ -26,7 +26,7 @@ use net_traits::response::{Response, ResponseBody, ResponseType}; use net_traits::{FetchTaskTarget, NetworkError, ReferrerPolicy, ResourceFetchTiming}; use servo_url::ServoUrl; use std::borrow::Cow; -use std::fs::{File, Metadata}; +use std::fs::File; use std::io::{BufReader, Seek, SeekFrom}; use std::mem; use std::ops::Bound; @@ -490,10 +490,34 @@ fn wait_for_response(response: &mut Response, target: Target, done_chan: &mut Do } } +/// Range header start and end values. +pub enum RangeRequestBounds { + /// The range bounds are known and set to final values. + Final(RelativePos), + /// We need extra information to set the range bounds. + /// i.e. buffer or file size. + Pending(u64), +} + +impl RangeRequestBounds { + pub fn get_final(&self, len: Option) -> RelativePos { + match self { + RangeRequestBounds::Final(pos) => pos.clone(), + RangeRequestBounds::Pending(offset) => RelativePos::from_opts( + if let Some(len) = len { + Some((len - u64::min(len, *offset)) as i64) + } else { + Some(0) + }, + None, + ), + } + } +} + /// Get the range bounds if the `Range` header is present. -fn get_range_bounds(range: Option, metadata: Option) -> RelativePos { - if let Some(ref range) = range - { +fn get_range_request_bounds(range: Option) -> RangeRequestBounds { + if let Some(ref range) = range { let (start, end) = match range .iter() .collect::, Bound)>>() @@ -505,18 +529,13 @@ fn get_range_bounds(range: Option, metadata: Option) -> Relativ (start, Some(i64::max(start as i64, end as i64))) }, Some(&(Bound::Unbounded, Bound::Included(offset))) => { - if let Some(metadata) = metadata { - // `offset` cannot be bigger than the file size. - (metadata.len() - u64::min(metadata.len(), offset), None) - } else { - (0, None) - } + return RangeRequestBounds::Pending(offset); }, _ => (0, None), }; - RelativePos::from_opts(Some(start as i64), end) + RangeRequestBounds::Final(RelativePos::from_opts(Some(start as i64), end)) } else { - RelativePos::from_opts(Some(0), None) + RangeRequestBounds::Final(RelativePos::from_opts(Some(0), None)) } } @@ -569,12 +588,17 @@ fn scheme_fetch( // Get range bounds (if any) and try to seek to the requested offset. // If seeking fails, bail out with a NetworkError. - let range = get_range_bounds(request.headers.typed_get::(), file.metadata().ok()); + let file_size = match file.metadata() { + Ok(metadata) => Some(metadata.len()), + Err(_) => None, + }; + let range = get_range_request_bounds(request.headers.typed_get::()); + let range = range.get_final(file_size); let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file); if reader.seek(SeekFrom::Start(range.start as u64)).is_err() { *response.body.lock().unwrap() = ResponseBody::Done(vec![]); return Response::network_error(NetworkError::Internal( - "Unexpected method for blob".into(), + "Unexpected method for file".into(), )); } @@ -588,11 +612,13 @@ fn scheme_fetch( *done_chan = Some((done_sender.clone(), done_receiver)); *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]); - fetch_file_in_chunks(done_sender, - reader, - response.body.clone(), - context.cancellation_listener.clone(), - range); + fetch_file_in_chunks( + done_sender, + reader, + response.body.clone(), + context.cancellation_listener.clone(), + range, + ); response } else { @@ -614,12 +640,14 @@ fn scheme_fetch( )); } - let range = get_range_bounds(request.headers.typed_get::(), None); + let range = get_range_request_bounds(request.headers.typed_get::()); let (id, origin) = match parse_blob_url(&url) { Ok((id, origin)) => (id, origin), Err(()) => { - return Response::network_error(NetworkError::Internal("Invalid blob url".into())); + return Response::network_error(NetworkError::Internal( + "Invalid blob url".into(), + )); }, }; @@ -628,14 +656,15 @@ fn scheme_fetch( *done_chan = Some((done_sender.clone(), done_receiver)); *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]); let check_url_validity = true; - if let Err(err) = context.filemanager.fetch_file(&done_sender, - context.cancellation_listener.clone(), - id, - check_url_validity, - origin, - &mut response, - range) - { + if let Err(err) = context.filemanager.fetch_file( + &done_sender, + context.cancellation_listener.clone(), + id, + check_url_validity, + origin, + &mut response, + range, + ) { let _ = done_sender.send(Data::Done); return Response::network_error(NetworkError::Internal(err)); }; diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index 032199d3f43..f70d774e95e 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use crate::fetch::methods::{CancellationListener, Data}; +use crate::fetch::methods::{CancellationListener, Data, RangeRequestBounds}; use embedder_traits::{EmbedderMsg, EmbedderProxy, FilterPattern}; use headers_ext::{ContentLength, ContentType, HeaderMap, HeaderMapExt}; use http::header::{self, HeaderValue}; @@ -113,7 +113,7 @@ impl FileManager { check_url_validity: bool, origin: FileOrigin, response: &mut Response, - range: RelativePos + range: RangeRequestBounds, ) -> Result<(), String> { self.store .fetch_blob_buf( @@ -532,13 +532,14 @@ impl FileManagerStore { cancellation_listener: Arc>, id: &Uuid, origin_in: &FileOrigin, - range: RelativePos, + range: RangeRequestBounds, check_url_validity: bool, response: &mut Response, ) -> Result<(), BlobURLStoreError> { let file_impl = self.get_impl(id, origin_in, check_url_validity)?; match file_impl { FileImpl::Memory(buf) => { + let range = range.get_final(Some(buf.size)); let range = range.to_abs_range(buf.size as usize); let len = range.len() as u64; @@ -567,9 +568,12 @@ impl FileManagerStore { let file = File::open(&metadata.path) .map_err(|e| BlobURLStoreError::External(e.to_string()))?; + let range = range.get_final(Some(metadata.size)); let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file); if reader.seek(SeekFrom::Start(range.start as u64)).is_err() { - return Err(BlobURLStoreError::External("Unexpected method for blob".into())); + return Err(BlobURLStoreError::External( + "Unexpected method for blob".into(), + )); } let filename = metadata @@ -585,11 +589,13 @@ impl FileManagerStore { filename, ); - fetch_file_in_chunks(done_sender.clone(), - reader, - response.body.clone(), - cancellation_listener, - range); + fetch_file_in_chunks( + done_sender.clone(), + reader, + response.body.clone(), + cancellation_listener, + range, + ); Ok(()) }, @@ -601,7 +607,7 @@ impl FileManagerStore { cancellation_listener, &parent_id, origin_in, - range.slice_inner(&inner_rel_pos), + RangeRequestBounds::Final(range.get_final(None).slice_inner(&inner_rel_pos)), false, response, ); @@ -794,9 +800,7 @@ pub fn fetch_file_in_chunks( let length = { let buffer = reader.fill_buf().unwrap().to_vec(); let mut buffer_len = buffer.len(); - if let ResponseBody::Receiving(ref mut body) = - *res_body.lock().unwrap() - { + if let ResponseBody::Receiving(ref mut body) = *res_body.lock().unwrap() { let offset = usize::min( { if let Some(end) = range.end { @@ -824,9 +828,7 @@ pub fn fetch_file_in_chunks( if length == 0 { let mut body = res_body.lock().unwrap(); let completed_body = match *body { - ResponseBody::Receiving(ref mut body) => { - mem::replace(body, vec![]) - }, + ResponseBody::Receiving(ref mut body) => mem::replace(body, vec![]), _ => vec![], }; *body = ResponseBody::Done(completed_body); diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index d9fa6745d77..730701a7e78 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -27246,7 +27246,7 @@ "testharness" ], "mozilla/range_request_blob_url.html": [ - "4e8b55d0b836dda9de14faead2ee1d6da354e8a4", + "b41f0b5382e3994db35f38b2c358a41b75551fe2", "testharness" ], "mozilla/range_request_file_url.html": [ From ff1e8aaa081ef917ecf8912ddd26b60040ff42fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Tue, 13 Nov 2018 08:55:10 +0100 Subject: [PATCH 08/13] Adapt fetch blob test to new way of fetching in chunks --- components/net/tests/fetch.rs | 42 ++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/components/net/tests/fetch.rs b/components/net/tests/fetch.rs index 51f25059ad1..9fc8c351f83 100644 --- a/components/net/tests/fetch.rs +++ b/components/net/tests/fetch.rs @@ -29,15 +29,16 @@ use mime::{self, Mime}; use msg::constellation_msg::TEST_PIPELINE_ID; use net::connector::create_ssl_connector_builder; use net::fetch::cors_cache::CorsCache; -use net::fetch::methods::{CancellationListener, FetchContext}; +use net::fetch::methods::{self, CancellationListener, FetchContext}; use net::filemanager_thread::FileManager; use net::hsts::HstsEntry; use net::test::HttpState; use net_traits::request::{Destination, Origin, RedirectMode, Referrer, Request, RequestMode}; use net_traits::response::{CacheState, Response, ResponseBody, ResponseType}; use net_traits::{ - IncludeSubdomains, NetworkError, ReferrerPolicy, ResourceFetchTiming, ResourceTimingType, + FetchTaskTarget, IncludeSubdomains, NetworkError, ReferrerPolicy, ResourceFetchTiming, ResourceTimingType, }; +use servo_channel::{channel, Sender}; use servo_url::{ImmutableOrigin, ServoUrl}; use std::fs::File; use std::io::Read; @@ -127,7 +128,27 @@ fn test_fetch_blob() { use ipc_channel::ipc; use net_traits::blob_url_store::BlobBuf; - let mut context = new_fetch_context(None, None); + struct FetchResponseCollector { + sender: Sender, + buffer: Vec, + expected: Vec, + } + + impl FetchTaskTarget for FetchResponseCollector { + fn process_request_body(&mut self, _: &Request) {} + fn process_request_eof(&mut self, _: &Request) {} + fn process_response(&mut self, _: &Response) {} + fn process_response_chunk(&mut self, chunk: Vec) { + self.buffer.extend_from_slice(chunk.as_slice()); + } + /// Fired when the response is fully fetched + fn process_response_eof(&mut self, response: &Response) { + assert_eq!(self.buffer, self.expected); + let _ = self.sender.send(response.clone()); + } + } + + let context = new_fetch_context(None, None); let bytes = b"content"; let blob_buf = BlobBuf { @@ -147,7 +168,18 @@ fn test_fetch_blob() { let url = ServoUrl::parse(&format!("blob:{}{}", origin.as_str(), id.to_simple())).unwrap(); let mut request = Request::new(url, Some(Origin::Origin(origin.origin())), None); - let fetch_response = fetch_with_context(&mut request, &mut context); + + let (sender, receiver) = channel(); + + let mut target = FetchResponseCollector { + sender, + buffer: vec![], + expected: bytes.to_vec(), + }; + + methods::fetch(&mut request, &mut target, &context); + + let fetch_response = receiver.recv().unwrap(); assert!(!fetch_response.is_network_error()); @@ -165,7 +197,7 @@ fn test_fetch_blob() { assert_eq!( *fetch_response.body.lock().unwrap(), - ResponseBody::Done(bytes.to_vec()) + ResponseBody::Receiving(vec![]) ); } From b96e5681aaabf7b49411aa3c1b56f86c7e8570e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Tue, 13 Nov 2018 16:57:41 +0100 Subject: [PATCH 09/13] Do not set Receiving body to Done when it's not needed --- components/net/fetch/methods.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index fe6400822ed..a15045bef4b 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -596,7 +596,6 @@ fn scheme_fetch( let range = range.get_final(file_size); let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file); if reader.seek(SeekFrom::Start(range.start as u64)).is_err() { - *response.body.lock().unwrap() = ResponseBody::Done(vec![]); return Response::network_error(NetworkError::Internal( "Unexpected method for file".into(), )); From 79d27cb7ca0267092496dbbef24844ca5ab14fb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Thu, 15 Nov 2018 06:45:13 +0100 Subject: [PATCH 10/13] Set response status for range requests to file and blob urls --- components/net/fetch/methods.rs | 70 ++++++++++++++++--- components/net/filemanager_thread.rs | 42 +++++++---- components/net_traits/blob_url_store.rs | 4 +- tests/wpt/mozilla/meta/MANIFEST.json | 2 +- .../tests/mozilla/range_request_blob_url.html | 15 ++-- 5 files changed, 96 insertions(+), 37 deletions(-) diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index a15045bef4b..19736de1136 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -18,7 +18,7 @@ use hyper::StatusCode; use ipc_channel::ipc::IpcReceiver; use mime::{self, Mime}; use mime_guess::guess_mime_type; -use net_traits::blob_url_store::parse_blob_url; +use net_traits::blob_url_store::{parse_blob_url, BlobURLStoreError}; use net_traits::filemanager_thread::RelativePos; use net_traits::request::{CredentialsMode, Destination, Referrer, Request, RequestMode}; use net_traits::request::{Origin, ResponseTainting, Window}; @@ -500,17 +500,24 @@ pub enum RangeRequestBounds { } impl RangeRequestBounds { - pub fn get_final(&self, len: Option) -> RelativePos { + pub fn get_final(&self, len: Option) -> Result { match self { - RangeRequestBounds::Final(pos) => pos.clone(), - RangeRequestBounds::Pending(offset) => RelativePos::from_opts( + RangeRequestBounds::Final(pos) => { + if let Some(len) = len { + if pos.start <= len as i64 { + return Ok(pos.clone()); + } + } + Err(()) + }, + RangeRequestBounds::Pending(offset) => Ok(RelativePos::from_opts( if let Some(len) = len { Some((len - u64::min(len, *offset)) as i64) } else { Some(0) }, None, - ), + )), } } } @@ -539,6 +546,18 @@ fn get_range_request_bounds(range: Option) -> RangeRequestBounds { } } +fn partial_content(response: &mut Response) { + let reason = "Partial Content".to_owned(); + response.status = Some((StatusCode::PARTIAL_CONTENT, reason.clone())); + response.raw_status = Some((StatusCode::PARTIAL_CONTENT.as_u16(), reason.into())); +} + +fn range_not_satisfiable_error(response: &mut Response) { + let reason = "Range Not Satisfiable".to_owned(); + response.status = Some((StatusCode::RANGE_NOT_SATISFIABLE, reason.clone())); + response.raw_status = Some((StatusCode::RANGE_NOT_SATISFIABLE.as_u16(), reason.into())); +} + /// [Scheme fetch](https://fetch.spec.whatwg.org#scheme-fetch) fn scheme_fetch( request: &mut Request, @@ -584,16 +603,24 @@ fn scheme_fetch( } if let Ok(file_path) = url.to_file_path() { if let Ok(file) = File::open(file_path.clone()) { - let mut response = Response::new(url, ResourceFetchTiming::new(request.timing_type())); - // Get range bounds (if any) and try to seek to the requested offset. // If seeking fails, bail out with a NetworkError. let file_size = match file.metadata() { Ok(metadata) => Some(metadata.len()), Err(_) => None, }; - let range = get_range_request_bounds(request.headers.typed_get::()); - let range = range.get_final(file_size); + + let mut response = Response::new(url, ResourceFetchTiming::new(request.timing_type())); + + let range_header = request.headers.typed_get::(); + let is_range_request = range_header.is_some(); + let range = match get_range_request_bounds(range_header).get_final(file_size) { + Ok(range) => range, + Err(_) => { + range_not_satisfiable_error(&mut response); + return response; + }, + }; let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file); if reader.seek(SeekFrom::Start(range.start as u64)).is_err() { return Response::network_error(NetworkError::Internal( @@ -601,6 +628,12 @@ fn scheme_fetch( )); } + // Set response status to 206 if Range header is present. + // At this point we should have already validated the header. + if is_range_request { + partial_content(&mut response); + } + // Set Content-Type header. let mime = guess_mime_type(file_path); response.headers.typed_insert(ContentType::from(mime)); @@ -631,7 +664,7 @@ fn scheme_fetch( }, "blob" => { - println!("Loading blob {}", url.as_str()); + debug!("Loading blob {}", url.as_str()); // Step 2. if request.method != Method::GET { return Response::network_error(NetworkError::Internal( @@ -639,7 +672,11 @@ fn scheme_fetch( )); } - let range = get_range_request_bounds(request.headers.typed_get::()); + let range_header = request.headers.typed_get::(); + let is_range_request = range_header.is_some(); + // We will get a final version of this range once we have + // the length of the data backing the blob. + let range = get_range_request_bounds(range_header); let (id, origin) = match parse_blob_url(&url) { Ok((id, origin)) => (id, origin), @@ -651,6 +688,10 @@ fn scheme_fetch( }; let mut response = Response::new(url); + if is_range_request { + partial_content(&mut response); + } + let (done_sender, done_receiver) = channel(); *done_chan = Some((done_sender.clone(), done_receiver)); *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]); @@ -665,6 +706,13 @@ fn scheme_fetch( range, ) { let _ = done_sender.send(Data::Done); + let err = match err { + BlobURLStoreError::InvalidRange => { + range_not_satisfiable_error(&mut response); + return response; + }, + _ => format!("{:?}", err), + }; return Response::network_error(NetworkError::Internal(err)); }; diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index f70d774e95e..3a639cbb741 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -114,18 +114,16 @@ impl FileManager { origin: FileOrigin, response: &mut Response, range: RangeRequestBounds, - ) -> Result<(), String> { - self.store - .fetch_blob_buf( - done_sender, - cancellation_listener, - &id, - &origin, - range, - check_url_validity, - response, - ) - .map_err(|e| format!("{:?}", e)) + ) -> Result<(), BlobURLStoreError> { + self.store.fetch_blob_buf( + done_sender, + cancellation_listener, + &id, + &origin, + range, + check_url_validity, + response, + ) } pub fn promote_memory( @@ -539,7 +537,13 @@ impl FileManagerStore { let file_impl = self.get_impl(id, origin_in, check_url_validity)?; match file_impl { FileImpl::Memory(buf) => { - let range = range.get_final(Some(buf.size)); + let range = match range.get_final(Some(buf.size)) { + Ok(range) => range, + Err(_) => { + return Err(BlobURLStoreError::InvalidRange); + }, + }; + let range = range.to_abs_range(buf.size as usize); let len = range.len() as u64; @@ -568,7 +572,13 @@ impl FileManagerStore { let file = File::open(&metadata.path) .map_err(|e| BlobURLStoreError::External(e.to_string()))?; - let range = range.get_final(Some(metadata.size)); + let range = match range.get_final(Some(metadata.size)) { + Ok(range) => range, + Err(_) => { + return Err(BlobURLStoreError::InvalidRange); + }, + }; + let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file); if reader.seek(SeekFrom::Start(range.start as u64)).is_err() { return Err(BlobURLStoreError::External( @@ -607,7 +617,9 @@ impl FileManagerStore { cancellation_listener, &parent_id, origin_in, - RangeRequestBounds::Final(range.get_final(None).slice_inner(&inner_rel_pos)), + RangeRequestBounds::Final( + RelativePos::full_range().slice_inner(&inner_rel_pos), + ), false, response, ); diff --git a/components/net_traits/blob_url_store.rs b/components/net_traits/blob_url_store.rs index a429c6880bc..c9bb670ee29 100644 --- a/components/net_traits/blob_url_store.rs +++ b/components/net_traits/blob_url_store.rs @@ -9,7 +9,7 @@ use url::Url; use uuid::Uuid; /// Errors returned to Blob URL Store request -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub enum BlobURLStoreError { /// Invalid File UUID InvalidFileID, @@ -17,6 +17,8 @@ pub enum BlobURLStoreError { InvalidOrigin, /// Invalid entry content InvalidEntry, + /// Invalid range + InvalidRange, /// External error, from like file system, I/O etc. External(String), } diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 730701a7e78..5233572da7d 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -27246,7 +27246,7 @@ "testharness" ], "mozilla/range_request_blob_url.html": [ - "b41f0b5382e3994db35f38b2c358a41b75551fe2", + "075397620e989dafc814c0ed2bca46bd476bccf6", "testharness" ], "mozilla/range_request_file_url.html": [ diff --git a/tests/wpt/mozilla/tests/mozilla/range_request_blob_url.html b/tests/wpt/mozilla/tests/mozilla/range_request_blob_url.html index b41f0b5382e..075397620e9 100644 --- a/tests/wpt/mozilla/tests/mozilla/range_request_blob_url.html +++ b/tests/wpt/mozilla/tests/mozilla/range_request_blob_url.html @@ -23,12 +23,10 @@ range: "bytes=0-100", status: 206, expected: "abcdefghijklmnopqrstuvwxyz" -/*}, { - XXX(ferjm): Range requests with an out of bounds start throw an exception - rather than responding with 416. +}, { range: "bytes=100-", status: 416, - expected: ""*/ + expected: "" }, { range: "bytes=-100", status: 206, @@ -43,11 +41,10 @@ } }) .then(response => { -// XXX(ferjm): Responses to range requests to blob urls have status 200 rather than 206. -// assert_equals(response.status, test.status); -// if (response.status != 206) { -// return ""; -// } + assert_equals(response.status, test.status); + if (response.status != 206) { + return ""; + } return response.text(); }) .then(response => { From ef57ecbdb7864d924322f59a80ebd3c8c947b50e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Thu, 15 Nov 2018 08:38:52 +0100 Subject: [PATCH 11/13] Tweak limits of file chunked read and send chunks not entire body as partial payloads --- components/net/filemanager_thread.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index 3a639cbb741..dce6da0ed0f 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -817,7 +817,7 @@ pub fn fetch_file_in_chunks( { if let Some(end) = range.end { let remaining_bytes = - end as usize - range.start as usize - body.len(); + end as usize - range.start as usize - body.len() + 1; if remaining_bytes <= FILE_CHUNK_SIZE { // This is the last chunk so we set buffer // len to 0 to break the reading loop. @@ -832,8 +832,9 @@ pub fn fetch_file_in_chunks( }, buffer.len(), ); - body.extend_from_slice(&buffer[0..offset]); - let _ = done_sender.send(Data::Payload(buffer)); + let chunk = &buffer[0..offset]; + body.extend_from_slice(chunk); + let _ = done_sender.send(Data::Payload(chunk.to_vec())); } buffer_len }; From 4561a97309d1b4c6e56dc9bfd51782f40a9c6cba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Mon, 26 Nov 2018 09:15:18 +0100 Subject: [PATCH 12/13] Use crossbeam channel instead of servo channel. Fix rebase issues and add comment --- components/net/fetch/methods.rs | 6 +++--- components/net/filemanager_thread.rs | 11 +++++++---- components/net/tests/fetch.rs | 3 +-- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index 19736de1136..bd9dfce2dab 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -640,7 +640,7 @@ fn scheme_fetch( // Setup channel to receive cross-thread messages about the file fetch // operation. - let (done_sender, done_receiver) = channel(); + let (done_sender, done_receiver) = unbounded(); *done_chan = Some((done_sender.clone(), done_receiver)); *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]); @@ -687,12 +687,12 @@ fn scheme_fetch( }, }; - let mut response = Response::new(url); + let mut response = Response::new(url, ResourceFetchTiming::new(request.timing_type())); if is_range_request { partial_content(&mut response); } - let (done_sender, done_receiver) = channel(); + let (done_sender, done_receiver) = unbounded(); *done_chan = Some((done_sender.clone(), done_receiver)); *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]); let check_url_validity = true; diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index dce6da0ed0f..f0779df632b 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -3,6 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use crate::fetch::methods::{CancellationListener, Data, RangeRequestBounds}; +use crossbeam_channel::Sender; use embedder_traits::{EmbedderMsg, EmbedderProxy, FilterPattern}; use headers_ext::{ContentLength, ContentType, HeaderMap, HeaderMapExt}; use http::header::{self, HeaderValue}; @@ -17,7 +18,6 @@ use net_traits::filemanager_thread::{ use net_traits::http_percent_encode; use net_traits::response::{Response, ResponseBody}; use servo_arc::Arc as ServoArc; -use servo_channel; use servo_config::prefs::PREFS; use std::collections::HashMap; use std::fs::File; @@ -107,7 +107,7 @@ impl FileManager { // in a separate thread. pub fn fetch_file( &self, - done_sender: &servo_channel::Sender, + done_sender: &Sender, cancellation_listener: Arc>, id: Uuid, check_url_validity: bool, @@ -526,7 +526,7 @@ impl FileManagerStore { fn fetch_blob_buf( &self, - done_sender: &servo_channel::Sender, + done_sender: &Sender, cancellation_listener: Arc>, id: &Uuid, origin_in: &FileOrigin, @@ -794,7 +794,7 @@ fn read_file_in_chunks( } pub fn fetch_file_in_chunks( - done_sender: servo_channel::Sender, + done_sender: Sender, mut reader: BufReader, res_body: ServoArc>, cancellation_listener: Arc>, @@ -816,6 +816,9 @@ pub fn fetch_file_in_chunks( let offset = usize::min( { if let Some(end) = range.end { + // HTTP Range requests are specified with closed ranges, + // while Rust uses half-open ranges. We add +1 here so + // we don't skip the last requested byte. let remaining_bytes = end as usize - range.start as usize - body.len() + 1; if remaining_bytes <= FILE_CHUNK_SIZE { diff --git a/components/net/tests/fetch.rs b/components/net/tests/fetch.rs index 9fc8c351f83..e0311a5e626 100644 --- a/components/net/tests/fetch.rs +++ b/components/net/tests/fetch.rs @@ -38,7 +38,6 @@ use net_traits::response::{CacheState, Response, ResponseBody, ResponseType}; use net_traits::{ FetchTaskTarget, IncludeSubdomains, NetworkError, ReferrerPolicy, ResourceFetchTiming, ResourceTimingType, }; -use servo_channel::{channel, Sender}; use servo_url::{ImmutableOrigin, ServoUrl}; use std::fs::File; use std::io::Read; @@ -169,7 +168,7 @@ fn test_fetch_blob() { let mut request = Request::new(url, Some(Origin::Origin(origin.origin())), None); - let (sender, receiver) = channel(); + let (sender, receiver) = unbounded(); let mut target = FetchResponseCollector { sender, From b23dd0587b5d31d34915fc3906e550d05108a185 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Mon, 26 Nov 2018 09:54:32 +0100 Subject: [PATCH 13/13] Add comment about incorrect tests for file url range requests. Fmt and manifest update --- components/net/fetch/methods.rs | 3 ++- components/net/tests/fetch.rs | 3 ++- tests/wpt/mozilla/meta/MANIFEST.json | 22 +++++++++---------- .../tests/mozilla/range_request_file_url.html | 4 ++++ 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index bd9dfce2dab..7eb963fcd5e 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -610,7 +610,8 @@ fn scheme_fetch( Err(_) => None, }; - let mut response = Response::new(url, ResourceFetchTiming::new(request.timing_type())); + let mut response = + Response::new(url, ResourceFetchTiming::new(request.timing_type())); let range_header = request.headers.typed_get::(); let is_range_request = range_header.is_some(); diff --git a/components/net/tests/fetch.rs b/components/net/tests/fetch.rs index e0311a5e626..6831ca7aa46 100644 --- a/components/net/tests/fetch.rs +++ b/components/net/tests/fetch.rs @@ -36,7 +36,8 @@ use net::test::HttpState; use net_traits::request::{Destination, Origin, RedirectMode, Referrer, Request, RequestMode}; use net_traits::response::{CacheState, Response, ResponseBody, ResponseType}; use net_traits::{ - FetchTaskTarget, IncludeSubdomains, NetworkError, ReferrerPolicy, ResourceFetchTiming, ResourceTimingType, + FetchTaskTarget, IncludeSubdomains, NetworkError, ReferrerPolicy, ResourceFetchTiming, + ResourceTimingType, }; use servo_url::{ImmutableOrigin, ServoUrl}; use std::fs::File; diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 5233572da7d..66b49e51489 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -14269,13 +14269,13 @@ "/_mozilla/mozilla/range_deleteContents.html", {} ] - ], + ], "mozilla/range_request_blob_url.html": [ [ - "/_mozilla/mozilla/range_request_blob_url.html", + "/_mozilla/mozilla/range_request_blob_url.html", {} ] - ], + ], "mozilla/range_request_file_url.html": [ [ "/_mozilla/mozilla/range_request_file_url.html", @@ -27070,7 +27070,7 @@ "testharness" ], "mozilla/interfaces.html": [ - "ad17e930ddb5bc2daecb86216efe8885ae399173", + "ad17e930ddb5bc2daecb86216efe8885ae399173", "testharness" ], "mozilla/interfaces.js": [ @@ -27078,7 +27078,7 @@ "support" ], "mozilla/interfaces.worker.js": [ - "a5f2e00f234ea66b80e8a9bd4dbbc5433926191f", + "a5f2e00f234ea66b80e8a9bd4dbbc5433926191f", "testharness" ], "mozilla/invalid-this.html": [ @@ -27244,13 +27244,13 @@ "mozilla/range_deleteContents.html": [ "8de03455bcb0d18258f76af20f58c14868fe1c21", "testharness" - ], + ], "mozilla/range_request_blob_url.html": [ - "075397620e989dafc814c0ed2bca46bd476bccf6", + "075397620e989dafc814c0ed2bca46bd476bccf6", "testharness" - ], + ], "mozilla/range_request_file_url.html": [ - "65fe13fe93d97cebc2846ff7d7deab3eb84c1787", + "4fd4ddc8b1a9959e90b243795267c220d6a05f5e", "testharness" ], "mozilla/referrer-policy/OWNERS": [ @@ -32914,11 +32914,11 @@ "testharness" ], "mozilla/window_performance.html": [ - "690870b7080e179481ca0255f7c30337e8b6636a", + "690870b7080e179481ca0255f7c30337e8b6636a", "testharness" ], "mozilla/window_performance_topLevelDomComplete.html": [ - "50bbc2917b5ac900b5061a0b2c30b6c1fef1067e", + "50bbc2917b5ac900b5061a0b2c30b6c1fef1067e", "testharness" ], "mozilla/window_requestAnimationFrame.html": [ diff --git a/tests/wpt/mozilla/tests/mozilla/range_request_file_url.html b/tests/wpt/mozilla/tests/mozilla/range_request_file_url.html index 65fe13fe93d..4fd4ddc8b1a 100644 --- a/tests/wpt/mozilla/tests/mozilla/range_request_file_url.html +++ b/tests/wpt/mozilla/tests/mozilla/range_request_file_url.html @@ -3,6 +3,10 @@