Fix zip files created on Windows being unusable on Linux/MacOS (missing executable bits on directories) #59
Conversation
9e2e73f to
8fc0fd6
Compare
| @@ -92,6 +92,14 @@ var BufferList = require("bl"); | |||
| if (entry.fileName !== expectedName) { | |||
| throw new Error("unexpected entry fileName: " + entry.fileName + ", expected: " + expectedName); | |||
| } | |||
| var mode = entry.externalFileAttributes >>> 16; | |||
| if (/\/$/.test(entry.fileName)) { | |||
There was a problem hiding this comment.
| if (/\/$/.test(entry.fileName)) { | |
| if (entry.fileName.startsWith('/')) { |
There was a problem hiding this comment.
The other spot in this file is using that regex to decide if it's a directory, so just matching that style, though I agree it's not particularly readable. So much so that your suggestion is actually backwards (should be fileName.endsWith('/')) ^_^. endsWith, however, is ES6+ only, and doesn't exist in Node v0.10, that this library is still being tested against.
…ng executable bits on directories) from thejoshwolfe/yazl#59
|
@Jimbly Hello, I'm forking yazl, would it be okay if I use the code from yours? I'm not sure how to interpret the license of a PR that hasn't been merged yet, so I'm asking first. If you don't want, I won't merge it. Thanks. |
|
@ijisol Feel free! Since it's in my fork, which is public with the same license, seems you'd be safe to do so =). If you cherry-pick the commit, it'd even get author attribution in your history, but I don't care if my name's not on it anyway. |
|
@Jimbly I didn't know there was |
…ng executable bits on directories) from thejoshwolfe/yazl#59
If a caller passes in a
modeon theaddEmptyDirectory()call, and that mode is taken directly from the (albeit platform-specific)fs.stat()'smode, we end up creating zip files with directories that do not have the executable bit, which makes them completely unopenable on some platforms (MacOS Finder), and creates bad directory entries on others (Linux).I also couldn't help but fix
setFileAttributesModeto always set either S_IFDIR or S_IFREG, since it seemed really weird that it was only sometimes setting it (only if mode === null). This isn't to fix any observed bug, so I'd be perfectly fine sending a PR without that part of the change if needed for some reason, but I'm guessing all zip clients ignore those two flags anyway, so it probably doesn't matter ^_^.This would resolve the following issues:
sindresorhus/gulp-zip#76
sindresorhus/gulp-zip#106
sindresorhus/gulp-zip#112
Some discussion about trying to work around it at a higher level here: sindresorhus/gulp-zip#117 . Ending conclusion was that there's probably no reasonable situation in which we would want
yazlto be creating zip files that cannot be opened on some platforms, so if it's writing platform-specific mode bits, it makes sense to massage them to be portable.