MQTT 3.1.1 support

[redlava]

I'm keen on using mosca in an upcoming project. The client I'm using requires a 3.1.1 broker but I gather mosca only supports 3.1.

[mcollina]

Mosca should support all of 3.1.1. If something is not working, please file a bug.

[psorowka]

I have two further thoughts on this one.

Mosca should support all of 3.1.1.

The Readme still claims Mosca to be 3.1 compliant. Is that for purpose?

2nd, I think there is at least one thing not 3.1.1. compliant, namely the clientId handler. The spec allows clients to provide an empty clientId, in which case the server should assign a random one. I think mosca doesn't do this yet.

[mcollina]

@psorowka you are right. It should be easy to implement that feature, would you like to send a PR for it?

[psorowka]

sure, I'll do it. I stumbled over that a couple of weeks ago but didn't alter the behavior because I thought you would want to stick to 3.1 for other reasons for now

[psorowka]

I prepared all changes but before we need support for this in the mqtt-packet module (see https://github.com/mqttjs/mqtt-packet/pull/2)

[mcollina]

Released mqtt-packet@3.3.0.

[ghost]

@mcollina @psorowka do you have any idea on what's missing for full 3.1.1 compliance?

[psorowka]

Here is a list of a few minor points (mostly error handling) that are imho not implemented yet, although spec requires them. All of them would be easy to implement.

The Server MUST process a second CONNECT Packet sent from a Client as a protocol violation and disconnect the Client.

currently, further CONNECT packets are ignored

The Server MUST respond to the CONNECT Packet with a CONNACK return code 0x01 (unacceptable protocol level) and then disconnect the Client if the Protocol Level is not supported by the Server

currently, ProtocolLevel is ignored

The Server MUST validate that the reserved flag in the CONNECT Control Packet is set to zero and disconnect the Client if it is not zero

that is not done.

The Server MUST treat a SUBSCRIBE packet as malformed and close the Network Connection if any of Reserved bits in the payload are non-zero, or QoS is not 0,1 or 2

that is not done.

(I guess this is not a complete list)

---

I am wondering whether in all those benchmark libs (malaria et.al.) is also something like a protocol linter?

[psorowka]

By the way, the above mentioned issue (handle empty client id) was fixed today by #308

[mcollina]

@psorowka thanks for the specific list :).Definitely I rather prefer to ignore things than to cutting connections, and that is probably why (and all of them were non-intentional).

[psorowka]

@mcollina right, all mentioned points are not required in order to support clients that incorporate 3.1.1. Imho the last missing feature for that was the clientId handling we just released. (which is optional according to the spec, however it was illegal to just use the empty clientId)

As soon as Mosca claims to be 3.1.1 compliant I find it crucial to support all edge-cases. Most of them refer to protocol robustness or future compatibility (reserved flags et.al.). Servers should stick to the spec, so that clients cannot be implemented lazily :)

But agreed, this is absolutely not top priority and I wouldn't have mentioned any of them if @bmcustodio hadn't asked for it.

[mcollina]

@psorowka the problem being a lot of clients/servers are not compliant. And sometimes some devices deployed in the field has a buggy/old MQTT implementation :(. Up to this point, I am trying to keep the code that supports MQTT 3.1 and 3.1.1 the same, but maybe I should revise that decision.

[psorowka]

I thought about that as well but I don't think a split would be wise at this point.

Actually, I have the impression that 3.1.1 is way more strict than 3.1. As we have the protocolVersion flag we can easily differentiate the 'strictness' of Mosca depending on what the clients submits. I think it is the responsibility of the servers to prevent the growth of buggy/non-spec-compatible clients by enforcing the spec, don't you agree?

[mcollina]

@psorowka I couldn't agree more! But there are many buggy clients out there :/.

[aug2uag]

There may be an issue with MQTT 3.1.1. The evidence is a NodeMCU client trying to communicate to a Mosca broker:

Mosca broker

var mosca = require('mosca')

var ascoltatore = {
    type: 'mongo',
    url: 'mongodb://localhost:27017/mqtt',
    pubsubCollection: 'ascoltatori',
    mongo: {}
};

var moscaSetting = {
    port: 1880,
    host: "127.0.0.1", // specify an host to bind to a single interface
    logger: {
        level: 'debug'
    },
    persistence: {
        factory: mosca.persistence.Mongo,
        url: 'mongodb://localhost:27017/mqtt'
      },
    backend: ascoltatore
};

var authenticate = function (client, username, password, callback) {
    console.log('ping');
    if (username == "test" && password.toString() == "test")
        callback(null, true);
    else
        callback(null, false);
}

var authorizePublish = function (client, topic, payload, callback) {
    callback(null, true);
}

var authorizeSubscribe = function (client, topic, callback) {
    callback(null, true);
}

var server = new mosca.Server(moscaSetting);

server.on('ready', setup);

function setup() {
    server.authenticate = authenticate;
    server.authorizePublish = authorizePublish;
    server.authorizeSubscribe = authorizeSubscribe;

    console.log('Mosca server is up and running.');
}

server.on("error", function (err) {
    console.log(err);
});

server.on('clientConnected', function (client) {
    console.log('Client Connected \t:= ', client.id);
});

server.on('published', function (packet, client) {
    console.log("Published :=", packet);
});

server.on('subscribed', function (topic, client) {
    console.log("Subscribed :=", client.packet);
});

server.on('unsubscribed', function (topic, client) {
    console.log('unsubscribed := ', topic);
});

server.on('clientDisconnecting', function (client) {
    console.log('clientDisconnecting := ', client.id);
});

server.on('clientDisconnected', function (client) {
    console.log('Client Disconnected     := ', client.id);
});

NodeMCU client

print('hello there')
function makeMQTT()
    print('do make MQTT')
    -- init mqtt client with keepalive timer 120sec
    m = mqtt.Client("clientid",120,"test","test")

    -- setup Last Will and Testament (optional)
    -- Broker will publish a message with:
    -- qos = 0,retain = 0,data = "offline" 
    -- to topic "/lwt" if client don't send keepalive packet
    m:lwt("/lwt","offline",0,0)

    m:on("connect",function(con) print ("connected") end)
    m:on("offline",function(con) print ("offline") end)

    -- on publish message receive event
    m:on("message",function(conn,topic,data) 
      print(topic .. ":" ) 
      if data ~= nil then
        print(data)
      end
    end)

    -- for secure: m:connect("127.0.0.1",1880,1)
    m:connect("127.0.0.1",1880,0,function(conn) 
        print("connected") 
    end)

    -- -- subscribe topic with qos = 0
    -- m:subscribe("/topic",0,function(conn) 
    --     print("subscribe success") 
    -- end)

    -- -- publish a message with data = hello,QoS = 0,retain = 0
    -- m:publish("/topic","hello",0,0,function(conn) 
    --     print("sent") 
    -- end)

    -- m:close();
    -- -- you can call m:connect again
end

tmr.alarm(0, 1000, 1, function()
    if wifi.sta.status() == 5 then
        tmr.stop(0)
        makeMQTT()
    end
end)

NodeMCU output

> dofile('mqtt.lua')
hello there
do make MQTT

Mosca output

$ node index.js 
{"name":"mosca","hostname":"Macintosh-4.local","pid":79142,"level":30,"mqtt":1880,"msg":"server started","time":"2015-08-15T03:02:58.359Z","v":0}
Mosca server is up and running.

There's no response from the broker, and the client crashes if it tries a pub/sub function as is. > PANIC: unprotected error in call to Lua API (mqtt.lua:30: not connected). I'm expecting "ping" or any other log to process when the client code is executed while the broker is running on localhost.

There's a thread on NodeMCU that does discuss the need to run MQTT 3.1.1. If something else is spotted, please comment. Thanks.

[aug2uag]

Hey, stupid mistake: needs wifi IP. All works!