Correct infinite loop in base parser

When sending a PCReply message with more than one ERO object, as per RFC540,
the PCEP base parser start infinite loop and stop with a java heap exception.

The problem is located in PCEPReplyMessageParser.java (pcep-base-parser
package) at line 217 in handleEro() method. handleEro() is called with the first
ERO object and remaining object to parse metrics associated with this ERO. The
parsing is done by parserPath() method, but this method expect to find only
metrics object. When a second ERO follow the last metric of the first ERO,
parsePath() method do nothing and handleEro() method start infinite loop at line
220 as the object list is not empty and parsePath() will not empty it.

This patch replace handleEro() method by handleEros() in order to parse
correctly multiple ERO in PCReply message. It also add a new test in
PCEPValidatorTest.java class: testPCRepMsgWithTwoEros() in order to verify
that a PCPep Message with two EROs is correctly parse.

JIRA: BGPCEP-851 PCEP base parser is unable to handle multiple ERO

Change-Id: I194b7cbb347a3b377e7b3000f93ef1d21a86f39a
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
(cherry picked from commit 367772ab24152242563a16d120441b353315c49f)

diff --git a/pcep/base-parser/src/main/java/org/opendaylight/protocol/pcep/parser/message/PCEPReplyMessageParser.java b/pcep/base-parser/src/main/java/org/opendaylight/protocol/pcep/parser/message/PCEPReplyMessageParser.java
index 7ebb12f612f6fb7c5b5222df942029e18f4c2d89..b962deb880d643e1aa4dbd81b1b180b531d1f804 100644 (file)
--- a/pcep/base-parser/src/main/java/org/opendaylight/protocol/pcep/parser/message/PCEPReplyMessageParser.java
+++ b/pcep/base-parser/src/main/java/org/opendaylight/protocol/pcep/parser/message/PCEPReplyMessageParser.java
@@ -186,7 +186,7 @@ public class PCEPReplyMessageParser extends AbstractMessageParser {
             if (objects.get(0) instanceof NoPath) {
                 res = handleNoPath((NoPath) objects.get(0), objects);
             } else if (objects.get(0) instanceof Ero) {
-                res = handleEro((Ero) objects.get(0), objects);
+                res = handleEros(objects);
             }
         }
         final List<MetricPce> metricPceList = new ArrayList<>();
@@ -214,13 +214,14 @@ public class PCEPReplyMessageParser extends AbstractMessageParser {
         return builder.build();
     }
 
-    private Result handleEro(final Ero ero, final List<Object> objects) {
-        objects.remove(0);
+    private Result handleEros(final List<Object> objects) {
         final SuccessBuilder builder = new SuccessBuilder();
         final List<Paths> paths = new ArrayList<>();
-        final PathsBuilder pBuilder = new PathsBuilder();
-        pBuilder.setEro(ero);
         while (!objects.isEmpty() && !(objects.get(0) instanceof PceId)) {
+            final PathsBuilder pBuilder = new PathsBuilder();
+            if (objects.get(0) instanceof Ero) {
+                pBuilder.setEro((Ero ) objects.remove(0));
+            }
             final List<VendorInformationObject> vendorInfoObjects = addVendorInformationObjects(objects);
             if (!vendorInfoObjects.isEmpty()) {
                 builder.setVendorInformationObject(vendorInfoObjects);

diff --git a/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/PCEPValidatorTest.java b/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/PCEPValidatorTest.java
index e654d3ac52454dfb15522dbb3315f6a34344f54b..92450a83ad6d24180914d680893c26e550683d12 100644 (file)
--- a/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/PCEPValidatorTest.java
+++ b/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/PCEPValidatorTest.java
@@ -40,6 +40,7 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4Address;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.iana.rev130816.EnterpriseNumber;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ieee754.rev130819.Float32;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.network.concepts.rev131125.Bandwidth;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.message.rev131007.CloseBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.message.rev131007.KeepaliveBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.message.rev131007.OpenBuilder;
@@ -53,6 +54,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.mes
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev131005.OfId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev131005.ProtocolVersion;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev131005.RequestId;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev131005.bandwidth.object.BandwidthBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev131005.close.message.CCloseMessageBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev131005.close.object.CCloseBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev131005.close.object.c.close.TlvsBuilder;
@@ -831,4 +833,43 @@ public class PCEPValidatorTest {
         parser.serializeMessage(new PcmonreqBuilder().setPcreqMessage(builder.build()).build(), buf);
         assertArrayEquals(result.array(), buf.array());
     }
+
+    @Test
+    public void testReplyMsgWithTwoEros() throws IOException, PCEPDeserializerException {
+        // Success Reply with two EROs: the first one is followed by Bandwidth Object and one Metric Object
+        ByteBuf result = Unpooled.wrappedBuffer(ByteArray.fileToBytes("src/test/resources/PCRep.7.bin"));
+
+        final PCEPReplyMessageParser parser = new PCEPReplyMessageParser(this.objectRegistry);
+
+        final PcrepMessageBuilder builder = new PcrepMessageBuilder();
+        RepliesBuilder rBuilder = new RepliesBuilder();
+
+        final List<Replies> replies = Lists.newArrayList();
+
+        final BandwidthBuilder bwBuilder = new BandwidthBuilder();
+        bwBuilder.setIgnore(false);
+        bwBuilder.setProcessingRule(false);
+        bwBuilder.setBandwidth(new Bandwidth(new byte[] { (byte) 0x47, (byte) 0x74, (byte) 0x24, (byte) 0x00 }));
+
+        rBuilder = new RepliesBuilder();
+        rBuilder.setRp(this.rpTrue);
+        final List<Paths> paths = Lists.newArrayList();
+        final PathsBuilder paBuilder1 = new PathsBuilder();
+        paBuilder1.setEro(this.ero);
+        paBuilder1.setBandwidth(bwBuilder.build());
+        paBuilder1.setMetrics(Lists.newArrayList(this.metrics));
+        paths.add(paBuilder1.build());
+        final PathsBuilder paBuilder2 = new PathsBuilder();
+        paBuilder2.setEro(this.ero);
+        paths.add(paBuilder2.build());
+        rBuilder.setResult(new SuccessCaseBuilder().setSuccess(new SuccessBuilder().setPaths(paths).build()).build()).build();
+        replies.add(rBuilder.build());
+        builder.setReplies(replies);
+
+        assertEquals(new PcrepBuilder().setPcrepMessage(builder.build()).build(), parser.parseMessage(result.slice(4,
+            result.readableBytes() - 4), Collections.emptyList()));
+        ByteBuf buf = Unpooled.buffer(result.readableBytes());
+        parser.serializeMessage(new PcrepBuilder().setPcrepMessage(builder.build()).build(), buf);
+        assertArrayEquals(result.array(), buf.array());
+    }
 }

diff --git a/pcep/impl/src/test/resources/PCRep.7.bin b/pcep/impl/src/test/resources/PCRep.7.bin
new file mode 100644 (file)
index 0000000..441f3a9
Binary files /dev/null and b/pcep/impl/src/test/resources/PCRep.7.bin differ