mirror of
https://github.com/crate-ci/typos.git
synced 2026-10-17 01:25:12 +02:00
fix(cli): Don't panic when canonicalize fails mid-walk
`walk_entry` canonicalizes each entry's path to look up its policy, and routes a failure through `report_result`, which reports the error and then substitutes `T::default()` -- for a `PathBuf` that is the empty path. `ConfigEngine` never saw that path during the walk, so the `policy()` call immediately after aborts the process: a debug build trips the `debug_assert!` on `path.is_absolute()`, a release build reaches the `expect` below it. Skip the entry after reporting the error instead, matching how a walk error is already handled a few lines above. A file removed by another process mid-scan is enough to reach this, so an abort here takes out an otherwise healthy run. Fixes #1444 and Closes #1577 Co-authored-by: Alexander Kireev <ak.chatman.media@gmail.com> Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
This commit is contained in:
co-authored by
Alexander Kireev
parent
25761c7bf3
commit
160d792805
Generated
+8
-1
@@ -960,7 +960,14 @@ fn walk_entry(
|
||||
(path, cwd)
|
||||
} else {
|
||||
let path = entry.path();
|
||||
let abs_path = report_result(path.canonicalize(), Some(path), reporter)?;
|
||||
let abs_path = match path.canonicalize() {
|
||||
Ok(abs_path) => abs_path,
|
||||
Err(err) => {
|
||||
report_error(err, Some(path), reporter)?;
|
||||
// Avoid a failed `engine.policy` lookup
|
||||
return Ok(());
|
||||
}
|
||||
};
|
||||
(path, abs_path)
|
||||
};
|
||||
let policy = engine.policy(&lookup_path);
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
//! Regression test for <https://github.com/crate-ci/typos/issues/1444>
|
||||
//!
|
||||
//! `walk_entry` canonicalizes each entry's path to look up its policy. When that fails,
|
||||
//! `report_result` reports the error and then hands back `PathBuf::default()`, an empty
|
||||
//! `walk_entry` canonicalizes each entry's path to look up its policy. When that failed,
|
||||
//! `report_result` reported the error and then handed back `PathBuf::default()`, an empty
|
||||
//! path that is never a key in `ConfigEngine`'s directory map, so the `policy()` call
|
||||
//! right after it panics with `` `walk()` should be called first ``.
|
||||
//! right after it panicked with `` `walk()` should be called first ``.
|
||||
#![cfg(unix)]
|
||||
|
||||
struct CollectingReporter {
|
||||
@@ -54,27 +54,18 @@ fn walk_path_reports_file_removed_mid_walk() {
|
||||
});
|
||||
let reporter = CollectingReporter::new();
|
||||
|
||||
let payload = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
||||
typos_cli::file::walk_path(
|
||||
builder.build(),
|
||||
&typos_cli::file::Typos,
|
||||
&engine,
|
||||
&reporter,
|
||||
false,
|
||||
)
|
||||
}))
|
||||
.expect_err("walk_path should panic on the failed policy lookup");
|
||||
let message = payload
|
||||
.downcast_ref::<String>()
|
||||
.map(String::as_str)
|
||||
.or_else(|| payload.downcast_ref::<&str>().copied());
|
||||
// `policy()` checks the path with a `debug_assert!` before reaching its `expect`.
|
||||
let expected = if cfg!(debug_assertions) {
|
||||
" is not absolute"
|
||||
} else {
|
||||
"`walk()` should be called first"
|
||||
};
|
||||
assert_eq!(message, Some(expected));
|
||||
let result = typos_cli::file::walk_path(
|
||||
builder.build(),
|
||||
&typos_cli::file::Typos,
|
||||
&engine,
|
||||
&reporter,
|
||||
false,
|
||||
);
|
||||
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"walk_path should not surface an ignore::Error: {result:?}"
|
||||
);
|
||||
assert!(
|
||||
!reporter.errors.lock().unwrap().is_empty(),
|
||||
"expected the vanished file's canonicalize() failure to be reported"
|
||||
|
||||
Reference in New Issue
Block a user