[Remote-SSH Bug]: Race condition when parsing ssh.exe -V output on Windows #10497
Description
Is there an existing issue for this bug?
- I have searched the existing issues
Required Troubleshooting Steps
- I have followed these troubleshooting steps
- I have tried both values of the
remote.SSH.useLocalServersetting (more info here)
Connect Locally
It connects successfully
->
No response
Expected Behavior
Connection with the remote-ssh extension should work without issues when the target host is reachable.
I have a hacky code workaround I've validated and a suggestion for how to fix this, please see below!
Steps To Reproduce
- Windows 10 in a corp environment
- Large number of strange antivirus programs installed
- Try to connect vs code to a remote linux server
- For some reason the 'exit' event of the ssh version string check subprocess fires before the stderr data of the ssh process is received
- Connection attempt hangs indefinitely
Remote-SSH Log
I'm unable to provide the log due to corporate restrictions.
Anything else?
The problem exists in the below code in the extension (ran the js through a beautifier to make it readable)
t.debug(`Checking ssh with "${e} -V"`);
const i = [],
s = [];
let a, c = e;
f.isWindows && (c.endsWith(".bat") || c.endsWith(".cmd")) && (c = `"${c}"`, a = {
shell: !0
});
const l = n.spawn(c, ["-V"], a);
l.stdout.on("data", (e => {
i.push(e), t.debug("stdout> " + e.toString())
})), l.stderr.on("data", (e => {
s.push(e), t.debug("> " + e.toString())
})), l.on("error", (e => {
t.debug("Got error from ssh: " + e.message), clearTimeout(d), r(I.NotFound)
}));
let u = !1;
const d = setTimeout((() => {
u = !0, t.debug("ssh is not exiting, continuing"), r(I.NotFound), l.kill()
}), 1e4);
l.on("exit", (e => { // XXX we get here before s.push above has been called with the version string data
if (u) return;
if (clearTimeout(d), e) return t.debug("ssh exited with code: " + e), void r(I.NotFound);
const n = Buffer.concat(i).toString("utf8").trim(),
o = Buffer.concat(s).toString("utf8").trim() || n;
o && (o.match(/OpenSSH_for_Windows/i) ? r(I.WindowsSsh) : o.match(/OpenSSH/i) ? r(I.Other) : (t.debug("ssh output did not match /OpenSSH/"), r(I.NotFound)))
}))
In my case, just calling r(I.WindowsSsh) on exit is sufficient to make connections work every time.
As a longer term fix, perhaps there is a way to use a promise instead of relying on the ordering of the stderr data and exit events to parse the version string. The nodejs docs say that 'When the 'exit' event is triggered, child process stdio streams might still be open.' but I'm unable to tell if that suggests that stderr may still contain data when 'exit' is triggered or not.
Additionally, it's not entirely clear to me why the failure mode is no progress instead of I.NotFound being passed to the result callback.