Repository: ncclient/ncclient

Issue:
Exception when NETCONF response contains an XML text node > 10,000,000 bytes
By default, libxml2 >= 2.7 restricts XML text nodes to a maximum of 10,000,000 bytes (~9.5Mbytes). In turn, lxml uses this libxml2 default, but offers the `huge_tree` option:
http://lxml.de/parsing.html#parser-options

If a NETCONF response contains an XML text node with > 10,000,000 bytes this causes the following exception to be raised:
```
    return self._conn.rpc(rpc_cmd_e)._NCElement__doc
  File "/usr/local/lib/python2.7/dist-packages/ncclient/manager.py", line 172, in wrapper
    return self.execute(op_cls, *args, **kwds)
  File "/usr/local/lib/python2.7/dist-packages/ncclient/manager.py", line 232, in execute
    raise_mode=self._raise_mode).request(*args, **kwds)
  File "/usr/local/lib/python2.7/dist-packages/ncclient/operations/third_party/juniper/rpc.py", line 44, in request
    return self._request(rpc)
  File "/usr/local/lib/python2.7/dist-packages/ncclient/operations/rpc.py", line 339, in _request
    return NCElement(self._reply, self._device_handler.transform_reply())
  File "/usr/local/lib/python2.7/dist-packages/ncclient/xml_.py", line 153, in __init__
    self.__doc = self.remove_namespaces(self.__result)
  File "/usr/local/lib/python2.7/dist-packages/ncclient/xml_.py", line 201, in remove_namespaces
    self.__root = etree.fromstring(str(self.__transform(etree.parse(StringIO(str(rpc_reply))))))
  File "src/lxml/lxml.etree.pyx", line 3427, in lxml.etree.parse (src/lxml/lxml.etree.c:81117)
  File "src/lxml/parser.pxi", line 1828, in lxml.etree._parseDocument (src/lxml/lxml.etree.c:118072)
  File "src/lxml/parser.pxi", line 1848, in lxml.etree._parseMemoryDocument (src/lxml/lxml.etree.c:118341)
  File "src/lxml/parser.pxi", line 1736, in lxml.etree._parseDoc (src/lxml/lxml.etree.c:117021)
  File "src/lxml/parser.pxi", line 1102, in lxml.etree._BaseParser._parseDoc (src/lxml/lxml.etree.c:111265)
  File "src/lxml/parser.pxi", line 595, in lxml.etree._ParserContext._handleParseResultDoc (src/lxml/lxml.etree.c:105109)
  File "src/lxml/parser.pxi", line 706, in lxml.etree._handleParseResult (src/lxml/lxml.etree.c:106817)
  File "src/lxml/parser.pxi", line 635, in lxml.etree._raiseParseError (src/lxml/lxml.etree.c:105671)
  File "<string>", line 106044
lxml.etree.XMLSyntaxError: xmlSAX2Characters: huge text node, line 106044, column 1
```

This has been encountered when trying to retrieve a large Junos configuration file in set or text format, but the general issue is not specific to Junos.


Discussion:

Patch:
diff --git a/docs/source/manager.rst b/docs/source/manager.rst
index 5aef64f..27dff7c 100644
--- a/docs/source/manager.rst
+++ b/docs/source/manager.rst
@@ -38,6 +38,8 @@ Presence of capabilities is verified to the extent possible, and you can expect
 
 .. autoclass:: Manager
 
+    .. autoattribute:: HUGE_TREE_DEFAULT
+
     .. automethod:: get_config(source, filter=None)
 
     .. automethod:: edit_config(target, config, default_operation=None, test_option=None, error_option=None)
@@ -86,6 +88,7 @@ Presence of capabilities is verified to the extent possible, and you can expect
 
     .. autoattribute:: connected
 
+    .. autoattribute:: huge_tree
 
 Special kinds of parameters
 ---------------------------
diff --git a/docs/source/operations.rst b/docs/source/operations.rst
index 8645a20..63bbe4d 100644
--- a/docs/source/operations.rst
+++ b/docs/source/operations.rst
@@ -11,7 +11,7 @@ Base classes
 ------------
 
 .. autoclass:: RPC
-    :members: DEPENDS, REPLY_CLS, _assert, _request, request, event, error, reply, raise_mode, is_async, timeout
+    :members: DEPENDS, REPLY_CLS, _assert, _request, request, event, error, reply, raise_mode, is_async, timeout, huge_tree
 
 .. autoclass:: RPCReply
     :members: xml, ok, error, errors, _parsing_hook
diff --git a/ncclient/manager.py b/ncclient/manager.py
index 09a8aaf..93909b9 100644
--- a/ncclient/manager.py
+++ b/ncclient/manager.py
@@ -198,11 +198,16 @@ class Manager(object):
 
    # __metaclass__ = OpExecutor
 
+
+    HUGE_TREE_DEFAULT = False
+    """Default for `huge_tree` support for XML parsing of RPC replies (defaults to False)"""
+
     def __init__(self, session, device_handler, timeout=30):
         self._session = session
         self._async_mode = False
         self._timeout = timeout
         self._raise_mode = operations.RaiseMode.ALL
+        self._huge_tree = self.HUGE_TREE_DEFAULT
         self._device_handler = device_handler
 
     def __enter__(self):
@@ -227,7 +232,8 @@ class Manager(object):
                    device_handler=self._device_handler,
                    async_mode=self._async_mode,
                    timeout=self._timeout,
-                   raise_mode=self._raise_mode).request(*args, **kwds)
+                   raise_mode=self._raise_mode,
+                   huge_tree=self._huge_tree).request(*args, **kwds)
 
     def locked(self, target):
         """Returns a context manager for a lock on a datastore, where
@@ -331,3 +337,13 @@ class Manager(object):
     exceptions. Valid values are the constants defined in
     :class:`~ncclient.operations.RaiseMode`.
     The default value is :attr:`~ncclient.operations.RaiseMode.ALL`."""
+
+    @property
+    def huge_tree(self):
+        """Whether `huge_tree` support for XML parsing of RPC replies is enabled (default=False)
+        The default value is configurable through :attr:`~ncclient.manager.Manager.HUGE_TREE_DEFAULT`"""
+        return self._huge_tree
+
+    @huge_tree.setter
+    def huge_tree(self, x):
+        self._huge_tree = x
diff --git a/ncclient/operations/rpc.py b/ncclient/operations/rpc.py
index d0d7f5c..64e0f3f 100644
--- a/ncclient/operations/rpc.py
+++ b/ncclient/operations/rpc.py
@@ -120,6 +120,10 @@ class RPCReply(object):
 
     """Represents an *rpc-reply*. Only concerns itself with whether the operation was successful.
 
+    *raw*: the raw unparsed reply
+
+    *huge_tree*: parse XML with very deep trees and very long text content
+
     .. note::
         If the reply has not yet been parsed there is an implicit, one-time parsing overhead to
         accessing some of the attributes defined by this class.
@@ -128,11 +132,12 @@ class RPCReply(object):
     ERROR_CLS = RPCError
     "Subclasses can specify a different error class, but it should be a subclass of `RPCError`."
 
-    def __init__(self, raw):
+    def __init__(self, raw, huge_tree=False):
         self._raw = raw
         self._parsed = False
         self._root = None
         self._errors = []
+        self._huge_tree = huge_tree
 
     def __repr__(self):
         return self._raw
@@ -140,7 +145,7 @@ class RPCReply(object):
     def parse(self):
         "Parses the *rpc-reply*."
         if self._parsed: return
-        root = self._root = to_ele(self._raw) # The <rpc-reply> element
+        root = self._root = to_ele(self._raw, huge_tree=self._huge_tree) # The <rpc-reply> element
         # Per RFC 4741 an <ok/> tag is sent when there are no errors or warnings
         ok = root.find(qualify("ok"))
         if ok is None:
@@ -267,7 +272,7 @@ class RPC(object):
     "By default :class:`RPCReply`. Subclasses can specify a :class:`RPCReply` subclass."
 
 
-    def __init__(self, session, device_handler, async_mode=False, timeout=30, raise_mode=RaiseMode.NONE):
+    def __init__(self, session, device_handler, async_mode=False, timeout=30, raise_mode=RaiseMode.NONE, huge_tree=False):
         """
         *session* is the :class:`~ncclient.transport.Session` instance
 
@@ -278,6 +283,8 @@ class RPC(object):
         *timeout* is the timeout for a synchronous request, see :attr:`timeout`
 
         *raise_mode* specifies the exception raising mode, see :attr:`raise_mode`
+
+        *huge_tree* parse xml with huge_tree support (e.g. for large text config retrieval), see :attr:`huge_tree`
         """
         self._session = session
         try:
@@ -288,6 +295,7 @@ class RPC(object):
         self._async = async_mode
         self._timeout = timeout
         self._raise_mode = raise_mode
+        self._huge_tree = huge_tree
         self._id = uuid4().urn # Keeps things simple instead of having a class attr with running ID that has to be locked
         self._listener = RPCReplyListener(session, device_handler)
         self._listener.register(self._id, self)
@@ -340,7 +348,7 @@ class RPC(object):
                         else:
                             raise self._reply.error
                 if self._device_handler.transform_reply():
-                    return NCElement(self._reply, self._device_handler.transform_reply())
+                    return NCElement(self._reply, self._device_handler.transform_reply(), huge_tree=self._huge_tree)
                 else:
                     return self._reply
             else:
@@ -361,7 +369,7 @@ class RPC(object):
 
     def deliver_reply(self, raw):
         # internal use
-        self._reply = self.REPLY_CLS(raw)
+        self._reply = self.REPLY_CLS(raw, huge_tree=self._huge_tree)
         self._event.set()
 
     def deliver_error(self, err):
@@ -424,3 +432,12 @@ class RPC(object):
 
     Irrelevant for asynchronous usage.
     """
+
+    @property
+    def huge_tree(self):
+        """Whether `huge_tree` support for XML parsing of RPC replies is enabled (default=False)"""
+        return self._huge_tree
+
+    @huge_tree.setter
+    def huge_tree(self, x):
+        self._huge_tree = x
diff --git a/ncclient/operations/third_party/juniper/rpc.py b/ncclient/operations/third_party/juniper/rpc.py
index 84d3f98..661935d 100644
--- a/ncclient/operations/third_party/juniper/rpc.py
+++ b/ncclient/operations/third_party/juniper/rpc.py
@@ -12,6 +12,9 @@ class GetConfiguration(RPC):
         node = new_ele('get-configuration', {'format':format})
         if filter is not None:
             node.append(filter)
+        if format !='xml':
+            # The entire config comes as a single text element and this requires huge_tree support for large configs
+            self._huge_tree = True
         return self._request(node)
 
 class LoadConfiguration(RPC):
diff --git a/ncclient/xml_.py b/ncclient/xml_.py
index ef6b548..84dbff0 100644
--- a/ncclient/xml_.py
+++ b/ncclient/xml_.py
@@ -31,6 +31,12 @@ from lxml import etree
 from ncclient import NCClientError
 
 parser = etree.XMLParser(recover=False)
+huge_parser = etree.XMLParser(recover=False, huge_tree=True)
+
+
+def _get_parser(huge_tree=False):
+    return huge_parser if huge_tree else parser
+
 
 class XMLError(NCClientError):
     pass
@@ -106,12 +112,17 @@ def to_xml(ele, encoding="UTF-8", pretty_print=False):
         return xml.decode('UTF-8') if xml.startswith(b'<?xml') \
             else '<?xml version="1.0" encoding="%s"?>%s' % (encoding, xml.decode('UTF-8'))
 
-def to_ele(x):
-    "Convert and return the :class:`~xml.etree.ElementTree.Element` for the XML document *x*. If *x* is already an :class:`~xml.etree.ElementTree.Element` simply returns that."
+
+def to_ele(x, huge_tree=False):
+    """Convert and return the :class:`~xml.etree.ElementTree.Element` for the XML document *x*. If *x* is already an :class:`~xml.etree.ElementTree.Element` simply returns that.
+
+    *huge_tree*: parse XML with very deep trees and very long text content
+    """
     if sys.version < '3':
-        return x if etree.iselement(x) else etree.fromstring(x, parser=parser)
+        return x if etree.iselement(x) else etree.fromstring(x, parser=_get_parser(huge_tree))
     else:
-        return x if etree.iselement(x) else etree.fromstring(x.encode('UTF-8'), parser=parser)
+        return x if etree.iselement(x) else etree.fromstring(x.encode('UTF-8'), parser=_get_parser(huge_tree))
+
 
 def parse_root(raw):
     "Efficiently parses the root element of a *raw* XML document, returning a tuple of its qualified name and attribute dictionary."
@@ -153,9 +164,10 @@ XPATH_NAMESPACES = {
 
 
 class NCElement(object):
-    def __init__(self, result, transform_reply):
+    def __init__(self, result, transform_reply, huge_tree=False):
         self.__result = result
         self.__transform_reply = transform_reply
+        self.__huge_tree = huge_tree
         if isinstance(transform_reply, types.FunctionType):
             self.__doc = self.__transform_reply(result._root)
         else:
@@ -191,7 +203,7 @@ class NCElement(object):
     @property
     def tostring(self):
         """return a pretty-printed string output for rpc reply"""
-        parser = etree.XMLParser(remove_blank_text=True)
+        parser = etree.XMLParser(remove_blank_text=True, huge_tree=self.__huge_tree)
         outputtree = etree.XML(etree.tostring(self.__doc), parser)
         return etree.tostring(outputtree, pretty_print=True)
 
@@ -203,10 +215,12 @@ class NCElement(object):
     def remove_namespaces(self, rpc_reply):
         """remove xmlns attributes from rpc reply"""
         self.__xslt=self.__transform_reply
-        self.__parser = etree.XMLParser(remove_blank_text=True)
+        self.__parser = etree.XMLParser(remove_blank_text=True, huge_tree=self.__huge_tree)
         self.__xslt_doc = etree.parse(io.BytesIO(self.__xslt), self.__parser)
         self.__transform = etree.XSLT(self.__xslt_doc)
-        self.__root = etree.fromstring(str(self.__transform(etree.parse(StringIO(str(rpc_reply))))))
+        self.__root = etree.fromstring(str(self.__transform(etree.parse(StringIO(str(rpc_reply)),
+                                                                        parser=self.__parser))),
+                                       parser=self.__parser)
         return self.__root
 
 


Test patch:
diff --git a/test/unit/operations/test_rpc.py b/test/unit/operations/test_rpc.py
index 131d297..92b8815 100644
--- a/test/unit/operations/test_rpc.py
+++ b/test/unit/operations/test_rpc.py
@@ -7,6 +7,7 @@ import ncclient.transport
 from ncclient.xml_ import *
 from ncclient.operations import RaiseMode
 from ncclient.capabilities import Capabilities
+from xml.sax.saxutils import escape
 import sys
 
 if sys.version >= '3':
@@ -78,6 +79,16 @@ xml4 = """<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" xmlns:junos
 </data>
 </rpc-reply>"""
 
+# Huge dummy text configuration to trigger "xml.etree.XMLSyntaxError: xmlSAX2Characters: huge text node"
+# - The maximum size of a single text node is `#define XML_MAX_TEXT_LENGTH 10000000`
+#   (https://gitlab.gnome.org/GNOME/libxml2/blob/master/include/libxml/parserInternals.h)
+# - The text should contain at least one special character that should be escaped (e.g. '>')
+huge_configuration_text = "-->\n" + "Huge configuration. " * 500000
+
+xml5_huge = """<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" xmlns:junos="http://xml.juniper.net/junos/18.2R1/junos" xmlns:nc="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="urn:uuid:b8b20a95-7822-4c59-97a3-44435d6fc6a7">
+<configuration-text xmlns="http://xml.juniper.net/xnm/1.1/xnm">%s</configuration-text>
+</rpc-reply>""" % escape(huge_configuration_text)
+
 
 class TestRPC(unittest.TestCase):
 
@@ -89,12 +100,22 @@ class TestRPC(unittest.TestCase):
         self.assertEqual(xml4, obj.xml)
         self.assertTrue(obj._parsed)
 
+    def test_rpc_reply_huge_text_node_exception(self):
+        obj = RPCReply(xml5_huge)
+        self.assertRaises(etree.XMLSyntaxError, obj.parse)
+
+    def test_rpc_reply_huge_text_node_workaround(self):
+        obj = RPCReply(xml5_huge, huge_tree=True)
+        obj.parse()
+        self.assertTrue(obj.ok)
+        self.assertFalse(obj.error)
+        self.assertEqual(xml5_huge, obj.xml)
+        self.assertTrue(obj._parsed)
+
     @patch('ncclient.transport.Session.send')
     @patch(patch_str)
     def test_rpc_send(self, mock_thread, mock_send):
-        device_handler = manager.make_device_handler({'name': 'junos'})
-        capabilities = Capabilities(device_handler.get_capabilities())
-        session = ncclient.transport.Session(capabilities)
+        device_handler, session = self._mock_device_handler_and_session()
         obj = RPC(session, device_handler, raise_mode=RaiseMode.ALL, timeout=0)
         reply = RPCReply(xml1)
         obj._reply = reply
@@ -120,9 +141,7 @@ class TestRPC(unittest.TestCase):
     @patch('ncclient.transport.Session.send')
     @patch(patch_str)
     def test_rpc_async(self, mock_thread, mock_send):
-        device_handler = manager.make_device_handler({'name': 'junos'})
-        capabilities = Capabilities(device_handler.get_capabilities())
-        session = ncclient.transport.Session(capabilities)
+        device_handler, session = self._mock_device_handler_and_session()
         obj = RPC(
             session,
             device_handler,
@@ -138,9 +157,7 @@ class TestRPC(unittest.TestCase):
     @patch('ncclient.transport.Session.send')
     @patch(patch_str)
     def test_rpc_timeout_error(self, mock_thread, mock_send):
-        device_handler = manager.make_device_handler({'name': 'junos'})
-        capabilities = Capabilities(device_handler.get_capabilities())
-        session = ncclient.transport.Session(capabilities)
+        device_handler, session = self._mock_device_handler_and_session()
         obj = RPC(session, device_handler, raise_mode=RaiseMode.ALL, timeout=0)
         reply = RPCReply(xml1)
         obj.deliver_reply(reply)
@@ -152,9 +169,7 @@ class TestRPC(unittest.TestCase):
     @patch('ncclient.transport.Session.send')
     @patch(patch_str)
     def test_rpc_rpcerror(self, mock_thread, mock_send):
-        device_handler = manager.make_device_handler({'name': 'junos'})
-        capabilities = Capabilities(device_handler.get_capabilities())
-        session = ncclient.transport.Session(capabilities)
+        device_handler, session = self._mock_device_handler_and_session()
         obj = RPC(session, device_handler, raise_mode=RaiseMode.ALL, timeout=0)
         reply = RPCReply(xml1)
         obj._reply = reply
@@ -168,11 +183,38 @@ class TestRPC(unittest.TestCase):
     @patch('ncclient.transport.Session.send')
     @patch(patch_str)
     def test_rpc_capability_error(self, mock_thread, mock_send):
-        device_handler = manager.make_device_handler({'name': 'junos'})
-        capabilities = Capabilities(device_handler.get_capabilities())
-        session = ncclient.transport.Session(capabilities)
+        device_handler, session = self._mock_device_handler_and_session()
         session._server_capabilities = [':running']
         obj = RPC(session, device_handler, raise_mode=RaiseMode.ALL, timeout=0)
         obj._assert(':running')
         self.assertRaises(MissingCapabilityError,
             obj._assert, ':candidate')
+
+    @patch('ncclient.transport.Session.send')
+    def test_rpc_huge_text_node_exception(self, mock_send):
+        device_handler, session = self._mock_device_handler_and_session()
+        obj = RPC(session, device_handler, raise_mode=RaiseMode.ALL, timeout=0)
+
+        obj.deliver_reply(xml5_huge)
+        node = new_ele("get-configuration", {'format': 'text'})
+        self.assertRaises(etree.XMLSyntaxError, obj._request, node)
+
+    @patch('ncclient.transport.Session.send')
+    def test_rpc_huge_text_node_workaround(self, mock_send):
+        device_handler, session = self._mock_device_handler_and_session()
+        obj = RPC(session, device_handler, raise_mode=RaiseMode.ALL, timeout=0, huge_tree=True)
+        self.assertTrue(obj.huge_tree)
+
+        obj.deliver_reply(xml5_huge)
+        node = new_ele("get-configuration", {'format': 'text'})
+        result = obj._request(node)
+
+        self.assertEqual(result.find('configuration-text').text, huge_configuration_text)
+        obj.huge_tree = False
+        self.assertFalse(obj.huge_tree)
+
+    def _mock_device_handler_and_session(self):
+        device_handler = manager.make_device_handler({'name': 'junos'})
+        capabilities = Capabilities(device_handler.get_capabilities())
+        session = ncclient.transport.Session(capabilities)
+        return device_handler, session
diff --git a/test/unit/operations/third_party/juniper/test_rpc.py b/test/unit/operations/third_party/juniper/test_rpc.py
index 09a2e76..d21e5dc 100644
--- a/test/unit/operations/third_party/juniper/test_rpc.py
+++ b/test/unit/operations/third_party/juniper/test_rpc.py
@@ -46,6 +46,19 @@ class TestRPC(unittest.TestCase):
         self.assertEqual(call.tag, node.tag)
         self.assertEqual(call.attrib, node.attrib)
 
+    @patch('ncclient.transport.SSHSession')
+    @patch('ncclient.operations.third_party.juniper.rpc.RPC._request')
+    def test_getconf_text_sets_huge_tree(self, mock_request, mock_session):
+        device_handler = manager.make_device_handler({'name': 'junos'})
+        session = ncclient.transport.SSHSession(device_handler)
+        obj = GetConfiguration(
+            session,
+            device_handler,
+            raise_mode=RaiseMode.ALL)
+        obj.huge_tree = False
+        obj.request(format='text')
+        self.assertTrue(obj.huge_tree)
+
     @patch('ncclient.transport.SSHSession')
     @patch('ncclient.operations.third_party.juniper.rpc.RPC._request')
     def test_loadconf_xml(self, mock_request, mock_session):
diff --git a/test/unit/test_manager.py b/test/unit/test_manager.py
index 55a9859..df5dde7 100644
--- a/test/unit/test_manager.py
+++ b/test/unit/test_manager.py
@@ -4,6 +4,7 @@ from ncclient import manager
 from ncclient.devices.junos import JunosDeviceHandler
 import logging
 
+
 class TestManager(unittest.TestCase):
 
     @patch('ncclient.transport.SSHSession')
@@ -67,7 +68,6 @@ class TestManager(unittest.TestCase):
                                     port=830,
                                     username='user',
                                     password='password',
-                                    timeout=10,
                                     hostkey_verify=False,
                                     allow_agent=False,
                                     ssh_config=ssh_config_path)
@@ -184,6 +184,29 @@ class TestManager(unittest.TestCase):
         conn = self._mock_manager()
         self.assertEqual(conn.connected, True)
 
+    @patch('ncclient.manager.Manager.HUGE_TREE_DEFAULT')
+    @patch('ncclient.transport.SSHSession')
+    @patch('ncclient.operations.rpc.RPC')
+    def test_manager_huge_node(self, mock_rpc, mock_session, default_value):
+
+        # Set default value to True only in this test through the default_value mock
+        default_value = True
+
+        # true should propagate all the way to the RPC
+        conn = self._mock_manager()
+        self.assertTrue(conn.huge_tree)
+        conn.execute(mock_rpc)
+        mock_rpc.assert_called_once()
+        self.assertTrue(mock_rpc.call_args[1]['huge_tree'])
+
+        # false should propagate all the way to the RPC
+        conn.huge_tree = False
+        self.assertFalse(conn.huge_tree)
+        mock_rpc.reset_mock()
+        conn.execute(mock_rpc)
+        mock_rpc.assert_called_once()
+        self.assertFalse(mock_rpc.call_args[1]['huge_tree'])
+
     def _mock_manager(self):
         conn = manager.connect(host='10.10.10.10',
                                     port=22,
@@ -209,11 +232,11 @@ class TestManager(unittest.TestCase):
                                     sock_fd=6,
                                     username='user',
                                     password='password',
-                                    timeout=10,
                                     device_params={'name': 'junos'},
                                     hostkey_verify=False, allow_agent=False)
         return conn
 
+
 if __name__ == "__main__":
     suite = unittest.TestLoader().loadTestsFromTestCase(TestManager)
     unittest.TextTestRunner(verbosity=2).run(suite)