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

Add support for UNIX / DOS permissions #200

Merged
merged 1 commit into from
Feb 19, 2015

Conversation

dduponchel
Copy link
Collaborator

Two new fields on ZipObject, unixPermissions and dosPermissions, hold the
UNIX or DOS permissions of the file. A new option of generate(),
platform (DOS or UNIX) controls the use of the permissions.

The default behavior is to generate DOS archives, without any
permissions, like before.

Bonus side-effect : Finder on mac doesn't use the DOS directory flag,
JSZip didn't properly recognize folders until now.

Fix #194 and #198.

Two new fields on ZipObject, `unixPermissions` and `dosPermissions`, hold the
UNIX or DOS permissions of the file. A new option of `generate()`,
`platform` (DOS or UNIX) controls the use of the permissions.

The default behavior is to generate DOS archives, without any
permissions, like before.

Bonus side-effect : Finder on mac doesn't use the DOS directory flag,
JSZip didn't properly recognize folders until now.

Fix Stuk#194 and Stuk#198.
}

return extFileAttr;
};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fun stuff :)

@Stuk
Copy link
Owner

Stuk commented Feb 19, 2015

Wow, great work.

Stuk added a commit that referenced this pull request Feb 19, 2015
Add support for UNIX / DOS permissions
@Stuk Stuk merged commit 6c63447 into Stuk:master Feb 19, 2015
@dduponchel
Copy link
Collaborator Author

Thanks !

@dduponchel dduponchel deleted the feature-permissions branch February 19, 2015 18:27
@dduponchel
Copy link
Collaborator Author

I chose simplified permissions (readOnly/executable/etc versus the full unix/dos permissions/mode) for two reasons :

  • It's easy to get and set (no need for binary mask, octal is forbidden in strict mode)
  • nodejs on windows returns a non executable mode for folders (if generated with platform:"UNIX", this will render it unusable on linux/mac)

Unfortunately, this simplification may hurt too many nodejs users. If they compress a 660 file, JSZip will transform it into a 644 file (it ignores the "group" and "other" part). Same for folders : 750 ? You will get 755.

The best solution may be to replace the *Permissions fields with their numerical representation (from nodejs' stats.mode for example) and using them directly. No more surprises, the user get what is on the disk.
For the first issue (get/set permissions), it may be too much for a zip library to fully parse the file mode (nodejs' fs.Stats don't do that either). To easily set it, we can allow a string ("755", "640" for example) and transform it.
For the second issue, this edge case may be just a documentation issue : something like "Warning : if you set platform in nodejs, be sure to use process.platform as value. <explanation here>".

Does that sounds good to you @Stuk ?

We also lack an options parameter on folder() to set permissions :-)

dduponchel added a commit to dduponchel/jszip that referenced this pull request Mar 8, 2015
This is an update for Stuk#200.

If we don't keep the original unix/dos permissions, this will lead to
hard-to-debug issues, where the permissions are not exactly like they
were specified.
This commit changes that and uses the exact file mode. It also updates the
default umask from 022 to 002 (with unixPermissions == null, files will
get 0664 and folders 0775).

To put a folder with custom permissions, folder() is not a good
candidate : its behavior is to re-use existing entries. Instead, this
commit documents the existing dir attribute.
This was referenced Mar 8, 2015
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

Successfully merging this pull request may close these issues.

[feature request] generate UNIX extra fields
2 participants