TooTallNate / node-telnet

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

Telnet server crashes when CTRL-C pressed in linux telnet client #1

Open doughsay opened 11 years ago

doughsay commented 11 years ago

Running the example in the README and connecting to it using "$ telnet localhost 23" on linux, and then hitting CTRL-C to exit the telnet session, causes the server to crash with this error:

/home/chris/src/test/node_modules/telnet/lib/telnet.js:153
  return COMMAND_IMPLS[command](bufs, i + 1, event)
                               ^
TypeError: Property '244' of object #<Object> is not a function
    at parseCommand (/home/chris/src/test/node_modules/telnet/lib/telnet.js:153:32)
    at parse (/home/chris/src/test/node_modules/telnet/lib/telnet.js:146:10)
    at Socket.<anonymous> (/home/chris/src/test/node_modules/telnet/lib/telnet.js:201:11)
    at Socket.emit (events.js:67:17)
    at TCP.onread (net.js:367:14)
TooTallNate commented 11 years ago

Thanks for the report. Weird thing - I've noticed that when you run a telnet server on port 23 (the default port), then a lot more telnet commands gets sent in both directions. But if you bind the telnet server to any other port, the telnet(1) client seems to think you're talking to a dumb server or something. Point being: it may work if you bind to some other port, however this is indeed a bug and I would like to get it fixed at some point.

doughsay commented 11 years ago

Shoot, I'm sorry I was a bit misleading in my pasted stuff. I was actually running it on port 9999. I just tried actually running it on 23 (which requires root apparently) and it behaves much differently. i.e. it duplicates all sent characters immediately (without waiting for newline), prints ^M on returns, and doesn't do anything when hitting CTRL-C. To get out of the session, the only thing to do is escape with CTRL^] and then CTRL-D.

I'm not very familiar with the telnet protocol, and I'm not exactly sure what behavior I'm looking for. I just don't want people to be able to kill the server by sending CTRL-C (since I will most likely be using a port other than 23). I will take a closer look at the code and possibly submit a pull request if I find a decent solution.

TooTallNate commented 11 years ago

A patch is definitely welcome. I have a lot of project's I'm trying to get done in my free time.

What OS are you using out of curiosity?

doughsay commented 11 years ago

Ubuntu 12.10, Linux 3.5.0.

I also just tried it on MacOS 10.7.5, and the behavior is the same. (i.e. it crashes the server if you send ctrl-c through the command line telnet program)

Hydrothermal commented 10 years ago

This is super old and I hate to disturb a dead issue, but on the off chance that somebody in the future stumbles into this problem, I discovered why this is happening/how to fix it. It's not actually an issue with the telnet server, but with net, which hiccups with the ECONNRESET error when a connected client quits unexpectedly. See the "net: don't suppress ECONNRESET (Ben Noordhuis)" line on the Node v0.9.10 release notes.

After reading through a pair of StackOverflow questions here and here, it turns out that you can easily catch the error without going into the telnet server's source at all. The second question's accepted answer provides this concise handler:

socket.on('error', function (exc) {
    sys.log("ignoring exception: " + exc);
});

This should work fine, although I would suggest something more along the lines of this:

client.on('error', function(e) {
    if(e.code === "ECONNRESET") {
        console.log("Client quit unexpectedly; ignoring exception.");
    } else {
        console.log("Exception encountered:");
        console.log(e.code);
        process.exit(1);
    }
});
loretoparisi commented 8 years ago

@Hydrothermal thanks I have added then the error handler so that the simplest example would be

telnet.createServer(function (client) {
  console.log("Starting telnet server");
  // make unicode characters work properly
  client.do.transmit_binary()

  // make the client emit 'window size' events
  client.do.window_size()

  // listen for the window size events from the client
  client.on('window size', function (e) {
    if (e.command === 'sb') {
      console.log('telnet window resized to %d x %d', e.width, e.height)
    }
  })

  client.on('error', function(e) {
    if(e.code === "ECONNRESET") {
        console.log("Client quit unexpectedly; ignoring exception.");
    } else {
        console.log("Exception encountered:");
        console.log(e.code);
        process.exit(1);
    }
  });

  // listen for the actual data from the client
  client.on('data', function (b) {
    client.write(b)
  })

  client.write('\nConnected to Telnet server!\n')

}).listen(8989)

But I get the same error indeed.

Hydrothermal commented 8 years ago

@loretoparisi That seems to work for me. Can you copy the text of your error here?

loretoparisi commented 8 years ago

@Hydrothermal I'm not sure what the error was, mostly due to a bad merge, I have noticed several separated merge, so at the end I put here all together, this works for every signal then

sorry too lazy to issue a PR, also not sure why it's not there already:

/**
 * Telnet server implementation.
 *
 * References:
 *  - http://tools.ietf.org/html/rfc854
 *  - http://support.microsoft.com/kb/231866
 *  - http://www.iana.org/assignments/telnet-options
 *
 */

// export the "telnet" convenience function directly
module.exports = exports = telnet

var net = require('net')
  , assert = require('assert')
  , debug = require('debug')('telnet')
  , Stream = require('stream')
  , Buffers = require('buffers')
  , Binary = require('binary')

var COMMANDS = {
    SE:   240 // end of subnegotiation parameters
  , EOR:  239 // end of record 
  , NOP:  241 // no operation
  , DM:   242 // data mark
  , BRK:  243 // break
  , IP:   244 // suspend (a.k.a. "interrupt process")
  , AO:   245 // abort output
  , AYT:  246 // are you there?
  , EC:   247 // erase character
  , EL:   248 // erase line
  , GA:   249 // go ahead
  , SB:   250 // subnegotiation
  , WILL: 251 // will
  , WONT: 252 // wont
  , DO:   253 // do
  , DONT: 254 // dont
  , IAC:  255 // interpret as command
}

var OPTIONS = {
    TRANSMIT_BINARY: 0         // http://tools.ietf.org/html/rfc856
  , ECHO: 1                    // http://tools.ietf.org/html/rfc857
  , SUPPRESS_GO_AHEAD: 3       // http://tools.ietf.org/html/rfc858
  , STATUS: 5                  // http://tools.ietf.org/html/rfc859
  , TIMING_MARK: 6             // http://tools.ietf.org/html/rfc860
  , TERMINAL_TYPE: 24          // http://tools.ietf.org/html/rfc1091
  , END_OF_RECORD: 25          // http://tools.ietf.org/html/rfc885
  , WINDOW_SIZE: 31            // http://tools.ietf.org/html/rfc1073
  , TERMINAL_SPEED: 32         // http://tools.ietf.org/html/rfc1079
  , REMOTE_FLOW_CONTROL: 33    // http://tools.ietf.org/html/rfc1372
  , TERMINAL_SPEED: 32         // http://tools.ietf.org/html/rfc1079
  , REMOTE_FLOW_CONTROL: 33    // http://tools.ietf.org/html/rfc1372
  , LINEMODE: 34               // http://tools.ietf.org/html/rfc1184
  , X_DISPLAY_LOCATION: 35     // http://tools.ietf.org/html/rfc1096
  , AUTHENTICATION: 37         // http://tools.ietf.org/html/rfc2941
  , ENVIRONMENT_VARIABLES: 39  // http://tools.ietf.org/html/rfc1572
}

var IAC_BUF = new Buffer([ COMMANDS.IAC ])

var COMMAND_NAMES = Object.keys(COMMANDS).reduce(function (names, name) {
  names[COMMANDS[name]] = name.toLowerCase()
  return names
}, {})

var OPTION_NAMES = Object.keys(OPTIONS).reduce(function (names, name) {
  names[OPTIONS[name]] = name.toLowerCase().replace(/_/g, ' ')
  return names
}, {})

var COMMAND_IMPLS = {}
;['do','dont','will','wont','sb'].forEach(function (command) {
  var code = COMMANDS[command.toUpperCase()]
  COMMAND_IMPLS[code] = function (bufs, i, event) {
    // needs to read 1 byte, for the command
    //console.error(command, bufs)
    if (bufs.length < (i+1)) return MORE
    return parseOption(bufs, i, event)
  }
})

// IAC
//   this will happen in "binary" mode, two IAC bytes needs to be translated
//   into 1 "data" event with a 1-length Buffer of value 255.
COMMAND_IMPLS[COMMANDS.IAC] = function (bufs, i, event) {
  event.buf = bufs.splice(0, i).toBuffer()
  event.data = Buffer([ 255 ])
  return event
}

COMMAND_IMPLS[COMMANDS.IP] = function (bufs, i, event) { 
  event.buf = bufs.splice(0,i).toBuffer()
  event.data = Buffer([ 255, 244 ])
 return event
}

COMMAND_IMPLS[COMMANDS.EOR] =
COMMAND_IMPLS[COMMANDS.SE] = 
COMMAND_IMPLS[COMMANDS.NOP] =
COMMAND_IMPLS[COMMANDS.DM] =
COMMAND_IMPLS[COMMANDS.BRK] =
COMMAND_IMPLS[COMMANDS.IP] =
COMMAND_IMPLS[COMMANDS.AO] =
COMMAND_IMPLS[COMMANDS.AYT] =
COMMAND_IMPLS[COMMANDS.EC] =
COMMAND_IMPLS[COMMANDS.EL] =
COMMAND_IMPLS[COMMANDS.GA] = function (bufs, i, event) {
    event.buf = bufs.splice(0,i).toBuffer()
    return event
}

var OPTION_IMPLS = {}
// these ones don't take any arguments
OPTION_IMPLS.NO_ARGS =
OPTION_IMPLS[OPTIONS.ECHO] =
OPTION_IMPLS[OPTIONS.STATUS] =
OPTION_IMPLS[OPTIONS.TIMING_MARK] =
OPTION_IMPLS[OPTIONS.LINEMODE] =
OPTION_IMPLS[OPTIONS.TRANSMIT_BINARY] =
OPTION_IMPLS[OPTIONS.AUTHENTICATION] =
OPTION_IMPLS[OPTIONS.TERMINAL_SPEED] =
OPTION_IMPLS[OPTIONS.TERMINAL_TYPE] =
OPTION_IMPLS[OPTIONS.END_OF_RECORD] =
OPTION_IMPLS[OPTIONS.REMOTE_FLOW_CONTROL] =
OPTION_IMPLS[OPTIONS.X_DISPLAY_LOCATION] =
OPTION_IMPLS[OPTIONS.SUPPRESS_GO_AHEAD] = function (bufs, i, event) {
  event.buf = bufs.splice(0, i).toBuffer()
  return event
}

OPTION_IMPLS[OPTIONS.WINDOW_SIZE] = function (bufs, i, event) {
  if (event.commandCode !== COMMANDS.SB) {
    OPTION_IMPLS.NO_ARGS(bufs, i, event)
  } else {
    // receiving a "resize" event
    if (bufs.length < 9) return MORE
    event.buf = bufs.splice(0, 9).toBuffer()
    Binary.parse(event.buf)
      .word8('iac1')
      .word8('sb')
      .word8('naws')
      .word16bu('width')
      .word16bu('height')
      .word8('iac2')
      .word8('se')
      .tap(function (vars) {
        //console.error(vars)
        assert(vars.iac1 === COMMANDS.IAC)
        assert(vars.iac2 === COMMANDS.IAC)
        assert(vars.sb === COMMANDS.SB)
        assert(vars.se === COMMANDS.SE)
        assert(vars.naws === OPTIONS.WINDOW_SIZE)
        event.cols = event.columns = event.width = vars.width
        event.rows = event.height = vars.height
      })
  }
  return event
}

OPTION_IMPLS[OPTIONS.ENVIRONMENT_VARIABLES] = function (bufs, i, event) {
  if (event.commandCode !== COMMANDS.SB) {
    OPTION_IMPLS.NO_ARGS(bufs, i, event)
  } else {
    // Minimum size should be 10:
    if (bufs.length < 10) return MORE
    event.buf = bufs.splice(0, bufs.length).toBuffer()
    Binary.parse(event.buf)
      .word8('iac1') // 255
      .word8('sb') // 250
      .word8('newenv') // 39
      .word8('send') // 2
      .word8('variable') // 3
      .scan('name', new Buffer([1]))
      .scan('value', new Buffer([255])) // unfortunately this eats the IAC
      // .word8('iac2') // eaten by the `value`
      .word8('se')
      .tap(function (vars) {
        assert(vars.iac1 === COMMANDS.IAC)
        assert(vars.sb === COMMANDS.SB)
        assert(vars.newenv === OPTIONS.ENVIRONMENT_VARIABLES)
        assert(vars.send === 0x02)
        assert(vars.variable === 0x03)
        assert(vars.name.length > 0)
        assert(vars.value.length > 0)
        // assert(vars.iac2 === COMMANDS.IAC) // eaten by the `value`
        assert(vars.se === COMMANDS.SE)
        event.name = vars.name.toString('ascii')
        event.value = vars.value.toString('ascii')
      })
  }
  return event
}

OPTION_IMPLS[OPTIONS.TERMINAL_TYPE] = function (bufs, i, event) {
  if (event.commandCode !== COMMANDS.SB) {
    OPTION_IMPLS.NO_ARGS(bufs, i, event)
  } else {
    // Minimum size should be 7:
    if (bufs.length < 7) return MORE
    event.buf = bufs.splice(0, bufs.length).toBuffer()
    Binary.parse(event.buf)
      .word8('iac1') // 255
      .word8('sb') // 250
      .word8('termtype') // 24
      .word8('is') // 0
      .scan('name', new Buffer([255]))
      // .word8('iac2') // eaten by the `value`
      .word8('se')
      .tap(function (vars) {
        assert(vars.iac1 === COMMANDS.IAC)
        assert(vars.sb === COMMANDS.SB)
        assert(vars.termtype === OPTIONS.TERMINAL_TYPE)
        assert(vars.is === 0x00)
        assert(vars.name.length > 0)
        // assert(vars.iac2 === COMMANDS.IAC) // eaten by the `value`
        assert(vars.se === COMMANDS.SE)
        // NOTE: Terminal name is always uppercase for some reason:
        event.name = vars.name.toString('ascii').toLowerCase()
      })
  }
  return event
}

var MORE = -123132

function parse(bufs) {
  assert(bufs.length >= 2) // IAC byte and whatever follows it
  assert(bufs.get(0) === COMMANDS.IAC)
  return parseCommand(bufs, 1, {})
}

function parseCommand (bufs, i, event) {
  var command = bufs.get(i)
  event.commandCode = command
  event.command = COMMAND_NAMES[command]
  return COMMAND_IMPLS[command](bufs, i + 1, event)
}

function parseOption (bufs, i, event) {
  var option = bufs.get(i)
  event.optionCode = option
  event.option = OPTION_NAMES[option]
  return OPTION_IMPLS[option](bufs, i + 1, event)
}

function Socket (input, output) {
  Stream.call(this)

  var bufs = Buffers()
    , self = this

  this.bufs = bufs
  this.input = input
  this.output = output
  this.convertLF = true

  // proxy "close", "end", "error"
  this.input.on('end', function () {
    self.emit('end')
  })
  this.input.on('close', function () {
    self.emit('close')
  })
  this.input.on('drain', function () {
    self.emit('drain')
  })
  this.input.on('error', function (e) {
    self.emit('error', e)
  })

  // this main 'data' listener
  this.input.on('data', function (b) {
    debug('incoming "data" event %j', b.toString('utf8'), b)
    bufs.push(b)

    var i
    while ((i = bufs.indexOf(IAC_BUF)) >= 0) {
      assert(bufs.length > (i+1))
      if (i > 0) {
        var data = bufs.splice(0, i).toBuffer()
        self.emit('data', data)
      }
      i = parse(bufs)
      if (i === MORE) {
        debug('need to wait for more...')
        break
      } else {
        debug('emitting event', i)
        self.emit('event', i)
        self.emit(i.command, i)
        if (i.option) {
          self.emit(i.option, i)
        }
        if (i.data) {
          self.emit('data', i.data)
        }
        if (i.option === 'terminal type' && i.command === 'will') {
          // Request that the client send its terminal type:
          self.output.write(new Buffer([
            COMMANDS.IAC,
            COMMANDS.SB,
            OPTIONS.TERMINAL_TYPE,
            1, // SEND
            COMMANDS.IAC,
            COMMANDS.SE
          ]))
        }
      }
    }
    if (i !== MORE && bufs.length > 0) {
      // we got regular data!
      var data = bufs.splice(0).toBuffer()
      self.emit('data', data)
    }
  })
}
require('util').inherits(Socket, Stream)
exports.Socket = Socket

;['do','dont','will','wont'].forEach(function (command) {
  function get () {
    return new Command(command, this)
  }
  Object.defineProperty(Socket.prototype, command, {
      get: get
    , enumerable: true
    , configurable: true
  })
})

// readable stuff
Object.defineProperty(Socket.prototype, 'readable', {
    get: function () { return this.input.readable }
  , set: function (v) { return this.input.readable = v }
  , enumerable: true
  , configurable: true
})
Socket.prototype.pause = function () {
  return this.input.pause.apply(this.output, arguments)
}
Socket.prototype.resume = function () {
  return this.input.resume.apply(this.output, arguments)
}

// writable stuff
Object.defineProperty(Socket.prototype, 'writable', {
    get: function () { return this.output.writable }
  , set: function (v) { return this.output.writable = v }
  , enumerable: true
  , configurable: true
})
Socket.prototype.write = function (b) {
  if (this.convertLF) {
    b = b.toString('utf8').replace(/\r?\n/g, '\r\n')
  }
  debug('writing outgoing data %j', b)
  return this.output.write.apply(this.output, arguments)
}
Socket.prototype.end = function () {
  return this.output.end.apply(this.output, arguments)
}
Socket.prototype.destroy = function () {
  return this.output.destroy.apply(this.output, arguments)
}
Socket.prototype.destroySoon = function () {
  return this.output.destroySoon.apply(this.output, arguments)
}

/**
 * Sends out a telnet command.
 */

function Command (command, client) {
  this.command = COMMANDS[command.toUpperCase()]
  this.client = client
  if (debug.enabled) {
    this.commandName = command
  }
}

Object.keys(OPTIONS).forEach(function (name) {
  var code = OPTIONS[name]
  Command.prototype[name.toLowerCase()] = function () {
    var buf = Buffer(3)
    buf[0] = COMMANDS.IAC
    buf[1] = this.command
    buf[2] = code
    debug('writing Command', this.commandName, name, buf)
    return this.client.output.write(buf)
  }
})

/**
 * Convenience function to create a Telnet TCP/IP server.
 * Listen for the "client" event.
 */

function telnet (cb) {
  var net = require('net')
    , server = net.createServer(conn)

  function conn (socket) {
    var client = new Socket(socket, socket)
    server.emit('client', client)
  }
  if (typeof cb === 'function') {
    server.on('client', cb)
  }
  return server
}

// if you're more old skool (like the net.createServer() syntax)...
exports.createServer = telnet

so it captures all signals:

n$ node telnetserver.js 
Telnet server
Starting telnet server
(node) Buffer.get is deprecated. Use array indexes instead.
telnet window resized to 181 x 52
telnet window resized to 177 x 51
telnet window resized to 171 x 50
telnet window resized to 166 x 49
telnet window resized to 165 x 48
telnet window resized to 167 x 49
telnet window resized to 170 x 50
telnet window resized to 172 x 51
telnet window resized to 173 x 51
telnet window resized to 174 x 52
INTERRUPT PROCESS
TIMING-MARK
CLIENT DISCONNECTED
codebling commented 8 years ago

FYI it seems like this issue is fixed by #16. See also https://github.com/chjj/node-telnet2 which is the source for #16.