Repository: ncclient/ncclient

Issue:
get_schema fails to fetch yang schema for junos
RPC response for `get-schema('junos-common-types')`

https://gist.github.com/ganeshrn/02d20623cd17f6cfac61f3210066cde5

As the rpc response root node does not have namespace `"urn:ietf:params:xml:ns:yang:ietf-netconf-monitoring"` the [root.find()](https://github.com/ncclient/ncclient/blob/master/ncclient/operations/retrieve.py#L76) call results in empty node and stactrace while find trying get the text of the node.
Error:
```
'NoneType' object has no attribute 'text'
```



Discussion:

Patch:
diff --git a/ncclient/devices/default.py b/ncclient/devices/default.py
index e292227..2a701e0 100644
--- a/ncclient/devices/default.py
+++ b/ncclient/devices/default.py
@@ -222,6 +222,16 @@ class DefaultDeviceHandler(object):
     def handle_connection_exceptions(self, sshsession):
         return False
 
+    def handle_reply_parsing_error(self, root, reply):
+        """
+        Hook for working around bugs in replies from devices (the root emelent can be "fixed")
+
+        :param root: the rpc reply root element
+        :param reply: the RPCReply object that is parsing 'root'
+
+        :return:
+        """
+        pass
 
     def transform_reply(self):
         return False
diff --git a/ncclient/devices/junos.py b/ncclient/devices/junos.py
index 5ad7f75..01e7501 100644
--- a/ncclient/devices/junos.py
+++ b/ncclient/devices/junos.py
@@ -11,20 +11,26 @@ All device-specific handlers derive from the DefaultDeviceHandler, which impleme
 generic information needed for interaction with a Netconf server.
 
 """
-
+import logging
 import re
 
 from lxml import etree
+from lxml.etree import QName
+
+from ncclient.operations.retrieve import GetSchemaReply
 from .default import DefaultDeviceHandler
 from ncclient.operations.third_party.juniper.rpc import GetConfiguration, LoadConfiguration, CompareConfiguration
 from ncclient.operations.third_party.juniper.rpc import ExecuteRpc, Command, Reboot, Halt, Commit, Rollback
 from ncclient.operations.rpc import RPCError
-from ncclient.xml_ import to_ele
+from ncclient.xml_ import to_ele, replace_namespace, BASE_NS_1_0, NETCONF_MONITORING_NS
 from ncclient.transport.third_party.junos.parser import JunosXMLParser
 from ncclient.transport.parser import DefaultXMLParser
 from ncclient.transport.parser import SAXParserHandler
 
 
+logger = logging.getLogger(__name__)
+
+
 class JunosDeviceHandler(DefaultDeviceHandler):
     """
     Juniper handler for device specific information.
@@ -33,6 +39,9 @@ class JunosDeviceHandler(DefaultDeviceHandler):
 
     def __init__(self, device_params):
         super(JunosDeviceHandler, self).__init__(device_params)
+        self.__reply_parsing_error_handler_by_cls = {
+            GetSchemaReply: fix_get_schema_reply
+        }
 
     def add_additional_operations(self):
         dict = {}
@@ -54,7 +63,7 @@ class JunosDeviceHandler(DefaultDeviceHandler):
         if 'routing-engine' in raw:
             raw = re.sub(r'<ok/>', '</routing-engine>\n<ok/>', raw)
             return raw
-        # check if error is during capabilites exchange itself
+        # check if error is during capabilities exchange itself
         elif re.search(r'<rpc-reply>.*?</rpc-reply>.*</hello>?', raw, re.M | re.S):
             errs = re.findall(
                 r'<rpc-error>.*?</rpc-error>', raw, re.M | re.S)
@@ -86,6 +95,14 @@ class JunosDeviceHandler(DefaultDeviceHandler):
         c.exec_command("xml-mode netconf need-trailer")
         return True
 
+    def handle_reply_parsing_error(self, root, reply):
+        reply_class = type(reply)
+
+        # Apply transform if found
+        transform_handler = self.__reply_parsing_error_handler_by_cls.get(reply_class)
+        if transform_handler is not None:
+            transform_handler(root)
+
     def transform_reply(self):
         reply = '''<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
         <xsl:output method="xml" indent="no"/>
@@ -126,3 +143,27 @@ class JunosDeviceHandler(DefaultDeviceHandler):
             return JunosXMLParser(session)
         else:
             return DefaultXMLParser(session)
+
+
+def fix_get_schema_reply(root):
+    # Workaround for wrong namespace of the data elem
+    # (issue with some Junos versions, might be corrected by Juniper at some point)
+
+    # get the data element, by local-name
+    data_elems = root.xpath('/nc:rpc-reply/*[local-name()="data"]', namespaces={'nc': BASE_NS_1_0})
+
+    if len(data_elems) != 1:
+        return  # Will not alter unexpected content
+
+    data_el = data_elems[0]
+    namespace = QName(data_el).namespace
+
+    if namespace == BASE_NS_1_0:
+        # With the default netconf setting, we may get "{BASE_NS_1_0}data"; warn and fix it
+        logger.warning("The device seems to run non-rfc compliant netconf. You may want to "
+                       "configure: 'set system services netconf rfc-compliant'")
+        replace_namespace(data_el, old_ns=BASE_NS_1_0, new_ns=NETCONF_MONITORING_NS)
+    elif namespace is None:
+        # With 'set system services netconf rfc-compliant' we may get "data" (no namespace); fix it
+        # There is no default xmlns and the data el is <data xmlns:ncm="NETCONF_MONITORING_NS">
+        replace_namespace(data_el, old_ns=None, new_ns=NETCONF_MONITORING_NS)
diff --git a/ncclient/operations/rpc.py b/ncclient/operations/rpc.py
index 375112f..2a6f32d 100644
--- a/ncclient/operations/rpc.py
+++ b/ncclient/operations/rpc.py
@@ -144,8 +144,9 @@ 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, huge_tree=False):
+    def __init__(self, raw, device_handler, huge_tree=False):
         self._raw = raw
+        self._device_handler = device_handler
         self._parsed = False
         self._root = None
         self._errors = []
@@ -167,7 +168,13 @@ class RPCReply(object):
                 for err in root.getiterator(error.tag):
                     # Process a particular <rpc-error>
                     self._errors.append(self.ERROR_CLS(err))
-        self._parsing_hook(root)
+        try:
+            self._parsing_hook(root)
+        except Exception as e:
+            # Apply device specific workaround and try again
+            self._device_handler.handle_reply_parsing_error(root, self)
+            self._parsing_hook(root)
+
         self._parsed = True
 
     def _parsing_hook(self, root):
@@ -380,7 +387,7 @@ class RPC(object):
 
     def deliver_reply(self, raw):
         # internal use
-        self._reply = self.REPLY_CLS(raw, huge_tree=self._huge_tree)
+        self._reply = self.REPLY_CLS(raw, self._device_handler, huge_tree=self._huge_tree)
         self._event.set()
 
     def deliver_error(self, err):
diff --git a/ncclient/xml_.py b/ncclient/xml_.py
index e47463d..8f58b35 100644
--- a/ncclient/xml_.py
+++ b/ncclient/xml_.py
@@ -262,6 +262,31 @@ def yang_action(name, attrs):
     node = new_ele('action', attrs={'xmlns': YANG_NS_1_0})
     return (node, sub_ele(node, name, attrs))
 
+
+def replace_namespace(root, old_ns, new_ns):
+    """
+    Substitute old_ns with new_ns for all the xml elements including and below root
+    :param root: top element (root for this change)
+    :param old_ns: old namespace
+    :param new_ns: new namespace
+    :return:
+    """
+    for elem in root.getiterator():
+        # Comments don't have a namespace
+        if elem.tag is not etree.Comment:
+            # handle tag
+            qtag = etree.QName(elem)
+            if qtag.namespace == old_ns:
+                elem.tag = etree.QName(new_ns, qtag.localname)
+
+            # handle attributes
+            attribs_dict = elem.attrib
+            for attr in attribs_dict.keys():
+                qattr = etree.QName(attr)
+                if qattr.namespace == old_ns:
+                    attribs_dict[etree.QName(new_ns, qattr.localname)] = attribs_dict.pop(attr)
+
+
 new_ele_nsmap = lambda tag, nsmap, attrs={}, **extra: etree.Element(qualify(tag), attrs, nsmap, **extra)
 
 new_ele = lambda tag, attrs={}, **extra: etree.Element(qualify(tag), attrs, **extra)


Test patch:
diff --git a/test/unit/operations/test_rpc.py b/test/unit/operations/test_rpc.py
index 32ffe69..a561dd8 100644
--- a/test/unit/operations/test_rpc.py
+++ b/test/unit/operations/test_rpc.py
@@ -118,7 +118,7 @@ xml7 = """<rpc-error xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
 class TestRPC(unittest.TestCase):
 
     def test_rpc_reply(self):
-        obj = RPCReply(xml4)
+        obj = RPCReply(xml4, self._mock_device_handler())
         obj.parse()
         self.assertTrue(obj.ok)
         self.assertFalse(obj.error)
@@ -126,11 +126,11 @@ class TestRPC(unittest.TestCase):
         self.assertTrue(obj._parsed)
 
     def test_rpc_reply_huge_text_node_exception(self):
-        obj = RPCReply(xml5_huge)
+        obj = RPCReply(xml5_huge, self._mock_device_handler())
         self.assertRaises(etree.XMLSyntaxError, obj.parse)
 
     def test_rpc_reply_huge_text_node_workaround(self):
-        obj = RPCReply(xml5_huge, huge_tree=True)
+        obj = RPCReply(xml5_huge, self._mock_device_handler(), huge_tree=True)
         obj.parse()
         self.assertTrue(obj.ok)
         self.assertFalse(obj.error)
@@ -142,7 +142,7 @@ class TestRPC(unittest.TestCase):
     def test_rpc_send(self, mock_thread, mock_send):
         device_handler, session = self._mock_device_handler_and_session()
         obj = RPC(session, device_handler, raise_mode=RaiseMode.ALL, timeout=0)
-        reply = RPCReply(xml1)
+        reply = RPCReply(xml1, device_handler)
         obj._reply = reply
         node = new_ele("commit")
         sub_ele(node, "confirmed")
@@ -168,7 +168,7 @@ class TestRPC(unittest.TestCase):
     def test_generic_rpc_send(self, mock_thread, mock_send):
         device_handler, session = self._mock_device_handler_and_session()
         obj = GenericRPC(session, device_handler, raise_mode=RaiseMode.ALL, timeout=0)
-        reply = RPCReply(xml1)
+        reply = RPCReply(xml1, device_handler)
         obj._reply = reply
         rpc_command = 'edit-config'
         filters = ('subtree', '<top xmlns="urn:mod1"/>')
@@ -203,7 +203,7 @@ class TestRPC(unittest.TestCase):
             raise_mode=RaiseMode.ALL,
             timeout=0,
             async_mode=True)
-        reply = RPCReply(xml1)
+        reply = RPCReply(xml1, device_handler)
         obj._reply = reply
         node = new_ele("commit")
         result = obj._request(node)
@@ -214,7 +214,7 @@ class TestRPC(unittest.TestCase):
     def test_rpc_timeout_error(self, mock_thread, mock_send):
         device_handler, session = self._mock_device_handler_and_session()
         obj = RPC(session, device_handler, raise_mode=RaiseMode.ALL, timeout=0)
-        reply = RPCReply(xml1)
+        reply = RPCReply(xml1, device_handler)
         obj.deliver_reply(reply)
         node = new_ele("commit")
         sub_ele(node, "confirmed")
@@ -226,7 +226,7 @@ class TestRPC(unittest.TestCase):
     def test_rpc_rpcerror(self, mock_thread, mock_send):
         device_handler, session = self._mock_device_handler_and_session()
         obj = RPC(session, device_handler, raise_mode=RaiseMode.ALL, timeout=0)
-        reply = RPCReply(xml1)
+        reply = RPCReply(xml1, device_handler)
         obj._reply = reply
         node = new_ele("commit")
         sub_ele(node, "confirmed")
@@ -312,6 +312,9 @@ class TestRPC(unittest.TestCase):
         obj.huge_tree = False
         self.assertFalse(obj.huge_tree)
 
+    def _mock_device_handler(self):
+        return manager.make_device_handler({'name': 'default'})
+
     def _mock_device_handler_and_session(self):
         device_handler = manager.make_device_handler({'name': 'junos'})
         capabilities = Capabilities(device_handler.get_capabilities())