danielbolan
commented
11 years ago Hi @opensourcejunkie, welcome to GitHub!
I implemented your suggestion in 61f070d1651edfb70521b37bf58d3056b400f443, though I did it a bit differently. I didn't use exceptions because I was worried that having parse in a try-catch block has a chance of masking other exceptions that could arise and might be better dealt with elsewhere (how big of an issue this is, I'm not sure). It also avoids the overhead of creating and throwing an error that gets immediately caught, however marginal of a gain that is. Still, it should handle invalid/unimplemented commands now, skipping over them and continuing on with the rest of the buffer. You were also right about why the server hangs.
TooTallNate
Like some of the other posters on here, I too was hit by the Ctr+C bug. @danielbolan, thanks for fixing the issue! I'll definitely be applying that patch to my app.
Before I saw the fix though, I attempted a resolution myself; my strategy was to generically handle all unsupported commands. Unfortunately, I am about three days old in Node, and I don't understand a whole lot about the telnet protocol, so I couldn't seem to get a complete fix. However, I suspect that completing the fix will be an easy one-liner for someone familiar with the Node API and telnet, so I'm posting the code anyway.
Essentially the problem is a missing index in the array of command handlers; I therefore just added a check to see if a handler is registered. If it isn't, it throws an error, which is caught in the socket's 'data' event handler, the one parsing the raw data.
The trouble is handling the error. Just continuing/breaking from the loop doesn't solve it; the server just hangs, and will not accept any new input. I suspect that the reason for this is because the command associated with Ctr+C is remaining in the buffer, and needs to be cleared out before it can move forward. I can't really seem to figure out how to do that...a little embarrasing, actually.
But, hopefully someone with a better knowledge of Node/telnet can finish this; should beef up the security on all fronts (if it can happen with Ctr+C, I'm guessing it can happen with other (invalid?) sequences). I doubt you'll want to approve the pull request until it's complete; perhaps merge it into a non-master branch?
Thanks so much for creating this; it's been a huge help! ~ ModeratelyTallNate