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

hashForDep with invalid input, would not error rather would produce a checksum. So restoring this for now, although we should remove in the next major version.

One deviation from the old behavior is to ensure the all garbage input doesn’t produce the same hash, rather it will get a hash based on inputs provided.

Context:

* emberjs/ember.js#17679
* emberjs/ember.js#17678

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);