Skip to content

Commit

Permalink
fix: Use a null prototype object for this.files
Browse files Browse the repository at this point in the history
This approach is taken to prevent overriding object methods that would
exist on a normal object Object.create({})
  • Loading branch information
MichaelAquilina committed Jun 14, 2021
1 parent b7f472d commit 2235749
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
5 changes: 4 additions & 1 deletion lib/index.js
Expand Up @@ -19,7 +19,10 @@ function JSZip() {
// "folder/" : {...},
// "folder/data.txt" : {...}
// }
this.files = {};
// NOTE: we use a null prototype because we do not
// want filenames like "toString" coming from a zip file
// to overwrite methods and attributes in a normal Object.
this.files = Object.create(null);

this.comment = null;

Expand Down
6 changes: 3 additions & 3 deletions lib/object.js
Expand Up @@ -179,16 +179,16 @@ var out = {
*/
forEach: function(cb) {
var filename, relativePath, file;
/* jshint ignore:start */
// ignore warning about unwanted properties because this.files is a null prototype object
for (filename in this.files) {
if (!this.files.hasOwnProperty(filename)) {
continue;
}
file = this.files[filename];
relativePath = filename.slice(this.root.length, filename.length);
if (relativePath && filename.slice(0, this.root.length) === this.root) { // the file is in the current root
cb(relativePath, file); // TODO reverse the parameters ? need to be clean AND consistent with the filter search fn...
}
}
/* jshint ignore:end */
},

/**
Expand Down

2 comments on commit 2235749

@igandrews
Copy link

Choose a reason for hiding this comment

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

Out of curiosity was there actually a report of such an issue as a file named toString that prompted this change? I ask because this actually introduced a bug in my code that uses jszip because i used hasOwnProperty on the files just like your own code above but now that throws because there is no such member.

@igandrews
Copy link

Choose a reason for hiding this comment

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

Nevermind. I see there was.

Please sign in to comment.