Fixed FIXME on MAC address serialization

 + Fix incorrect log message formatting

Change-Id: Ifc523e1e67e1e29ee3c1271995ca79d6edc8b223
Signed-off-by: Michal Polkorab <michal.polkorab@pantheon.sk>

diff --git a/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/OFEncoder.java b/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/OFEncoder.java
index 6080003f99297b53374b8a0e7980df9e69d7112f..493fa00791f2edf6fb6c4bd5930b9af98382cae7 100644 (file)
--- a/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/OFEncoder.java
+++ b/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/OFEncoder.java
@@ -40,7 +40,7 @@ public class OFEncoder extends MessageToByteEncoder<MessageListenerWrapper> {
         try {
             serializationFactory.messageToBuffer(wrapper.getMsg().getVersion(), out, wrapper.getMsg());
         } catch(Exception e) {
-            LOGGER.warn("Message serialization failed: {}", e.getMessage());
+            LOGGER.warn("Message serialization failed ", e);
             Future<Void> newFailedFuture = ctx.newFailedFuture(e);
             wrapper.getListener().operationComplete(newFailedFuture);
             out.clear();

diff --git a/util/src/main/java/org/opendaylight/openflowjava/util/ByteBufUtils.java b/util/src/main/java/org/opendaylight/openflowjava/util/ByteBufUtils.java
index f03f19638427b12bb9a065cb4b367203723bc9a2..444cffc504ad9ffcdcc2e571f15d599012c603f0 100644 (file)
--- a/util/src/main/java/org/opendaylight/openflowjava/util/ByteBufUtils.java
+++ b/util/src/main/java/org/opendaylight/openflowjava/util/ByteBufUtils.java
@@ -226,24 +226,40 @@ public abstract class ByteBufUtils {
      * @param macAddress
      * @return byte representation of mac address
      * @see {@link MacAddress}
-     *
-     * FIXME: this method does not support shortened values, e.g.
-     *        "0:1:2:3:4:5", only "00:11:22:33:44:55".
      */
     public static byte[] macAddressToBytes(final String macAddress) {
         final byte[] result = new byte[EncodeConstants.MAC_ADDRESS_LENGTH];
         final char[] mac = macAddress.toCharArray();
 
-        int offset = 0;
-        for (int i = 0; i < EncodeConstants.MAC_ADDRESS_LENGTH - 1; ++i) {
-            result[i] = UnsignedBytes.checkedCast(
-                    (hexValue(mac[offset++]) << 4) | hexValue(mac[offset++]));
-            Preconditions.checkArgument(mac[offset] == ':', "Invalid value: %s", macAddress);
-            offset++;
-        }
+        try {
+            int offset = 0;
+            for (int i = 0; i < EncodeConstants.MAC_ADDRESS_LENGTH - 1; ++i) {
+                if (mac[offset + EncodeConstants.SIZE_OF_BYTE_IN_BYTES] == ':') {
+                    result[i] = UnsignedBytes.checkedCast(hexValue(mac[offset]));
+                    offset++;
+                } else {
+                    result[i] = UnsignedBytes.checkedCast(
+                            (hexValue(mac[offset]) << 4) | hexValue(mac[offset +1]));
+                    offset += 2;
+                }
+                Preconditions.checkArgument(mac[offset] == ':', "Invalid value: %s", macAddress);
+                offset++;
+            }
 
-        result[EncodeConstants.MAC_ADDRESS_LENGTH - 1] =
-                UnsignedBytes.checkedCast(hexValue(mac[offset++]) << 4 | hexValue(mac[offset]));
+            if (offset == (mac.length - 1)) {
+                result[EncodeConstants.MAC_ADDRESS_LENGTH - 1] = UnsignedBytes.checkedCast(hexValue(mac[offset]));
+            } else {
+                result[EncodeConstants.MAC_ADDRESS_LENGTH - 1] =
+                        UnsignedBytes.checkedCast(hexValue(mac[offset]) << 4 | hexValue(mac[offset +1]));
+                offset++;
+            }
+            if (offset != (mac.length -1)) {
+                throw new IllegalArgumentException("Incorrect MAC address length");
+            }
+        } catch (Exception e) {
+            throw new IllegalArgumentException("Unable to serialize MAC address for input: " + macAddress
+                    + ". \n" + e);
+        }
         return result;
     }
 

diff --git a/util/src/test/java/org/opendaylight/openflowjava/util/ByteBufUtilsTest.java b/util/src/test/java/org/opendaylight/openflowjava/util/ByteBufUtilsTest.java
index 6ca0e7b0dc4716dea87acc3577503fbf230b7ee0..9bbb0d743b874c43c45ba4bd736ea50603556e50 100644 (file)
--- a/util/src/test/java/org/opendaylight/openflowjava/util/ByteBufUtilsTest.java
+++ b/util/src/test/java/org/opendaylight/openflowjava/util/ByteBufUtilsTest.java
@@ -223,8 +223,14 @@ public class ByteBufUtilsTest {
     public void testMacToBytes() {
         Assert.assertArrayEquals("Wrong byte array", new byte[]{0, 1, 2, 3, (byte) 255, 5},
                 ByteBufUtils.macAddressToBytes("00:01:02:03:FF:05"));
-        Assert.assertArrayEquals("Wrong byte array", new byte[]{11, 1, 2, 3, (byte) 255, 10},
-                ByteBufUtils.macAddressToBytes("0b:01:02:03:FF:0a"));
+        Assert.assertArrayEquals("Wrong byte array", new byte[]{1, 2, 3, 4, (byte) 255, 5},
+                ByteBufUtils.macAddressToBytes("01:02:03:04:FF:05"));
+        Assert.assertArrayEquals("Wrong byte array", new byte[]{1, 2, 3, 4, (byte) 255, 5},
+                ByteBufUtils.macAddressToBytes("1:2:3:4:FF:5"));
+        Assert.assertArrayEquals("Wrong byte array", new byte[]{1, 2, 3, 4, 5, (byte) 255},
+                ByteBufUtils.macAddressToBytes("1:2:3:4:5:FF"));
+        Assert.assertArrayEquals("Wrong byte array", new byte[]{1, 15, 3, 4, 5, 6},
+                ByteBufUtils.macAddressToBytes("1:F:3:4:5:6"));
     }
 
     /**
@@ -236,6 +242,70 @@ public class ByteBufUtilsTest {
                 ByteBufUtils.macAddressToBytes("00:01:02:03:FF:0G"));
     }
 
+    /**
+     * Test of {@link ByteBufUtils#macAddressToBytes(String)}
+     */
+    @Test(expected=IllegalArgumentException.class)
+    public void testMacToBytesTooShort() {
+        ByteBufUtils.macAddressToBytes("00:01:02:03:FF");
+    }
+
+    /**
+     * Test of {@link ByteBufUtils#macAddressToBytes(String)}
+     */
+    @Test(expected=IllegalArgumentException.class)
+    public void testMacToBytesTooShort2() {
+        ByteBufUtils.macAddressToBytes("00:01:02:03:FF:");
+    }
+
+    /**
+     * Test of {@link ByteBufUtils#macAddressToBytes(String)}
+     */
+    @Test(expected=IllegalArgumentException.class)
+    public void testIncorrectMacToBytes() {
+        ByteBufUtils.macAddressToBytes("00:01:02:03:FF::");
+    }
+
+    /**
+     * Test of {@link ByteBufUtils#macAddressToBytes(String)}
+     */
+    @Test(expected=IllegalArgumentException.class)
+    public void testIncorrectMacToBytes2() {
+        ByteBufUtils.macAddressToBytes("00:01:02:03:FF:::");
+    }
+
+    /**
+     * Test of {@link ByteBufUtils#macAddressToBytes(String)}
+     */
+    @Test(expected=IllegalArgumentException.class)
+    public void testMacToBytesTooLong() {
+        ByteBufUtils.macAddressToBytes("00:01:02:03:FF:05:85");
+    }
+
+    /**
+     * Test of {@link ByteBufUtils#macAddressToBytes(String)}
+     */
+    @Test(expected=IllegalArgumentException.class)
+    public void testMacToBytesInvalidOctet() {
+        ByteBufUtils.macAddressToBytes("00:01:02:03:FF:05d");
+    }
+
+    /**
+     * Test of {@link ByteBufUtils#macAddressToBytes(String)}
+     */
+    @Test(expected=IllegalArgumentException.class)
+    public void testMacToBytesInvalidOctet2() {
+        ByteBufUtils.macAddressToBytes("00:01:rr:03:FF:05");
+    }
+
+    /**
+     * Test of {@link ByteBufUtils#macAddressToBytes(String)}
+     */
+    @Test(expected=IllegalArgumentException.class)
+    public void testMacToBytesInvalidOctet3() {
+        ByteBufUtils.macAddressToBytes("00:01:05d:03:FF:02");
+    }
+
     /**
      * Test of {@link ByteBufUtils#macAddressToString(byte[])}
      */