moscajs / mosca

MQTT broker as a module
mosca.io
3.2k stars 508 forks source link

mqtt over websocket and vanilla websocket cohabitat #605

Closed namgk closed 7 years ago

namgk commented 7 years ago

Hi there,

It seems that currently you cannot serve one http server with 2 websocket connections, one for vanilla ws and one for mqtt over ws.

What I mean by this:

const server = http.createServer();

const wss = new ws.Server({
  server:server,
  path:'/test',
  perMessageDeflate: false
})

var mqttServ = new mosca.Server({})
mqttServ.attachHttpServer(server);

When you have this setting, all ws clients cannot connect and throwing "Error: reserved fields must be empty". I guess the mosca mqtt server is interfering with the vanilla ws connection.

Indeed, when I change the following line to add a path to the mqtt over ws, it works like a charm:

https://github.com/mcollina/mosca/blob/ab4b35dc3bf27f224af4db244c1db02bd1620d4b/lib/server.js#L615

Server.prototype.attachHttpServer = function(server) {
  var that = this;
  ws.createServer({ server: server, path: '/mqttws' }, function(stream) {
    var conn = new Connection(stream);
    new Client(conn, that);
  });
};

The client would connect seemlessly using:

var client = mqtt.connect('ws://localhost:3333/mqttws');
mcollina commented 7 years ago

can you please send a PR, adding a path  option to attachHttpServer? So we can maintain the current behavior, and support also your use-case.

namgk commented 7 years ago

I'd like to, but there is a design decision, either we hard coded the path there or we have to change the attachHttpServer API to include the path configuration.

What's your thought?

mcollina commented 7 years ago

attachHttpServer can have the path  as an optional parameter.