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

create files with a literal `/` in the name #130

Closed
SheetJSDev opened this Issue May 13, 2014 · 9 comments

Comments

Projects
None yet
3 participants
@SheetJSDev
Contributor

SheetJSDev commented May 13, 2014

OPC requires file names like "foo/bar", but jszip seems to interpret those as folder names:

zip.file('foo/bar', 'baz');
zip.folder('foo').file('bar', 'baz') // equivalent

All of the checks seem to directly look for a slash character. Ideally something like "foo//bar" or "foo/bar" or some other expression would allow for a literal slash in the name

@dduponchel

This comment has been minimized.

Show comment
Hide comment
@dduponchel

dduponchel May 13, 2014

Collaborator

zip.file('foo/bar', 'baz'); will creates 2 entries (in memory and when generating a zip file) : foo/ (no content, marked as directory) and foo/bar (with content). Do you have an example of what you're trying to generate ?

Collaborator

dduponchel commented May 13, 2014

zip.file('foo/bar', 'baz'); will creates 2 entries (in memory and when generating a zip file) : foo/ (no content, marked as directory) and foo/bar (with content). Do you have an example of what you're trying to generate ?

@SheetJSDev

This comment has been minimized.

Show comment
Hide comment
@SheetJSDev

SheetJSDev May 13, 2014

Contributor

@dduponchel Take a peek at https://github.com/sheetjs/test_files/blob/master/AutoFilter.xlsx?raw=true (generated by Excel 2011) and note that directory entries do not show up (at least, according to zip.vim). This is true of zip files from 2007, 2010, 2011, and 2013.

The quote which led me to conclude that Excel wasn't generating individual folders comes from ECMA376 Part II (Open Packaging Conventions) Section 9 (Package Model) Subsection 9.1.1 (Part Names):

The part name "/hello/world/doc.xml" contains three segments: "hello", "world", and "doc.xml" ... Note that segments are not explicitly represented as folders in the package model, and no directory of folders exists in the package model

Contributor

SheetJSDev commented May 13, 2014

@dduponchel Take a peek at https://github.com/sheetjs/test_files/blob/master/AutoFilter.xlsx?raw=true (generated by Excel 2011) and note that directory entries do not show up (at least, according to zip.vim). This is true of zip files from 2007, 2010, 2011, and 2013.

The quote which led me to conclude that Excel wasn't generating individual folders comes from ECMA376 Part II (Open Packaging Conventions) Section 9 (Package Model) Subsection 9.1.1 (Part Names):

The part name "/hello/world/doc.xml" contains three segments: "hello", "world", and "doc.xml" ... Note that segments are not explicitly represented as folders in the package model, and no directory of folders exists in the package model

@dduponchel

This comment has been minimized.

Show comment
Hide comment
@dduponchel

dduponchel May 13, 2014

Collaborator

Indeed, Excel doesn't generate sub folders. The change itself is small : we just need a condition on this folderAdd call (and maybe one on remove() to have a consistent behavior and remove individual entries).
I don't think that a special syntax is a good idea. We could add an options object on the JSZip instance (and the default values on the JSZip object) and a boolean value for this behavior.

Collaborator

dduponchel commented May 13, 2014

Indeed, Excel doesn't generate sub folders. The change itself is small : we just need a condition on this folderAdd call (and maybe one on remove() to have a consistent behavior and remove individual entries).
I don't think that a special syntax is a good idea. We could add an options object on the JSZip instance (and the default values on the JSZip object) and a boolean value for this behavior.

@SheetJSDev

This comment has been minimized.

Show comment
Hide comment
@SheetJSDev

SheetJSDev May 13, 2014

Contributor

@dduponchel alternatively you could disable the magic folder-creation behavior entirely (and require people to explicitly create the folder first).

Contributor

SheetJSDev commented May 13, 2014

@dduponchel alternatively you could disable the magic folder-creation behavior entirely (and require people to explicitly create the folder first).

@dduponchel

This comment has been minimized.

Show comment
Hide comment
@dduponchel

dduponchel May 14, 2014

Collaborator

We could. I tested with some programs (unzip, 7z, the default zip program in windows XP and windows 7) with a zip file with "missing" folders and they all create sub folders when necessary.
But there are a lot of software reading zip files (archive managers, xlsx readers, epub readers, etc) and I can't predict if the result with a zip file with and without extra folder entries. What's your experience with xlsx readers and extra folders ?

Collaborator

dduponchel commented May 14, 2014

We could. I tested with some programs (unzip, 7z, the default zip program in windows XP and windows 7) with a zip file with "missing" folders and they all create sub folders when necessary.
But there are a lot of software reading zip files (archive managers, xlsx readers, epub readers, etc) and I can't predict if the result with a zip file with and without extra folder entries. What's your experience with xlsx readers and extra folders ?

@SheetJSDev

This comment has been minimized.

Show comment
Hide comment
@SheetJSDev

SheetJSDev May 15, 2014

Contributor

Numbers for iOS is showing some strange behavior. Writing the paths in a different order seems to confuse the parser only if the directory entries show up after the [Content_Types].xml subfile. However, reworking the order of writing does seem to resolve the issue.

Food for thought: other JS zip implementations (for example, https://www.npmjs.org/package/archiver) and implementations in other languages (https://rubygems.org/gems/rubyzip) don't create those directory entries.

Contributor

SheetJSDev commented May 15, 2014

Numbers for iOS is showing some strange behavior. Writing the paths in a different order seems to confuse the parser only if the directory entries show up after the [Content_Types].xml subfile. However, reworking the order of writing does seem to resolve the issue.

Food for thought: other JS zip implementations (for example, https://www.npmjs.org/package/archiver) and implementations in other languages (https://rubygems.org/gems/rubyzip) don't create those directory entries.

@Mithgol

This comment has been minimized.

Show comment
Hide comment
@Mithgol

Mithgol May 15, 2014

Contributor

Wait, what does “don't create those directory entries” mean exactly? “Don't create directory entries that are files with / in names”? Or “don't make zip entries that are automatically created directories”?

Contributor

Mithgol commented May 15, 2014

Wait, what does “don't create those directory entries” mean exactly? “Don't create directory entries that are files with / in names”? Or “don't make zip entries that are automatically created directories”?

@SheetJSDev

This comment has been minimized.

Show comment
Hide comment
@SheetJSDev

SheetJSDev May 15, 2014

Contributor

@Mithgol consider this script:

var JSZip = require('jszip');
var fs = require('fs');
var data = fs.readFileSync('old.zip');
var zip = new JSZip(data);
fs.writeFileSync('new.zip', zip.generate({type:'nodebuffer'}));

All it does is parse old.zip and save as new.zip.

Now run this and take a peek at what zip.vim reports:

vim -d old.zip new.zip

I'm not sure why, but those extra directory entries are confusing some readers.

Contributor

SheetJSDev commented May 15, 2014

@Mithgol consider this script:

var JSZip = require('jszip');
var fs = require('fs');
var data = fs.readFileSync('old.zip');
var zip = new JSZip(data);
fs.writeFileSync('new.zip', zip.generate({type:'nodebuffer'}));

All it does is parse old.zip and save as new.zip.

Now run this and take a peek at what zip.vim reports:

vim -d old.zip new.zip

I'm not sure why, but those extra directory entries are confusing some readers.

@dduponchel

This comment has been minimized.

Show comment
Hide comment
@dduponchel

dduponchel May 16, 2014

Collaborator

I've tested some archive managers (windows 7 "compressed folders", winzip, winrar, 7zip, izarc) on this :

  • they all support my two test zips (with and without folders)
  • izarc and winrar have an option to remove them at creation time
  • 7zip, winrar and winzip create sub folders
  • izarc does not
  • windows does sometimes (it seems to skip the first levels)

I've also checked the java API, which doesn't create sub folders.

It seems that changing the default behavior shouldn't break anything.

Collaborator

dduponchel commented May 16, 2014

I've tested some archive managers (windows 7 "compressed folders", winzip, winrar, 7zip, izarc) on this :

  • they all support my two test zips (with and without folders)
  • izarc and winrar have an option to remove them at creation time
  • 7zip, winrar and winzip create sub folders
  • izarc does not
  • windows does sometimes (it seems to skip the first levels)

I've also checked the java API, which doesn't create sub folders.

It seems that changing the default behavior shouldn't break anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment