mcavage / node-ssh-agent

Node.js client library for interacting with the OpenSSH Agent
MIT License
23 stars 9 forks source link

fix possible undefined timeout #5

Closed bahamas10 closed 9 years ago

bahamas10 commented 9 years ago

as is, if options is not supplied, this.timeout will be undefined which can cause an error to be throw when socket.setTimeout is called with undefined.

from node-manta

TypeError: msecs must be a number
    at Object.exports.enroll (timers.js:156:11)
    at Socket.setTimeout (net.js:329:12)
    at SSHAgentClient._request (/Users/dave/dev/node-manta/node_modules/ssh-agent/lib/ssh_agent_client.js:300:10)
    at SSHAgentClient.requestIdentities (/Users/dave/dev/node-manta/node_modules/ssh-agent/lib/ssh_agent_client.js:186:15)
    at sshAgentGetKey (/Users/dave/dev/node-manta/lib/auth.js:216:12)
    at Object.checkAgentForKey [as func] (/Users/dave/dev/node-manta/lib/auth.js:450:17)
    at /Users/dave/dev/node-manta/node_modules/vasync/lib/vasync.js:191:20
    at process._tickCallback (node.js:355:11)
    at Function.Module.runMain (module.js:503:11)
    at startup (node.js:129:16)
trentm commented 9 years ago

@bahamas10 To clarify, that error only happens with node 0.12, right?

bahamas10 commented 9 years ago

@trentm correct. adding some debugging i can see that this.timeout is undefined, but node 8 doesn't seem to have a problem accepting that as a value.

dave - sjc1-dave-01 sunos ~/dev/node-manta (git:master) $ node -v
v0.8.26
dave - sjc1-dave-01 sunos ~/dev/node-manta (git:master) $ ./bin/mls 
timeout => undefined
...
bahamas10 commented 9 years ago

it looks like this change https://github.com/joyent/node/commit/f34757398fcc393685b4dfbcbdc692fb38332d6c exposed this bug.

looking at the previous logic, it seems as though the timeout was just completely ignored (because of the invalid msecs argument)

trentm commented 9 years ago

@pfmooney Do you have creds to commit here? You have creds for the npm registry for ssh-agent.