Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Guard against mime.lookup('foo.constructor') trying to return the lookup table's constructor #24

Merged
merged 3 commits into from

2 participants

Joshua Holbrook Robert Kieffer
Joshua Holbrook

See jfhbrook/node-ecstatic#15 for a more in-depth explanation of the issue.

Thanks! ^__^

Robert Kieffer
Owner

Thanks for the request! Can we inline the hasOwnProperty() check instead of defining the has() method? (Does has() really add that much value?)

if (mime.types.hasOwnProperty(ext) {
...
Joshua Holbrook

Yes, inlining it should work. I added a test, so you can check too.

Robert Kieffer broofa merged commit 39ef52a into from
Robert Kieffer broofa referenced this pull request from a commit
Robert Kieffer patchup README and #24 68f653f
Robert Kieffer
Owner

Oh, sweet, just discovered the "Object.create(null)" trick that creates an Object instance that doesn't inherit from Object.prototype.

See comments in http://www.devthought.com/2012/01/18/an-object-is-not-a-hash/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 16 additions and 2 deletions.
  1. +11 −1 mime.js
  2. +5 −1 test.js
12 mime.js
View
@@ -1,6 +1,10 @@
var path = require('path'),
fs = require('fs');
+function has (obj, key) {
+ return Object.prototype.hasOwnProperty.call(obj, key);
+}
+
var mime = module.exports = {
/** Map of extension to mime type */
types: {},
@@ -60,7 +64,13 @@ var mime = module.exports = {
*/
lookup: function(path, fallback) {
var ext = path.replace(/.*[\.\/]/, '').toLowerCase();
- return mime.types[ext] || fallback || mime.default_type;
+
+ if (has(mime.types, ext)) {
+ return mime.types[ext];
+ }
+ else {
+ return fallback || mime.default_type;
+ }
},
/**
6 test.js
View
@@ -23,6 +23,10 @@ exports["test mime lookup"] = function(test) {
// fallback
test.equal('fallback', mime.lookup('text.fallback', 'fallback'));
+ // an object is not a hash
+ // http://www.devthought.com/2012/01/18/an-object-is-not-a-hash/
+ test.equal('application/octet-stream', mime.lookup('text.constructor'));
+
test.finish();
};
@@ -75,5 +79,5 @@ exports["test charset lookup"] = function(test) {
};
if (module == require.main) {
- require('async_testing').run(__filename, process.ARGV);
+ require('async_testing').run(__filename, process.argv);
}
Something went wrong with that request. Please try again.