Skip to content

Commit

Permalink
fix: ModuleSpecifier removes relative path parts (denoland#6762)
Browse files Browse the repository at this point in the history
  • Loading branch information
bartlomieju committed Jul 16, 2020
1 parent b0f2bd4 commit 98e0ed5
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 12 deletions.
2 changes: 2 additions & 0 deletions cli/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ pub fn write_file_2<T: AsRef<[u8]>>(
file.write_all(data.as_ref())
}

/// IMPORTANT: This method is duplicated in core/module_specifier.rs
///
/// Normalize all itermediate components of the path (ie. remove "./" and "../" components).
/// Similar to `fs::canonicalize()` but doesn't resolve symlinks.
///
Expand Down
11 changes: 1 addition & 10 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,16 +440,7 @@ async fn bundle_command(
source_file: String,
out_file: Option<PathBuf>,
) -> Result<(), ErrBox> {
let mut module_specifier =
ModuleSpecifier::resolve_url_or_path(&source_file)?;
let url = module_specifier.as_url();

// TODO(bartlomieju): fix this hack in ModuleSpecifier
if url.scheme() == "file" {
let a = deno_fs::normalize_path(&url.to_file_path().unwrap());
let u = Url::from_file_path(a).unwrap();
module_specifier = ModuleSpecifier::from(u)
}
let module_specifier = ModuleSpecifier::resolve_url_or_path(&source_file)?;

debug!(">>>>> bundle START");
let global_state = GlobalState::new(flags)?;
Expand Down
67 changes: 65 additions & 2 deletions core/module_specifier.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::env::current_dir;
use std::error::Error;
use std::fmt;
use std::path::Component;
use std::path::Path;
use std::path::PathBuf;
use url::ParseError;
use url::Url;
Expand Down Expand Up @@ -150,6 +152,7 @@ impl ModuleSpecifier {
path_str: &str,
) -> Result<ModuleSpecifier, ModuleResolutionError> {
let path = current_dir().unwrap().join(path_str);
let path = normalize_path(&path);
Url::from_file_path(path.clone())
.map(ModuleSpecifier)
.map_err(|()| ModuleResolutionError::InvalidPath(path))
Expand Down Expand Up @@ -203,6 +206,39 @@ impl PartialEq<String> for ModuleSpecifier {
}
}

/// Normalize all itermediate components of the path (ie. remove "./" and "../" components).
/// Similar to `fs::canonicalize()` but doesn't resolve symlinks.
///
/// Taken from Cargo
/// https://github.com/rust-lang/cargo/blob/af307a38c20a753ec60f0ad18be5abed3db3c9ac/src/cargo/util/paths.rs#L60-L85
pub fn normalize_path(path: &Path) -> PathBuf {
let mut components = path.components().peekable();
let mut ret =
if let Some(c @ Component::Prefix(..)) = components.peek().cloned() {
components.next();
PathBuf::from(c.as_os_str())
} else {
PathBuf::new()
};

for component in components {
match component {
Component::Prefix(..) => unreachable!(),
Component::RootDir => {
ret.push(component.as_os_str());
}
Component::CurDir => {}
Component::ParentDir => {
ret.pop();
}
Component::Normal(c) => {
ret.push(c);
}
}
}
ret
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -415,7 +451,12 @@ mod tests {
);
tests.extend(vec![
(r"/deno/tests/006_url_imports.ts", expected_url.to_string()),
(r"\deno\tests\006_url_imports.ts", expected_url),
(r"\deno\tests\006_url_imports.ts", expected_url.to_string()),
(
r"\deno\..\deno\tests\006_url_imports.ts",
expected_url.to_string(),
),
(r"\deno\.\tests\006_url_imports.ts", expected_url),
]);

// Relative local path.
Expand Down Expand Up @@ -450,7 +491,12 @@ mod tests {
let expected_url = format!("file:https://{}/tests/006_url_imports.ts", cwd_str);
tests.extend(vec![
("tests/006_url_imports.ts", expected_url.to_string()),
("./tests/006_url_imports.ts", expected_url),
("./tests/006_url_imports.ts", expected_url.to_string()),
(
"tests/../tests/006_url_imports.ts",
expected_url.to_string(),
),
("tests/./006_url_imports.ts", expected_url),
]);
}

Expand Down Expand Up @@ -511,4 +557,21 @@ mod tests {
assert_eq!(result, expected);
}
}

#[test]
fn test_normalize_path() {
assert_eq!(normalize_path(Path::new("a/../b")), PathBuf::from("b"));
assert_eq!(normalize_path(Path::new("a/./b/")), PathBuf::from("a/b/"));
assert_eq!(
normalize_path(Path::new("a/./b/../c")),
PathBuf::from("a/c")
);

if cfg!(windows) {
assert_eq!(
normalize_path(Path::new("C:\\a\\.\\b\\..\\c")),
PathBuf::from("C:\\a\\c")
);
}
}
}

0 comments on commit 98e0ed5

Please sign in to comment.