Skip to content

Commit 0350ad3

Browse files
cosmicexplorercosmicexplorer
committed
set --large-file automatically and improve eyre messages
1 parent faca509 commit 0350ad3

File tree

3 files changed

+166
-51
lines changed

3 files changed

+166
-51
lines changed

‎cli/src/args.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -275,13 +275,16 @@ processing all flags.
275275
--large-file [true|false]
276276
Whether to enable large file support.
277277
This may take up more space for records, but allows files over 32 bits in length to be
278-
written, up to 64 bit sizes.
278+
written, up to 64 bit sizes. File arguments over 32 bits in length (either provided
279+
explicitly or encountered when traversing a recursive directory) will have this flag set
280+
automatically, without affecting the sticky value for later options. Therefore, this
281+
option likely never has to be set explicitly by the user.
279282
280283
Non-sticky attributes:
281284
These flags only apply to the next entry after them, and may not be repeated.
282285
283286
-n, --name <name>
284-
The name to apply to the entry.
287+
The name to apply to the entry. This must be UTF-8 encoded.
285288
286289
-s, --symlink
287290
Make the next entry into a symlink entry.
@@ -295,8 +298,9 @@ Each of these flags creates an entry in the output zip archive.
295298
Create a directory entry.
296299
A name must be provided beforehand with -n.
297300
298-
-i, --imm <immediate>
299-
Write an entry containing this data.
301+
-i, --immediate <data>
302+
Write an entry containing the data in the argument, which need not be UTF-8 encoded
303+
but will exit early upon encountering any null bytes.
300304
A name must be provided beforehand with -n.
301305
302306
-f, --file <path>

‎cli/src/compress.rs‎

Lines changed: 156 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
use std::{
22
fs,
33
io::{self, Cursor, IsTerminal, Seek, Write},
4+
mem,
45
path::Path,
56
};
67

7-
use color_eyre::eyre::{eyre, Report, WrapErr};
8+
use color_eyre::eyre::{bail, eyre, Report, WrapErr};
89

910
use zip::{
1011
unstable::path_to_string,
1112
write::{SimpleFileOptions, ZipWriter},
12-
CompressionMethod,
13+
CompressionMethod, ZIP64_BYTES_THR,
1314
};
1415

1516
use crate::args::*;
@@ -61,10 +62,11 @@ fn enter_recursive_dir_entries(
6162
)?;
6263
writer
6364
.add_directory(&base_dirname, options)
64-
.wrap_err_with(|| eyre!("{base_dirname}"))?;
65+
.wrap_err_with(|| eyre!("error adding top-level directory entry {base_dirname}"))?;
6566

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

8385
let file_type = dir_entry
8486
.file_type()
85-
.wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?;
87+
.wrap_err_with(|| eyre!("failed to read file type for dir entry {dir_entry:?}"))?;
8688
if file_type.is_symlink() {
8789
let target: String = path_to_string(
8890
fs::read_link(dir_entry.path())
89-
.wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?,
91+
.wrap_err_with(|| eyre!("failed to read symlink from {dir_entry:?}"))?,
9092
)
9193
.into();
94+
if target.len() > ZIP64_BYTES_THR.try_into().unwrap() {
95+
bail!(
96+
"symlink target for {full_path} is over {ZIP64_BYTES_THR} bytes (was: {})",
97+
target.len()
98+
);
99+
}
92100
writeln!(
93101
err,
94102
"writing recursive symlink entry with name {full_path:?} and target {target:?}"
95103
)?;
96104
writer
97105
.add_symlink(&full_path, &target, options)
98-
.wrap_err_with(|| eyre!("{full_path}->{target}"))?;
106+
.wrap_err_with(|| eyre!("error adding symlink from {full_path}->{target}"))?;
99107
} else if file_type.is_file() {
100108
writeln!(err, "writing recursive file entry with name {full_path:?}")?;
109+
let mut f = fs::File::open(dir_entry.path()).wrap_err_with(|| {
110+
eyre!("error opening file for {full_path} from dir entry {dir_entry:?}")
111+
})?;
112+
/* Get the length of the file before reading it and set large_file if needed. */
113+
let input_len: u64 = f
114+
.metadata()
115+
.wrap_err_with(|| eyre!("error reading file metadata for {f:?}"))?
116+
.len();
117+
let maybe_large_file_options = if input_len > ZIP64_BYTES_THR {
118+
writeln!(
119+
err,
120+
"temporarily ensuring .large_file(true) for current entry"
121+
)?;
122+
options.large_file(true)
123+
} else {
124+
options
125+
};
101126
writer
102-
.start_file(&full_path, options)
103-
.wrap_err_with(|| eyre!("{full_path}"))?;
104-
let mut f = fs::File::open(dir_entry.path())
105-
.wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?;
106-
io::copy(&mut f, writer)
107-
.wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?;
127+
.start_file(&full_path, maybe_large_file_options)
128+
.wrap_err_with(|| eyre!("error creating file entry for {full_path}"))?;
129+
io::copy(&mut f, writer).wrap_err_with(|| {
130+
eyre!("error copying content for {full_path} from file {f:?}")
131+
})?;
108132
} else {
109133
assert!(file_type.is_dir());
110134
writeln!(
@@ -113,13 +137,14 @@ fn enter_recursive_dir_entries(
113137
)?;
114138
writer
115139
.add_directory(&full_path, options)
116-
.wrap_err_with(|| eyre!("{full_path}"))?;
140+
.wrap_err_with(|| eyre!("failed to create directory entry {full_path}"))?;
117141
writeln!(
118142
err,
119143
"adding subdirectories depth-first for recursive directory entry {entry_basename:?}"
120144
)?;
121-
let new_readdir = fs::read_dir(dir_entry.path())
122-
.wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?;
145+
let new_readdir = fs::read_dir(dir_entry.path()).wrap_err_with(|| {
146+
eyre!("failed to read recursive directory contents from {dir_entry:?}")
147+
})?;
123148
readdir_stack.push((new_readdir, entry_basename));
124149
}
125150
}
@@ -142,7 +167,9 @@ pub fn execute_compress(
142167
Some(path) => {
143168
writeln!(err, "writing compressed zip to output file path {path:?}")?;
144169
OutputHandle::File(
145-
fs::File::create(&path).wrap_err_with(|| eyre!("{}", path.display()))?,
170+
fs::File::create(&path).wrap_err_with(|| {
171+
eyre!("failed to create output file at {}", path.display())
172+
})?,
146173
)
147174
}
148175
None => {
@@ -213,7 +240,7 @@ pub fn execute_compress(
213240
});
214241
writer
215242
.add_directory(&dirname, options)
216-
.wrap_err_with(|| eyre!("{dirname}"))?;
243+
.wrap_err_with(|| eyre!("failed to create dir entry {dirname}"))?;
217244
}
218245
CompressionArg::Symlink => {
219246
writeln!(err, "setting symlink flag for next entry")?;
@@ -227,6 +254,17 @@ pub fn execute_compress(
227254
let name = last_name.take().unwrap_or_else(|| {
228255
Compress::exit_arg_invalid("no name provided for immediate data {data:?}")
229256
});
257+
/* It's highly unlikely any OS allows process args of this length, so even though
258+
* we're using rust's env::args_os() and it would be very impressive for an attacker
259+
* to get CLI args to overflow, it seems likely to be inefficient in any case, and
260+
* very unlikely to be useful, so exit with a clear error. */
261+
if data.len() > ZIP64_BYTES_THR.try_into().unwrap() {
262+
bail!(
263+
"length of immediate data argument is {}; use a file for inputs over {} bytes",
264+
data.len(),
265+
ZIP64_BYTES_THR
266+
);
267+
};
230268
if symlink_flag {
231269
/* This is a symlink entry. */
232270
let target = data.into_string().unwrap_or_else(|target| {
@@ -241,7 +279,9 @@ pub fn execute_compress(
241279
/* TODO: .add_symlink() should support OsString targets! */
242280
writer
243281
.add_symlink(&name, &target, options)
244-
.wrap_err_with(|| eyre!("{name}->{target}"))?;
282+
.wrap_err_with(|| {
283+
eyre!("failed to created symlink entry {name}->{target}")
284+
})?;
245285
symlink_flag = false;
246286
} else {
247287
/* This is a file entry. */
@@ -252,10 +292,13 @@ pub fn execute_compress(
252292
let data = data.into_encoded_bytes();
253293
writer
254294
.start_file(&name, options)
255-
.wrap_err_with(|| eyre!("{name}"))?;
256-
writer
257-
.write_all(data.as_ref())
258-
.wrap_err_with(|| eyre!("{name}"))?;
295+
.wrap_err_with(|| eyre!("failed to create file entry {name}"))?;
296+
writer.write_all(data.as_ref()).wrap_err_with(|| {
297+
eyre!(
298+
"failed writing immediate data of length {} to file entry {name}",
299+
data.len()
300+
)
301+
})?;
259302
}
260303
}
261304
CompressionArg::FilePath(path) => {
@@ -264,27 +307,56 @@ pub fn execute_compress(
264307
.unwrap_or_else(|| path_to_string(&path).into());
265308
if symlink_flag {
266309
/* This is a symlink entry. */
267-
let target: String = path_to_string(
268-
fs::read_link(&path).wrap_err_with(|| eyre!("{}", path.display()))?,
269-
)
270-
.into();
310+
let target: String =
311+
path_to_string(fs::read_link(&path).wrap_err_with(|| {
312+
eyre!("failed to read symlink from path {}", path.display())
313+
})?)
314+
.into();
315+
/* Similarly to immediate data arguments, we're simply not going to support
316+
* symlinks over this length, which should be impossible anyway. */
317+
if target.len() > ZIP64_BYTES_THR.try_into().unwrap() {
318+
bail!(
319+
"symlink target for {name} is over {ZIP64_BYTES_THR} bytes (was: {})",
320+
target.len()
321+
);
322+
}
271323
writeln!(err, "writing symlink entry from path {path:?} with name {name:?} and target {target:?}")?;
272324
writer
273325
.add_symlink(&name, &target, options)
274-
.wrap_err_with(|| eyre!("{name}->{target}"))?;
326+
.wrap_err_with(|| {
327+
eyre!("failed to create symlink entry for {name}->{target}")
328+
})?;
275329
symlink_flag = false;
276330
} else {
277331
/* This is a file entry. */
278332
writeln!(
279333
err,
280334
"writing file entry from path {path:?} with name {name:?}"
281335
)?;
336+
let mut f = fs::File::open(&path).wrap_err_with(|| {
337+
eyre!("error opening file for {name} at {}", path.display())
338+
})?;
339+
/* Get the length of the file before reading it and set large_file if needed. */
340+
let input_len: u64 = f
341+
.metadata()
342+
.wrap_err_with(|| eyre!("error reading file metadata for {f:?}"))?
343+
.len();
344+
writeln!(err, "entry is {input_len} bytes long")?;
345+
let maybe_large_file_options = if input_len > ZIP64_BYTES_THR {
346+
writeln!(
347+
err,
348+
"temporarily ensuring .large_file(true) for current entry"
349+
)?;
350+
options.large_file(true)
351+
} else {
352+
options
353+
};
282354
writer
283-
.start_file(&name, options)
284-
.wrap_err_with(|| eyre!("{name}"))?;
285-
let mut f =
286-
fs::File::open(&path).wrap_err_with(|| eyre!("{}", path.display()))?;
287-
io::copy(&mut f, &mut writer).wrap_err_with(|| eyre!("{}", path.display()))?;
355+
.start_file(&name, maybe_large_file_options)
356+
.wrap_err_with(|| eyre!("error creating file entry for {name}"))?;
357+
io::copy(&mut f, &mut writer).wrap_err_with(|| {
358+
eyre!("error copying content for {name} from file {f:?}")
359+
})?;
288360
}
289361
}
290362
CompressionArg::RecursiveDirPath(r) => {
@@ -296,7 +368,7 @@ pub fn execute_compress(
296368
"writing recursive dir entries for path {r:?} with name {last_name:?}"
297369
)?;
298370
enter_recursive_dir_entries(err, last_name.take(), &r, &mut writer, options)
299-
.wrap_err_with(|| eyre!("{}", r.display()))?;
371+
.wrap_err_with(|| eyre!("failed to read recursive dir {}", r.display()))?;
300372
}
301373
}
302374
}
@@ -310,47 +382,84 @@ pub fn execute_compress(
310382
}
311383
for pos_arg in positional_paths.into_iter() {
312384
let file_type = fs::symlink_metadata(&pos_arg)
313-
.wrap_err_with(|| eyre!("{}", pos_arg.display()))?
385+
.wrap_err_with(|| eyre!("failed to read metadata from path {}", pos_arg.display()))?
314386
.file_type();
315387
if file_type.is_symlink() {
316-
let target =
317-
fs::read_link(&pos_arg).wrap_err_with(|| eyre!("{}", pos_arg.display()))?;
388+
let target = fs::read_link(&pos_arg).wrap_err_with(|| {
389+
eyre!("failed to read symlink content from {}", pos_arg.display())
390+
})?;
318391
writeln!(
319392
err,
320393
"writing positional symlink entry with path {pos_arg:?} and target {target:?}"
321394
)?;
322395
writer
323396
.add_symlink_from_path(&pos_arg, &target, options)
324-
.wrap_err_with(|| eyre!("{}->{}", pos_arg.display(), target.display()))?;
397+
.wrap_err_with(|| {
398+
eyre!(
399+
"failed to create symlink entry for {}->{}",
400+
pos_arg.display(),
401+
target.display()
402+
)
403+
})?;
325404
} else if file_type.is_file() {
326405
writeln!(err, "writing positional file entry with path {pos_arg:?}")?;
406+
let mut f = fs::File::open(&pos_arg)
407+
.wrap_err_with(|| eyre!("failed to open file at {}", pos_arg.display()))?;
408+
/* Get the length of the file before reading it and set large_file if needed. */
409+
let input_len: u64 = f
410+
.metadata()
411+
.wrap_err_with(|| eyre!("error reading file metadata for {f:?}"))?
412+
.len();
413+
let maybe_large_file_options = if input_len > ZIP64_BYTES_THR {
414+
writeln!(
415+
err,
416+
"temporarily ensuring .large_file(true) for current entry"
417+
)?;
418+
options.large_file(true)
419+
} else {
420+
options
421+
};
327422
writer
328-
.start_file_from_path(&pos_arg, options)
329-
.wrap_err_with(|| eyre!("{}", pos_arg.display()))?;
330-
let mut f =
331-
fs::File::open(&pos_arg).wrap_err_with(|| eyre!("{}", pos_arg.display()))?;
332-
io::copy(&mut f, &mut writer).wrap_err_with(|| eyre!("{}", pos_arg.display()))?;
423+
.start_file_from_path(&pos_arg, maybe_large_file_options)
424+
.wrap_err_with(|| eyre!("failed to create file entry {}", pos_arg.display()))?;
425+
io::copy(&mut f, &mut writer)
426+
.wrap_err_with(|| eyre!("failed to copy file contents from {f:?}"))?;
333427
} else {
334428
assert!(file_type.is_dir());
335429
writeln!(
336430
err,
337431
"writing positional recursive dir entry for {pos_arg:?}"
338432
)?;
339-
enter_recursive_dir_entries(err, None, &pos_arg, &mut writer, options)
340-
.wrap_err_with(|| eyre!("{}", pos_arg.display()))?;
433+
enter_recursive_dir_entries(err, None, &pos_arg, &mut writer, options).wrap_err_with(
434+
|| {
435+
eyre!(
436+
"failed to insert recursive directory entries from {}",
437+
pos_arg.display()
438+
)
439+
},
440+
)?;
341441
}
342442
}
343443

344444
let handle = writer
345445
.finish()
346446
.wrap_err("failed to write zip to output handle")?;
347447
match handle {
348-
OutputHandle::File(_) => (),
448+
OutputHandle::File(f) => {
449+
let archive_len: u64 = f
450+
.metadata()
451+
.wrap_err_with(|| eyre!("failed reading metadata from file {f:?}"))?
452+
.len();
453+
writeln!(err, "file archive {f:?} was {archive_len} bytes")?;
454+
mem::drop(f); /* Superfluous explicit drop. */
455+
}
349456
OutputHandle::InMem(mut cursor) => {
457+
let archive_len: u64 = cursor.position();
458+
writeln!(err, "in-memory archive was {archive_len} bytes")?;
350459
cursor.rewind().wrap_err("failed to rewind cursor")?;
351460
let mut stdout = io::stdout().lock();
352461
io::copy(&mut cursor, &mut stdout)
353-
.wrap_err("failed to copy contents of cursor to stdout")?;
462+
.wrap_err("failed to copy {archive_len} byte archive to stdout")?;
354463
}
355464
}
356465

‎src/write.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ impl<'a> arbitrary::Arbitrary<'a> for EncryptWith<'a> {
253253
}
254254

255255
/// Metadata for a file to be written
256+
/* TODO: add accessors for this data as well so options can be introspected! */
256257
#[derive(Clone, Debug, Copy, Eq, PartialEq)]
257258
pub struct FileOptions<'k, T: FileOptionExtension> {
258259
pub(crate) compression_method: CompressionMethod,
@@ -1396,6 +1397,7 @@ impl<W: Write + Seek> ZipWriter<W> {
13961397
/// implementations may materialize a symlink as a regular file, possibly with the
13971398
/// content incorrectly set to the symlink target. For maximum portability, consider
13981399
/// storing a regular file instead.
1400+
/* TODO: support OsStr instead of just str, for non-unicode paths. */
13991401
pub fn add_symlink<N: ToString, T: ToString, E: FileOptionExtension>(
14001402
&mut self,
14011403
name: N,

0 commit comments

Comments
 (0)