From d8cd470281d752abf095e650d15e14daa2551bc1 Mon Sep 17 00:00:00 2001 From: ChanTsune <41658782+ChanTsune@users.noreply.github.com> Date: Sun, 8 Feb 2026 13:54:08 +0900 Subject: [PATCH 1/2] :boom: Skip entries whose -s substitution yields empty path Add empty-path detection to PathnameEditor so that entries whose pathname becomes empty (or `.`-only) after substitution or stripping are skipped, matching bsdtar behavior. - Add `is_effectively_empty_path` guard to `edit_entry_name` and `edit_hardlink` (checked after transform and after strip) - Log skipped entries at debug level for diagnostics - Document why `edit_symlink` intentionally lacks the same guard --- cli/src/command/core.rs | 4 +- cli/src/command/core/path.rs | 61 ++++++++++++++-------- cli/tests/cli/stdio.rs | 1 + cli/tests/cli/stdio/option_substitution.rs | 46 ++++++++++++++++ 4 files changed, 89 insertions(+), 23 deletions(-) create mode 100644 cli/tests/cli/stdio/option_substitution.rs diff --git a/cli/src/command/core.rs b/cli/src/command/core.rs index f43567f02..600f304ec 100644 --- a/cli/src/command/core.rs +++ b/cli/src/command/core.rs @@ -851,11 +851,13 @@ pub(crate) fn create_entry( metadata, } = item; let Some(entry_name) = pathname_editor.edit_entry_name(path) else { + log::debug!("Skip: {}", path.display()); return Ok(None); }; match store_as { StoreAs::Hardlink(source) => { let Some(reference) = pathname_editor.edit_hardlink(source) else { + log::debug!("Skip: {}", path.display()); return Ok(None); }; let entry = EntryBuilder::new_hard_link(entry_name, reference)?; @@ -1688,7 +1690,7 @@ fn transform_normal_entry( // Apply path transformation let original_name = entry.header().path(); let Some(new_name) = pathname_editor.edit_entry_name(original_name.as_ref()) else { - // Entry path was stripped away entirely + log::debug!("Skip: {original_name}"); return Ok(None); }; diff --git a/cli/src/command/core/path.rs b/cli/src/command/core/path.rs index 3f89ab7c6..34bfb761d 100644 --- a/cli/src/command/core/path.rs +++ b/cli/src/command/core/path.rs @@ -1,7 +1,7 @@ use pna::{EntryName, EntryReference}; use std::{ borrow::Cow, - path::{Path, PathBuf}, + path::{Component, Path, PathBuf}, }; use super::PathTransformers; @@ -19,11 +19,6 @@ pub(crate) struct PathnameEditor { } impl PathnameEditor { - /// Create a PathnameEditor with the given configuration. - /// - /// `strip_components` sets the number of leading path components to remove when editing paths (use `None` to disable stripping). - /// `transformers` supplies optional path transformations to apply before stripping. - /// `absolute_paths` controls whether absolute paths are preserved (true) or sanitized (false). #[inline] pub(crate) const fn new( strip_components: Option, @@ -37,9 +32,10 @@ impl PathnameEditor { } } - /// Edit the pathname for a regular archive entry using the editor's configured transformers, strip count, and absolute-path policy. + /// Edit the pathname for a regular archive entry. /// - /// The method applies any path transformations first (bsdtar order), then strips the configured number of leading components; if stripping removes the entire path, the entry is skipped. The resulting path is converted to an `EntryName` that preserves a root component, and is sanitized unless the editor is configured to preserve absolute paths. + /// Returns `None` (skip the entry) when the path becomes empty after + /// transformation or after stripping. pub(crate) fn edit_entry_name(&self, path: &Path) -> Option { // bsdtar order: substitution first, then strip let transformed: Cow<'_, Path> = if let Some(t) = &self.transformers { @@ -47,7 +43,13 @@ impl PathnameEditor { } else { Cow::Borrowed(path) }; + if is_effectively_empty_path(&transformed) { + return None; + } let stripped = strip_components(&transformed, self.strip_components)?; + if is_effectively_empty_path(&stripped) { + return None; + } let entry_name = EntryName::from_path_lossy_preserve_root(&stripped); if self.absolute_paths { Some(entry_name) @@ -56,19 +58,10 @@ impl PathnameEditor { } } - /// Edit a hardlink target pathname according to the editor's configuration. - /// - /// The method applies any configured path transformers, then strips the configured - /// number of leading components (if set). The resulting path is converted to an - /// `EntryReference` (preserving a leading root if present). If the editor is - /// configured to preserve absolute paths, the preserved `EntryReference` is - /// returned; otherwise a sanitized `EntryReference` is returned. If stripping - /// removes all components, `None` is returned to indicate the entry should be skipped. + /// Edit a hardlink target pathname. /// - /// # Returns - /// - /// `Some(EntryReference)` with the edited reference, or `None` if the path was - /// stripped away (entry should be skipped). + /// Returns `None` (skip the entry) when the target becomes empty after + /// transformation or after stripping. pub(crate) fn edit_hardlink(&self, target: &Path) -> Option { // bsdtar order: substitution first, then strip let transformed: Cow<'_, Path> = if let Some(t) = &self.transformers { @@ -80,7 +73,13 @@ impl PathnameEditor { } else { Cow::Borrowed(target) }; + if is_effectively_empty_path(&transformed) { + return None; + } let stripped = strip_components(&transformed, self.strip_components)?; + if is_effectively_empty_path(&stripped) { + return None; + } let entry_reference = EntryReference::from_path_lossy_preserve_root(&stripped); if self.absolute_paths { Some(entry_reference) @@ -89,9 +88,11 @@ impl PathnameEditor { } } - /// Edit a symlink target path by applying any configured transformations and then either preserving or sanitizing absolute paths. + /// Edit a symlink target path. /// - /// Strip-component rules are not applied to symlink targets; only the editor's optional transformers and its absolute-paths configuration affect the result. + /// Unlike entry names and hardlink targets, symlink targets are never + /// skipped: the containing entry's name is validated separately via + /// `edit_entry_name`, and bsdtar does not strip or skip symlink targets. pub(crate) fn edit_symlink(&self, target: &Path) -> EntryReference { // Note: symlinks do not have strip_components applied (matching bsdtar behavior) let transformed: Cow<'_, Path> = if let Some(t) = &self.transformers { @@ -126,6 +127,11 @@ fn strip_components(path: &Path, count: Option) -> Option> Some(Cow::from(PathBuf::from_iter(components.skip(count)))) } +#[inline] +fn is_effectively_empty_path(path: &Path) -> bool { + path.components().all(|c| matches!(c, Component::CurDir)) +} + #[cfg(test)] mod tests { use super::*; @@ -215,4 +221,15 @@ mod tests { let result = editor.edit_symlink(Path::new("a/b/c")); assert_eq!(result.as_str(), "a/b/c"); // Not stripped } + + #[test] + fn editor_skips_empty_or_curdir_paths() { + let editor = PathnameEditor::new(None, None, false); + assert!(editor.edit_entry_name(Path::new("")).is_none()); + assert!(editor.edit_entry_name(Path::new(".")).is_none()); + assert!(editor.edit_entry_name(Path::new("./.")).is_none()); + assert!(editor.edit_hardlink(Path::new("")).is_none()); + assert!(editor.edit_hardlink(Path::new(".")).is_none()); + assert!(editor.edit_hardlink(Path::new("./.")).is_none()); + } } diff --git a/cli/tests/cli/stdio.rs b/cli/tests/cli/stdio.rs index 44a68cbeb..f2b6e9ddc 100644 --- a/cli/tests/cli/stdio.rs +++ b/cli/tests/cli/stdio.rs @@ -10,6 +10,7 @@ mod option_mac_metadata; mod option_no_recursive; mod option_options; mod option_same_permissions; +mod option_substitution; mod option_update; mod strip_components; mod unlink_first; diff --git a/cli/tests/cli/stdio/option_substitution.rs b/cli/tests/cli/stdio/option_substitution.rs new file mode 100644 index 000000000..91397cf9b --- /dev/null +++ b/cli/tests/cli/stdio/option_substitution.rs @@ -0,0 +1,46 @@ +use crate::utils::setup; +use assert_cmd::cargo::cargo_bin_cmd; +use std::fs; + +/// Precondition: A single file is archived with a substitution rule that yields an empty pathname. +/// Action: Create and list archive via stdio with `-s ,in/d1/foo,,`. +/// Expectation: Entry is skipped (no blank pathname entry appears in list output). +#[test] +fn stdio_substitution_empty_name_is_skipped() { + setup(); + fs::create_dir_all("stdio_substitution_empty_name_is_skipped/in/d1").unwrap(); + fs::write("stdio_substitution_empty_name_is_skipped/in/d1/foo", b"foo").unwrap(); + + let mut create = cargo_bin_cmd!("pna"); + create + .args([ + "--quiet", + "experimental", + "stdio", + "--unstable", + "-c", + "-f", + "stdio_substitution_empty_name_is_skipped/archive.pna", + "-C", + "stdio_substitution_empty_name_is_skipped", + "-s", + ",in/d1/foo,,", + "in/d1/foo", + ]) + .assert() + .success(); + + let mut list = cargo_bin_cmd!("pna"); + list.args([ + "--quiet", + "experimental", + "stdio", + "--unstable", + "-t", + "-f", + "stdio_substitution_empty_name_is_skipped/archive.pna", + ]) + .assert() + .success() + .stdout(""); +} From 26b84f299689bee8c622f32b779abfad0eaa3ddc Mon Sep 17 00:00:00 2001 From: ChanTsune <41658782+ChanTsune@users.noreply.github.com> Date: Sun, 8 Feb 2026 13:53:32 +0900 Subject: [PATCH 2/2] :boom: Fail stdio create when no input paths are given Previously `pna experimental stdio -c -f out.pna` with no positional arguments silently created an empty archive. Bail with an actionable error message, matching bsdtar behavior. --- cli/src/command/stdio.rs | 3 +++ cli/tests/cli/stdio.rs | 1 + cli/tests/cli/stdio/missing_file.rs | 22 ++++++++++++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 cli/tests/cli/stdio/missing_file.rs diff --git a/cli/src/command/stdio.rs b/cli/src/command/stdio.rs index fcda25c17..27c875c32 100644 --- a/cli/src/command/stdio.rs +++ b/cli/src/command/stdio.rs @@ -847,6 +847,9 @@ fn run_create_archive(args: StdioCommand) -> anyhow::Result<()> { if let Some(path) = args.files_from { files.extend(read_paths(path, args.null)?); } + if files.is_empty() { + anyhow::bail!("create mode requires at least one input path or @archive source"); + } let mut exclude = args.exclude; if let Some(p) = args.exclude_from { diff --git a/cli/tests/cli/stdio.rs b/cli/tests/cli/stdio.rs index f2b6e9ddc..1924243cd 100644 --- a/cli/tests/cli/stdio.rs +++ b/cli/tests/cli/stdio.rs @@ -1,6 +1,7 @@ mod archive_inclusion; mod exclude_vcs; mod files_from; +mod missing_file; #[cfg(unix)] mod mtree; mod option_auto_compress; diff --git a/cli/tests/cli/stdio/missing_file.rs b/cli/tests/cli/stdio/missing_file.rs new file mode 100644 index 000000000..a4a54f06e --- /dev/null +++ b/cli/tests/cli/stdio/missing_file.rs @@ -0,0 +1,22 @@ +use crate::utils::setup; +use assert_cmd::cargo::cargo_bin_cmd; + +/// Precondition: No input paths are provided. +/// Action: Run `pna experimental stdio -c -f ...` without positional paths. +/// Expectation: Command fails similarly to bsdtar's "missing file" handling. +#[test] +fn stdio_create_without_inputs_fails() { + setup(); + + let mut cmd = cargo_bin_cmd!("pna"); + cmd.args([ + "--quiet", + "experimental", + "stdio", + "--unstable", + "-c", + "-f", + "stdio_create_without_inputs_fails.pna", + ]); + cmd.assert().failure(); +}