Skip to content

Commit

Permalink
set --large-file automatically and improve eyre messages
Browse files Browse the repository at this point in the history
  • Loading branch information
@cosmicexplorer
cosmicexplorer committed Aug 15, 2024
1 parent faca509 commit 0350ad3
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 51 deletions.
12 changes: 8 additions & 4 deletions cli/src/args.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,13 +275,16 @@ processing all flags.
--large-file [true|false]
Whether to enable large file support.
This may take up more space for records, but allows files over 32 bits in length to be
written, up to 64 bit sizes.
written, up to 64 bit sizes. File arguments over 32 bits in length (either provided
explicitly or encountered when traversing a recursive directory) will have this flag set
automatically, without affecting the sticky value for later options. Therefore, this
option likely never has to be set explicitly by the user.
Non-sticky attributes:
These flags only apply to the next entry after them, and may not be repeated.
-n, --name <name>
The name to apply to the entry.
The name to apply to the entry. This must be UTF-8 encoded.
-s, --symlink
Make the next entry into a symlink entry.
Expand All @@ -295,8 +298,9 @@ Each of these flags creates an entry in the output zip archive.
Create a directory entry.
A name must be provided beforehand with -n.
-i, --imm <immediate>
Write an entry containing this data.
-i, --immediate <data>
Write an entry containing the data in the argument, which need not be UTF-8 encoded
but will exit early upon encountering any null bytes.
A name must be provided beforehand with -n.
-f, --file <path>
Expand Down
203 changes: 156 additions & 47 deletions cli/src/compress.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
use std::{
fs,
io::{self, Cursor, IsTerminal, Seek, Write},
mem,
path::Path,
};

use color_eyre::eyre::{eyre, Report, WrapErr};
use color_eyre::eyre::{bail, eyre, Report, WrapErr};

use zip::{
unstable::path_to_string,
write::{SimpleFileOptions, ZipWriter},
CompressionMethod,
CompressionMethod, ZIP64_BYTES_THR,
};

use crate::args::*;
Expand Down Expand Up @@ -61,10 +62,11 @@ fn enter_recursive_dir_entries(
)?;
writer
.add_directory(&base_dirname, options)
.wrap_err_with(|| eyre!("{base_dirname}"))?;
.wrap_err_with(|| eyre!("error adding top-level directory entry {base_dirname}"))?;

let mut readdir_stack: Vec<(fs::ReadDir, String)> = vec![(
fs::read_dir(root).wrap_err_with(|| eyre!("{}", root.display()))?,
fs::read_dir(root)
.wrap_err_with(|| eyre!("error reading directory contents for {}", root.display()))?,
base_dirname,
)];
while let Some((mut readdir, top_component)) = readdir_stack.pop() {
Expand All @@ -82,29 +84,51 @@ fn enter_recursive_dir_entries(

let file_type = dir_entry
.file_type()
.wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?;
.wrap_err_with(|| eyre!("failed to read file type for dir entry {dir_entry:?}"))?;
if file_type.is_symlink() {
let target: String = path_to_string(
fs::read_link(dir_entry.path())
.wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?,
.wrap_err_with(|| eyre!("failed to read symlink from {dir_entry:?}"))?,
)
.into();
if target.len() > ZIP64_BYTES_THR.try_into().unwrap() {
bail!(
"symlink target for {full_path} is over {ZIP64_BYTES_THR} bytes (was: {})",
target.len()
);
}
writeln!(
err,
"writing recursive symlink entry with name {full_path:?} and target {target:?}"
)?;
writer
.add_symlink(&full_path, &target, options)
.wrap_err_with(|| eyre!("{full_path}->{target}"))?;
.wrap_err_with(|| eyre!("error adding symlink from {full_path}->{target}"))?;
} else if file_type.is_file() {
writeln!(err, "writing recursive file entry with name {full_path:?}")?;
let mut f = fs::File::open(dir_entry.path()).wrap_err_with(|| {
eyre!("error opening file for {full_path} from dir entry {dir_entry:?}")
})?;
/* Get the length of the file before reading it and set large_file if needed. */
let input_len: u64 = f
.metadata()
.wrap_err_with(|| eyre!("error reading file metadata for {f:?}"))?
.len();
let maybe_large_file_options = if input_len > ZIP64_BYTES_THR {
writeln!(
err,
"temporarily ensuring .large_file(true) for current entry"
)?;
options.large_file(true)
} else {
options
};
writer
.start_file(&full_path, options)
.wrap_err_with(|| eyre!("{full_path}"))?;
let mut f = fs::File::open(dir_entry.path())
.wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?;
io::copy(&mut f, writer)
.wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?;
.start_file(&full_path, maybe_large_file_options)
.wrap_err_with(|| eyre!("error creating file entry for {full_path}"))?;
io::copy(&mut f, writer).wrap_err_with(|| {
eyre!("error copying content for {full_path} from file {f:?}")
})?;
} else {
assert!(file_type.is_dir());
writeln!(
Expand All @@ -113,13 +137,14 @@ fn enter_recursive_dir_entries(
)?;
writer
.add_directory(&full_path, options)
.wrap_err_with(|| eyre!("{full_path}"))?;
.wrap_err_with(|| eyre!("failed to create directory entry {full_path}"))?;
writeln!(
err,
"adding subdirectories depth-first for recursive directory entry {entry_basename:?}"
)?;
let new_readdir = fs::read_dir(dir_entry.path())
.wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?;
let new_readdir = fs::read_dir(dir_entry.path()).wrap_err_with(|| {
eyre!("failed to read recursive directory contents from {dir_entry:?}")
})?;
readdir_stack.push((new_readdir, entry_basename));
}
}
Expand All @@ -142,7 +167,9 @@ pub fn execute_compress(
Some(path) => {
writeln!(err, "writing compressed zip to output file path {path:?}")?;
OutputHandle::File(
fs::File::create(&path).wrap_err_with(|| eyre!("{}", path.display()))?,
fs::File::create(&path).wrap_err_with(|| {
eyre!("failed to create output file at {}", path.display())
})?,
)
}
None => {
Expand Down Expand Up @@ -213,7 +240,7 @@ pub fn execute_compress(
});
writer
.add_directory(&dirname, options)
.wrap_err_with(|| eyre!("{dirname}"))?;
.wrap_err_with(|| eyre!("failed to create dir entry {dirname}"))?;
}
CompressionArg::Symlink => {
writeln!(err, "setting symlink flag for next entry")?;
Expand All @@ -227,6 +254,17 @@ pub fn execute_compress(
let name = last_name.take().unwrap_or_else(|| {
Compress::exit_arg_invalid("no name provided for immediate data {data:?}")
});
/* It's highly unlikely any OS allows process args of this length, so even though
* we're using rust's env::args_os() and it would be very impressive for an attacker
* to get CLI args to overflow, it seems likely to be inefficient in any case, and
* very unlikely to be useful, so exit with a clear error. */
if data.len() > ZIP64_BYTES_THR.try_into().unwrap() {
bail!(
"length of immediate data argument is {}; use a file for inputs over {} bytes",
data.len(),
ZIP64_BYTES_THR
);
};
if symlink_flag {
/* This is a symlink entry. */
let target = data.into_string().unwrap_or_else(|target| {
Expand All @@ -241,7 +279,9 @@ pub fn execute_compress(
/* TODO: .add_symlink() should support OsString targets! */

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
writer
.add_symlink(&name, &target, options)
.wrap_err_with(|| eyre!("{name}->{target}"))?;
.wrap_err_with(|| {
eyre!("failed to created symlink entry {name}->{target}")
})?;
symlink_flag = false;
} else {
/* This is a file entry. */
Expand All @@ -252,10 +292,13 @@ pub fn execute_compress(
let data = data.into_encoded_bytes();
writer
.start_file(&name, options)
.wrap_err_with(|| eyre!("{name}"))?;
writer
.write_all(data.as_ref())
.wrap_err_with(|| eyre!("{name}"))?;
.wrap_err_with(|| eyre!("failed to create file entry {name}"))?;
writer.write_all(data.as_ref()).wrap_err_with(|| {
eyre!(
"failed writing immediate data of length {} to file entry {name}",
data.len()
)
})?;
}
}
CompressionArg::FilePath(path) => {
Expand All @@ -264,27 +307,56 @@ pub fn execute_compress(
.unwrap_or_else(|| path_to_string(&path).into());
if symlink_flag {
/* This is a symlink entry. */
let target: String = path_to_string(
fs::read_link(&path).wrap_err_with(|| eyre!("{}", path.display()))?,
)
.into();
let target: String =
path_to_string(fs::read_link(&path).wrap_err_with(|| {
eyre!("failed to read symlink from path {}", path.display())
})?)
.into();
/* Similarly to immediate data arguments, we're simply not going to support
* symlinks over this length, which should be impossible anyway. */
if target.len() > ZIP64_BYTES_THR.try_into().unwrap() {
bail!(
"symlink target for {name} is over {ZIP64_BYTES_THR} bytes (was: {})",
target.len()
);
}
writeln!(err, "writing symlink entry from path {path:?} with name {name:?} and target {target:?}")?;
writer
.add_symlink(&name, &target, options)
.wrap_err_with(|| eyre!("{name}->{target}"))?;
.wrap_err_with(|| {
eyre!("failed to create symlink entry for {name}->{target}")
})?;
symlink_flag = false;
} else {
/* This is a file entry. */
writeln!(
err,
"writing file entry from path {path:?} with name {name:?}"
)?;
let mut f = fs::File::open(&path).wrap_err_with(|| {
eyre!("error opening file for {name} at {}", path.display())
})?;
/* Get the length of the file before reading it and set large_file if needed. */
let input_len: u64 = f
.metadata()
.wrap_err_with(|| eyre!("error reading file metadata for {f:?}"))?
.len();
writeln!(err, "entry is {input_len} bytes long")?;
let maybe_large_file_options = if input_len > ZIP64_BYTES_THR {
writeln!(
err,
"temporarily ensuring .large_file(true) for current entry"
)?;
options.large_file(true)
} else {
options
};
writer
.start_file(&name, options)
.wrap_err_with(|| eyre!("{name}"))?;
let mut f =
fs::File::open(&path).wrap_err_with(|| eyre!("{}", path.display()))?;
io::copy(&mut f, &mut writer).wrap_err_with(|| eyre!("{}", path.display()))?;
.start_file(&name, maybe_large_file_options)
.wrap_err_with(|| eyre!("error creating file entry for {name}"))?;
io::copy(&mut f, &mut writer).wrap_err_with(|| {
eyre!("error copying content for {name} from file {f:?}")
})?;
}
}
CompressionArg::RecursiveDirPath(r) => {
Expand All @@ -296,7 +368,7 @@ pub fn execute_compress(
"writing recursive dir entries for path {r:?} with name {last_name:?}"
)?;
enter_recursive_dir_entries(err, last_name.take(), &r, &mut writer, options)
.wrap_err_with(|| eyre!("{}", r.display()))?;
.wrap_err_with(|| eyre!("failed to read recursive dir {}", r.display()))?;
}
}
}
Expand All @@ -310,47 +382,84 @@ pub fn execute_compress(
}
for pos_arg in positional_paths.into_iter() {
let file_type = fs::symlink_metadata(&pos_arg)
.wrap_err_with(|| eyre!("{}", pos_arg.display()))?
.wrap_err_with(|| eyre!("failed to read metadata from path {}", pos_arg.display()))?
.file_type();
if file_type.is_symlink() {
let target =
fs::read_link(&pos_arg).wrap_err_with(|| eyre!("{}", pos_arg.display()))?;
let target = fs::read_link(&pos_arg).wrap_err_with(|| {
eyre!("failed to read symlink content from {}", pos_arg.display())
})?;
writeln!(
err,
"writing positional symlink entry with path {pos_arg:?} and target {target:?}"
)?;
writer
.add_symlink_from_path(&pos_arg, &target, options)
.wrap_err_with(|| eyre!("{}->{}", pos_arg.display(), target.display()))?;
.wrap_err_with(|| {
eyre!(
"failed to create symlink entry for {}->{}",
pos_arg.display(),
target.display()
)
})?;
} else if file_type.is_file() {
writeln!(err, "writing positional file entry with path {pos_arg:?}")?;
let mut f = fs::File::open(&pos_arg)
.wrap_err_with(|| eyre!("failed to open file at {}", pos_arg.display()))?;
/* Get the length of the file before reading it and set large_file if needed. */
let input_len: u64 = f
.metadata()
.wrap_err_with(|| eyre!("error reading file metadata for {f:?}"))?
.len();
let maybe_large_file_options = if input_len > ZIP64_BYTES_THR {
writeln!(
err,
"temporarily ensuring .large_file(true) for current entry"
)?;
options.large_file(true)
} else {
options
};
writer
.start_file_from_path(&pos_arg, options)
.wrap_err_with(|| eyre!("{}", pos_arg.display()))?;
let mut f =
fs::File::open(&pos_arg).wrap_err_with(|| eyre!("{}", pos_arg.display()))?;
io::copy(&mut f, &mut writer).wrap_err_with(|| eyre!("{}", pos_arg.display()))?;
.start_file_from_path(&pos_arg, maybe_large_file_options)
.wrap_err_with(|| eyre!("failed to create file entry {}", pos_arg.display()))?;
io::copy(&mut f, &mut writer)
.wrap_err_with(|| eyre!("failed to copy file contents from {f:?}"))?;
} else {
assert!(file_type.is_dir());
writeln!(
err,
"writing positional recursive dir entry for {pos_arg:?}"
)?;
enter_recursive_dir_entries(err, None, &pos_arg, &mut writer, options)
.wrap_err_with(|| eyre!("{}", pos_arg.display()))?;
enter_recursive_dir_entries(err, None, &pos_arg, &mut writer, options).wrap_err_with(
|| {
eyre!(
"failed to insert recursive directory entries from {}",
pos_arg.display()
)
},
)?;
}
}

let handle = writer
.finish()
.wrap_err("failed to write zip to output handle")?;
match handle {
OutputHandle::File(_) => (),
OutputHandle::File(f) => {
let archive_len: u64 = f
.metadata()
.wrap_err_with(|| eyre!("failed reading metadata from file {f:?}"))?
.len();
writeln!(err, "file archive {f:?} was {archive_len} bytes")?;
mem::drop(f); /* Superfluous explicit drop. */
}
OutputHandle::InMem(mut cursor) => {
let archive_len: u64 = cursor.position();
writeln!(err, "in-memory archive was {archive_len} bytes")?;
cursor.rewind().wrap_err("failed to rewind cursor")?;
let mut stdout = io::stdout().lock();
io::copy(&mut cursor, &mut stdout)
.wrap_err("failed to copy contents of cursor to stdout")?;
.wrap_err("failed to copy {archive_len} byte archive to stdout")?;
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/write.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ impl<'a> arbitrary::Arbitrary<'a> for EncryptWith<'a> {
}

/// Metadata for a file to be written
/* TODO: add accessors for this data as well so options can be introspected! */

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
#[derive(Clone, Debug, Copy, Eq, PartialEq)]
pub struct FileOptions<'k, T: FileOptionExtension> {
pub(crate) compression_method: CompressionMethod,
Expand Down Expand Up @@ -1396,6 +1397,7 @@ impl<W: Write + Seek> ZipWriter<W> {
/// implementations may materialize a symlink as a regular file, possibly with the
/// content incorrectly set to the symlink target. For maximum portability, consider
/// storing a regular file instead.
/* TODO: support OsStr instead of just str, for non-unicode paths. */

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
pub fn add_symlink<N: ToString, T: ToString, E: FileOptionExtension>(
&mut self,
name: N,
Expand Down

0 comments on commit 0350ad3

Please sign in to comment.