Merge pull request #56 from stefanpenner/restore-strange-functionality

[BUGFIX] restore old behavior – hashing garbage input should not error

lib/module-entry.js

@@ -17,7 +17,7 @@ var crypto = require('crypto');
  */
 module.exports = ModuleEntry;
 
-function ModuleEntry(name, version, rootDir, sourceHash) {
+function ModuleEntry(name, version, rootDir, sourceHash, isValid) {
 
   // The name of the module, from its package.json file.
   this.name = name;
@@ -43,6 +43,8 @@ function ModuleEntry(name, version, rootDir, sourceHash) {
   // to the dependencies listed in the package.json file.
   // See locate() for initialization of this value.
   this._dependencies = Object.create(null);
+
+  this.isValid = isValid === false ? false : true;
 }
 
 ModuleEntry.prototype.addDependency = function(dependency, moduleEntry) {
@@ -115,6 +117,17 @@ ModuleEntry.prototype.getHash = function(heimdallNode) {
   return (this._hash = hash);
 };
 
+/*
+ * This exists to maintain compatibbility with some unexpected old behavior.
+ * Specifically invalid input, would still produce a hash.
+ *
+ * This is clearly not helpful, but to fix a regression we will restore this
+ * behavior, but the next major version (2.x.x) we should remove support for
+ * this, and instead error with a helpful error message
+ */
+ModuleEntry.buildInvalidModule = function(name, dir) {
+  return new this(name, '0.0.0' , dir, crypto.createHash('sha1').update([name, dir].filter(Boolean).join('\x00')).digest('hex'), false);
+};
 
 /*
  * Compute and return the ModuleEntry for a given name/dir pair. This is done
@@ -139,7 +152,7 @@ ModuleEntry.locate = function(caches, name, dir, hashTreeFn) {
     if (realPathKey !== null) {
       return caches.MODULE_ENTRY.get(realPathKey);
     } else {
-      return null;
+      return this.buildInvalidModule(name, dir);
     }
   }
 
@@ -150,8 +163,7 @@ ModuleEntry.locate = function(caches, name, dir, hashTreeFn) {
   var realPath = resolvePackagePath(name, dir, caches);;
 
   if (realPath === null) {
-    caches.PATH.set(nameDirKey, null);
-    return null;
+      return this.buildInvalidModule(name, dir);
   }
 
   // We have a path to a file that supposedly is package.json. We need to be sure

tests/hash-for-dep-test.js

@@ -69,6 +69,11 @@ describe('hashForDep', function() {
     assert.equal(result, 'b2d270f1274267a5fe29a49b5d44bb86125977f9', 'Expected sha1');
   });
 
+  it('maintains compatibility for now, not erroring for garbage input', function() {
+    expect(hashForDep('garbage-non-existing')).to.eql('c5c7d7981c22e790055a9b6e98a2972b8d14a599');
+    expect(hashForDep('garbage-non-existing', 'with-garbage-basedir')).to.eql('507cb109ec6bd0781751f7eb65faf98183c79375');
+  });
+
   describe('cache', function() {
     it('caches', function() {
       expect(hashForDep._cache.size).to.eql(0);
@@ -129,7 +134,7 @@ describe('hashForDep', function() {
 
       expect(hashForDep._cache.size).to.eql(4);
       expect(hashForDep._caches.MODULE_ENTRY.size).to.eql(4);
-      expect(hashForDep._caches.PATH.size).to.eql(10);
+      expect(hashForDep._caches.PATH.size).to.eql(9);
     });
   });
 });

tests/module-entry-test.js

@@ -35,7 +35,7 @@ describe('ModuleEntry', function() {
 
   it('._gatherDependencies', function() {
     // gatherDependencies only occurs during getHash().
-    var moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, hashTree);  
+    var moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, hashTree);
 
     var dependencies = Object.create(null);
     moduleEntry._gatherDependencies(dependencies);
@@ -62,8 +62,8 @@ describe('ModuleEntry', function() {
 
     assert.equal(moduleEntry.rootDir,
                  FOO_DIR_PATH,
-                 'rootDir must be in node_modules/foo');
-    assert.equal(Object.keys(moduleEntry._dependencies).length, 2, 'foo._dependencies must have 2 entries');
+      'rootDir must be in node_modules/foo');
+    assert.equal(Object.keys(moduleEntry._dependencies).length, 3, 'foo._dependencies must have 2 entries');
 
     // XXX we should check which dependencies they are.
 
@@ -73,7 +73,7 @@ describe('ModuleEntry', function() {
     moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, hashTree);
 
     var hash2 = moduleEntry.getHash();
-    
+
     assert.equal(hash1, hash2, 'getHash should return the same hash after clearing and recalculating');
   });
 
@@ -84,23 +84,27 @@ describe('ModuleEntry', function() {
 
     var moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, newHashFn);
     assert.equal(moduleEntry.rootDir, FOO_DIR_PATH, 'rootDir must be in node_modules/foo');
-    assert.equal(Object.keys(moduleEntry._dependencies).length, 2, 'foo._dependencies must have 2 entries');
+    assert.equal(Object.keys(moduleEntry._dependencies).length, 3, 'foo._dependencies must have 2 entries');
 
     var hash1 = moduleEntry.getHash();
 
     caches = new CacheGroup();
     moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, newHashFn);
 
     var hash2 = moduleEntry.getHash();
-    
+
     assert.equal(hash1, hash2, 'getHash should return the same hash after clearing and recalculating');
   });
 
   it('.locate (class method) with invalid package', function() {
     var moduleEntry = ModuleEntry.locate(caches, 'foo2', fixturesPath, hashTree);
-    assert.isNull(moduleEntry);
+    var moduleEntry2 = ModuleEntry.locate(caches, 'foo2', fixturesPath, hashTree);
+    assert.isNotNull(moduleEntry);
+    var hash1 = moduleEntry.getHash();
+    assert.isOk(typeof hash1 === 'string');
+    assert.equal(hash1, moduleEntry2.getHash());
   });
-  
+
   it('.getHash', function() {
     var moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, hashTree);