search

TooTallNate / node-telnet

Telnet implementation for Node.js
87 stars 30 forks source link

A fix to invalid commands causing node-telnet to crash irrevocably. #10

Open dazhazit opened 9 years ago

dazhazit commented 9 years ago

I've been waiting for a few months for a fix to this so I did it myself. OK, try/catch could have been used but it seemed a cleaner solution to put in guard code where the crash is happening and just ignore the command code or option code entirely.

dazhazit commented 9 years ago

One unit test fail on one version of node and we have a pull failure? No offence but the unit tests would never have passed on v0.8.28 with even the original master as AFAIK stream.end() does not accept a function in that version. So I guess it's time to bugfix the unit tests too? http://nodejs.org/docs/v0.8.28/api/stream.html#stream_stream_end_string_encoding

georgefrick commented 9 years ago

I assume this is a case such as sending command EL will crash the host? It's better to add the handling for these then to swallow the error.

  1. add EOF, SUSP, and ABORT, as 236, 237, 238. then:

;['eof','susp','abort','ec','el'].forEach(function (command) { var code = COMMANDS[command.toUpperCase()] COMMAND_IMPLS[code] = function (bufs, i, event) { // Needs to be converted to a NOP, we don't want to act on it. event.buf = bufs.splice(0, i).toBuffer() event.data = Buffer([ 241 ]) return event } })

Really, additional linemode support should be added; but this code will prevent crashes. I also agree about the unit tests. Due to lack of reply, a fork is probably in order.