Details
Critical Description
I tried to use the MQTT.js library in a browser but it didn't connect to ActiveMQ using MQTT-WebSocket transport.
Debugging MQTT.js and ActiveMQ I found that the org.apache.activemq.transport.ws.jetty9.MQTTSocket class (in activemq-http module) assumes that a single WebSocket data frame always contains a single MQTT Control Packet.
This is the cause of MQTT.js failure because MQTT.js makes several send over WebSocket for a single MQTT Control Packet (header, length etc).
Accordingly to MQTT OASIS specification ( http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718127 ):
A single WebSocket data frame can contain multiple or partial MQTT Control Packets. The receiver MUST NOT assume that MQTT Control Packets are aligned on WebSocket frame boundaries.
And RFC 6455 ( https://www.ietf.org/rfc/rfc6455.txt ):
The WebSocket message does not necessarily correspond to a particular network layer framing, as a fragmented message may be coalesced or split by an intermediary.
So I think that ActiveMQ should be fixed to be compliant to the WebSocket and MQTT specifications.
I attached the diff patch file for MQTTSocket which fixes the problem and a new activemq-http module MQTT-WebSocket unit test for this issue (the same of MQTTWSTransportTest but which splits MQTT Frame Packet in multiple WebSocket frames):
MQTTWSPartialFrameConnection.java
MQTTWSPartialFrameTest.java
When writing the unit test I found also a bug in the class org.apache.activemq.transport.ws.MQTTWSConnection.java.
All the occurrences of:
ByteSequence payload = wireFormat.marshal(command.encode()); connection.getRemote().sendBytes(ByteBuffer.wrap(payload.data));
should be replaced with:
ByteSequence payload = wireFormat.marshal(command.encode()); connection.getRemote().sendBytes(ByteBuffer.wrap(payload.data, payload.offset, payload.length));
That is because of the payload.data bytes array buffer in ByteSequenze is bigger than the real MQTT frame packed bytes size, so sending all the payload.data bytes array, ActiveMQ receives much more extra zero-extra bytes. With the current implementation, those extra zero-bytes are ignored and this makes wrongly passes the test (while it should fail because there are "extra bytes" which doesn't respect the MQTT protocol).
I attached also the diff patch file for the MQTTWSConnection file.