Skip to content

Commit

Permalink
chore: remove No*Permissions structs (#12316)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lucacasonato authored Oct 4, 2021
1 parent c6ae41f commit 64a7187
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 70 deletions.
13 changes: 0 additions & 13 deletions ext/fetch/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
8 changes: 0 additions & 8 deletions ext/ffi/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 0 additions & 20 deletions ext/net/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: AsRef<str>>(
&mut self,
_host: &(T, Option<u16>),
) -> 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
Expand Down
18 changes: 16 additions & 2 deletions ext/timers/benches/timers_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Extension> {
vec![
deno_webidl::init(),
deno_url::init(),
deno_web::init(BlobStore::default(), None),
deno_timers::init::<deno_timers::NoTimersPermission>(),
deno_timers::init::<Permissions>(),
Extension::builder()
.js(vec![
("setup",
Expand All @@ -21,7 +35,7 @@ fn setup() -> Vec<Extension> {
),
])
.state(|state| {
state.put(deno_timers::NoTimersPermission{});
state.put(Permissions{});
Ok(())
})
.build()
Expand Down
9 changes: 0 additions & 9 deletions ext/timers/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<P: TimersPermission + 'static>() -> Extension {
Extension::builder()
.js(include_js_files!(
Expand Down
9 changes: 0 additions & 9 deletions ext/websocket/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,6 @@ pub trait WebSocketPermissions {
/// would override previously used alias.
pub struct UnsafelyIgnoreCertificateErrors(Option<Vec<String>>);

/// 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<MaybeTlsStream<TcpStream>>;
pub enum WebSocketStreamType {
Client {
Expand Down
84 changes: 75 additions & 9 deletions runtime/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,36 +41,102 @@ 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<T: AsRef<str>>(
&mut self,
_host: &(T, Option<u16>),
) -> 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<PathBuf>) {
let extensions: Vec<Extension> = vec![
deno_webidl::init(),
deno_console::init(),
deno_url::init(),
deno_tls::init(),
deno_web::init(deno_web::BlobStore::default(), Default::default()),
deno_fetch::init::<deno_fetch::NoFetchPermissions>(
deno_fetch::init::<Permissions>(
"".to_owned(),
None,
None,
None,
None,
None,
),
deno_websocket::init::<deno_websocket::NoWebSocketPermissions>(
"".to_owned(),
None,
None,
),
deno_websocket::init::<Permissions>("".to_owned(), None, None),
deno_webstorage::init(None),
deno_crypto::init(None),
deno_webgpu::init(false),
deno_timers::init::<deno_timers::NoTimersPermission>(),
deno_timers::init::<Permissions>(),
deno_broadcast_channel::init(
deno_broadcast_channel::InMemoryBroadcastChannel::default(),
false, // No --unstable.
),
deno_ffi::init::<deno_ffi::NoFfiPermissions>(false),
deno_net::init::<deno_net::NoNetPermissions>(
deno_ffi::init::<Permissions>(false),
deno_net::init::<Permissions>(
None, false, // No --unstable.
None,
),
Expand Down

0 comments on commit 64a7187

Please sign in to comment.