Merge pull request #113 from legraphista/fix/known_hosts-fingerprint-duplication-error/1.0

Fix: regarding PR #110 edge cases

Author: Zhen Li

src/v1/internal/ch-node.js

@@ -75,17 +75,27 @@ function loadFingerprint( serverId, knownHostsPath, cb ) {
   });
 }
 
-function storeFingerprint(serverId, knownHostsPath, fingerprint) {
+const _lockFingerprintFromAppending = {};
+function storeFingerprint( serverId, knownHostsPath, fingerprint, cb ) {
+  // we check if the serverId has been appended
+  if(!!_lockFingerprintFromAppending[serverId]){
+    // if it has, we ignore it
+    return cb(null);
+  }
+
   // If file doesn't exist, create full path to it
   try {
     fs.accessSync(knownHostsPath);
   } catch (_) {
     mkFullPath(path.dirname(knownHostsPath));
   }
+
   fs.appendFile(knownHostsPath, serverId + " " + fingerprint + EOL, "utf8", (err) => {
+    delete _lockFingerprintFromAppending[serverId];
     if (err) {
       console.log(err);
     }
+    return cb(err);
   });
 }
 
@@ -152,8 +162,12 @@ const TrustStrategy = {
         if( knownFingerprint === serverFingerprint ) {
           onSuccess();
         } else if( knownFingerprint == null ) {
-          storeFingerprint( serverId, knownHostsPath, serverFingerprint );
-          onSuccess();
+          storeFingerprint( serverId, knownHostsPath, serverFingerprint, (err) => {
+            if (err) {
+              return onFailure(err);
+            }
+            return onSuccess();
+          });
         } else {
           onFailure(newError("Database encryption certificate has changed, and no longer " +
             "matches the certificate stored for " + serverId + " in `" + knownHostsPath +

test/internal/tls.test.js

@@ -180,6 +180,60 @@ describe('trust-on-first-use', function() {
       done();
     });
   });
+
+  it('should not duplicate fingerprint entries', function(done) {
+    // Assuming we only run this test on NodeJS with TOFU support
+    if( !hasFeature("trust_on_first_use") ) {
+      done();
+      return;
+    }
+
+    // Given
+    var knownHostsPath = "build/known_hosts";
+    if( fs.existsSync(knownHostsPath) ) {
+      fs.unlinkSync(knownHostsPath);
+    }
+    fs.writeFileSync(knownHostsPath, '');
+
+    driver = neo4j.driver("bolt://localhost", neo4j.auth.basic("neo4j", "neo4j"), {
+      encrypted: true,
+      trust: "TRUST_ON_FIRST_USE",
+      knownHosts: knownHostsPath
+    });
+
+    // When
+    driver.session();
+    driver.session();
+
+    // Then
+    setTimeout(function() {
+      var lines = {};
+      fs.readFileSync(knownHostsPath, 'utf8')
+          .split('\n')
+          .filter(function(line) {
+            return !! (line.trim());
+          })
+          .forEach(function(line) {
+            if (!lines[line]) {
+              lines[line] = 0;
+            }
+            lines[line]++;
+          });
+
+      var duplicatedLines = Object
+          .keys(lines)
+          .map(function(line) {
+            return lines[line];
+          })
+          .filter(function(count) {
+            return count > 1;
+          })
+          .length;
+
+      expect( duplicatedLines ).toBe( 0 );
+      done();
+    }, 1000);
+  });
 
   it('should should give helpful error if database cert does not match stored certificate', function(done) {
     // Assuming we only run this test on NodeJS with TOFU support