Bug 8824 - NETCONF request hangs when rpc-rply has invalid xml

The decoder handler is blindly throwing the SAXException but
nobody is there to intercept it. Also, as we're in the netty
world, to be able to propagate the exception, we're using the
NetconfMessage POJO.
That latest has been modified to accept either the decoded message
or an exception, if exception is thrown.

Change-Id: I62af5a885cc4e9f459c4aa71871b7d9331c4b946
Signed-off-by: Alexis de Talhouët <adetalhouet89@gmail.com>

diff --git a/netconf/netconf-api/src/main/java/org/opendaylight/netconf/api/FailedNetconfMessage.java b/netconf/netconf-api/src/main/java/org/opendaylight/netconf/api/FailedNetconfMessage.java
new file mode 100644 (file)
index 0000000..e7a1cad
--- /dev/null
+++ b/netconf/netconf-api/src/main/java/org/opendaylight/netconf/api/FailedNetconfMessage.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 2017 Bell Canada. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.opendaylight.netconf.api;
+
+/**
+ * FailedNetconfMessage represents a wrapper around NetconfMessage.
+ */
+public class FailedNetconfMessage extends NetconfMessage {
+
+    private Throwable exception;
+
+    public FailedNetconfMessage(final Throwable exception) {
+        this.exception = exception;
+    }
+
+    public Throwable getException() {
+        return this.exception;
+    }
+}

diff --git a/netconf/netconf-api/src/main/java/org/opendaylight/netconf/api/NetconfMessage.java b/netconf/netconf-api/src/main/java/org/opendaylight/netconf/api/NetconfMessage.java
index 957c0475a50d3143860f924e7b9f3f5919aefa22..a60a880bf4d70cd04963e502f3e95476e98a6bb2 100644 (file)
--- a/netconf/netconf-api/src/main/java/org/opendaylight/netconf/api/NetconfMessage.java
+++ b/netconf/netconf-api/src/main/java/org/opendaylight/netconf/api/NetconfMessage.java
@@ -41,6 +41,11 @@ public class NetconfMessage {
 
     private final Document doc;
 
+    public NetconfMessage() {
+        // Required for FailedNetconfMessage
+        this.doc = null;
+    }
+
     public NetconfMessage(final Document doc) {
         this.doc = doc;
     }

diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfXMLToMessageDecoder.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfXMLToMessageDecoder.java
index fcb92f0e14dca2ded4e4ebd2002fc9f5371da5af..75d3a258a93f4e122a90f86795b174046710ea3f 100644 (file)
--- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfXMLToMessageDecoder.java
+++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfXMLToMessageDecoder.java
@@ -16,10 +16,12 @@ import io.netty.handler.codec.ByteToMessageDecoder;
 import java.io.IOException;
 import java.util.List;
 import org.opendaylight.controller.config.util.xml.XmlUtil;
+import org.opendaylight.netconf.api.FailedNetconfMessage;
 import org.opendaylight.netconf.api.NetconfMessage;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.xml.sax.SAXException;
+import org.xml.sax.SAXParseException;
 
 public final class NetconfXMLToMessageDecoder extends ByteToMessageDecoder {
     private static final Logger LOG = LoggerFactory.getLogger(NetconfXMLToMessageDecoder.class);
@@ -67,7 +69,16 @@ public final class NetconfXMLToMessageDecoder extends ByteToMessageDecoder {
             }
         }
         if (in.isReadable()) {
-            out.add(new NetconfMessage(XmlUtil.readXmlToDocument(new ByteBufInputStream(in))));
+            NetconfMessage msg;
+
+            try {
+                msg = new NetconfMessage(XmlUtil.readXmlToDocument(new ByteBufInputStream(in)));
+            } catch (SAXParseException exception) {
+                LOG.error("Failed to parse received message", exception);
+                msg = new FailedNetconfMessage(exception);
+            }
+
+            out.add(msg);
         } else {
             LOG.debug("No more content in incoming buffer.");
         }

diff --git a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/NetconfXMLToMessageDecoderTest.java b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/NetconfXMLToMessageDecoderTest.java
index 37b7b2a9cd0609a00a4f3cdbdf0da06fef4f5699..1891800a9f8d4ffb4247e6292a610b3905d2c3d9 100644 (file)
--- a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/NetconfXMLToMessageDecoderTest.java
+++ b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/NetconfXMLToMessageDecoderTest.java
@@ -9,11 +9,14 @@
 package org.opendaylight.netconf.nettyutil.handler;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import com.google.common.collect.Lists;
 import io.netty.buffer.Unpooled;
 import java.util.ArrayList;
 import org.junit.Test;
+import org.opendaylight.netconf.api.FailedNetconfMessage;
+import org.opendaylight.netconf.api.NetconfMessage;
 import org.xml.sax.SAXParseException;
 
 public class NetconfXMLToMessageDecoderTest {
@@ -55,12 +58,14 @@ public class NetconfXMLToMessageDecoderTest {
         assertEquals(1, out.size());
     }
 
-    @Test(expected=SAXParseException.class)
+    @Test
     public void testDecodeGibberish() throws Exception {
         /* Test that we reject inputs where we cannot find the xml start '<' character */
         final ArrayList<Object> out = Lists.newArrayList();
         new NetconfXMLToMessageDecoder().decode(null, Unpooled.wrappedBuffer("\r\n?xml version>".getBytes()), out);
         assertEquals(1, out.size());
+        assertTrue(FailedNetconfMessage.class.isInstance(out.get(0)));
+        assertTrue(((FailedNetconfMessage) out.get(0)).getException().getClass().isAssignableFrom(SAXParseException.class));
     }
 
     @Test

diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java
index 00dd66f260f8e3e75b4316b68129f2ce56028bf1..3811b7289151876d980baa7888b6983b4b44e14f 100644 (file)
--- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java
+++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java
@@ -22,8 +22,11 @@ import java.util.concurrent.Semaphore;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
+
+import org.opendaylight.controller.config.util.xml.DocumentedException;
 import org.opendaylight.controller.config.util.xml.XmlElement;
 import org.opendaylight.controller.config.util.xml.XmlUtil;
+import org.opendaylight.netconf.api.FailedNetconfMessage;
 import org.opendaylight.netconf.api.NetconfDocumentedException;
 import org.opendaylight.netconf.api.NetconfMessage;
 import org.opendaylight.netconf.api.NetconfTerminationReason;
@@ -287,6 +290,11 @@ public class NetconfDeviceCommunicator implements NetconfClientSessionListener,
 
         if( request != null ) {
 
+            if (FailedNetconfMessage.class.isInstance(message)) {
+                request.future.set(NetconfMessageTransformUtil.toRpcResult((FailedNetconfMessage) message));
+                return;
+            }
+
             LOG.debug("{}: Message received {}", id, message);
 
             if(LOG.isTraceEnabled()) {
@@ -395,6 +403,10 @@ public class NetconfDeviceCommunicator implements NetconfClientSessionListener,
     }
 
     private static boolean isNotification(final NetconfMessage message) {
+        if (message.getDocument() == null) {
+            // We have no message, which mean we have a FailedNetconfMessage
+            return false;
+        }
         final XmlElement xmle = XmlElement.fromDomDocument(message.getDocument());
         return XmlNetconfConstants.NOTIFICATION_ELEMENT_NAME.equals(xmle.getName()) ;
     }

diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfMessageTransformUtil.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfMessageTransformUtil.java
index 2b1476f8220394f953d25f76814e09f858ce48d6..040ec2ba021f6b589903bc4556de48913089ce13 100644 (file)
--- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfMessageTransformUtil.java
+++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfMessageTransformUtil.java
@@ -28,6 +28,7 @@ import javax.xml.transform.dom.DOMSource;
 import org.opendaylight.controller.config.util.xml.DocumentedException;
 import org.opendaylight.controller.config.util.xml.XmlElement;
 import org.opendaylight.controller.config.util.xml.XmlUtil;
+import org.opendaylight.netconf.api.FailedNetconfMessage;
 import org.opendaylight.netconf.api.NetconfDocumentedException;
 import org.opendaylight.netconf.api.NetconfMessage;
 import org.opendaylight.netconf.notifications.NetconfNotification;
@@ -41,6 +42,7 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.not
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.common.RpcError;
 import org.opendaylight.yangtools.yang.common.RpcError.ErrorSeverity;
+import org.opendaylight.yangtools.yang.common.RpcResult;
 import org.opendaylight.yangtools.yang.common.RpcResultBuilder;
 import org.opendaylight.yangtools.yang.data.api.ModifyAction;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
@@ -380,4 +382,16 @@ public class NetconfMessageTransformUtil {
             }
         }
     }
+
+    public static RpcResult<NetconfMessage> toRpcResult(final FailedNetconfMessage message) {
+        return RpcResultBuilder.<NetconfMessage>failed()
+                .withRpcError(
+                        toRpcError(
+                                new NetconfDocumentedException(
+                                        message.getException().getMessage(),
+                                        DocumentedException.ErrorType.APPLICATION,
+                                        DocumentedException.ErrorTag.MALFORMED_MESSAGE,
+                                        DocumentedException.ErrorSeverity.ERROR)))
+                .build();
+    }
 }