Fix/known hosts fingerprint duplication error/upstream 1.0

[legraphista]

Fixes issue #107 & #108
Fix: loadFingerprint callback only called once
Fix: storeFingerprint does not duplicate information in the known_hosts file
Add: test cases

[zhenlineo]

The two newly added tests failed on our test server.
The error messages are:

Failures: 
1) trust-on-first-use should not throw an error if the host file contains two host duplicates
1.1) Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.
         at null._onTimeout (/opt/teamcity-agent/work/724ebdf14baddd87/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1909:23)
         at Timer.listOnTimeout (timers.js:92:15)

2) trust-on-first-use should not duplicate fingerprint entries
2.1) Expected 2 to be 1.
     Error: Expected 2 to be 1.
         at stack (/opt/teamcity-agent/work/724ebdf14baddd87/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1577:17)
         at buildExpectationResult (/opt/teamcity-agent/work/724ebdf14baddd87/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1547:14)
         at Spec.Env.expectationResultFactory (/opt/teamcity-agent/work/724ebdf14baddd87/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:638:18)
         at Spec.addExpectationResult (/opt/teamcity-agent/work/724ebdf14baddd87/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:330:34)
         at Expectation.addExpectationResult (/opt/teamcity-agent/work/724ebdf14baddd87/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:588:21)
         at Expectation.toBe (/opt/teamcity-agent/work/724ebdf14baddd87/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1501:12)
         at null._onTimeout (/opt/teamcity-agent/work/724ebdf14baddd87/test/internal/tls.test.js:167:76)
         at Timer.listOnTimeout (timers.js:92:15)

129 specs, 2 failures

I fixed the first error in #111, which is just because the server failed to connect to 7687 within timeout as the cert you created in the test is not real.

However the second one I have not think of a way to fix. The file DID have two fingerprints sometimes. I am guessing it might because the lock var cannot ensure the two processes seeing the change to the lock var immediately (a.k.a. the lock var is not atomic). So in #111 I ignored your second change. You are most welcome to submit your new changes if you know why this could happen.

To run the tests locally, the lazy way is run ./runTests.sh (or .\runTests.ps1 powershell script with admin right for windows). It will download and start/stop a server for you automatically. But make sure you have no real server running on 7474 in case the test clean your important db data in tests.

Thanks again for helping us!
Zhen

[legraphista]

Its weird, I did run the tests locally and none failed. I tend to believe that somehow the tls.test.js did not run for me. I think it's because I ran npm run test without npm i first.

As for the lock var, NodeJS is single-threaded so there is no concept of mutex or atomicity, but running multiple node instances of the same code will not share the lock var and thus it cannot ensure uniqueness.

[legraphista]

@zhenlineo The issue were the tests, I have fixed them in the last commit

[zhenlineo]

I actually changed that line to expect( fs.readFileSync(knownHostsPath, 'utf8').split('\n').filter(v=>v!='').length ).toBe( 1 ); this morning but it still shows two to me sometime. The error happens around 1 out of 4 runs. Let me try this new one out tomorrow. Thanks a lot again.

[zhenlineo]

• Currently test server failed with an unrelated tck error. Working on a fix for the test server now so that this pr could get in without any error.

[legraphista]

I will look into this, see if i can dig up a deeper issue

[legraphista]

I have reworked the test to check for duplicate lines, but I cannot reproduce the occasional 1 in 4 line duplicates that you are experiencing for should not duplicate fingerprint entries test.

[zhenlineo]

It is still wrong in my local runs, but I will merge this PR and let the whole build system to go over the tests. Will let you know if the test is also flaky on test server. Otherwise, it might just my local settings is somehow wrong.

[legraphista]

Can you please detail a bit your setup, maybe I can reproduce it in a VM. Like OS and node version. I ran on OSX 10.11 with node 4.4.5 and 6.3.

[zhenlineo]

I have OSX 10.11 with node 4.4.5. I got the error if I run with fdescribe on trust-on-first-use tests only. The whole build server accept the change without any error. So maybe just some thing wrong with my own machine.

[legraphista]

@zhenlineo Using fdescribe yielded the issue that you were describing. I have fixed and pushed it to the same branch. I will open another pull request and try do describe the issue.