add a fetch token for blob urls

This commit is contained in:
Gregory Terzian 2020-02-13 00:18:14 +08:00
parent 4c5ec9da27
commit 895500e68c
10 changed files with 167 additions and 45 deletions

View file

@ -18,7 +18,7 @@ use hyper::StatusCode;
use ipc_channel::ipc::IpcReceiver;
use mime::{self, Mime};
use net_traits::blob_url_store::{parse_blob_url, BlobURLStoreError};
use net_traits::filemanager_thread::RelativePos;
use net_traits::filemanager_thread::{FileTokenCheck, RelativePos};
use net_traits::request::{
is_cors_safelisted_method, is_cors_safelisted_request_header, Origin, ResponseTainting, Window,
};
@ -56,6 +56,7 @@ pub struct FetchContext {
pub user_agent: Cow<'static, str>,
pub devtools_chan: Option<Sender<DevtoolsControlMsg>>,
pub filemanager: FileManager,
pub file_token: FileTokenCheck,
pub cancellation_listener: Arc<Mutex<CancellationListener>>,
pub timing: ServoArc<Mutex<ResourceFetchTiming>>,
}
@ -755,12 +756,12 @@ fn scheme_fetch(
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;
if let Err(err) = context.filemanager.fetch_file(
&done_sender,
context.cancellation_listener.clone(),
id,
check_url_validity,
&context.file_token,
origin,
&mut response,
range,

View file

@ -11,14 +11,16 @@ use http::header::{self, HeaderValue};
use ipc_channel::ipc::{self, IpcSender};
use mime::{self, Mime};
use net_traits::blob_url_store::{BlobBuf, BlobURLStoreError};
use net_traits::filemanager_thread::{FileManagerResult, FileManagerThreadMsg, FileOrigin};
use net_traits::filemanager_thread::{
FileManagerResult, FileManagerThreadMsg, FileOrigin, FileTokenCheck,
};
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 std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::fs::File;
use std::io::{BufRead, BufReader, Read, Seek, SeekFrom};
use std::mem;
@ -46,6 +48,9 @@ struct FileStoreEntry {
/// by the user with createObjectURL. Validity can be revoked as well.
/// (The UUID is the one that maps to this entry in `FileManagerStore`)
is_valid_url: AtomicBool,
/// UUIDs of fetch instances that acquired an interest in this file,
/// when the url was still valid.
outstanding_tokens: HashSet<Uuid>,
}
#[derive(Clone)]
@ -91,7 +96,6 @@ impl FileManager {
&self,
sender: IpcSender<FileManagerResult<ReadFileProgress>>,
id: Uuid,
check_url_validity: bool,
origin: FileOrigin,
) {
let store = self.store.clone();
@ -99,7 +103,7 @@ impl FileManager {
.upgrade()
.and_then(|pool| {
pool.spawn(move || {
if let Err(e) = store.try_read_file(&sender, id, check_url_validity, origin) {
if let Err(e) = store.try_read_file(&sender, id, origin) {
let _ = sender.send(Err(FileManagerThreadError::BlobURLStoreError(e)));
}
});
@ -110,6 +114,14 @@ impl FileManager {
});
}
pub fn get_token_for_file(&self, file_id: &Uuid) -> FileTokenCheck {
self.store.get_token_for_file(file_id)
}
pub fn invalidate_token(&self, token: &FileTokenCheck, file_id: &Uuid) {
self.store.invalidate_token(token, file_id);
}
// Read a file for the Fetch implementation.
// It gets the required headers synchronously and reads the actual content
// in a separate thread.
@ -118,7 +130,7 @@ impl FileManager {
done_sender: &Sender<Data>,
cancellation_listener: Arc<Mutex<CancellationListener>>,
id: Uuid,
check_url_validity: bool,
file_token: &FileTokenCheck,
origin: FileOrigin,
response: &mut Response,
range: RangeRequestBounds,
@ -127,9 +139,9 @@ impl FileManager {
done_sender,
cancellation_listener,
&id,
file_token,
&origin,
range,
check_url_validity,
response,
)
}
@ -175,8 +187,8 @@ impl FileManager {
);
});
},
FileManagerThreadMsg::ReadFile(sender, id, check_url_validity, origin) => {
self.read_file(sender, id, check_url_validity, origin);
FileManagerThreadMsg::ReadFile(sender, id, origin) => {
self.read_file(sender, id, origin);
},
FileManagerThreadMsg::PromoteMemory(id, blob_buf, set_valid, origin) => {
self.promote_memory(id, blob_buf, set_valid, origin);
@ -273,12 +285,12 @@ impl FileManager {
done_sender: &Sender<Data>,
cancellation_listener: Arc<Mutex<CancellationListener>>,
id: &Uuid,
file_token: &FileTokenCheck,
origin_in: &FileOrigin,
range: RangeRequestBounds,
check_url_validity: bool,
response: &mut Response,
) -> Result<(), BlobURLStoreError> {
let file_impl = self.store.get_impl(id, origin_in, check_url_validity)?;
let file_impl = self.store.get_impl(id, file_token, origin_in)?;
match file_impl {
FileImpl::Memory(buf) => {
let range = match range.get_final(Some(buf.size)) {
@ -362,11 +374,11 @@ impl FileManager {
done_sender,
cancellation_listener,
&parent_id,
file_token,
origin_in,
RangeRequestBounds::Final(
RelativePos::full_range().slice_inner(&inner_rel_pos),
),
false,
response,
);
},
@ -392,19 +404,23 @@ impl FileManagerStore {
pub fn get_impl(
&self,
id: &Uuid,
file_token: &FileTokenCheck,
origin_in: &FileOrigin,
check_url_validity: bool,
) -> Result<FileImpl, BlobURLStoreError> {
match self.entries.read().unwrap().get(id) {
Some(ref entry) => {
if *origin_in != *entry.origin {
Err(BlobURLStoreError::InvalidOrigin)
} else {
let is_valid = entry.is_valid_url.load(Ordering::Acquire);
if check_url_validity && !is_valid {
Err(BlobURLStoreError::InvalidFileID)
} else {
Ok(entry.file_impl.clone())
match file_token {
FileTokenCheck::NotRequired => Ok(entry.file_impl.clone()),
FileTokenCheck::Required(token) => {
if entry.outstanding_tokens.contains(token) {
return Ok(entry.file_impl.clone());
}
Err(BlobURLStoreError::InvalidFileID)
},
FileTokenCheck::ShouldFail => Err(BlobURLStoreError::InvalidFileID),
}
}
},
@ -412,6 +428,58 @@ impl FileManagerStore {
}
}
pub fn invalidate_token(&self, token: &FileTokenCheck, file_id: &Uuid) {
if let FileTokenCheck::Required(token) = token {
let mut entries = self.entries.write().unwrap();
if let Some(entry) = entries.get_mut(file_id) {
entry.outstanding_tokens.remove(token);
// Check if there are references left.
let zero_refs = entry.refs.load(Ordering::Acquire) == 0;
// Check if no other fetch has acquired a token for this file.
let no_outstanding_tokens = entry.outstanding_tokens.len() == 0;
// Check if there is still a blob URL outstanding.
let valid = entry.is_valid_url.load(Ordering::Acquire);
// Can we remove this file?
let do_remove = zero_refs && no_outstanding_tokens && !valid;
if do_remove {
entries.remove(&file_id);
}
}
}
}
pub fn get_token_for_file(&self, file_id: &Uuid) -> FileTokenCheck {
let mut entries = self.entries.write().unwrap();
let parent_id = match entries.get(file_id) {
Some(entry) => {
if let FileImpl::Sliced(ref parent_id, _) = entry.file_impl {
Some(parent_id.clone())
} else {
None
}
},
None => return FileTokenCheck::ShouldFail,
};
let file_id = match parent_id.as_ref() {
Some(id) => id,
None => file_id,
};
if let Some(entry) = entries.get_mut(file_id) {
if !entry.is_valid_url.load(Ordering::Acquire) {
return FileTokenCheck::ShouldFail;
}
let token = Uuid::new_v4();
entry.outstanding_tokens.insert(token.clone());
return FileTokenCheck::Required(token);
}
FileTokenCheck::ShouldFail
}
fn insert(&self, id: Uuid, entry: FileStoreEntry) {
self.entries.write().unwrap().insert(id, entry);
}
@ -453,6 +521,7 @@ impl FileManagerStore {
// Valid here since AddSlicedURLEntry implies URL creation
// from a BlobImpl::Sliced
is_valid_url: AtomicBool::new(true),
outstanding_tokens: Default::default(),
},
);
@ -604,6 +673,7 @@ impl FileManagerStore {
refs: AtomicUsize::new(1),
// Invalid here since create_entry is called by file selection
is_valid_url: AtomicBool::new(false),
outstanding_tokens: Default::default(),
},
);
@ -626,11 +696,11 @@ impl FileManagerStore {
&self,
sender: &IpcSender<FileManagerResult<ReadFileProgress>>,
id: &Uuid,
file_token: &FileTokenCheck,
origin_in: &FileOrigin,
rel_pos: RelativePos,
check_url_validity: bool,
) -> Result<(), BlobURLStoreError> {
let file_impl = self.get_impl(id, origin_in, check_url_validity)?;
let file_impl = self.get_impl(id, file_token, origin_in)?;
match file_impl {
FileImpl::Memory(buf) => {
let range = rel_pos.to_abs_range(buf.size as usize);
@ -686,9 +756,9 @@ impl FileManagerStore {
self.get_blob_buf(
sender,
&parent_id,
file_token,
origin_in,
rel_pos.slice_inner(&inner_rel_pos),
false,
)
},
}
@ -699,15 +769,14 @@ impl FileManagerStore {
&self,
sender: &IpcSender<FileManagerResult<ReadFileProgress>>,
id: Uuid,
check_url_validity: bool,
origin_in: FileOrigin,
) -> Result<(), BlobURLStoreError> {
self.get_blob_buf(
sender,
&id,
&FileTokenCheck::NotRequired,
&origin_in,
RelativePos::full_range(),
check_url_validity,
)
}
@ -724,10 +793,17 @@ impl FileManagerStore {
// last reference, and if it has a reference to parent id
// dec_ref on parent later if necessary
let is_valid = entry.is_valid_url.load(Ordering::Acquire);
// Check if no fetch has acquired a token for this file.
let no_outstanding_tokens = entry.outstanding_tokens.len() == 0;
// Can we remove this file?
let do_remove = !is_valid && no_outstanding_tokens;
if let FileImpl::Sliced(ref parent_id, _) = entry.file_impl {
(!is_valid, Some(parent_id.clone()))
(do_remove, Some(parent_id.clone()))
} else {
(!is_valid, None)
(do_remove, None)
}
}
} else {
@ -762,6 +838,7 @@ impl FileManagerStore {
file_impl: FileImpl::Memory(blob_buf),
refs: AtomicUsize::new(1),
is_valid_url: AtomicBool::new(set_valid),
outstanding_tokens: Default::default(),
},
);
},
@ -786,10 +863,16 @@ impl FileManagerStore {
// and store entry id holders
let zero_refs = entry.refs.load(Ordering::Acquire) == 0;
// Check if no fetch has acquired a token for this file.
let no_outstanding_tokens = entry.outstanding_tokens.len() == 0;
// Can we remove this file?
let do_remove = zero_refs && no_outstanding_tokens;
if let FileImpl::Sliced(ref parent_id, _) = entry.file_impl {
(zero_refs, Some(parent_id.clone()), Ok(()))
(do_remove, Some(parent_id.clone()), Ok(()))
} else {
(zero_refs, None, Ok(()))
(do_remove, None, Ok(()))
}
} else {
(false, None, Ok(()))

View file

@ -22,6 +22,8 @@ use embedder_traits::EmbedderProxy;
use hyper_serde::Serde;
use ipc_channel::ipc::{self, IpcReceiver, IpcReceiverSet, IpcSender};
use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
use net_traits::blob_url_store::parse_blob_url;
use net_traits::filemanager_thread::FileTokenCheck;
use net_traits::request::{Destination, RequestBuilder};
use net_traits::response::{Response, ResponseInit};
use net_traits::storage_thread::StorageThreadMsg;
@ -621,8 +623,31 @@ impl CoreResourceManager {
_ => ResourceTimingType::Resource,
};
let mut request = request_builder.build();
let url = request.current_url();
// In the case of a valid blob URL, acquiring a token granting access to a file,
// regardless if the URL is revoked after token acquisition.
//
// TODO: to make more tests pass, acquire this token earlier,
// probably in a separate message flow.
//
// In such a setup, the token would not be acquired here,
// but could instead be contained in the actual CoreResourceMsg::Fetch message.
//
// See https://github.com/servo/servo/issues/25226
let (file_token, blob_url_file_id) = match url.scheme() {
"blob" => {
if let Ok((id, _)) = parse_blob_url(&url) {
(self.filemanager.get_token_for_file(&id), Some(id))
} else {
(FileTokenCheck::ShouldFail, None)
}
},
_ => (FileTokenCheck::NotRequired, None),
};
self.thread_pool.spawn(move || {
let mut request = request_builder.build();
// XXXManishearth: Check origin against pipeline id (also ensure that the mode is allowed)
// todo load context / mimesniff in fetch
// todo referrer policy?
@ -632,6 +657,7 @@ impl CoreResourceManager {
user_agent: ua,
devtools_chan: dc,
filemanager: filemanager,
file_token,
cancellation_listener: Arc::new(Mutex::new(CancellationListener::new(cancel_chan))),
timing: ServoArc::new(Mutex::new(ResourceFetchTiming::new(request.timing_type()))),
};
@ -651,6 +677,13 @@ impl CoreResourceManager {
},
None => fetch(&mut request, &mut sender, &context),
};
// Remove token after fetch.
if let Some(id) = blob_url_file_id.as_ref() {
context
.filemanager
.invalidate_token(&context.file_token, id);
}
});
}

View file

@ -29,6 +29,7 @@ use net::filemanager_thread::FileManager;
use net::hsts::HstsEntry;
use net::resource_thread::CoreResourceThreadPool;
use net::test::HttpState;
use net_traits::filemanager_thread::FileTokenCheck;
use net_traits::request::{
Destination, Origin, RedirectMode, Referrer, Request, RequestBuilder, RequestMode,
};
@ -683,6 +684,7 @@ fn test_fetch_with_hsts() {
user_agent: DEFAULT_USER_AGENT.into(),
devtools_chan: None,
filemanager: FileManager::new(create_embedder_proxy(), Weak::new()),
file_token: FileTokenCheck::NotRequired,
cancellation_listener: Arc::new(Mutex::new(CancellationListener::new(None))),
timing: ServoArc::new(Mutex::new(ResourceFetchTiming::new(
ResourceTimingType::Navigation,
@ -735,6 +737,7 @@ fn test_load_adds_host_to_hsts_list_when_url_is_https() {
user_agent: DEFAULT_USER_AGENT.into(),
devtools_chan: None,
filemanager: FileManager::new(create_embedder_proxy(), Weak::new()),
file_token: FileTokenCheck::NotRequired,
cancellation_listener: Arc::new(Mutex::new(CancellationListener::new(None))),
timing: ServoArc::new(Mutex::new(ResourceFetchTiming::new(
ResourceTimingType::Navigation,

View file

@ -59,7 +59,6 @@ fn test_filemanager() {
filemanager.handle(FileManagerThreadMsg::ReadFile(
tx2,
selected.id.clone(),
false,
origin.clone(),
));
@ -113,7 +112,6 @@ fn test_filemanager() {
filemanager.handle(FileManagerThreadMsg::ReadFile(
tx2,
selected.id.clone(),
false,
origin.clone(),
));

View file

@ -35,6 +35,7 @@ use net::fetch::methods::{self, CancellationListener, FetchContext};
use net::filemanager_thread::FileManager;
use net::resource_thread::CoreResourceThreadPool;
use net::test::HttpState;
use net_traits::filemanager_thread::FileTokenCheck;
use net_traits::request::Request;
use net_traits::response::Response;
use net_traits::{FetchTaskTarget, ResourceFetchTiming, ResourceTimingType};
@ -98,6 +99,7 @@ fn new_fetch_context(
user_agent: DEFAULT_USER_AGENT.into(),
devtools_chan: dc,
filemanager: FileManager::new(sender, pool_handle.unwrap_or_else(|| Weak::new())),
file_token: FileTokenCheck::NotRequired,
cancellation_listener: Arc::new(Mutex::new(CancellationListener::new(None))),
timing: ServoArc::new(Mutex::new(ResourceFetchTiming::new(
ResourceTimingType::Navigation,

View file

@ -16,6 +16,20 @@ use uuid::Uuid;
/// File manager store entry's origin
pub type FileOrigin = String;
/// A token modulating access to a file for a blob URL.
pub enum FileTokenCheck {
/// Checking against a token not required,
/// used for accessing a file
/// that isn't linked to from a blob URL.
NotRequired,
/// Checking against token required.
Required(Uuid),
/// Request should always fail,
/// used for cases when a check is required,
/// but no token could be acquired.
ShouldFail,
}
/// Relative slice positions of a sequence,
/// whose semantic should be consistent with (start, end) parameters in
/// <https://w3c.github.io/FileAPI/#dfn-slice>
@ -125,7 +139,6 @@ pub enum FileManagerThreadMsg {
ReadFile(
IpcSender<FileManagerResult<ReadFileProgress>>,
Uuid,
bool,
FileOrigin,
),

View file

@ -1380,8 +1380,7 @@ impl GlobalScope {
let resource_threads = self.resource_threads();
let (chan, recv) = profile_ipc::channel(self.time_profiler_chan().clone()).unwrap();
let origin = get_blob_origin(&self.get_url());
let check_url_validity = false;
let msg = FileManagerThreadMsg::ReadFile(chan, id, check_url_validity, origin);
let msg = FileManagerThreadMsg::ReadFile(chan, id, origin);
let _ = resource_threads.send(CoreResourceMsg::ToFileManager(msg));
recv
}

View file

@ -1,5 +1,4 @@
[url-in-tags-revoke.window.html]
expected: TIMEOUT
[Fetching a blob URL immediately before revoking it works in an iframe.]
expected: FAIL
@ -15,6 +14,3 @@
[Opening a blob URL in a new window by clicking an <a> tag works immediately before revoking the URL.]
expected: FAIL
[Fetching a blob URL immediately before revoking it works in <script> tags.]
expected: TIMEOUT

View file

@ -2,14 +2,8 @@
[Revoke blob URL after creating Request, will fetch]
expected: FAIL
[Revoke blob URL after calling fetch, fetch should succeed]
expected: FAIL
[url-with-fetch.any.html]
[Revoke blob URL after creating Request, will fetch]
expected: FAIL
[Revoke blob URL after calling fetch, fetch should succeed]
expected: FAIL