From 6b5a0a96e4ca6119466db523103c36be6fc5f162 Mon Sep 17 00:00:00 2001 From: Jeff Bailey Date: Sat, 4 Apr 2026 23:27:21 +0000 Subject: [PATCH] tar: support -f - for stdin/stdout archives When the archive path is "-", read archives from stdin for list and extract, and write archive data to stdout for create. Route create status output to stderr when writing the archive to stdout so verbose output does not corrupt the stream. Add CLI tests for create-to-stdout, list-from-stdin, extract-from-stdin, and the stderr routing cases. --- src/uu/tar/src/operations/create.rs | 26 +++--- src/uu/tar/src/operations/extract.rs | 15 +--- src/uu/tar/src/operations/list.rs | 9 +- src/uu/tar/src/tar.rs | 43 +++++++++- src/uu/tar/tests/test_errors.rs | 5 +- tests/by-util/test_tar.rs | 119 +++++++++++++++++++++++++++ 6 files changed, 181 insertions(+), 36 deletions(-) diff --git a/src/uu/tar/src/operations/create.rs b/src/uu/tar/src/operations/create.rs index 6e639dc..8f21cc6 100644 --- a/src/uu/tar/src/operations/create.rs +++ b/src/uu/tar/src/operations/create.rs @@ -5,7 +5,8 @@ use crate::errors::TarError; use std::collections::VecDeque; -use std::fs::{self, File}; +use std::fs; +use std::io::Write; use std::path::Component::{self, ParentDir, Prefix, RootDir}; use std::path::{self, Path, PathBuf}; use tar::Builder; @@ -25,15 +26,14 @@ use uucore::error::UResult; /// - The archive file cannot be created /// - Any input file cannot be read /// - Files cannot be added due to I/O or permission errors -pub fn create_archive(archive_path: &Path, files: &[&Path], verbose: bool) -> UResult<()> { - // Create the output file - let file = File::create(archive_path).map_err(|e| TarError::CannotCreateArchive { - path: archive_path.to_path_buf(), - source: e, - })?; - +pub fn create_archive( + output: Box, + mut status_output: Box, + files: &[&Path], + verbose: bool, +) -> UResult<()> { // Create Builder instance - let mut builder = Builder::new(file); + let mut builder = Builder::new(output); // Add each file or directory to the archive for &path in files { @@ -58,7 +58,7 @@ pub fn create_archive(archive_path: &Path, files: &[&Path], verbose: bool) -> UR }) .collect::>() .join("\n"); - println!("{to_print}"); + writeln!(status_output, "{to_print}")?; } // Normalize path if needed (so far, handles only absolute paths) @@ -70,7 +70,11 @@ pub fn create_archive(archive_path: &Path, files: &[&Path], verbose: bool) -> UR [..original_components.len() - normalized_components.len()] .iter() .collect(); - println!("Removing leading `{}' from member names", removed.display()); + writeln!( + status_output, + "Removing leading `{}' from member names", + removed.display() + )?; } normalized diff --git a/src/uu/tar/src/operations/extract.rs b/src/uu/tar/src/operations/extract.rs index be14b11..cfefa44 100644 --- a/src/uu/tar/src/operations/extract.rs +++ b/src/uu/tar/src/operations/extract.rs @@ -4,8 +4,7 @@ // file that was distributed with this source code. use crate::errors::TarError; -use std::fs::File; -use std::path::Path; +use std::io::Read; use tar::Archive; use uucore::error::UResult; @@ -22,17 +21,9 @@ use uucore::error::UResult; /// - The archive file cannot be opened /// - The archive format is invalid /// - Files cannot be extracted due to I/O or permission errors -pub fn extract_archive(archive_path: &Path, verbose: bool) -> UResult<()> { - // Open the archive file - let file = File::open(archive_path).map_err(|e| TarError::from_io_error(e, archive_path))?; - +pub fn extract_archive(input: Box, verbose: bool) -> UResult<()> { // Create Archive instance - let mut archive = Archive::new(file); - - // Extract to current directory - if verbose { - println!("Extracting archive: {}", archive_path.display()); - } + let mut archive = Archive::new(input); // Iterate through entries for verbose output and error handling for entry_result in archive.entries().map_err(TarError::CannotReadEntries)? { diff --git a/src/uu/tar/src/operations/list.rs b/src/uu/tar/src/operations/list.rs index 98f1566..ad6a4f1 100644 --- a/src/uu/tar/src/operations/list.rs +++ b/src/uu/tar/src/operations/list.rs @@ -5,17 +5,14 @@ use crate::errors::TarError; use chrono::{TimeZone, Utc}; -use std::fs::File; -use std::path::Path; +use std::io::Read; use tar::Archive; use uucore::error::UResult; use uucore::fs::display_permissions_unix; /// List the contents of a tar archive, printing one entry per line. -pub fn list_archive(archive_path: &Path, verbose: bool) -> UResult<()> { - let file: File = - File::open(archive_path).map_err(|e| TarError::from_io_error(e, archive_path))?; - let mut archive = Archive::new(file); +pub fn list_archive(input: Box, verbose: bool) -> UResult<()> { + let mut archive = Archive::new(input); for entry_result in archive.entries().map_err(TarError::CannotReadEntries)? { let entry = entry_result.map_err(TarError::CannotReadEntry)?; diff --git a/src/uu/tar/src/tar.rs b/src/uu/tar/src/tar.rs index 16e29d1..72c7341 100644 --- a/src/uu/tar/src/tar.rs +++ b/src/uu/tar/src/tar.rs @@ -6,7 +6,10 @@ pub mod errors; mod operations; +use crate::errors::TarError; use clap::{arg, crate_version, ArgAction, Command}; +use std::fs::File; +use std::io; use std::path::{Path, PathBuf}; use uucore::error::UResult; use uucore::format_usage; @@ -138,7 +141,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { uucore::error::USimpleError::new(64, "option requires an argument -- 'f'") })?; - return operations::extract::extract_archive(archive_path, verbose); + let input: Box = if archive_path == Path::new("-") { + Box::new(io::stdin()) + } else { + Box::new( + File::open(archive_path).map_err(|e| TarError::from_io_error(e, archive_path))?, + ) + }; + + return operations::extract::extract_archive(input, verbose); } // Handle create operation @@ -159,7 +170,25 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { )); } - return operations::create::create_archive(archive_path, &files, verbose); + let output_is_stdout = archive_path == Path::new("-"); + let output: Box = if output_is_stdout { + Box::new(io::stdout()) + } else { + Box::new( + File::create(archive_path).map_err(|e| TarError::CannotCreateArchive { + path: archive_path.clone(), + source: e, + })?, + ) + }; + + let status_output: Box = if output_is_stdout { + Box::new(io::stderr()) + } else { + Box::new(io::stdout()) + }; + + return operations::create::create_archive(output, status_output, &files, verbose); } // Handle list operation @@ -168,7 +197,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { uucore::error::USimpleError::new(64, "option requires an argument -- 'f'") })?; - return operations::list::list_archive(archive_path, verbose); + let input: Box = if archive_path == Path::new("-") { + Box::new(io::stdin()) + } else { + Box::new( + File::open(archive_path).map_err(|e| TarError::from_io_error(e, archive_path))?, + ) + }; + + return operations::list::list_archive(input, verbose); } // If no operation specified, show error diff --git a/src/uu/tar/tests/test_errors.rs b/src/uu/tar/tests/test_errors.rs index 98b7acb..8bb41a0 100644 --- a/src/uu/tar/tests/test_errors.rs +++ b/src/uu/tar/tests/test_errors.rs @@ -44,10 +44,7 @@ fn test_tar_error_code() { .code(), 2 ); - assert_eq!( - TarError::Io(io::Error::new(io::ErrorKind::Other, "test")).code(), - 2 - ); + assert_eq!(TarError::Io(io::Error::other("test")).code(), 2); } #[test] diff --git a/tests/by-util/test_tar.rs b/tests/by-util/test_tar.rs index 6b8e506..15dcd83 100644 --- a/tests/by-util/test_tar.rs +++ b/tests/by-util/test_tar.rs @@ -145,6 +145,17 @@ fn test_create_verbose() { assert!(at.file_exists("archive.tar")); } +#[test] +fn test_create_verbose_to_stdout_uses_stderr() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.write("file.txt", "hello"); + + ucmd.args(&["-cvf", "-", "file.txt"]) + .succeeds() + .stderr_contains("file.txt"); +} + #[test] fn test_create_empty_archive_fails() { new_ucmd!() @@ -752,3 +763,111 @@ fn test_list_conflicts_with_extract() { .code_is(2) .stderr_contains("cannot be used with"); } + +// stdin/stdout (-f -) tests + +#[test] +fn test_create_to_stdout() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("file.txt", "hello"); + + let result = ucmd.args(&["-cf", "-", "file.txt"]).succeeds(); + let bytes = result.stdout(); + // A tar archive is at least two blocks: one header + two end-of-archive blocks + assert!( + bytes.len() >= TAR_BLOCK_SIZE, + "stdout should contain tar data (got {} bytes)", + bytes.len() + ); + // The first 100 bytes of a tar header are the file name + let header_prefix = std::str::from_utf8(&bytes[..100]).unwrap_or(""); + assert!( + header_prefix.contains("file.txt"), + "tar header should contain filename" + ); +} + +#[test] +fn test_create_verbose_to_stdout_keeps_stdout_as_tar_data() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("file.txt", "hello"); + + let result = ucmd.args(&["-cvf", "-", "file.txt"]).succeeds(); + let bytes = result.stdout(); + let header_prefix = std::str::from_utf8(&bytes[..100]).unwrap_or(""); + + assert!( + bytes.len() >= TAR_BLOCK_SIZE, + "stdout should contain tar data (got {} bytes)", + bytes.len() + ); + assert!( + header_prefix.contains("file.txt"), + "tar header should contain filename" + ); + assert_eq!(&bytes[..8], b"file.txt"); +} + +#[test] +fn test_create_absolute_path_to_stdout_uses_stderr_for_normalization_notice() { + let (at, mut ucmd) = at_and_ucmd!(); + + let mut file_abs_path = PathBuf::from(at.root_dir_resolved()); + file_abs_path.push("file1.txt"); + + at.write(&file_abs_path.display().to_string(), "content1"); + + let result = ucmd + .args(&["-cf", "-", &file_abs_path.display().to_string()]) + .succeeds(); + result.stderr_contains("Removing leading"); + let bytes = result.stdout(); + let header_prefix = std::str::from_utf8(&bytes[..100]).unwrap_or(""); + + assert!( + bytes.len() >= TAR_BLOCK_SIZE, + "stdout should contain tar data (got {} bytes)", + bytes.len() + ); + assert!( + !header_prefix.contains("Removing leading"), + "stdout should not contain normalization notices" + ); +} + +#[test] +fn test_list_from_stdin() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("file.txt", "hello"); + + // Create archive to a file in the same sandbox + new_ucmd!() + .args(&["-cf", &at.plus_as_string("archive.tar"), "file.txt"]) + .current_dir(at.as_string()) + .succeeds(); + + let archive_bytes = at.read_bytes("archive.tar"); + ucmd.args(&["-tf", "-"]) + .pipe_in(archive_bytes) + .succeeds() + .stdout_contains("file.txt"); +} + +#[test] +fn test_extract_from_stdin() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("file.txt", "hello world"); + + // Create archive to a file in the same sandbox + new_ucmd!() + .args(&["-cf", &at.plus_as_string("archive.tar"), "file.txt"]) + .current_dir(at.as_string()) + .succeeds(); + + at.remove("file.txt"); + + let archive_bytes = at.read_bytes("archive.tar"); + ucmd.args(&["-xf", "-"]).pipe_in(archive_bytes).succeeds(); + + assert_eq!(at.read("file.txt"), "hello world"); +}