From fac64478157ee563b185edb5734688e4523df3a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Date: Sat, 7 Jan 2023 17:25:34 +0100
Subject: refactor(permissions): add PermissionsContainer struct for internal
 mutability (#17134)

Turns out we were cloning permissions which after prompting were discarded,
so the state of permissions was never preserved. To handle that we need to store
all permissions behind "Arc<Mutex<>>" (because there are situations where we
need to send them to other thread).

Testing and benching code still uses "Permissions" in most places - it's undesirable
to share the same permission set between various test/bench files - otherwise
granting or revoking permissions in one file would influence behavior of other test
files.
---
 cli/ops/bench.rs   | 16 ++++++++++------
 cli/ops/testing.rs | 16 ++++++++++------
 2 files changed, 20 insertions(+), 12 deletions(-)

(limited to 'cli/ops')

diff --git a/cli/ops/bench.rs b/cli/ops/bench.rs
index 6fc881f61..3b27ffa7e 100644
--- a/cli/ops/bench.rs
+++ b/cli/ops/bench.rs
@@ -9,7 +9,7 @@ use deno_core::ModuleSpecifier;
 use deno_core::OpState;
 use deno_runtime::permissions::create_child_permissions;
 use deno_runtime::permissions::ChildPermissionsArg;
-use deno_runtime::permissions::Permissions;
+use deno_runtime::permissions::PermissionsContainer;
 use serde::Deserialize;
 use serde::Serialize;
 use std::sync::atomic::AtomicUsize;
@@ -40,7 +40,7 @@ pub fn init(
 }
 
 #[derive(Clone)]
-struct PermissionsHolder(Uuid, Permissions);
+struct PermissionsHolder(Uuid, PermissionsContainer);
 
 #[op]
 pub fn op_pledge_test_permissions(
@@ -48,8 +48,12 @@ pub fn op_pledge_test_permissions(
   args: ChildPermissionsArg,
 ) -> Result<Uuid, AnyError> {
   let token = Uuid::new_v4();
-  let parent_permissions = state.borrow_mut::<Permissions>();
-  let worker_permissions = create_child_permissions(parent_permissions, args)?;
+  let parent_permissions = state.borrow_mut::<PermissionsContainer>();
+  let worker_permissions = {
+    let mut parent_permissions = parent_permissions.0.lock();
+    let perms = create_child_permissions(&mut parent_permissions, args)?;
+    PermissionsContainer::new(perms)
+  };
   let parent_permissions = parent_permissions.clone();
 
   if state.try_take::<PermissionsHolder>().is_some() {
@@ -59,7 +63,7 @@ pub fn op_pledge_test_permissions(
   state.put::<PermissionsHolder>(PermissionsHolder(token, parent_permissions));
 
   // NOTE: This call overrides current permission set for the worker
-  state.put::<Permissions>(worker_permissions);
+  state.put::<PermissionsContainer>(worker_permissions);
 
   Ok(token)
 }
@@ -75,7 +79,7 @@ pub fn op_restore_test_permissions(
     }
 
     let permissions = permissions_holder.1;
-    state.put::<Permissions>(permissions);
+    state.put::<PermissionsContainer>(permissions);
     Ok(())
   } else {
     Err(generic_error("no permissions to restore"))
diff --git a/cli/ops/testing.rs b/cli/ops/testing.rs
index b6fd29fc7..e5c0a2b99 100644
--- a/cli/ops/testing.rs
+++ b/cli/ops/testing.rs
@@ -17,7 +17,7 @@ use deno_core::ModuleSpecifier;
 use deno_core::OpState;
 use deno_runtime::permissions::create_child_permissions;
 use deno_runtime::permissions::ChildPermissionsArg;
-use deno_runtime::permissions::Permissions;
+use deno_runtime::permissions::PermissionsContainer;
 use serde::Deserialize;
 use serde::Deserializer;
 use serde::Serialize;
@@ -50,7 +50,7 @@ pub fn init(
 }
 
 #[derive(Clone)]
-struct PermissionsHolder(Uuid, Permissions);
+struct PermissionsHolder(Uuid, PermissionsContainer);
 
 #[op]
 pub fn op_pledge_test_permissions(
@@ -58,8 +58,12 @@ pub fn op_pledge_test_permissions(
   args: ChildPermissionsArg,
 ) -> Result<Uuid, AnyError> {
   let token = Uuid::new_v4();
-  let parent_permissions = state.borrow_mut::<Permissions>();
-  let worker_permissions = create_child_permissions(parent_permissions, args)?;
+  let parent_permissions = state.borrow_mut::<PermissionsContainer>();
+  let worker_permissions = {
+    let mut parent_permissions = parent_permissions.0.lock();
+    let perms = create_child_permissions(&mut parent_permissions, args)?;
+    PermissionsContainer::new(perms)
+  };
   let parent_permissions = parent_permissions.clone();
 
   if state.try_take::<PermissionsHolder>().is_some() {
@@ -68,7 +72,7 @@ pub fn op_pledge_test_permissions(
   state.put::<PermissionsHolder>(PermissionsHolder(token, parent_permissions));
 
   // NOTE: This call overrides current permission set for the worker
-  state.put::<Permissions>(worker_permissions);
+  state.put::<PermissionsContainer>(worker_permissions);
 
   Ok(token)
 }
@@ -84,7 +88,7 @@ pub fn op_restore_test_permissions(
     }
 
     let permissions = permissions_holder.1;
-    state.put::<Permissions>(permissions);
+    state.put::<PermissionsContainer>(permissions);
     Ok(())
   } else {
     Err(generic_error("no permissions to restore"))
-- 
cgit v1.2.3