summaryrefslogtreecommitdiff
path: root/cli
diff options
context:
space:
mode:
authorNayeem Rahman <nayeemrmn99@gmail.com>2023-04-26 21:23:28 +0100
committerGitHub <noreply@github.com>2023-04-26 16:23:28 -0400
commit3d8a4d3b81e107bbb152ad69047f64d16ca800f3 (patch)
tree52c8d5e655e3b4bc51aabee1bbfe428a3b7b394a /cli
parentc2f5c096925e2fc303f6ea1c36cdba38748c9217 (diff)
feat(cli): don't check permissions for statically analyzable dynamic imports (#18713)
Closes #17697 Closes #17658
Diffstat (limited to 'cli')
-rw-r--r--cli/cache/mod.rs17
-rw-r--r--cli/graph_util.rs11
-rw-r--r--cli/lsp/testing/execution.rs1
-rw-r--r--cli/module_loader.rs17
-rw-r--r--cli/tests/integration/run_tests.rs5
-rw-r--r--cli/tests/testdata/dynamic_import/empty_1.ts0
-rw-r--r--cli/tests/testdata/dynamic_import/empty_2.ts0
-rw-r--r--cli/tests/testdata/dynamic_import/permissions_remote_remote.ts2
-rw-r--r--cli/tests/testdata/dynamic_import/static_analysis_no_permissions.ts13
-rw-r--r--cli/tests/testdata/dynamic_import/static_analysis_no_permissions.ts.out2
-rw-r--r--cli/tests/testdata/run/error_015_dynamic_import_permissions.js2
-rw-r--r--cli/tests/testdata/run/error_015_dynamic_import_permissions.out2
-rw-r--r--cli/tests/testdata/workers/dynamic_remote.ts2
-rw-r--r--cli/tests/testdata/workers/permissions_dynamic_remote.ts.out2
-rw-r--r--cli/tools/bench.rs7
-rw-r--r--cli/tools/test.rs9
16 files changed, 40 insertions, 52 deletions
diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs
index 24712d08a..40d74ff66 100644
--- a/cli/cache/mod.rs
+++ b/cli/cache/mod.rs
@@ -45,10 +45,9 @@ pub const CACHE_PERM: u32 = 0o644;
/// a concise interface to the DENO_DIR when building module graphs.
pub struct FetchCacher {
emit_cache: EmitCache,
- dynamic_permissions: PermissionsContainer,
file_fetcher: Arc<FileFetcher>,
file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
- root_permissions: PermissionsContainer,
+ permissions: PermissionsContainer,
cache_info_enabled: bool,
maybe_local_node_modules_url: Option<ModuleSpecifier>,
}
@@ -58,16 +57,14 @@ impl FetchCacher {
emit_cache: EmitCache,
file_fetcher: Arc<FileFetcher>,
file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
- root_permissions: PermissionsContainer,
- dynamic_permissions: PermissionsContainer,
+ permissions: PermissionsContainer,
maybe_local_node_modules_url: Option<ModuleSpecifier>,
) -> Self {
Self {
emit_cache,
- dynamic_permissions,
file_fetcher,
file_header_overrides,
- root_permissions,
+ permissions,
cache_info_enabled: false,
maybe_local_node_modules_url,
}
@@ -105,7 +102,7 @@ impl Loader for FetchCacher {
fn load(
&mut self,
specifier: &ModuleSpecifier,
- is_dynamic: bool,
+ _is_dynamic: bool,
) -> LoadFuture {
if let Some(node_modules_url) = self.maybe_local_node_modules_url.as_ref() {
// The specifier might be in a completely different symlinked tree than
@@ -124,11 +121,7 @@ impl Loader for FetchCacher {
}
}
- let permissions = if is_dynamic {
- self.dynamic_permissions.clone()
- } else {
- self.root_permissions.clone()
- };
+ let permissions = self.permissions.clone();
let file_fetcher = self.file_fetcher.clone();
let file_header_overrides = self.file_header_overrides.clone();
let specifier = specifier.clone();
diff --git a/cli/graph_util.rs b/cli/graph_util.rs
index d5bc6ac0d..f9dafbb57 100644
--- a/cli/graph_util.rs
+++ b/cli/graph_util.rs
@@ -314,23 +314,18 @@ impl ModuleGraphBuilder {
/// Creates the default loader used for creating a graph.
pub fn create_graph_loader(&self) -> cache::FetchCacher {
- self.create_fetch_cacher(
- PermissionsContainer::allow_all(),
- PermissionsContainer::allow_all(),
- )
+ self.create_fetch_cacher(PermissionsContainer::allow_all())
}
pub fn create_fetch_cacher(
&self,
- root_permissions: PermissionsContainer,
- dynamic_permissions: PermissionsContainer,
+ permissions: PermissionsContainer,
) -> cache::FetchCacher {
cache::FetchCacher::new(
self.emit_cache.clone(),
self.file_fetcher.clone(),
self.options.resolve_file_header_overrides(),
- root_permissions,
- dynamic_permissions,
+ permissions,
self.options.node_modules_dir_specifier(),
)
}
diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs
index 020dd5c08..5e5a3788a 100644
--- a/cli/lsp/testing/execution.rs
+++ b/cli/lsp/testing/execution.rs
@@ -226,7 +226,6 @@ impl TestRun {
Permissions::from_options(&ps.options.permissions_options())?;
test::check_specifiers(
&ps,
- permissions.clone(),
self
.queue
.iter()
diff --git a/cli/module_loader.rs b/cli/module_loader.rs
index 979898374..e4b8b616d 100644
--- a/cli/module_loader.rs
+++ b/cli/module_loader.rs
@@ -109,15 +109,12 @@ impl ModuleLoadPreparer {
roots: Vec<ModuleSpecifier>,
is_dynamic: bool,
lib: TsTypeLib,
- root_permissions: PermissionsContainer,
- dynamic_permissions: PermissionsContainer,
+ permissions: PermissionsContainer,
) -> Result<(), AnyError> {
log::debug!("Preparing module load.");
let _pb_clear_guard = self.progress_bar.clear_guard();
- let mut cache = self
- .module_graph_builder
- .create_fetch_cacher(root_permissions, dynamic_permissions);
+ let mut cache = self.module_graph_builder.create_fetch_cacher(permissions);
let maybe_imports = self.options.to_maybe_imports()?;
let graph_resolver = self.resolver.as_graph_resolver();
let graph_npm_resolver = self.resolver.as_graph_npm_resolver();
@@ -216,7 +213,6 @@ impl ModuleLoadPreparer {
false,
lib,
PermissionsContainer::allow_all(),
- PermissionsContainer::allow_all(),
)
.await
}
@@ -537,7 +533,6 @@ impl ModuleLoader for CliModuleLoader {
let specifier = specifier.clone();
let module_load_preparer = self.module_load_preparer.clone();
- let dynamic_permissions = self.dynamic_permissions.clone();
let root_permissions = if is_dynamic {
self.dynamic_permissions.clone()
} else {
@@ -547,13 +542,7 @@ impl ModuleLoader for CliModuleLoader {
async move {
module_load_preparer
- .prepare_module_load(
- vec![specifier],
- is_dynamic,
- lib,
- root_permissions,
- dynamic_permissions,
- )
+ .prepare_module_load(vec![specifier], is_dynamic, lib, root_permissions)
.await
}
.boxed_local()
diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs
index cc37cf523..d946b6a1c 100644
--- a/cli/tests/integration/run_tests.rs
+++ b/cli/tests/integration/run_tests.rs
@@ -2661,6 +2661,11 @@ mod permissions {
});
}
+ itest!(dynamic_import_static_analysis_no_permissions {
+ args: "run --quiet --reload --no-prompt dynamic_import/static_analysis_no_permissions.ts",
+ output: "dynamic_import/static_analysis_no_permissions.ts.out",
+ });
+
itest!(dynamic_import_permissions_remote_remote {
args: "run --quiet --reload --allow-net=localhost:4545 dynamic_import/permissions_remote_remote.ts",
output: "dynamic_import/permissions_remote_remote.ts.out",
diff --git a/cli/tests/testdata/dynamic_import/empty_1.ts b/cli/tests/testdata/dynamic_import/empty_1.ts
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/cli/tests/testdata/dynamic_import/empty_1.ts
diff --git a/cli/tests/testdata/dynamic_import/empty_2.ts b/cli/tests/testdata/dynamic_import/empty_2.ts
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/cli/tests/testdata/dynamic_import/empty_2.ts
diff --git a/cli/tests/testdata/dynamic_import/permissions_remote_remote.ts b/cli/tests/testdata/dynamic_import/permissions_remote_remote.ts
index 0033bcccc..65a254191 100644
--- a/cli/tests/testdata/dynamic_import/permissions_remote_remote.ts
+++ b/cli/tests/testdata/dynamic_import/permissions_remote_remote.ts
@@ -1,3 +1,3 @@
await import(
- "http://localhost:4545/dynamic_import/static_remote.ts"
+ "" + "http://localhost:4545/dynamic_import/static_remote.ts"
);
diff --git a/cli/tests/testdata/dynamic_import/static_analysis_no_permissions.ts b/cli/tests/testdata/dynamic_import/static_analysis_no_permissions.ts
new file mode 100644
index 000000000..de75ba87b
--- /dev/null
+++ b/cli/tests/testdata/dynamic_import/static_analysis_no_permissions.ts
@@ -0,0 +1,13 @@
+try {
+ await import("./empty_1.ts");
+ console.log("✅ Succeeded importing statically analyzable specifier");
+} catch {
+ console.log("❌ Failed importing statically analyzable specifier");
+}
+
+try {
+ await import("" + "./empty_2.ts");
+ console.log("❌ Succeeded importing non-statically analyzable specifier");
+} catch {
+ console.log("✅ Failed importing non-statically analyzable specifier");
+}
diff --git a/cli/tests/testdata/dynamic_import/static_analysis_no_permissions.ts.out b/cli/tests/testdata/dynamic_import/static_analysis_no_permissions.ts.out
new file mode 100644
index 000000000..ba9249ab0
--- /dev/null
+++ b/cli/tests/testdata/dynamic_import/static_analysis_no_permissions.ts.out
@@ -0,0 +1,2 @@
+✅ Succeeded importing statically analyzable specifier
+✅ Failed importing non-statically analyzable specifier
diff --git a/cli/tests/testdata/run/error_015_dynamic_import_permissions.js b/cli/tests/testdata/run/error_015_dynamic_import_permissions.js
index 73da56fd8..47961cf63 100644
--- a/cli/tests/testdata/run/error_015_dynamic_import_permissions.js
+++ b/cli/tests/testdata/run/error_015_dynamic_import_permissions.js
@@ -1,3 +1,3 @@
(async () => {
- await import("http://localhost:4545/subdir/mod4.js");
+ await import("" + "http://localhost:4545/subdir/mod4.js");
})();
diff --git a/cli/tests/testdata/run/error_015_dynamic_import_permissions.out b/cli/tests/testdata/run/error_015_dynamic_import_permissions.out
index ef54f331b..87ce43e9c 100644
--- a/cli/tests/testdata/run/error_015_dynamic_import_permissions.out
+++ b/cli/tests/testdata/run/error_015_dynamic_import_permissions.out
@@ -1,4 +1,4 @@
error: Uncaught (in promise) TypeError: Requires net access to "localhost:4545", run again with the --allow-net flag
- await import("http://localhost:4545/subdir/mod4.js");
+ await import("" + "http://localhost:4545/subdir/mod4.js");
^
at async file://[WILDCARD]/error_015_dynamic_import_permissions.js:2:3
diff --git a/cli/tests/testdata/workers/dynamic_remote.ts b/cli/tests/testdata/workers/dynamic_remote.ts
index 381c7f374..54e4a4714 100644
--- a/cli/tests/testdata/workers/dynamic_remote.ts
+++ b/cli/tests/testdata/workers/dynamic_remote.ts
@@ -1,2 +1,2 @@
// This file doesn't really exist, but it doesn't matter, a "PermissionsDenied" error should be thrown.
-await import("https://example.com/some/file.ts");
+await import("" + "https://example.com/some/file.ts");
diff --git a/cli/tests/testdata/workers/permissions_dynamic_remote.ts.out b/cli/tests/testdata/workers/permissions_dynamic_remote.ts.out
index cd1884c7e..91f3cc6d5 100644
--- a/cli/tests/testdata/workers/permissions_dynamic_remote.ts.out
+++ b/cli/tests/testdata/workers/permissions_dynamic_remote.ts.out
@@ -1,5 +1,5 @@
error: Uncaught (in worker "") (in promise) TypeError: Requires net access to "example.com", run again with the --allow-net flag
-await import("https://example.com/some/file.ts");
+await import("" + "https://example.com/some/file.ts");
^
at async http://localhost:4545/workers/dynamic_remote.ts:2:1
[WILDCARD]error: Uncaught (in promise) Error: Unhandled error in child worker.
diff --git a/cli/tools/bench.rs b/cli/tools/bench.rs
index 9930bcc77..962b1ac17 100644
--- a/cli/tools/bench.rs
+++ b/cli/tools/bench.rs
@@ -418,7 +418,6 @@ impl BenchReporter for ConsoleReporter {
/// Type check a collection of module and document specifiers.
async fn check_specifiers(
ps: &ProcState,
- permissions: Permissions,
specifiers: Vec<ModuleSpecifier>,
) -> Result<(), AnyError> {
let lib = ps.options.ts_type_lib_window();
@@ -428,10 +427,8 @@ async fn check_specifiers(
false,
lib,
PermissionsContainer::allow_all(),
- PermissionsContainer::new(permissions),
)
.await?;
-
Ok(())
}
@@ -654,7 +651,7 @@ pub async fn run_benchmarks(
return Err(generic_error("No bench modules found"));
}
- check_specifiers(&ps, permissions.clone(), specifiers.clone()).await?;
+ check_specifiers(&ps, specifiers.clone()).await?;
if bench_options.no_run {
return Ok(());
@@ -813,7 +810,7 @@ pub async fn run_benchmarks_with_watch(
.filter(|specifier| modules_to_reload.contains(specifier))
.collect::<Vec<ModuleSpecifier>>();
- check_specifiers(&ps, permissions.clone(), specifiers.clone()).await?;
+ check_specifiers(&ps, specifiers.clone()).await?;
if bench_options.no_run {
return Ok(());
diff --git a/cli/tools/test.rs b/cli/tools/test.rs
index 17d1cebf3..268f3b4b9 100644
--- a/cli/tools/test.rs
+++ b/cli/tools/test.rs
@@ -1230,7 +1230,6 @@ async fn fetch_inline_files(
/// Type check a collection of module and document specifiers.
pub async fn check_specifiers(
ps: &ProcState,
- permissions: Permissions,
specifiers: Vec<(ModuleSpecifier, TestMode)>,
) -> Result<(), AnyError> {
let lib = ps.options.ts_type_lib_window();
@@ -1265,7 +1264,6 @@ pub async fn check_specifiers(
false,
lib,
PermissionsContainer::new(Permissions::allow_all()),
- PermissionsContainer::new(permissions.clone()),
)
.await?;
}
@@ -1287,7 +1285,6 @@ pub async fn check_specifiers(
false,
lib,
PermissionsContainer::allow_all(),
- PermissionsContainer::new(permissions),
)
.await?;
@@ -1648,8 +1645,7 @@ pub async fn run_tests(
return Err(generic_error("No test modules found"));
}
- check_specifiers(&ps, permissions.clone(), specifiers_with_mode.clone())
- .await?;
+ check_specifiers(&ps, specifiers_with_mode.clone()).await?;
if test_options.no_run {
return Ok(());
@@ -1821,8 +1817,7 @@ pub async fn run_tests_with_watch(
.filter(|(specifier, _)| modules_to_reload.contains(specifier))
.collect::<Vec<(ModuleSpecifier, TestMode)>>();
- check_specifiers(&ps, permissions.clone(), specifiers_with_mode.clone())
- .await?;
+ check_specifiers(&ps, specifiers_with_mode.clone()).await?;
if test_options.no_run {
return Ok(());