Sanitize flowspec serialization

We have a rather useless indirection in serializeNlri, which deals with
variadic arguments between plain and l3vpn NLRIs. Refactor the
interation by keeping only the basic in AbstractFlowspecNlriParser and
providing archetype-specific serialization methods.

Change-Id: Ifce9fcdd3380d77288f8f7b615aa0b2c966b2369
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/AbstractFlowspecIpNlriParser.java b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/AbstractFlowspecIpNlriParser.java
index 918def30bd6abe183d8eb8ac91858e5c0c74160a..56fdf6fbcef9a1c1213567884941c6760b9afdb2 100644 (file)
--- a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/AbstractFlowspecIpNlriParser.java
+++ b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/AbstractFlowspecIpNlriParser.java
@@ -8,6 +8,7 @@
 package org.opendaylight.protocol.bgp.flowspec;
 
 import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
 import java.util.List;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
@@ -38,4 +39,11 @@ abstract class AbstractFlowspecIpNlriParser extends AbstractFlowspecNlriParser {
 
     abstract @NonNull DestinationType createWithdrawnDestinationType(List<Flowspec> flowspecList,
         @Nullable PathId pathId);
+
+    protected final void serializeNlri(final List<Flowspec> flowspecList, final @Nullable PathId pathId,
+            final @NonNull ByteBuf buffer) {
+        final var nlri = Unpooled.buffer();
+        serializeNlri(flowspecList, nlri);
+        appendNlri(pathId, nlri, buffer);
+    }
 }

diff --git a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/AbstractFlowspecNlriParser.java b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/AbstractFlowspecNlriParser.java
index 1e09cdbc246a86fd6b6e06936d2199a23a9e09fe..8f1f6848fefdacb1b4f3405df88a7ca851964082 100644 (file)
--- a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/AbstractFlowspecNlriParser.java
+++ b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/AbstractFlowspecNlriParser.java
@@ -7,13 +7,12 @@
  */
 package org.opendaylight.protocol.bgp.flowspec;
 
+import static com.google.common.base.Preconditions.checkState;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
-import com.google.common.base.Preconditions;
 import io.netty.buffer.ByteBuf;
-import io.netty.buffer.Unpooled;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
@@ -184,11 +183,6 @@ public abstract class AbstractFlowspecNlriParser implements NlriParser, NlriSeri
         }
     }
 
-    protected void serializeNlri(final Object @NonNull [] nlriFields, final @NonNull ByteBuf buffer) {
-        final List<Flowspec> flowspecList = (List<Flowspec>) nlriFields[0];
-        serializeNlri(flowspecList, buffer);
-    }
-
     protected final void serializeNlri(final List<Flowspec> flowspecList, final @NonNull ByteBuf buffer) {
         if (flowspecList != null) {
             for (final Flowspec flow : flowspecList) {
@@ -198,27 +192,23 @@ public abstract class AbstractFlowspecNlriParser implements NlriParser, NlriSeri
     }
 
     /**
-     * Serializes Flowspec NLRI to ByteBuf.
+     * Appends a Flowspec NLRI buffer to an output ByteBuf.
      *
-     * @param nlriFields NLRI fields to be serialized
      * @param pathId path ID
-     * @param buffer where flowspec NLRI will be serialized
+     * @param nlri NLRI to be appended
+     * @param output where flowspec NLRI will be appended
      */
-    protected final void serializeNlri(final Object @NonNull[] nlriFields, final @Nullable PathId pathId,
-            final @NonNull ByteBuf buffer) {
-        final ByteBuf nlriByteBuf = Unpooled.buffer();
-        PathIdUtil.writePathId(pathId, buffer);
-
-        serializeNlri(nlriFields, nlriByteBuf);
+    protected static final void appendNlri(final @Nullable PathId pathId, final @NonNull ByteBuf nlri,
+            final @NonNull ByteBuf output) {
+        checkState(nlri.readableBytes() <= MAX_NLRI_LENGTH, "Maximum length of Flowspec NLRI reached.");
+        PathIdUtil.writePathId(pathId, output);
 
-        Preconditions.checkState(nlriByteBuf.readableBytes() <= MAX_NLRI_LENGTH,
-                "Maximum length of Flowspec NLRI reached.");
-        if (nlriByteBuf.readableBytes() <= MAX_NLRI_LENGTH_ONE_BYTE) {
-            buffer.writeByte(nlriByteBuf.readableBytes());
+        if (nlri.readableBytes() <= MAX_NLRI_LENGTH_ONE_BYTE) {
+            output.writeByte(nlri.readableBytes());
         } else {
-            buffer.writeShort(nlriByteBuf.readableBytes() + LENGTH_MAGIC);
+            output.writeShort(nlri.readableBytes() + LENGTH_MAGIC);
         }
-        buffer.writeBytes(nlriByteBuf);
+        output.writeBytes(nlri);
     }
 
     public String stringNlri(final DataContainerNode flowspec) {
@@ -554,13 +544,12 @@ public abstract class AbstractFlowspecNlriParser implements NlriParser, NlriSeri
 
     public static int readNlriLength(final @NonNull ByteBuf nlri) {
         requireNonNull(nlri, "NLRI information cannot be null");
-        Preconditions.checkState(nlri.isReadable(), "NLRI Byte buffer is not readable.");
+        checkState(nlri.isReadable(), "NLRI Byte buffer is not readable.");
         int length = nlri.readUnsignedByte();
         if (length >= MAX_NLRI_LENGTH_ONE_BYTE) {
             length = (length << Byte.SIZE | nlri.readUnsignedByte()) & MAX_NLRI_LENGTH;
         }
-        Preconditions.checkState(length > 0 && length <= nlri.readableBytes(),
-                "Invalid flowspec NLRI length %s", length);
+        checkState(length > 0 && length <= nlri.readableBytes(), "Invalid flowspec NLRI length %s", length);
         return length;
     }
 
@@ -579,8 +568,8 @@ public abstract class AbstractFlowspecNlriParser implements NlriParser, NlriSeri
 
         while (nlri.isReadable()) {
             int nlriLength = readNlriLength(nlri);
-            Preconditions.checkState(nlriLength > 0 && nlriLength <= nlri.readableBytes(),
-                    "Invalid flowspec NLRI length %s", nlriLength);
+            checkState(nlriLength > 0 && nlriLength <= nlri.readableBytes(),
+                "Invalid flowspec NLRI length %s", nlriLength);
             LOG.trace("Flowspec NLRI length is {}", nlriLength);
 
             while (nlriLength > 0) {
@@ -591,8 +580,7 @@ public abstract class AbstractFlowspecNlriParser implements NlriParser, NlriSeri
                 final int flowspecTypeLength = readableLength - nlri.readableBytes();
                 nlriLength -= flowspecTypeLength;
             }
-            Preconditions.checkState(nlriLength == 0,
-                    "Remain NLRI length should be 0 instead of %s", nlriLength);
+            checkState(nlriLength == 0, "Remain NLRI length should be 0 instead of %s", nlriLength);
         }
 
         return fss;

diff --git a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv4NlriParser.java b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv4NlriParser.java
index 64a56ef235c91e0acd150f7257a18ff24ace8cd9..098026a3740e24907dd12ef904b782dbc633dcbc 100644 (file)
--- a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv4NlriParser.java
+++ b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv4NlriParser.java
@@ -55,7 +55,7 @@ public final class SimpleFlowspecIpv4NlriParser extends AbstractFlowspecIpNlriPa
             final DestinationFlowspecIpv4 destFlowspec = ((org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns
                     .yang.bgp.flowspec.rev200120.update.attributes.mp.reach.nlri.advertized.routes.destination.type
                     .DestinationFlowspecCase) dstType).getDestinationFlowspecIpv4();
-            serializeNlri(new Object[] {destFlowspec.getFlowspec()}, destFlowspec.getPathId(), byteAggregator);
+            serializeNlri(destFlowspec.getFlowspec(), destFlowspec.getPathId(), byteAggregator);
         }
     }
 
@@ -76,7 +76,7 @@ public final class SimpleFlowspecIpv4NlriParser extends AbstractFlowspecIpNlriPa
             final DestinationFlowspecIpv4 destFlowspec = ((org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns
                     .yang.bgp.flowspec.rev200120.update.attributes.mp.unreach.nlri.withdrawn.routes.destination.type
                     .DestinationFlowspecCase) dstType).getDestinationFlowspecIpv4();
-            serializeNlri(new Object[] {destFlowspec.getFlowspec()}, destFlowspec.getPathId(), byteAggregator);
+            serializeNlri(destFlowspec.getFlowspec(), destFlowspec.getPathId(), byteAggregator);
         }
     }
 }

diff --git a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv6NlriParser.java b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv6NlriParser.java
index 9317adb1226bbc47a6c2532f2a1a5b87b6a95571..ac004d6221840b8de1d1abf54e358aa6c6d12073 100644 (file)
--- a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv6NlriParser.java
+++ b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv6NlriParser.java
@@ -65,7 +65,7 @@ public final class SimpleFlowspecIpv6NlriParser extends AbstractFlowspecIpNlriPa
             final DestinationFlowspecIpv6 destFlowspec = ((org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns
                     .yang.bgp.flowspec.rev200120.update.attributes.mp.reach.nlri.advertized.routes.destination.type
                     .DestinationFlowspecIpv6Case) dstType).getDestinationFlowspecIpv6();
-            serializeNlri(new Object[] {destFlowspec.getFlowspec()}, destFlowspec.getPathId(), byteAggregator);
+            serializeNlri(destFlowspec.getFlowspec(), destFlowspec.getPathId(), byteAggregator);
         }
     }
 
@@ -76,7 +76,7 @@ public final class SimpleFlowspecIpv6NlriParser extends AbstractFlowspecIpNlriPa
             final DestinationFlowspecIpv6 destFlowspec = ((org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns
                     .yang.bgp.flowspec.rev200120.update.attributes.mp.unreach.nlri.withdrawn.routes.destination.type
                     .DestinationFlowspecIpv6Case) dstType).getDestinationFlowspecIpv6();
-            serializeNlri(new Object[] {destFlowspec.getFlowspec()}, destFlowspec.getPathId(), byteAggregator);
+            serializeNlri(destFlowspec.getFlowspec(), destFlowspec.getPathId(), byteAggregator);
         }
     }
 }

diff --git a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/AbstractFlowspecL3vpnNlriParser.java b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/AbstractFlowspecL3vpnNlriParser.java
index 7759d3469a68ac0e3ae8ecadf95556651d86148b..394737dee202e74c768466cf2e9fcc93f47a1dde 100644 (file)
--- a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/AbstractFlowspecL3vpnNlriParser.java
+++ b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/AbstractFlowspecL3vpnNlriParser.java
@@ -10,7 +10,9 @@ package org.opendaylight.protocol.bgp.flowspec.l3vpn;
 import static java.util.Objects.requireNonNull;
 import static org.opendaylight.bgp.concepts.RouteDistinguisherUtil.extractRouteDistinguisher;
 
+import com.google.common.annotations.VisibleForTesting;
 import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
 import java.util.ArrayList;
 import java.util.List;
 import org.eclipse.jdt.annotation.NonNull;
@@ -59,12 +61,13 @@ public abstract class AbstractFlowspecL3vpnNlriParser extends AbstractFlowspecNl
         return rd;
     }
 
-    @Override
-    protected final void serializeNlri(final Object[] nlriFields, final ByteBuf buffer) {
-        final RouteDistinguisher rd = requireNonNull((RouteDistinguisher) nlriFields[0]);
-        RouteDistinguisherUtil.serializeRouteDistinquisher(rd, buffer);
-        final List<Flowspec> flowspecList = (List<Flowspec>) nlriFields[1];
-        serializeNlri(flowspecList, buffer);
+    @VisibleForTesting
+    public final void serializeNlri(final @NonNull RouteDistinguisher rd, final List<Flowspec> flowspecList,
+            final @Nullable PathId pathId, final @NonNull ByteBuf buffer) {
+        final var nlri = Unpooled.buffer();
+        RouteDistinguisherUtil.serializeRouteDistinquisher(rd, nlri);
+        serializeNlri(flowspecList, nlri);
+        appendNlri(pathId, nlri, buffer);
     }
 
     @Override

diff --git a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/ipv4/FlowspecL3vpnIpv4NlriParser.java b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/ipv4/FlowspecL3vpnIpv4NlriParser.java
index 981f7e6fc011c436f7642ebe7418112a3799c824..f6e557b3f00f7c5fffd7d5e4e7a0eb3515497693 100644 (file)
--- a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/ipv4/FlowspecL3vpnIpv4NlriParser.java
+++ b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/ipv4/FlowspecL3vpnIpv4NlriParser.java
@@ -77,10 +77,8 @@ public final class FlowspecL3vpnIpv4NlriParser extends AbstractFlowspecL3vpnNlri
                     .ns.yang.bgp.flowspec.rev200120.update.attributes.mp.reach.nlri.advertized.routes.destination.type
                     .DestinationFlowspecL3vpnIpv4Case) dstType).getDestinationFlowspecL3vpnIpv4();
             serializeNlri(
-                new Object[] {
-                    destFlowspec.getRouteDistinguisher(),
-                    destFlowspec.getFlowspec()
-                },
+                destFlowspec.getRouteDistinguisher(),
+                destFlowspec.getFlowspec(),
                 destFlowspec.getPathId(),
                 byteAggregator
             );
@@ -95,10 +93,8 @@ public final class FlowspecL3vpnIpv4NlriParser extends AbstractFlowspecL3vpnNlri
                     .ns.yang.bgp.flowspec.rev200120.update.attributes.mp.unreach.nlri.withdrawn.routes.destination.type
                     .DestinationFlowspecL3vpnIpv4Case) dstType).getDestinationFlowspecL3vpnIpv4();
             serializeNlri(
-                new Object[] {
-                    destFlowspec.getRouteDistinguisher(),
-                    destFlowspec.getFlowspec()
-                },
+                destFlowspec.getRouteDistinguisher(),
+                destFlowspec.getFlowspec(),
                 destFlowspec.getPathId(),
                 byteAggregator
             );

diff --git a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/ipv6/FlowspecL3vpnIpv6NlriParser.java b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/ipv6/FlowspecL3vpnIpv6NlriParser.java
index 2080001b74b196b3e2c4a4d5b2c90aeb85c0c26a..a06638b7febc3826e5928ba7e0f7f5b40cb29e42 100644 (file)
--- a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/ipv6/FlowspecL3vpnIpv6NlriParser.java
+++ b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/ipv6/FlowspecL3vpnIpv6NlriParser.java
@@ -78,10 +78,8 @@ public final class FlowspecL3vpnIpv6NlriParser extends AbstractFlowspecL3vpnNlri
                     .ns.yang.bgp.flowspec.rev200120.update.attributes.mp.reach.nlri.advertized.routes.destination.type
                     .DestinationFlowspecL3vpnIpv6Case) dstType).getDestinationFlowspecL3vpnIpv6();
             serializeNlri(
-                new Object[] {
-                    destFlowspec.getRouteDistinguisher(),
-                    destFlowspec.getFlowspec()
-                },
+                destFlowspec.getRouteDistinguisher(),
+                destFlowspec.getFlowspec(),
                 destFlowspec.getPathId(),
                 byteAggregator
             );
@@ -96,10 +94,8 @@ public final class FlowspecL3vpnIpv6NlriParser extends AbstractFlowspecL3vpnNlri
                     .ns.yang.bgp.flowspec.rev200120.update.attributes.mp.unreach.nlri.withdrawn.routes.destination.type
                     .DestinationFlowspecL3vpnIpv6Case) dstType).getDestinationFlowspecL3vpnIpv6();
             serializeNlri(
-                new Object[] {
-                    destFlowspec.getRouteDistinguisher(),
-                    destFlowspec.getFlowspec()
-                },
+                destFlowspec.getRouteDistinguisher(),
+                destFlowspec.getFlowspec(),
                 destFlowspec.getPathId(),
                 byteAggregator
             );

diff --git a/bgp/extensions/flowspec/src/test/java/org/opendaylight/protocol/bgp/flowspec/FlowspecL3vpnIpv4NlriParserTest.java b/bgp/extensions/flowspec/src/test/java/org/opendaylight/protocol/bgp/flowspec/FlowspecL3vpnIpv4NlriParserTest.java
index 186fdbddf05d7b66be47ad536453f07d17959b56..0ae045cfea6427a1437bef8855a76702ee6c9838 100644 (file)
--- a/bgp/extensions/flowspec/src/test/java/org/opendaylight/protocol/bgp/flowspec/FlowspecL3vpnIpv4NlriParserTest.java
+++ b/bgp/extensions/flowspec/src/test/java/org/opendaylight/protocol/bgp/flowspec/FlowspecL3vpnIpv4NlriParserTest.java
@@ -439,7 +439,7 @@ public class FlowspecL3vpnIpv4NlriParserTest {
         );
 
         final ByteBuf buffer = Unpooled.buffer();
-        parser.serializeNlri(new Object[] {rd, flows}, null, buffer);
+        parser.serializeNlri(rd, flows, null, buffer);
         assertArrayEquals(UNREACHED_NLRI, ByteArray.readAllBytes(buffer));
 
         parser.serializeAttribute(new AttributesBuilder()
@@ -573,7 +573,7 @@ public class FlowspecL3vpnIpv4NlriParserTest {
         );
 
         final ByteBuf buffer = Unpooled.buffer();
-        parser.serializeNlri(new Object[] {rd, flows}, PATH_ID, buffer);
+        parser.serializeNlri(rd, flows, PATH_ID, buffer);
         assertArrayEquals(UNREACHED_NLRI_ADD_PATH, ByteArray.readAllBytes(buffer));
 
         parser.serializeAttribute(new AttributesBuilder()

diff --git a/bgp/extensions/flowspec/src/test/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv4NlriParserTest.java b/bgp/extensions/flowspec/src/test/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv4NlriParserTest.java
index 0b80c47fe2b5d972458be25a0a3bdd9bc9df1909..4f1355fb0e79cf5e66af60c3edcad7dd9d8fc9b5 100644 (file)
--- a/bgp/extensions/flowspec/src/test/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv4NlriParserTest.java
+++ b/bgp/extensions/flowspec/src/test/java/org/opendaylight/protocol/bgp/flowspec/SimpleFlowspecIpv4NlriParserTest.java
@@ -403,7 +403,7 @@ public class SimpleFlowspecIpv4NlriParserTest {
         checkUnreachFlows(flows, icmpType, icmpCode, tcp, packet, dscp, fragment);
 
         final ByteBuf buffer = Unpooled.buffer();
-        parser.serializeNlri(new Object[] {flows}, null, buffer);
+        parser.serializeNlri(flows, null, buffer);
         assertArrayEquals(UNREACHED_NLRI, ByteArray.readAllBytes(buffer));
 
         parser.serializeAttribute(new AttributesBuilder()
@@ -538,7 +538,7 @@ public class SimpleFlowspecIpv4NlriParserTest {
 
 
         final ByteBuf buffer = Unpooled.buffer();
-        parser.serializeNlri(new Object[] {flows}, PATH_ID, buffer);
+        parser.serializeNlri(flows, PATH_ID, buffer);
         assertArrayEquals(UNREACHED_NLRI_ADD_PATH, ByteArray.readAllBytes(buffer));
 
         parser.serializeAttribute(new AttributesBuilder()