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>

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 9381fafb5bc9e0ef2c762184c5f0c7ff2d4bb11a..32e76266959b1953c477bf7aa86f02ebec421348 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 ee8caa59c288cc1e13b47bb65d7f13bba649db19..b6d26297df8ee02ce2254091fd3f8641bff3e5d8 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.Ipv4AddressNoZone;
 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