Fixing QOSType encoding and decoding of FAILURE qos (0x80). This was causing bug with decoding a SubAck with failure qos type. See Appendix B: MQTT-3.9.3-2

netty_parser/src/main/java/org/eclipse/moquette/parser/netty/DemuxDecoder.java

@@ -73,7 +73,11 @@ abstract class DemuxDecoder {
 
         message.setMessageType(messageType);
         message.setDupFlag(dupFlag);
-        message.setQos(AbstractMessage.QOSType.values()[qosLevel]);
+        try {
+            message.setQos(AbstractMessage.QOSType.valueOf(qosLevel));
+        } catch(IllegalArgumentException e) {
+            throw new CorruptedFrameException(String.format("Received an invalid QOS: %s", e.getMessage()), e);
+        }
         message.setRetainFlag(retainFlag);
         message.setRemainingLength(remainingLength);
         return true;

netty_parser/src/main/java/org/eclipse/moquette/parser/netty/SubAckDecoder.java

@@ -49,7 +49,7 @@ class SubAckDecoder extends DemuxDecoder {
         }
         for (int i = 0; i < remainingLength; i++) {
             byte qos = in.readByte();
-            message.addType(AbstractMessage.QOSType.values()[qos]);
+            message.addType(AbstractMessage.QOSType.valueOf(qos));
         }
         
         out.add(message);

netty_parser/src/main/java/org/eclipse/moquette/parser/netty/SubAckEncoder.java

@@ -40,8 +40,7 @@ class SubAckEncoder extends DemuxEncoder<SubAckMessage> {
             buff.writeBytes(Utils.encodeRemainingLength(variableHeaderSize));
             buff.writeShort(message.getMessageID());
             for (QOSType c : message.types()) {
-                int qosValue = (c == QOSType.FAILURE) ? qosValue = 0x80 : c.ordinal();
-                buff.writeByte(qosValue);
+                buff.writeByte(c.byteValue());
             }
 
             out.writeBytes(buff);

netty_parser/src/main/java/org/eclipse/moquette/parser/netty/Utils.java

@@ -184,7 +184,7 @@ public class Utils {
             flags |= 0x01;
         }
         
-        flags |= ((message.getQos().ordinal() & 0x03) << 1);
+        flags |= ((message.getQos().byteValue() & 0x03) << 1);
         return flags;
     }
     

netty_parser/src/test/java/org/eclipse/moquette/parser/netty/SubAckDecoderTest.java

@@ -81,13 +81,31 @@ public class SubAckDecoderTest {
         assertEquals(AbstractMessage.QOSType.LEAST_ONE, message.types().get(0));
     }
 
+    @Test
+    public void testEncodeWithFailureQOS() throws Exception {
+        initHeaderQos(m_buff, 0xAABB, AbstractMessage.QOSType.FAILURE);
+
+        //Exercise
+        m_msgdec.decode(null, m_buff, m_results);
+
+        //Verify
+        assertFalse(m_results.isEmpty());
+        SubAckMessage message = (SubAckMessage)m_results.get(0);
+        assertNotNull(message);
+        assertEquals(0xAABB, message.getMessageID().intValue());
+        List<AbstractMessage.QOSType> qoses = message.types();
+        assertEquals(1, qoses.size());
+        assertEquals(AbstractMessage.QOSType.FAILURE, qoses.get(0));
+        assertEquals(AbstractMessage.SUBACK, message.getMessageType());
+    }
+
     private void initHeaderQos(ByteBuf buff, int messageID, AbstractMessage.QOSType... qoss) throws IllegalAccessException {
         buff.clear().writeByte(AbstractMessage.SUBACK << 4).
                 writeBytes(Utils.encodeRemainingLength(2 + qoss.length));
         
         buff.writeShort(messageID);
         for (AbstractMessage.QOSType qos : qoss) {
-            buff.writeByte(qos.ordinal());
+            buff.writeByte(qos.byteValue());
         }
     }
 }

netty_parser/src/test/java/org/eclipse/moquette/parser/netty/SubAckEncoderTest.java

@@ -49,7 +49,7 @@ public class SubAckEncoderTest {
         //Exercise
         m_encoder.encode(m_mockedContext, msg, m_out);
     }
-    
+
     @Test
     public void testEncodeWithMultipleQos() throws Exception {
         SubAckMessage msg = new SubAckMessage();
@@ -70,8 +70,29 @@ public class SubAckEncoderTest {
         //Variable part
         assertEquals((byte)0xAA, m_out.readByte()); //MessageID MSB
         assertEquals((byte)0xBB, m_out.readByte()); //MessageID LSB
-        assertEquals((byte)AbstractMessage.QOSType.MOST_ONE.ordinal(), m_out.readByte());
-        assertEquals((byte)AbstractMessage.QOSType.LEAST_ONE.ordinal(), m_out.readByte());
-        assertEquals((byte)AbstractMessage.QOSType.EXACTLY_ONCE.ordinal(), m_out.readByte());
+        assertEquals(AbstractMessage.QOSType.MOST_ONE.byteValue(), m_out.readByte());
+        assertEquals(AbstractMessage.QOSType.LEAST_ONE.byteValue(), m_out.readByte());
+        assertEquals(AbstractMessage.QOSType.EXACTLY_ONCE.byteValue(), m_out.readByte());
+    }
+
+    @Test
+    public void testEncodeWithFailureQOS() throws Exception {
+        SubAckMessage msg = new SubAckMessage();
+
+        int messageID = 0xAABB;
+        msg.setMessageID(messageID);
+        msg.addType(AbstractMessage.QOSType.FAILURE);
+
+        //Exercise
+        m_encoder.encode(m_mockedContext, msg, m_out);
+
+        //Verify
+        assertEquals((byte) (AbstractMessage.SUBACK << 4 ), m_out.readByte()); //1 byte
+        assertEquals(3, m_out.readByte()); //remaining length
+
+        //Variable part
+        assertEquals((byte)0xAA, m_out.readByte()); //MessageID MSB
+        assertEquals((byte)0xBB, m_out.readByte()); //MessageID LSB
+        assertEquals((byte)0x80, m_out.readByte()); //Failure return code
     }
 }