From: Alexis de Talhouët Date: Fri, 7 Jul 2017 17:00:43 +0000 (-0400) Subject: Bug 8824 - NETCONF request hangs when rpc-rply has invalid xml X-Git-Tag: release/carbon-sr2~8^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=cdc6e07df71958198c279e291ba1f6889fc86797;p=netconf.git 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 --- 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 index 0000000000..e7a1cade43 --- /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 957c0475a5..a60a880bf4 100644 --- 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 fcb92f0e14..75d3a258a9 100644 --- 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 37b7b2a9cd..1891800a9f 100644 --- 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 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 00dd66f260..3811b72891 100644 --- 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 2b1476f822..040ec2ba02 100644 --- 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 toRpcResult(final FailedNetconfMessage message) { + return RpcResultBuilder.failed() + .withRpcError( + toRpcError( + new NetconfDocumentedException( + message.getException().getMessage(), + DocumentedException.ErrorType.APPLICATION, + DocumentedException.ErrorTag.MALFORMED_MESSAGE, + DocumentedException.ErrorSeverity.ERROR))) + .build(); + } }