Review fixes

4 files changed, 68 insertions, 98 deletions

diff --git a/src/org/traccar/protocol/NavisFrameDecoder.java b/src/org/traccar/protocol/NavisFrameDecoder.java
index ff367ba77..9a3b3d45c 100644
--- a/src/org/traccar/protocol/NavisFrameDecoder.java
+++ b/src/org/traccar/protocol/NavisFrameDecoder.java
@@ -44,6 +44,10 @@ public class NavisFrameDecoder extends BaseFrameDecoder {
             return null;
         }
 
+        if (buf.readableBytes() > MAX_FRAME_LENGTH) {
+            throw new TooLongFrameException();
+        }
+
         if (buf.getByte(buf.readerIndex()) == 0x7F) {
             // FLEX keep alive frame
             return buf.readRetainedSlice(1);
@@ -51,10 +55,6 @@ public class NavisFrameDecoder extends BaseFrameDecoder {
 
         if (protocolDecoder.getFlexDataSize() > 0) {
             // FLEX frame
-            if (buf.readableBytes() > MAX_FRAME_LENGTH) {
-                throw new TooLongFrameException();
-            }
-
             if (buf.readableBytes() > FLEX_HEADER_LENGTH) {
                 int length = 0;
                 String type = buf.toString(buf.readerIndex(), 2, StandardCharsets.US_ASCII);
@@ -101,9 +101,6 @@ public class NavisFrameDecoder extends BaseFrameDecoder {
 
             int length = buf.getUnsignedShortLE(buf.readerIndex() + NTCB_LENGTH_OFFSET);
             if (buf.readableBytes() >= NTCB_HEADER_LENGTH + length) {
-                if (buf.readableBytes() > MAX_FRAME_LENGTH) {
-                    throw new TooLongFrameException();
-                }
                 return buf.readRetainedSlice(NTCB_HEADER_LENGTH + length);
             }
         }
diff --git a/src/org/traccar/protocol/NavisProtocolDecoder.java b/src/org/traccar/protocol/NavisProtocolDecoder.java
index 912e47369..d3b3f9932 100644
--- a/src/org/traccar/protocol/NavisProtocolDecoder.java
+++ b/src/org/traccar/protocol/NavisProtocolDecoder.java
@@ -18,8 +18,6 @@ package org.traccar.protocol;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import io.netty.channel.Channel;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.traccar.BaseProtocolDecoder;
 import org.traccar.DeviceSession;
 import org.traccar.NetworkMessage;
@@ -44,7 +42,6 @@ public class NavisProtocolDecoder extends BaseProtocolDecoder {
     private int flexDataSize; // bytes
     private int flexBitfieldDataSize; // bits
     private final byte[] flexBitfield;
-    private static final Logger LOGGER = LoggerFactory.getLogger(NavisProtocolDecoder.class);
     private static final int[] FLEX_FIELDS_SIZES = {4, 2, 4, 1, 1, 1, 1, 1, 4, 4, 4, 4, 4, 2, 4, 4, 2, 2,
         2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 1, 1, 1, 1, 4, 4, 2, 2, 4, 2, 2, 2, 2, 2, 2, 2, 1, 1, 1, 1, 1, 1, 1,
         1, 2, 4, 2, 1, 4, 2, 2, 2, 2, 2, 1, 1, 1, 2, 4, 2, 1, /* FLEX 2.0 */ 8, 2, 1, 16, 4, 2, 4, 37, 1,
@@ -323,12 +320,6 @@ public class NavisProtocolDecoder extends BaseProtocolDecoder {
         return BitUtil.check(flexBitfield[byteIndex], 7 - bitIndex);
     }
 
-    private void skipFlexField(int index, ByteBuf buf) {
-        if (index < FLEX_FIELDS_SIZES.length) {
-            buf.skipBytes(FLEX_FIELDS_SIZES[index]);
-        }
-    }
-
     private ParseResult parseFlexPosition(DeviceSession deviceSession, ByteBuf buf) {
 
         Position position = new Position(getProtocolName());
@@ -342,7 +333,6 @@ public class NavisProtocolDecoder extends BaseProtocolDecoder {
 
         for (int i = 0; i < flexBitfieldDataSize; i++) {
             if (!checkFlexBitfield(i)) {
-                // skip FLEX field
                 continue;
             }
 
@@ -458,29 +448,14 @@ public class NavisProtocolDecoder extends BaseProtocolDecoder {
                 case 51:
                     position.set(Position.PREFIX_TEMP + (i - 43), buf.readByte());
                     break;
-                case 68: // CAN Speed
+                case 68:
                     position.set("can-speed", buf.readUnsignedByte());
                     break;
                 // FLEX 2.0
                 case 69:
                     int satVisible = 0;
                     for (int k = 0; k < 8; k++) {
-                        switch (k) {
-                            case 0:
-                                int satVisibleGLONASS = buf.readUnsignedByte();
-                                position.set("sat-visible-glonass", satVisibleGLONASS);
-                                satVisible += satVisibleGLONASS;
-                                break;
-                            case 1:
-                                int satVisibleGPS = buf.readUnsignedByte();
-                                position.set("sat-visible-gps", satVisibleGPS);
-                                satVisible += satVisibleGPS;
-                                break;
-                            default:
-                                // Don't detalize Galileo, Compass, Beidou, DORIS, IRNSS, QZSS
-                                satVisible += buf.readUnsignedByte();
-                                break;
-                }
+                        satVisible += buf.readUnsignedByte();
                     }
                     position.set(Position.KEY_SATELLITES_VISIBLE, satVisible);
                     break;
@@ -489,7 +464,9 @@ public class NavisProtocolDecoder extends BaseProtocolDecoder {
                     position.set(Position.KEY_PDOP, buf.readUnsignedByte() * 0.1);
                     break;
                 default:
-                    skipFlexField(i, buf);
+                    if (i < FLEX_FIELDS_SIZES.length) {
+                        buf.skipBytes(FLEX_FIELDS_SIZES[i]);
+                    }
                     break;
             }
         }
@@ -505,7 +482,6 @@ public class NavisProtocolDecoder extends BaseProtocolDecoder {
         long index = 0;
         int length = buf.readUnsignedShort();
 
-        // Check buffer size and struct version
         if (length <= buf.readableBytes() && buf.readUnsignedByte() == 0x0A) {
             buf.readUnsignedByte(); // length of static part
 
@@ -586,12 +562,10 @@ public class NavisProtocolDecoder extends BaseProtocolDecoder {
             return null;
         }
 
-        byte flexProtocolVersion, flexStructVersion;
-
-        flexProtocolVersion = (byte) buf.readUnsignedByte();
-        flexStructVersion = (byte) buf.readUnsignedByte();
-        if ((flexProtocolVersion == (byte) 0x0A || flexProtocolVersion == (byte) 0x14)
-            && (flexStructVersion == (byte) 0x0A || flexStructVersion == (byte) 0x14)) {
+        byte flexProtocolVersion = (byte) buf.readUnsignedByte();
+        byte flexStructVersion = (byte) buf.readUnsignedByte();
+        if ((flexProtocolVersion == 0x0A || flexProtocolVersion == 0x14)
+            && (flexStructVersion == 0x0A || flexStructVersion == 0x14)) {
 
             flexBitfieldDataSize = buf.readUnsignedByte();
             if (flexBitfieldDataSize > 122) {
@@ -674,20 +648,16 @@ public class NavisProtocolDecoder extends BaseProtocolDecoder {
         } else {
             DeviceSession deviceSession = getDeviceSession(channel, remoteAddress);
             if (deviceSession != null) {
-                try {
-                    switch (type) {
-                        case "*>A":
-                            return processNtcbArray(deviceSession, channel, buf);
-                        case "*>T":
-                            return processNtcbSingle(deviceSession, channel, buf);
-                        case "*>F": // "*>FLEX"
-                            buf.skipBytes(3);
-                            return processFlexNegotiation(channel, buf);
-                        default:
-                            break;
-                    }
-                } catch (IndexOutOfBoundsException error) {
-                    LOGGER.warn("Navis NTCB message parsing error", error);
+                switch (type) {
+                    case "*>A":
+                        return processNtcbArray(deviceSession, channel, buf);
+                    case "*>T":
+                        return processNtcbSingle(deviceSession, channel, buf);
+                    case "*>F":
+                        buf.skipBytes(3);
+                        return processFlexNegotiation(channel, buf);
+                    default:
+                        break;
                 }
             }
         }
@@ -707,24 +677,20 @@ public class NavisProtocolDecoder extends BaseProtocolDecoder {
 
         DeviceSession deviceSession = getDeviceSession(channel, remoteAddress);
         if (deviceSession != null) {
-            try {
-                switch (type) {
-                    // FLEX 1.0
-                    case "~A":
-                        return processFlexArray(this::parseFlexPosition, type, deviceSession, channel, buf);
-                    case "~T":
-                    case "~C":
-                        return processFlexSingle(this::parseFlexPosition, type, deviceSession, channel, buf);
-                    // FLEX 2.0 (Extra packages)
-                    case "~E":
-                        return processFlexArray(this::parseFlex20Position, type, deviceSession, channel, buf);
-                    case "~X":
-                        return processFlexSingle(this::parseFlex20Position, type, deviceSession, channel, buf);
-                    default:
-                        break;
-                }
-            } catch (IndexOutOfBoundsException error) {
-                LOGGER.warn("Navis FLEX message parsing error", error);
+            switch (type) {
+                // FLEX 1.0
+                case "~A":
+                    return processFlexArray(this::parseFlexPosition, type, deviceSession, channel, buf);
+                case "~T":
+                case "~C":
+                    return processFlexSingle(this::parseFlexPosition, type, deviceSession, channel, buf);
+                // FLEX 2.0 (Extra packages)
+                case "~E":
+                    return processFlexArray(this::parseFlex20Position, type, deviceSession, channel, buf);
+                case "~X":
+                    return processFlexSingle(this::parseFlex20Position, type, deviceSession, channel, buf);
+                default:
+                    break;
             }
         }
 
diff --git a/test/org/traccar/protocol/NavisFrameDecoderTest.java b/test/org/traccar/protocol/NavisFrameDecoderTest.java
index 069f03d1b..5e8d8fd36 100644
--- a/test/org/traccar/protocol/NavisFrameDecoderTest.java
+++ b/test/org/traccar/protocol/NavisFrameDecoderTest.java
@@ -7,30 +7,33 @@ import org.junit.Test;
 public class NavisFrameDecoderTest extends ProtocolTest {
 
     @Test
-    public void testDecode() throws Exception {
+    public void testDecodeNtcb() throws Exception {
 
         NavisProtocolDecoder protocolDecoder = new NavisProtocolDecoder(null);
         NavisFrameDecoder frameDecoder = new NavisFrameDecoder(protocolDecoder);
 
-        // Handshake
-        verifyNull(protocolDecoder, binary(
-                "404E5443010000007B000000130044342A3E533A383631373835303035323035303739"));
-
-        // NTCB F5.2 - Single
         verifyFrame(binary(
                 "404e5443010000000000000059009adb2a3e54250000000000ff1500040b0a1008291838001200760ee600000000000000000000000f1500040b0a10ac20703fb1aec23f00000000320149668f430000000000000000000000000000000000000000000000f3808080"),
                 frameDecoder.decode(null, null, binary("404e5443010000000000000059009adb2a3e54250000000000ff1500040b0a1008291838001200760ee600000000000000000000000f1500040b0a10ac20703fb1aec23f00000000320149668f430000000000000000000000000000000000000000000000f3808080")));
 
-        // FLEX 1.0 - Negotiation
+    }
+
+    @Test
+    public void testDecodeFlex10() throws Exception {
+
+        NavisProtocolDecoder protocolDecoder = new NavisProtocolDecoder(null);
+        NavisFrameDecoder frameDecoder = new NavisFrameDecoder(protocolDecoder);
+
+        verifyNull(protocolDecoder, binary(
+                "404e544301000000c9b5f602130046c52a3e533a383639363936303439373232383235"));
+
         verifyNull(protocolDecoder, binary(
                 "404e544301000000aaecf6021300c8712a3e464c4558b00a0a45ffff300a08080f8388"));
 
-        // FLEX 1.0 - Single
         verifyFrame(binary(
                 "7e54040000000400000030129957405c000b00632f9857405ccace03021e129101a103000000000000c4005ba3fe3b00000000120046100000000000001aff7f000080bfffff80000080bfffffffff9f"),
                 frameDecoder.decode(null, null, binary("7e54040000000400000030129957405c000b00632f9857405ccace03021e129101a103000000000000c4005ba3fe3b00000000120046100000000000001aff7f000080bfffff80000080bfffffffff9f")));
 
-        // FLEX 1.0 - Array (1 message)
         verifyFrame(binary(
                 "7e4101080000000917c057405c002b001833c057405cbbce030225129101a00300007c6102408900400c1b3cfce3b23a12004710e000000000001bff7f000080bfffff80000080bfffffffffb2"),
                 frameDecoder.decode(null, null, binary("7e4101080000000917c057405c002b001833c057405cbbce030225129101a00300007c6102408900400c1b3cfce3b23a12004710e000000000001bff7f000080bfffff80000080bfffffffffb2")));
diff --git a/test/org/traccar/protocol/NavisProtocolDecoderTest.java b/test/org/traccar/protocol/NavisProtocolDecoderTest.java
index 5bf0c84bc..33a6bab24 100644
--- a/test/org/traccar/protocol/NavisProtocolDecoderTest.java
+++ b/test/org/traccar/protocol/NavisProtocolDecoderTest.java
@@ -7,20 +7,20 @@ import org.junit.Test;
 public class NavisProtocolDecoderTest extends ProtocolTest {
 
     @Test
-    public void testDecode() throws Exception {
+    public void testDecodeNtcb() throws Exception {
 
         NavisProtocolDecoder decoder = new NavisProtocolDecoder(null);
 
-        // Handshake
         verifyNull(decoder, binary(
                 "404E5443010000007B000000130044342A3E533A383631373835303035323035303739"));
 
-        // NTCB F5.2 - Single
+        verifyNull(decoder, binary(
+                "404E5443010000007B000000130047372A3E533A383631373835303035313236303639"));
+
         verifyPosition(decoder, binary(
                 "404e5443010000000000000059009adb2a3e54250000000000ff1500040b0a1008291838001200760ee600000000000000000000000f1500040b0a10ac20703fb1aec23f00000000320149668f430000000000000000000000000000000000000000000000f3808080"),
                 position("2016-11-11 21:00:04.000", true, 53.74336, 87.14437));
 
-        // NTCB 5.2 - Array (1 message)
         verifyPositions(decoder, binary(
                 "404e544300000000040000005a00c6812a3e410125e3a60700011705071503011030210c0000fa200910e6000000000000000000000001082106150010ae97643f88a39f3f0000000090001fcc6c450000000000000000000000000000000000000000000000f6808080"));
 
@@ -30,44 +30,48 @@ public class NavisProtocolDecoderTest extends ProtocolTest {
         verifyPositions(decoder, binary(
                 "404E5443010000007B0000005A0050692A3E410125DB0E00000015110707110A0C0880630000AA39A2381600020000000000000000000C110708110A0CB389793F1AEF263F00000000120034F516440000000000000000000000FAFF000000FAFF000000FAFF80808080"));
 
-        // NTCB 6.0 - Single
         verifyPosition(decoder, binary(
                 "404e544301000000cdfbf5027200852e2a3e5406aa170000c11116162410001310a9110e80996b281003000a0008000000000000000000d207d207ffffff00fbff00fbff00fbff00fbff00fbff00fbff00fbff2d808080ffffffffffff2b161624100013509b0302b0f89201830500000000000037002fb8cf43eed5843a35003500"),
                 position("2019-01-16 22:22:36.000", true, 56.31800, 44.01523));
 
-        // NTCB 6.0 - Array (1 message)
         verifyPositions(decoder, binary(
                 "404e54430100000045635902730081972a3e4101060b7e0e000b171328050d00133029110e00bc6141100200000000000000000000000000d207d307ffffff00fbff00fbff00fbff00fbff00fbff00fbff00fbff02808080ffffffffffff4f1328050d001371cd0302c5109101a60300000000000000003d1b37470000000096009600"));
+    }
+
+    @Test
+    public void testDecodeFlex10() throws Exception {
+
+        NavisProtocolDecoder decoder = new NavisProtocolDecoder(null);
+
+        verifyNull(decoder, binary(
+                "404e544301000000c9b5f602130046c52a3e533a383639363936303439373232383235"));
 
-        // FLEX 1.0 - Negotiation
         verifyNull(decoder, binary(
                 "404e544301000000aaecf6021300c8712a3e464c4558b00a0a45ffff300a08080f8388"));
 
-        // FLEX 1.0 - Single
         verifyPosition(decoder, binary(
                 "7e54040000000400000030129957405c000b00632f9857405ccace03021e129101a103000000000000c4005ba3fe3b00000000120046100000000000001aff7f000080bfffff80000080bfffffffff9f"),
                 position("2019-01-17 10:23:20.000", true, 56.33996, 43.80762));
 
-        // FLEX 1.0 - Array (1 message)
         verifyPositions(decoder, binary(
                 "7e4101080000000917c057405c002b001833c057405cbbce030225129101a00300007c6102408900400c1b3cfce3b23a12004710e000000000001bff7f000080bfffff80000080bfffffffffb2"));
+    }
 
-        decoder = new NavisProtocolDecoder(null);
+    @Test
+    public void testDecodeFlex20() throws Exception {
+
+        NavisProtocolDecoder decoder = new NavisProtocolDecoder(null);
 
-        // Handshake
         verifyNull(decoder, binary(
-                "404E5443010000007B000000130047372A3E533A383631373835303035313236303639"));
+                "404e544301000000a9eef602130043fb2a3e533a383639363936303439373337333835"));
 
-        // FLEX 2.0 - Negotiation
         verifyNull(decoder, binary(
                 "404e544301000000a9eef6021a003f8e2a3e464c4558b014147afffff008080800000e00000000000000"));
 
-        // FLEX 2.0 - Single
         verifyPosition(decoder, binary(
                 "7e5428000000280000002111d16b435c00a900154bd16b435ce19e030259f6920133050000b7623e429300c9e7f03f2ba45a3e1f001f007b6c5910850f0100001629080a000000000000060947"),
                 position("2019-01-19 18:26:25.000", true, 56.31952, 44.01423));
 
-        // FLEX 2.0 - Array (1 message)
         verifyPositions(decoder, binary(
                 "7e4101270000000b17b16b435c00a9000d4bb26b435caaa2030229f29201620500000000000093004493d53fee892d3e1f001f00ac6c591081f00000001700080a0000000000000609f2"));
     }