Rewrite the code of frame decoder.

Added tests with invalid packets.

2 files changed, 44 insertions, 18 deletions

diff --git a/src/org/traccar/protocol/Arnavi4FrameDecoder.java b/src/org/traccar/protocol/Arnavi4FrameDecoder.java
index bb130e6bb..ee4aee32a 100644
--- a/src/org/traccar/protocol/Arnavi4FrameDecoder.java
+++ b/src/org/traccar/protocol/Arnavi4FrameDecoder.java
@@ -22,17 +22,18 @@ import org.jboss.netty.handler.codec.frame.FrameDecoder;
 
 public class Arnavi4FrameDecoder extends FrameDecoder {
 
-    static final int PACKET_MIN_LENGTH = 4;
+    private static final int PACKET_MINIMUM_LENGTH = 4;
+    private static final int RECORD_MINIMUM_LENGTH = 8;
 
-    static final byte HEADER_START_SIGN = (byte) 0xFF;
+    static final byte HEADER_START_SIGN = (byte) 0xff;
     static final byte HEADER_VERSION_1 = 0x22;
     static final byte HEADER_VERSION_2 = 0x23;
-    static final int HEADER_LENGTH = 10;
+    private static final int HEADER_LENGTH = 10;
 
-    static final byte PACKAGE_START_SIGN = 0x5B;
-    static final byte PACKAGE_END_SIGN = 0x5D;
-    static final int PACKAGE_MIN_PARCEL_NUMBER = 0x01;
-    static final int PACKAGE_MAX_PARCEL_NUMBER = 0xFB;
+    private static final byte PACKAGE_START_SIGN = 0x5b;
+    private static final byte PACKAGE_END_SIGN = 0x5d;
+    private static final int PACKAGE_MIN_PARCEL_NUMBER = 0x01;
+    private static final int PACKAGE_MAX_PARCEL_NUMBER = 0xfb;
 
     @Override
     protected Object decode(
@@ -40,31 +41,37 @@ public class Arnavi4FrameDecoder extends FrameDecoder {
             Channel channel,
             ChannelBuffer buf) throws Exception {
 
-        if (buf.readableBytes() < PACKET_MIN_LENGTH) {
+        if (buf.readableBytes() < PACKET_MINIMUM_LENGTH) {
             return null;
         }
 
         if (buf.getByte(0) == HEADER_START_SIGN
                 && buf.readableBytes() == HEADER_LENGTH
                 && (buf.getByte(1) == HEADER_VERSION_1 || buf.getByte(1) == HEADER_VERSION_2)) {
+
             return buf.readBytes(HEADER_LENGTH);
         }
 
-        int index = buf.getByte(1) & 0xFF; // parcel number
+        int index = buf.getUnsignedByte(1); // parcel number
         if (buf.getByte(0) == PACKAGE_START_SIGN
                 && index >= PACKAGE_MIN_PARCEL_NUMBER && index <= PACKAGE_MAX_PARCEL_NUMBER) {
 
-            // package: 1 byte start sign, 1 byte parcel number, packets, 1 byte end sign
-            int packetStartIndex = 2;
-            while (packetStartIndex + 8 < buf.readableBytes() && buf.getByte(packetStartIndex) != PACKAGE_END_SIGN) {
-                // packet: 1 byte type, 2 bytes length, 4 bytes unixtime, length-bytes data, 1 byte crc
-                packetStartIndex += buf.getUnsignedShort(packetStartIndex + 1) + 8;
+            int bufferPosition = 2; // start sign + parcel number
+            while (bufferPosition + RECORD_MINIMUM_LENGTH < buf.readableBytes()
+                    && buf.getByte(bufferPosition) != PACKAGE_END_SIGN) {
+
+                int dataLength = buf.getUnsignedShort(bufferPosition + 1);
+                bufferPosition += RECORD_MINIMUM_LENGTH + dataLength; // type + data length + unixtime + data + crc
             }
 
-            return buf.readBytes(packetStartIndex + 1);
+            if (bufferPosition < buf.readableBytes()
+                    && buf.getByte(bufferPosition) == PACKAGE_END_SIGN) {
+
+                return buf.readBytes(bufferPosition + 1); // end sign
+            }
         }
 
         return null;
     }
 
-}
+}
\ No newline at end of file
diff --git a/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java b/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java
index 93d818f56..2203cdafc 100644
--- a/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java
+++ b/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java
@@ -9,7 +9,7 @@ import java.nio.ByteOrder;
 public class Arnavi4FrameDecoderTest extends ProtocolTest {
 
     @Test
-    public void testDecode() throws Exception {
+    public void testDecodeValidPackets() throws Exception {
 
         Arnavi4FrameDecoder decoder = new Arnavi4FrameDecoder();
 
@@ -21,7 +21,7 @@ public class Arnavi4FrameDecoderTest extends ProtocolTest {
                 binary(ByteOrder.LITTLE_ENDIAN, "ff23f30c45f5c90f0300"),
                 decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "ff23f30c45f5c90f0300")));
 
-        Assert.assertEquals( // Valid PACKAGE with answer to server on file transfer.
+        Assert.assertEquals( // Valid PACKAGE with answer to server on file transfer
                 null,
                 decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5bfd005d")));
 
@@ -43,4 +43,23 @@ public class Arnavi4FrameDecoderTest extends ProtocolTest {
 
     }
 
+    @Test
+    public void testDecodeInvalidPackets() throws Exception {
+
+        Arnavi4FrameDecoder decoder = new Arnavi4FrameDecoder();
+
+        Assert.assertEquals( // Invalid PACKAGE with one DATA packet (missing last byte with end sign)
+                null,
+                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa3701000029")));
+
+        Assert.assertEquals( // Invalid PACKAGE with two DATA packet (missing last 10 bytes)
+                null,
+                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa3701000029012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d00")));
+
+        Assert.assertEquals( // Valid PACKAGE with useless extra bytes at the end
+                binary(ByteOrder.LITTLE_ENDIAN, "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa37010000295d"),
+                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa37010000295d010203040506070809")));
+
+    }
+
 }
\ No newline at end of file