jbertram commented on a change in pull request #3907:
URL: https://github.com/apache/activemq-artemis/pull/3907#discussion_r792954441
##########
File path:
artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java
##########
@@ -185,45 +243,83 @@ public void handleBuffer(RemotingConnection connection,
ActiveMQBuffer buffer) {
@Override
public void addChannelHandlers(ChannelPipeline pipeline) {
pipeline.addLast(MqttEncoder.INSTANCE);
- pipeline.addLast(new MqttDecoder(MQTTUtil.MAX_MESSAGE_SIZE));
+ /*
+ * If we use the value from getMaximumPacketSize() here anytime a client
sends a packet that's too large it
+ * will receive a DISCONNECT with a reason code of 0x81 instead of 0x95
like it should according to the spec.
+ * See
https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901086:
+ *
+ * If a Server receives a packet whose size exceeds this limit, this
is a Protocol Error, the Server uses
+ * DISCONNECT with Reason Code 0x95 (Packet too large)...
+ *
+ * Therefore we check manually in
org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.handlePublish
+ */
+ pipeline.addLast(new MqttDecoder(MQTTUtil.MAX_PACKET_SIZE));
pipeline.addLast(new MQTTProtocolHandler(server, this));
}
/**
- * The protocol handler passes us an 8 byte long array from the transport.
We sniff these first 8 bytes to see
- * if they match the first 8 bytes from MQTT Connect packet. In many other
protocols the protocol name is the first
- * thing sent on the wire. However, in MQTT the protocol name doesn't come
until later on in the CONNECT packet.
- *
- * In order to fully identify MQTT protocol via protocol name, we need up
to 12 bytes. However, we can use other
- * information from the connect packet to infer that the MQTT protocol is
being used. This is enough to identify MQTT
- * and add the Netty codec in the pipeline. The Netty codec takes care of
things from here.
- *
- * MQTT CONNECT PACKET: See MQTT 3.1.1 Spec for more info.
- *
- * Byte 1: Fixed Header Packet Type. 0b0001000 (16) = MQTT Connect
- * Byte 2-[N]: Remaining length of the Connect Packet (encoded with 1-4
bytes).
- *
- * The next set of bytes represents the UTF8 encoded string MQTT (MQTT
3.1.1) or MQIsdp (MQTT 3.1)
- * Byte N: UTF8 MSB must be 0
- * Byte N+1: UTF8 LSB must be (4(MQTT) or 6(MQIsdp))
- * Byte N+1: M (first char from the protocol name).
- *
- * Max no bytes used in the sequence = 8.
+ * Relevant portions of the specs we support:
+ * MQTT 3.1 -
https://public.dhe.ibm.com/software/dw/webservices/ws-mqtt/mqtt-v3r1.html#connect
+ * MQTT 3.1.1 -
http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718028
+ * MQTT 5 -
https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901033
*/
@Override
public boolean isProtocol(byte[] array) {
ByteBuf buf = Unpooled.wrappedBuffer(array);
- if (!(buf.readByte() == 16 && validateRemainingLength(buf) &&
buf.readByte() == (byte) 0)) return false;
+ // Parse "fixed header"
+ if (!(readByte(buf) == 16 && validateRemainingLength(buf) &&
readByte(buf) == (byte) 0)) {
+ return false;
+ }
+
+ // Start parsing the Protocol Name
+ byte b = readByte(buf); // LSB
+
+ // MQTT 3.1.1 & 5
+ if (b == 4 &&
+ (readByte(buf) != 77 || // M
+ readByte(buf) != 81 || // Q
+ readByte(buf) != 84 || // T
+ readByte(buf) != 84)) { // T
+ return false;
+ }
+
+ // MQTT 3.1
+ if (b == 6 &&
+ (readByte(buf) != 77 || // M
+ readByte(buf) != 81 || // Q
+ readByte(buf) != 73 || // I
+ readByte(buf) != 115 || // s
+ readByte(buf) != 100 || // d
+ readByte(buf) != 112)) { // p
+ return false;
+ }
+
+ // Protocol Version
+ b = readByte(buf);
Review comment:
You're right. I didn't catch this because I don't actually have a
low-level client implementation where I can pass a bad version and validate the
response from the broker. I'll omit the version check here so the
spec-compliant response can be returned later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
