From: Gwenael Lambrouin Date: Thu, 21 May 2015 13:32:35 +0000 (+0200) Subject: BUG-2838: Trim leading whitespaces from incoming NETCONF messages. X-Git-Tag: release/lithium~49 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=81441c1681143050118195459a1a731cf9be21bb BUG-2838: Trim leading whitespaces from incoming NETCONF messages. Some network devices (such as Cisco routers) send RPC replies with a leading newline before the XML declaration. The OpenDaylight controller does not accept those replies: the XML parsing fails. This patch fixes the NETCONF messages before they are sent to the XML parser: it removes all the spurious characters at the beginning of the messages. Change-Id: Ibc6eb6dc5bad6252a3c9bed73d3db83814aff501 Signed-off-by: Gwenael Lambrouin Signed-off-by: Maros Marsalek (cherry picked from commit eb3ebced9d3471644dece3e4e27cca9451db0685) --- diff --git a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfXMLToMessageDecoder.java b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfXMLToMessageDecoder.java index 1dc8e0233b..5ae9bb4b1c 100644 --- a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfXMLToMessageDecoder.java +++ b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfXMLToMessageDecoder.java @@ -10,6 +10,7 @@ package org.opendaylight.controller.netconf.nettyutil.handler; import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufInputStream; import io.netty.buffer.ByteBufUtil; +import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.ByteToMessageDecoder; import java.io.IOException; @@ -30,9 +31,56 @@ public final class NetconfXMLToMessageDecoder extends ByteToMessageDecoder { LOG.trace("Received to decode: {}", ByteBufUtil.hexDump(in)); } + /* According to the XML 1.0 specifications, when there is an XML declaration + * at the beginning of an XML document, it is invalid to have + * white spaces before that declaration (reminder: a XML declaration looks like: + * ). In contrast, when there is no XML declaration, + * it is valid to have white spaces at the beginning of the document. + * + * When they send a NETCONF message, several NETCONF servers start with a new line (either + * LF or CRLF), presumably to improve readability in interactive sessions with a human being. + * Some NETCONF servers send an XML declaration, some others do not. + * + * If a server starts a NETCONF message with white spaces and follows with an XML + * declaration, XmlUtil.readXmlToDocument() will fail because this is invalid XML. + * But in the spirit of the "NETCONF over SSH" RFC 4742 and to improve interoperability, we want + * to accept those messages. + * + * To do this, the following code strips the leading bytes before the start of the XML messages. + */ + + // Skip all leading whitespaces by moving the reader index to the first non whitespace character + while (in.isReadable()) { + if (!isWhitespace(in.readByte())) { + // return reader index to the first non whitespace character + in.readerIndex(in.readerIndex() - 1); + break; + } + } + + // Warn about leading whitespaces + if (in.readerIndex() != 0 && LOG.isWarnEnabled()) { + final byte[] strippedBytes = new byte[in.readerIndex()]; + in.getBytes(0, strippedBytes, 0, in.readerIndex()); + LOG.warn("XML message with unwanted leading bytes detected. Discarded the {} leading byte(s): '{}'", + in.readerIndex(), ByteBufUtil.hexDump(Unpooled.wrappedBuffer(strippedBytes))); + } + } + if (in.isReadable()) { out.add(new NetconfMessage(XmlUtil.readXmlToDocument(new ByteBufInputStream(in)))); } else { LOG.debug("No more content in incoming buffer."); } } + + /** + * Check whether a byte is whitespace/control character. Considered whitespace characters:
+ * SPACE, \t, \n, \v, \r, \f + * + * @param b byte to check + * @return true if the byte is a whitespace/control character + */ + private static boolean isWhitespace(final byte b) { + return b <= 0x0d && b >= 0x09 || b == 0x20; + } } diff --git a/opendaylight/netconf/netconf-netty-util/src/test/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfXMLToMessageDecoderTest.java b/opendaylight/netconf/netconf-netty-util/src/test/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfXMLToMessageDecoderTest.java index f85a38769f..aaa92e8050 100644 --- a/opendaylight/netconf/netconf-netty-util/src/test/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfXMLToMessageDecoderTest.java +++ b/opendaylight/netconf/netconf-netty-util/src/test/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfXMLToMessageDecoderTest.java @@ -14,6 +14,7 @@ import com.google.common.collect.Lists; import io.netty.buffer.Unpooled; import java.util.ArrayList; import org.junit.Test; +import org.xml.sax.SAXParseException; public class NetconfXMLToMessageDecoderTest { @@ -30,4 +31,61 @@ public class NetconfXMLToMessageDecoderTest { new NetconfXMLToMessageDecoder().decode(null, Unpooled.wrappedBuffer("".getBytes()), out); assertEquals(1, out.size()); } -} \ No newline at end of file + + @Test + public void testDecodeWithLeadingLFAndXmlDecl() throws Exception { + /* Test that we accept XML documents with a line feed (0x0a) before the + * XML declaration in the XML prologue. + * A leading LF is the case reported in BUG-2838. + */ + final ArrayList out = Lists.newArrayList(); + new NetconfXMLToMessageDecoder().decode(null, Unpooled.wrappedBuffer("\n".getBytes()), out); + assertEquals(1, out.size()); + } + + @Test + public void testDecodeWithLeadingCRLFAndXmlDecl() throws Exception { + /* Test that we accept XML documents with both a carriage return and + * line feed (0x0d 0x0a) before the XML declaration in the XML prologue. + * Leading CRLF can be seen with some Cisco routers + * (eg CSR1000V running IOS 15.4(1)S) + */ + final ArrayList out = Lists.newArrayList(); + new NetconfXMLToMessageDecoder().decode(null, Unpooled.wrappedBuffer("\r\n".getBytes()), out); + assertEquals(1, out.size()); + } + + @Test(expected=SAXParseException.class) + 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()); + } + + @Test + public void testDecodeOnlyWhitespaces() throws Exception { + /* Test that we handle properly a bunch of whitespaces. + */ + final ArrayList out = Lists.newArrayList(); + new NetconfXMLToMessageDecoder().decode(null, Unpooled.wrappedBuffer("\r\n".getBytes()), out); + assertEquals(0, out.size()); + } + + @Test + public void testDecodeWithAllWhitespaces() throws Exception { + /* Test that every whitespace we want to skip is actually skipped. + */ + + final ArrayList out = Lists.newArrayList(); + byte whitespaces[] = {' ', '\t', '\n', '\r', '\f', 0x0b /* vertical tab */}; + new NetconfXMLToMessageDecoder().decode( + null, + Unpooled.copiedBuffer( + Unpooled.wrappedBuffer(whitespaces), + Unpooled.wrappedBuffer("".getBytes())), + out); + assertEquals(1, out.size()); + } +} +