From 64a7187238c4f291f254bd6eb58138a3a6534898 Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Mon, 4 Oct 2021 22:56:24 +0200 Subject: [PATCH] chore: remove No*Permissions structs (#12316) These are confusing. They say they are "for users that don't care about permissions", but that isn't correct. `NoTimersPermissions` disables permissions instead of enabling them. I would argue that implementors should decide what permissions they want themselves, and not take our opinionated permissions struct. --- ext/fetch/lib.rs | 13 ----- ext/ffi/lib.rs | 8 --- ext/net/lib.rs | 20 -------- ext/timers/benches/timers_ops.rs | 18 ++++++- ext/timers/lib.rs | 9 ---- ext/websocket/lib.rs | 9 ---- runtime/build.rs | 84 ++++++++++++++++++++++++++++---- 7 files changed, 91 insertions(+), 70 deletions(-) diff --git a/ext/fetch/lib.rs b/ext/fetch/lib.rs index b422c274133227..dfb1ce059f009f 100644 --- a/ext/fetch/lib.rs +++ b/ext/fetch/lib.rs @@ -120,19 +120,6 @@ pub trait FetchPermissions { fn check_read(&mut self, _p: &Path) -> Result<(), AnyError>; } -/// For use with `op_fetch` when the user does not want permissions. -pub struct NoFetchPermissions; - -impl FetchPermissions for NoFetchPermissions { - fn check_net_url(&mut self, _url: &Url) -> Result<(), AnyError> { - Ok(()) - } - - fn check_read(&mut self, _p: &Path) -> Result<(), AnyError> { - Ok(()) - } -} - pub fn get_declaration() -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("lib.deno_fetch.d.ts") } diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs index 673f8347223f57..e0430366cf880d 100644 --- a/ext/ffi/lib.rs +++ b/ext/ffi/lib.rs @@ -37,14 +37,6 @@ pub trait FfiPermissions { fn check(&mut self, path: &str) -> Result<(), AnyError>; } -pub struct NoFfiPermissions; - -impl FfiPermissions for NoFfiPermissions { - fn check(&mut self, _path: &str) -> Result<(), AnyError> { - Ok(()) - } -} - struct Symbol { cif: libffi::middle::Cif, ptr: libffi::middle::CodePtr, diff --git a/ext/net/lib.rs b/ext/net/lib.rs index 8909c6f40f90ff..ad14c15d88422b 100644 --- a/ext/net/lib.rs +++ b/ext/net/lib.rs @@ -26,26 +26,6 @@ pub trait NetPermissions { fn check_write(&mut self, _p: &Path) -> Result<(), AnyError>; } -/// For use with this crate when the user does not want permission checks. -pub struct NoNetPermissions; - -impl NetPermissions for NoNetPermissions { - fn check_net>( - &mut self, - _host: &(T, Option), - ) -> Result<(), AnyError> { - Ok(()) - } - - fn check_read(&mut self, _p: &Path) -> Result<(), AnyError> { - Ok(()) - } - - fn check_write(&mut self, _p: &Path) -> Result<(), AnyError> { - Ok(()) - } -} - /// `UnstableChecker` is a struct so it can be placed inside `GothamState`; /// using type alias for a bool could work, but there's a high chance /// that there might be another type alias pointing to a bool, which diff --git a/ext/timers/benches/timers_ops.rs b/ext/timers/benches/timers_ops.rs index 269d9627d8a279..8d13d5807f8903 100644 --- a/ext/timers/benches/timers_ops.rs +++ b/ext/timers/benches/timers_ops.rs @@ -5,12 +5,26 @@ use deno_bench_util::bencher::{benchmark_group, Bencher}; use deno_bench_util::{bench_js_async, bench_js_sync}; use deno_web::BlobStore; +struct Permissions; + +impl deno_timers::TimersPermission for Permissions { + fn allow_hrtime(&mut self) -> bool { + true + } + fn check_unstable( + &self, + _state: &deno_core::OpState, + _api_name: &'static str, + ) { + } +} + fn setup() -> Vec { vec![ deno_webidl::init(), deno_url::init(), deno_web::init(BlobStore::default(), None), - deno_timers::init::(), + deno_timers::init::(), Extension::builder() .js(vec![ ("setup", @@ -21,7 +35,7 @@ fn setup() -> Vec { ), ]) .state(|state| { - state.put(deno_timers::NoTimersPermission{}); + state.put(Permissions{}); Ok(()) }) .build() diff --git a/ext/timers/lib.rs b/ext/timers/lib.rs index 2b9948d1fd7c13..384627c57dd634 100644 --- a/ext/timers/lib.rs +++ b/ext/timers/lib.rs @@ -31,15 +31,6 @@ pub trait TimersPermission { fn check_unstable(&self, state: &OpState, api_name: &'static str); } -pub struct NoTimersPermission; - -impl TimersPermission for NoTimersPermission { - fn allow_hrtime(&mut self) -> bool { - false - } - fn check_unstable(&self, _: &OpState, _: &'static str) {} -} - pub fn init() -> Extension { Extension::builder() .js(include_js_files!( diff --git a/ext/websocket/lib.rs b/ext/websocket/lib.rs index dbb88dc8db1a21..ebb2186d04cb02 100644 --- a/ext/websocket/lib.rs +++ b/ext/websocket/lib.rs @@ -59,15 +59,6 @@ pub trait WebSocketPermissions { /// would override previously used alias. pub struct UnsafelyIgnoreCertificateErrors(Option>); -/// For use with `op_websocket_*` when the user does not want permissions. -pub struct NoWebSocketPermissions; - -impl WebSocketPermissions for NoWebSocketPermissions { - fn check_net_url(&mut self, _url: &url::Url) -> Result<(), AnyError> { - Ok(()) - } -} - type WsStream = WebSocketStream>; pub enum WebSocketStreamType { Client { diff --git a/runtime/build.rs b/runtime/build.rs index 52933f19b9b74c..920aafc9f127e7 100644 --- a/runtime/build.rs +++ b/runtime/build.rs @@ -41,6 +41,76 @@ mod not_docs { println!("Snapshot written to: {} ", snapshot_path.display()); } + struct Permissions; + + impl deno_fetch::FetchPermissions for Permissions { + fn check_net_url( + &mut self, + _url: &deno_core::url::Url, + ) -> Result<(), deno_core::error::AnyError> { + unreachable!("snapshotting!") + } + + fn check_read( + &mut self, + _p: &Path, + ) -> Result<(), deno_core::error::AnyError> { + unreachable!("snapshotting!") + } + } + + impl deno_websocket::WebSocketPermissions for Permissions { + fn check_net_url( + &mut self, + _url: &deno_core::url::Url, + ) -> Result<(), deno_core::error::AnyError> { + unreachable!("snapshotting!") + } + } + + impl deno_timers::TimersPermission for Permissions { + fn allow_hrtime(&mut self) -> bool { + unreachable!("snapshotting!") + } + + fn check_unstable( + &self, + _state: &deno_core::OpState, + _api_name: &'static str, + ) { + unreachable!("snapshotting!") + } + } + + impl deno_ffi::FfiPermissions for Permissions { + fn check(&mut self, _path: &str) -> Result<(), deno_core::error::AnyError> { + unreachable!("snapshotting!") + } + } + + impl deno_net::NetPermissions for Permissions { + fn check_net>( + &mut self, + _host: &(T, Option), + ) -> Result<(), deno_core::error::AnyError> { + unreachable!("snapshotting!") + } + + fn check_read( + &mut self, + _p: &Path, + ) -> Result<(), deno_core::error::AnyError> { + unreachable!("snapshotting!") + } + + fn check_write( + &mut self, + _p: &Path, + ) -> Result<(), deno_core::error::AnyError> { + unreachable!("snapshotting!") + } + } + fn create_runtime_snapshot(snapshot_path: &Path, files: Vec) { let extensions: Vec = vec![ deno_webidl::init(), @@ -48,7 +118,7 @@ mod not_docs { deno_url::init(), deno_tls::init(), deno_web::init(deno_web::BlobStore::default(), Default::default()), - deno_fetch::init::( + deno_fetch::init::( "".to_owned(), None, None, @@ -56,21 +126,17 @@ mod not_docs { None, None, ), - deno_websocket::init::( - "".to_owned(), - None, - None, - ), + deno_websocket::init::("".to_owned(), None, None), deno_webstorage::init(None), deno_crypto::init(None), deno_webgpu::init(false), - deno_timers::init::(), + deno_timers::init::(), deno_broadcast_channel::init( deno_broadcast_channel::InMemoryBroadcastChannel::default(), false, // No --unstable. ), - deno_ffi::init::(false), - deno_net::init::( + deno_ffi::init::(false), + deno_net::init::( None, false, // No --unstable. None, ),