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

Fix/known hosts fingerprint duplication error/upstream 1.0

Author: Zhen Li

src/v1/internal/ch-node.js

@@ -45,7 +45,7 @@ function loadFingerprint( serverId, knownHostsPath, cb ) {
   require('readline').createInterface({
     input: fs.createReadStream(knownHostsPath)
   }).on('line', (line)  => {
-    if( line.startsWith( serverId )) {
+    if( !found && line.startsWith( serverId )) {
       found = true;
       cb( line.split(" ")[1] );
     }
@@ -56,12 +56,30 @@ function loadFingerprint( serverId, knownHostsPath, cb ) {
   });
 }
 
-function storeFingerprint(serverId, knownHostsPath, fingerprint) {
+const _lockFingerprintFromAppending = {};
+function storeFingerprint( serverId, knownHostsPath, fingerprint ) {
+  // we check if the serverId has been appended
+  if(!!_lockFingerprintFromAppending[serverId]){
+    // if it has, we ignore it
+    return;
+  }
+
+  // we make the line as appended
+  // ( 1 is more efficient to store than true because true is an oddball )
+  _lockFingerprintFromAppending[serverId] = 1;
+
+  // we append to file
   fs.appendFile(knownHostsPath, serverId + " " + fingerprint + EOL, "utf8", (err) => {
     if (err) {
       console.log(err);
     }
   });
+
+  // since the error occurs in the span of one tick
+  // after one tick we clean up to not interfere with anything else
+  setImmediate(() => {
+    delete _lockFingerprintFromAppending[serverId];
+  });
 }
 
 const TrustStrategy = {

test/internal/tls.test.js

@@ -76,6 +76,44 @@ describe('trust-on-first-use', function() {
 
   var driver;
 
+  it('should not throw an error if the host file contains two host duplicates', function(done) {
+    'use strict';
+    // 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);
+    }
+
+    driver = neo4j.driver("bolt://localhost", neo4j.auth.basic("neo4j", "neo4j"), {
+      encrypted: true,
+      trust: "TRUST_ON_FIRST_USE",
+      knownHosts: knownHostsPath
+    });
+
+    driver.session(); // write into the knownHost file
+
+    // duplicate the same serverId twice
+    setTimeout(function() {
+      var text = fs.readFileSync(knownHostsPath, 'utf8');
+      fs.writeFileSync(knownHostsPath, text + text);
+    }, 1000);
+
+    // When
+    setTimeout(function() {
+      driver.session().run("RETURN true AS a").then( function(data) {
+        // Then we get to here.
+        expect( data.records[0].get('a') ).toBe( true );
+        done();
+      });
+    }, 2000);
+  });
+
   it('should accept previously un-seen hosts', function(done) {
     // Assuming we only run this test on NodeJS with TOFU support
     if( !hasFeature("trust_on_first_use") ) {
@@ -104,6 +142,59 @@ describe('trust-on-first-use', function() {
     });
   });
 
+  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();
+
+    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
     if( !hasFeature("trust_on_first_use") ) {