chjj / tty.js

A terminal for your browser, using node/express/socket.io
MIT License
4.19k stars 480 forks source link

Using the programmatic interface can log the wrong port #135

Open Page- opened 9 years ago

Page- commented 9 years ago

When calling

var tty = require('tty.js');

var app = tty.createServer({
  shell: 'bash',
}).listen(80, null, function(){});

you get [tty.js] Listening on port 8080. logged, but it is actually listening on port 80.

The reason is that https://github.com/chjj/tty.js/blob/0cee00a7010028dc7f908a921f4161d873f70c17/lib/tty.js#L49 uses self.conf.port but we overrode it with a custom port in the listen call.

thatkookooguy commented 9 years ago

In order for this to work properly, you're supposed to change port by adding a "port" attribute inside the config.json file. By doing it that way, it should also change the message written to the terminal https://github.com/chjj/tty.js/blob/0cee00a7010028dc7f908a921f4161d873f70c17/lib/config.js#L99

Page- commented 9 years ago

Changing it in the config.json is great if running tty.js directly from the command line, the issue is when calling it from the programmatic interface, for instance you might start multiple tty instances on different ports in order to offer different users or chroots, etc

thatkookooguy commented 9 years ago

what about doing it like this? this doesn't work?

var app_one = tty.createServer({"port": "1234"});
var app_two = tty.createServer({"port": "4321"});

you can give each createServer a different config file, no? Instead of changing it in the listen command.

inside your example:

var tty = require('tty.js');

var app = tty.createServer({
    shell: 'bash',
    port: 1234,
}).listen();

It works the same as the config.json (and got the same attributes). the config.json is just parsed and passed on in the same way, as can be seen by these two lines from tty.js/bin/tty.js:

var conf = tty.config.readConfig()
 , app = tty.createServer(conf);