Skip to content

Commit

Permalink
Silently ignore absolute paths without base (fixes #320) (#338)
Browse files Browse the repository at this point in the history
  • Loading branch information
mre authored Sep 20, 2021
1 parent 712bdfa commit 3b41c4c
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 31 deletions.
18 changes: 18 additions & 0 deletions lychee-bin/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,4 +474,22 @@ mod cli {

Ok(())
}

/// If base-dir is not set, don't throw an error in case we encounter
/// an absolute local link within a file (e.g. `/about`).
#[test]
fn test_ignore_absolute_local_links_without_base() -> Result<()> {
let mut cmd = main_command();

let offline_dir = fixtures_path().join("offline");

cmd.arg("--offline")
.arg(&offline_dir.join("index.html"))
.env_clear()
.assert()
.success()
.stdout(contains("Total............0"));

Ok(())
}
}
23 changes: 17 additions & 6 deletions lychee-lib/src/extract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,14 @@ pub(crate) fn extract_links(
// Silently ignore anchor links for now
continue;
}
let url = create_uri_from_path(root, base, &link)?;
Request::new(Uri { url }, input_content.input.clone())
match create_uri_from_path(root, base, &link)? {
Some(url) => Request::new(Uri { url }, input_content.input.clone()),
None => {
// In case we cannot create a URI from a path but we didn't receive an error,
// it means that some preconditions were not met, e.g. the `base_url` wasn't set.
continue;
}
}
} else {
info!("Handling of {} not implemented yet", &link);
continue;
Expand Down Expand Up @@ -123,7 +129,7 @@ fn extract_links_from_plaintext(input: &str) -> Vec<String> {
.collect()
}

fn create_uri_from_path(src: &Path, base: &Option<Base>, dst: &str) -> Result<Url> {
fn create_uri_from_path(src: &Path, base: &Option<Base>, dst: &str) -> Result<Option<Url>> {
let dst = url::remove_get_params_and_fragment(dst);
// Avoid double-encoding already encoded destination paths by removing any
// potential encoding (e.g. `web%20site` becomes `web site`).
Expand All @@ -135,8 +141,13 @@ fn create_uri_from_path(src: &Path, base: &Option<Base>, dst: &str) -> Result<Ur
// `from_file_path` at the moment) while `dst` is left untouched and simply
// appended to the end.
let decoded = percent_decode_str(dst).decode_utf8()?.to_string();
let path = path::resolve(src, &PathBuf::from(decoded), base)?;
Url::from_file_path(&path).map_err(|_e| ErrorKind::InvalidUrl(path))
let resolved = path::resolve(src, &PathBuf::from(decoded), base)?;
match resolved {
Some(path) => Url::from_file_path(&path)
.map(Some)
.map_err(|_e| ErrorKind::InvalidUrl(path)),
None => Ok(None),
}
}

#[cfg(test)]
Expand Down Expand Up @@ -166,7 +177,7 @@ mod test {
fn test_create_uri_from_path() {
let result =
create_uri_from_path(&PathBuf::from("/README.md"), &None, "test+encoding").unwrap();
assert_eq!(result.as_str(), "file:///test+encoding");
assert_eq!(result.unwrap().as_str(), "file:///test+encoding");
}

fn load_fixture(filename: &str) -> String {
Expand Down
55 changes: 30 additions & 25 deletions lychee-lib/src/helpers/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,30 @@ fn dirname(src: &Path) -> PathBuf {
}

// Resolve `dst` that was linked to from within `src`
pub(crate) fn resolve(src: &Path, dst: &Path, base: &Option<Base>) -> Result<PathBuf> {
if dst.is_relative() {
// Find `dst` in the parent directory of `src`
if let Some(parent) = src.parent() {
let rel_path = parent.join(dst.to_path_buf());
return absolute_path(&rel_path);
// Returns Ok(None) in case of an absolute local link without a `base_url`
pub(crate) fn resolve(src: &Path, dst: &Path, base: &Option<Base>) -> Result<Option<PathBuf>> {
let resolved = match dst {
relative if dst.is_relative() => {
// Find `dst` in the parent directory of `src`
let parent = match src.parent() {
Some(parent) => parent,
None => return Err(ErrorKind::FileNotFound(relative.to_path_buf())),
};
parent.join(relative.to_path_buf())
}
}
if dst.is_absolute() {
// Absolute local links (leading slash) require the `base_url` to
// define the document root.
let base = get_base_dir(base).ok_or_else(|| {
ErrorKind::InvalidBase(
"<empty>".to_string(),
format!("Found absolute local link {:?} but no base directory was set. Set with `--base`.", dst)
)
})?;
let abs_path = join(dirname(&base), dst);
return absolute_path(&abs_path);
}
Err(ErrorKind::FileNotFound(dst.to_path_buf()))
absolute if dst.is_absolute() => {
// Absolute local links (leading slash) require the `base_url` to
// define the document root. Silently ignore the link in case we
// `base_url` is not defined.
let base = match get_base_dir(base) {
Some(path) => path,
None => return Ok(None),
};
join(dirname(&base), absolute)
}
_ => return Err(ErrorKind::FileNotFound(dst.to_path_buf())),
};
Ok(Some(absolute_path(&resolved)?))
}

// A cumbersome way to concatenate paths without checking their
Expand All @@ -79,7 +82,7 @@ mod test_path {
let abs_path = PathBuf::from("./foo.html");
assert_eq!(
resolve(&dummy, &abs_path, &None)?,
env::current_dir()?.join("foo.html")
Some(env::current_dir()?.join("foo.html"))
);
Ok(())
}
Expand All @@ -92,7 +95,7 @@ mod test_path {
let abs_path = PathBuf::from("./foo.html");
assert_eq!(
resolve(&dummy, &abs_path, &None)?,
env::current_dir()?.join("foo.html")
Some(env::current_dir()?.join("foo.html"))
);
Ok(())
}
Expand All @@ -105,7 +108,7 @@ mod test_path {
let abs_path = PathBuf::from("./foo.html");
assert_eq!(
resolve(&abs_index, &abs_path, &None)?,
PathBuf::from("/path/to/foo.html")
Some(PathBuf::from("/path/to/foo.html"))
);
Ok(())
}
Expand All @@ -120,7 +123,7 @@ mod test_path {
let base = Some(Base::Local(PathBuf::from("/some/absolute/base/dir")));
assert_eq!(
resolve(&dummy, &abs_path, &base)?,
PathBuf::from("/some/absolute/base/dir/foo.html")
Some(PathBuf::from("/some/absolute/base/dir/foo.html"))
);
Ok(())
}
Expand All @@ -134,7 +137,9 @@ mod test_path {
let base = Some(Base::Local(PathBuf::from("/some/absolute/base/dir")));
assert_eq!(
resolve(&abs_index, &abs_path, &base)?,
PathBuf::from("/some/absolute/base/dir/other/path/to/foo.html")
Some(PathBuf::from(
"/some/absolute/base/dir/other/path/to/foo.html"
))
);
Ok(())
}
Expand Down

0 comments on commit 3b41c4c

Please sign in to comment.