Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zero length files written incorrectly #7

Closed
danyocom opened this issue Apr 15, 2014 · 18 comments
Closed

Zero length files written incorrectly #7

danyocom opened this issue Apr 15, 2014 · 18 comments

Comments

@danyocom
Copy link

The file headers for zero length files should have a compression method of 0 (zero) (same as directories). Otherwise various zip utilities choke/complain.

See section 4.3.8 in https://www.pkware.com/documents/casestudies/APPNOTE.TXT

@ctalkington
Copy link
Member

this should be the case already, unless im missing something.

the test for this situation passes 7zip verify tests,

@ctalkington
Copy link
Member

nvm, i see what you mean. there really isn't a good way to detect this with streams as the data size isnt known til after its been piped. i know the size itself is set after such via a builtin feature of zip that allows putting size and CRC in file descriptor record. However, im pretty sure this isn't possible with compression method.

@danyocom
Copy link
Author

Right, not sure how you would do this with a stream. But with buffers and strings you can determine the length and set the compression method accordingly.

@ctalkington
Copy link
Member

im not sure that section of APPNOTE explains what you think.

Zero-byte files, directories, and other file types that contain no content MUST not include file data.

thats saying you shouldnt have actual content for the file, im fairly sure that streaming a zero length doesn't create any content. the size defaults to 0 and even zlib deflateraw doesnt append headers so should be no real output actually piped.

what program is the issue?

@ctalkington
Copy link
Member

also to note, i go by APPNOTE 2.0 in most cases for compat reasons haven't explored higher until we get into zip64 and such.

@danyocom
Copy link
Author

I have problems on android and MacOS with zero length files compressed using archiver unless I set the compression method to Zero.

if (source.length === 0) { data.compressionMethod = 0; }

in 'ZipStream.prototype._appendBuffer' before writing the file header.

@danyocom
Copy link
Author

@ctalkington
Copy link
Member

yah, has the base features of zip and wide compat at this point.

https://github.com/ctalkington/node-zip-stream/blob/master/APPNOTE.txt

@ctalkington
Copy link
Member

yah, we can def do that for buffer as its more static. the only way with a stream would be to read ahead but that may not work in every case.

@ctalkington
Copy link
Member

0.3.1 should address the Buffer side of this.

@danyocom
Copy link
Author

Thanks for your prompt response! I was expecting the support issue to sit for days-weeks.

@ctalkington
Copy link
Member

there are some issues that end up like that due to lack of response or debug-ability but most the time, i shoot for bug fixes within the first 72 hours of report.

@ctalkington
Copy link
Member

i'm going to think on the Streams side of this for a bit, not liking current solutions coming to mind.

@danyocom
Copy link
Author

So while doing some testing I discovered that zero length files worked in node-archiver 0.4.10 (which zip-stream seems to be based on) but are broken in 0.5.2+.

So the fix I came up with yesterday could be a 'workaround' and the proper implementation could have been in archiver 0.4.10 or vise versa.

The difference seems to be that in 4.10 if the source file is zero bytes the the file is compressed and written anyway. In 0.5.2 if the file is zero bytes the the raw bytes (0 in this case are written) and the compression is not done.

So I guess the header and continents just need to match.
If compressionMethod is 0 then then zero bytes should be written.
If the compression method is 8 then the compressed version of zero bytes should be written.

Again, I am only looking at the buffer implementation not streams. (it could be that streams were never broken ... I did not check)

@ctalkington
Copy link
Member

yah code has diverged a bit from archiver 0.4.10. however, the processStream that its writing to, should have the same effect as its just zlib.deflateRaw extended which has no zlib header so not sure what it was really writing before honestly. i think storing empty files is an acceptable solution but i honestly never had any issues with way zero files. tested with 7zip, total commander, and unzip on windows 7. do you have the exact error/warning you were getting?

EDIT: you can see what changed between archiver 0.4.10 and 0.5.2 here: archiverjs/node-archiver@0.4.10...0.5.2

@danyocom
Copy link
Author

I have a zero length file in my archive called _ZEROFILE created using zip-stream. This is the output of the 7Zip test (before the compression method = 0 fix).

screen shot 2014-04-16 at 11 34 22 am

@ctalkington
Copy link
Member

what code do you use to create the _ZEROFILE?

@ctalkington
Copy link
Member

closing for non-response. please open a new issue if you still have issues with newest version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants