Duplicate filename, but what?

[frederikhors]

I'm getting this error:

invalid Zip archive: Duplicate filename

but I cannot understand what is the file duplicated.

Is it possible to write the filename in the error?

[Pr0methean]

Fixing this will require that ZipError::InvalidArchive wrap a Cow<'static, str> rather than &'static str so that it can include a message generated at runtime, but without the overhead of allocating a string at runtime when it's not doing so.

[Pr0methean]

Started work on a solution, but running into some macro errors that I either don't know or have forgotten how to solve.

[Rapptz]

I ran into this issue as well. I'd really like to know what the duplicate is or how to avoid this.

Started work on a solution, but running into some macro errors that I either don't know or have forgotten how to solve.

I think the error you're getting is because you're expanding the argument list into a tuple rather than just letting it expand as arguments to the format! macro.

[Bogay]

Hi, I'm interested in solving this. After spending some time digging through the code in commit 4185739, I believe that I can solve the macro issue. If it's OK, I would like to open a PR. However, there are a few things I want to address:

1. I found that besides InvalidArchive, other error variants are also added more fields. But in this commit they are not used. Should I include them in my PR to add more context to other types of error?

zip2/src/result.rs

Lines 17 to 38 in 4185739

	/// Error type for Zip
	#[derive(Debug, Display, Error)]
	#[non_exhaustive]
	pub enum ZipError {
	/// i/o error: {0}
	Io(#[from] io::Error),
	/// invalid Zip archive: {0}
	InvalidArchive(Cow<'static, str>),
	/// unsupported Zip archive: {0}
	UnsupportedArchive(Cow<'static, str>),
	/// specified file not found in archive
	FileNotFound(Box<str>),
	/// The password provided is incorrect
	InvalidPassword {
	filename: Box<str>,
	password: Option<Box<str>>
	},
	}

2. The invalid! macro returns ZipResult, so it can't be used in functions such as Option::ok_or. Following code can't be compiled because it needs a ZipError

zip2/src/read.rs

Lines 410 to 417 in 4185739

	new_files.values_mut().try_for_each(|f| {
	/* This is probably the only really important thing to change. */
	f.header_start = f.header_start.checked_add(new_initial_header_start).ok_or(
	invalid!("new header start from merge would have been {} bytes, which is too large",
	f.header_start as u128 + new_initial_header_start as u128))?;
	/* This is only ever used internally to cache metadata lookups (it's not part of the
	* zip spec), and 0 is the sentinel value. */
	f.central_header_start = 0;

3. There are some const InvalidArchive and after it wraps a Cow these statements should be ZipError::InvalidArchive(Cow::Borrow("...")). Maybe I can adjust the invalid! macro to make it const? or another const fn to create a const error literal.

zip2/src/spec.rs

Lines 266 to 267 in e074e09

	const WRONG_MAGIC_ERROR: ZipError =
	ZipError::InvalidArchive("Invalid digital signature header");

[Pr0methean]

Sure, a PR would be great. Do whatever makes the invalid! macro as widely usable as possible, and add a second macro if you need; we can always refactor later.

[Bogay]

I've opened a draft PR, but I noticed an issue with the CI. Currently, the CI only runs fmt and check against zip2, so the code inside fuzz_write and fuzz_read isn't formatted. This results in some unrelated style changes appearing in my PR. Would it make sense to add these checks to the CI in a separate PR first?

[Pr0methean]

Sure, sounds good.

[Bogay]

PR opened (#283). I ran into some clippy problems, so it took longer than expected...