Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not panic running invalid file specifier #25530

Merged
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
641523d
fix
yazan-abdalrahman Sep 9, 2024
fa9feea
Merge branch 'main' into handle-invalid-path-error-#20062
yazan-abdalrahman Sep 9, 2024
e8f7211
fix
yazan-abdalrahman Sep 9, 2024
105ffb2
Merge remote-tracking branch 'origin/handle-invalid-path-error-#20062…
yazan-abdalrahman Sep 9, 2024
817319e
fix lint
yazan-abdalrahman Sep 9, 2024
59987b4
fix fmt
yazan-abdalrahman Sep 9, 2024
1334392
Merge branch 'main' into handle-invalid-path-error-#20062
yazan-abdalrahman Sep 9, 2024
ceea466
fix test
yazan-abdalrahman Sep 10, 2024
cd4d31b
Merge remote-tracking branch 'origin/handle-invalid-path-error-#20062…
yazan-abdalrahman Sep 10, 2024
727276b
Merge branch 'main' into handle-invalid-path-error-#20062
yazan-abdalrahman Sep 10, 2024
6a7b0f0
fix lint
yazan-abdalrahman Sep 10, 2024
1bc0db7
Merge remote-tracking branch 'origin/handle-invalid-path-error-#20062…
yazan-abdalrahman Sep 10, 2024
c4ec979
fix test
yazan-abdalrahman Sep 10, 2024
50ab8b3
fix test
yazan-abdalrahman Sep 10, 2024
e8fdc89
fix test
yazan-abdalrahman Sep 10, 2024
364d6cf
fix test
yazan-abdalrahman Sep 10, 2024
8ee44f2
fix test
yazan-abdalrahman Sep 10, 2024
312881f
Merge branch 'main' into handle-invalid-path-error-#20062
yazan-abdalrahman Sep 11, 2024
6b44a6f
fix test
yazan-abdalrahman Sep 11, 2024
e598bbc
Merge branch 'main' into handle-invalid-path-error-#20062
yazan-abdalrahman Sep 11, 2024
0834561
Trigger Build
yazan-abdalrahman Sep 11, 2024
ab72394
Merge remote-tracking branch 'origin/handle-invalid-path-error-#20062…
yazan-abdalrahman Sep 11, 2024
5b71f1b
fix test
yazan-abdalrahman Sep 12, 2024
b3776c4
Merge branch 'main' into handle-invalid-path-error-#20062
yazan-abdalrahman Sep 12, 2024
89ff3e0
Merge branch 'main' into handle-invalid-path-error-#20062
yazan-abdalrahman Sep 16, 2024
68a5d15
new fix
yazan-abdalrahman Sep 17, 2024
fb5c69c
Merge branch 'main' of https://github.com/yazan-abdalrahman/deno into…
yazan-abdalrahman Sep 17, 2024
2415d40
resolve conflict
yazan-abdalrahman Sep 17, 2024
bb0e521
lint
yazan-abdalrahman Sep 17, 2024
e9aa1ae
Trigger Build
yazan-abdalrahman Sep 17, 2024
2dded62
Merge branch 'main' into handle-invalid-path-error-#20062
dsherret Sep 17, 2024
b24f967
fix merge
dsherret Sep 17, 2024
4422694
reverts
dsherret Sep 17, 2024
2e24eaf
another revert
dsherret Sep 17, 2024
2390d3e
fix check_invalid_specifiers test
yazan-abdalrahman Sep 18, 2024
3b06919
Merge branch 'main' into handle-invalid-path-error-#20062
yazan-abdalrahman Sep 18, 2024
904b630
fix handle_invalid_path_error
yazan-abdalrahman Sep 18, 2024
b1b0f4f
Merge remote-tracking branch 'origin/handle-invalid-path-error-#20062…
yazan-abdalrahman Sep 18, 2024
459d96b
fmt
yazan-abdalrahman Sep 18, 2024
c31344f
fix
yazan-abdalrahman Sep 18, 2024
234b3f0
Merge branch 'main' into handle-invalid-path-error-#20062
yazan-abdalrahman Sep 18, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use deno_core::url::Url;
use deno_graph::GraphKind;
use deno_runtime::deno_permissions::parse_sys_kind;
use deno_runtime::deno_permissions::PermissionsOptions;
use deno_runtime::fs_util::specifier_to_file_path;
use log::debug;
use log::Level;
use serde::Deserialize;
Expand Down Expand Up @@ -916,7 +917,7 @@ impl Flags {
if module_specifier.scheme() == "file"
|| module_specifier.scheme() == "npm"
{
if let Ok(p) = module_specifier.to_file_path() {
if let Ok(p) = specifier_to_file_path(&module_specifier) {
Some(vec![p.parent().unwrap().to_path_buf()])
} else {
Some(vec![current_dir.to_path_buf()])
Expand Down
2 changes: 1 addition & 1 deletion cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use deno_graph::source::LoaderChecksum;
use deno_graph::JsrLoadError;
use deno_graph::ModuleLoadError;
use deno_graph::WorkspaceFastCheckOption;
use deno_runtime::fs_util::specifier_to_file_path;

use deno_core::error::custom_error;
use deno_core::error::AnyError;
Expand All @@ -42,6 +41,7 @@ use deno_graph::ResolutionError;
use deno_graph::SpecifierError;
use deno_runtime::deno_fs::FileSystem;
use deno_runtime::deno_node;
use deno_runtime::fs_util::specifier_to_file_path;
use deno_semver::jsr::JsrDepPackageReq;
use deno_semver::package::PackageNv;
use deno_semver::Version;
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use super::urls::url_to_uri;
use crate::args::jsr_url;
use crate::tools::lint::CliLinter;
use deno_lint::diagnostic::LintDiagnosticRange;
use deno_runtime::fs_util::specifier_to_file_path;

use deno_ast::SourceRange;
use deno_ast::SourceRangedForSpanned;
Expand All @@ -25,6 +24,7 @@ use deno_core::serde_json;
use deno_core::serde_json::json;
use deno_core::ModuleSpecifier;
use deno_runtime::deno_node::PathClean;
use deno_runtime::fs_util::specifier_to_file_path;
use deno_semver::jsr::JsrPackageNvReference;
use deno_semver::jsr::JsrPackageReqReference;
use deno_semver::npm::NpmPackageReqReference;
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ use crate::cache::LocalLspHttpCache;
use crate::lsp::config::Config;
use crate::lsp::logging::lsp_log;
use crate::lsp::logging::lsp_warn;
use deno_runtime::fs_util::specifier_to_file_path;

use deno_core::url::Url;
use deno_core::ModuleSpecifier;
use deno_runtime::fs_util::specifier_to_file_path;
use std::collections::BTreeMap;
use std::fs;
use std::path::Path;
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use crate::util::path::relative_specifier;
use deno_graph::source::ResolutionMode;
use deno_graph::Range;
use deno_runtime::deno_node::SUPPORTED_BUILTIN_NODE_MODULES;
use deno_runtime::fs_util::specifier_to_file_path;

use deno_ast::LineAndColumnIndex;
use deno_ast::SourceTextInfo;
Expand All @@ -30,6 +29,7 @@ use deno_core::serde::Serialize;
use deno_core::serde_json::json;
use deno_core::url::Position;
use deno_core::ModuleSpecifier;
use deno_runtime::fs_util::specifier_to_file_path;
use deno_semver::jsr::JsrPackageReqReference;
use deno_semver::package::PackageNv;
use import_map::ImportMap;
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use super::tsc;
use super::tsc::AssetDocument;

use crate::graph_util::CliJsrUrlProvider;
use deno_runtime::fs_util::specifier_to_file_path;

use dashmap::DashMap;
use deno_ast::swc::visit::VisitWith;
Expand All @@ -28,6 +27,7 @@ use deno_core::ModuleSpecifier;
use deno_graph::source::ResolutionMode;
use deno_graph::Resolution;
use deno_runtime::deno_node;
use deno_runtime::fs_util::specifier_to_file_path;
use deno_semver::jsr::JsrPackageReqReference;
use deno_semver::npm::NpmPackageReqReference;
use deno_semver::package::PackageReq;
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use deno_graph::GraphKind;
use deno_graph::Resolution;
use deno_runtime::deno_tls::rustls::RootCertStore;
use deno_runtime::deno_tls::RootCertStoreProvider;
use deno_runtime::fs_util::specifier_to_file_path;
use deno_semver::jsr::JsrPackageReqReference;
use indexmap::Equivalent;
use indexmap::IndexSet;
Expand Down Expand Up @@ -112,7 +113,6 @@ use crate::util::fs::remove_dir_all_if_exists;
use crate::util::path::is_importable_ext;
use crate::util::path::to_percent_decoded_str;
use crate::util::sync::AsyncFlag;
use deno_runtime::fs_util::specifier_to_file_path;

struct LspRootCertStoreProvider(RootCertStore);

Expand Down
46 changes: 23 additions & 23 deletions cli/lsp/resolver.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,5 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use crate::args::create_default_npmrc;
use crate::args::CacheSetting;
use crate::args::CliLockfile;
use crate::args::NpmInstallDepsProvider;
use crate::graph_util::CliJsrUrlProvider;
use crate::http_util::HttpClientProvider;
use crate::lsp::config::Config;
use crate::lsp::config::ConfigData;
use crate::lsp::logging::lsp_warn;
use crate::npm::create_cli_npm_resolver_for_lsp;
use crate::npm::CliNpmResolver;
use crate::npm::CliNpmResolverByonmCreateOptions;
use crate::npm::CliNpmResolverCreateOptions;
use crate::npm::CliNpmResolverManagedCreateOptions;
use crate::npm::CliNpmResolverManagedSnapshotOption;
use crate::npm::ManagedCliNpmResolver;
use crate::resolver::CjsResolutionStore;
use crate::resolver::CliGraphResolver;
use crate::resolver::CliGraphResolverOptions;
use crate::resolver::CliNodeResolver;
use crate::resolver::WorkerCliNpmGraphResolver;
use crate::util::progress_bar::ProgressBar;
use crate::util::progress_bar::ProgressBarStyle;
use dashmap::DashMap;
use deno_ast::MediaType;
use deno_cache_dir::HttpCache;
Expand Down Expand Up @@ -55,6 +32,29 @@ use std::sync::Arc;

use super::cache::LspCache;
use super::jsr::JsrCacheResolver;
use crate::args::create_default_npmrc;
use crate::args::CacheSetting;
use crate::args::CliLockfile;
use crate::args::NpmInstallDepsProvider;
use crate::graph_util::CliJsrUrlProvider;
use crate::http_util::HttpClientProvider;
use crate::lsp::config::Config;
use crate::lsp::config::ConfigData;
use crate::lsp::logging::lsp_warn;
use crate::npm::create_cli_npm_resolver_for_lsp;
use crate::npm::CliNpmResolver;
use crate::npm::CliNpmResolverByonmCreateOptions;
use crate::npm::CliNpmResolverCreateOptions;
use crate::npm::CliNpmResolverManagedCreateOptions;
use crate::npm::CliNpmResolverManagedSnapshotOption;
use crate::npm::ManagedCliNpmResolver;
use crate::resolver::CjsResolutionStore;
use crate::resolver::CliGraphResolver;
use crate::resolver::CliGraphResolverOptions;
use crate::resolver::CliNodeResolver;
use crate::resolver::WorkerCliNpmGraphResolver;
use crate::util::progress_bar::ProgressBar;
use crate::util::progress_bar::ProgressBarStyle;

#[derive(Debug, Clone)]
struct LspScopeResolver {
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ use deno_core::convert::ToV8;
use deno_core::error::StdAnyError;
use deno_core::futures::stream::FuturesOrdered;
use deno_core::futures::StreamExt;
use deno_runtime::fs_util::specifier_to_file_path;

use dashmap::DashMap;
use deno_ast::MediaType;
Expand All @@ -63,6 +62,7 @@ use deno_core::ModuleSpecifier;
use deno_core::OpState;
use deno_core::PollEventLoopOptions;
use deno_core::RuntimeOptions;
use deno_runtime::fs_util::specifier_to_file_path;
use deno_runtime::inspector_server::InspectorServer;
use deno_runtime::tokio_util::create_basic_runtime;
use indexmap::IndexMap;
Expand Down
2 changes: 1 addition & 1 deletion cli/npm/byonm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use deno_runtime::deno_node::NodePermissions;
use deno_runtime::deno_node::NodeRequireResolver;
use deno_runtime::deno_node::NpmProcessStateProvider;
use deno_runtime::deno_node::PackageJson;
use deno_runtime::fs_util::specifier_to_file_path;
use deno_semver::package::PackageReq;
use deno_semver::Version;
use node_resolver::errors::PackageFolderResolveError;
Expand All @@ -28,7 +29,6 @@ use node_resolver::NpmResolver;
use crate::args::NpmProcessState;
use crate::args::NpmProcessStateKind;
use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs;
use deno_runtime::fs_util::specifier_to_file_path;

use super::managed::normalize_pkg_name_for_node_modules_deno_folder;
use super::CliNpmResolver;
Expand Down
67 changes: 3 additions & 64 deletions runtime/fs_util.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use deno_ast::ModuleSpecifier;
use deno_core::anyhow::Context;
use deno_core::error::uri_error;
use deno_core::error::AnyError;
pub use deno_core::normalize_path;
use std::path::Path;
use std::path::PathBuf;

pub use deno_core::normalize_path;
pub use deno_permissions::specifier_to_file_path;

#[inline]
pub fn resolve_from_cwd(path: &Path) -> Result<PathBuf, AnyError> {
if path.is_absolute() {
Expand All @@ -20,49 +20,6 @@ pub fn resolve_from_cwd(path: &Path) -> Result<PathBuf, AnyError> {
}
}

/// Attempts to convert a specifier to a file path. By default, uses the Url
/// crate's `to_file_path()` method, but falls back to try and resolve unix-style
/// paths on Windows.
pub fn specifier_to_file_path(
specifier: &ModuleSpecifier,
) -> Result<PathBuf, AnyError> {
let result = if specifier.scheme() != "file" {
Err(())
} else if cfg!(windows) {
match specifier.to_file_path() {
Ok(path) => Ok(path),
Err(()) => {
// This might be a unix-style path which is used in the tests even on Windows.
// Attempt to see if we can convert it to a `PathBuf`. This code should be removed
// once/if https://github.com/servo/rust-url/issues/730 is implemented.
if specifier.scheme() == "file"
&& specifier.host().is_none()
&& specifier.port().is_none()
&& specifier.path_segments().is_some()
{
let path_str = specifier.path();
match String::from_utf8(
percent_encoding::percent_decode(path_str.as_bytes()).collect(),
) {
Ok(path_str) => Ok(PathBuf::from(path_str)),
Err(_) => Err(()),
}
} else {
Err(())
}
}
}
} else {
specifier.to_file_path()
};
match result {
Ok(path) => Ok(path),
Err(()) => Err(uri_error(format!(
"Invalid file path.\n Specifier: {specifier}"
))),
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -114,22 +71,4 @@ mod tests {
let absolute_expected = cwd.join(expected);
assert_eq!(resolve_from_cwd(expected).unwrap(), absolute_expected);
}

#[test]
fn test_specifier_to_file_path() {
run_success_test("file:///", "/");
run_success_test("file:///test", "/test");
run_success_test("file:///dir/test/test.txt", "/dir/test/test.txt");
run_success_test(
"file:///dir/test%20test/test.txt",
"/dir/test test/test.txt",
);

fn run_success_test(specifier: &str, expected_path: &str) {
let result =
specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap())
.unwrap();
assert_eq!(result, PathBuf::from(expected_path));
}
}
}
1 change: 1 addition & 0 deletions runtime/permissions/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ fqdn = "0.3.4"
libc.workspace = true
log.workspace = true
once_cell.workspace = true
percent-encoding = { version = "2.3.1", features = [] }
serde.workspace = true
which.workspace = true

Expand Down
Loading
Loading