Do not attempt to parse empty RPC/action reply

An RPC or action can legally result in an empty reply,
even if it has some optional content in its schema.

Detect </ok> reply and do not attempt to parse it as
YANG data.

JIRA: NETCONF-568
JIRA: NETCONF-644
Change-Id: I1f0b5c349f418824b9b0b3e1f01f16824b1b8df4
Signed-off-by: Anna Bencurova <Anna.Bencurova@pantheon.tech>
Signed-off-by: Sanjana B <sanjana.b@verizon.com>
Signed-off-by: Tibor Král <tibor.kral@pantheon.tech>
(cherry picked from commit df25a540e0985695d3d0f93c68a977a4de729329)

diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformer.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformer.java
index 0406b95db74b3c8bd45110fd5bc728e107a09a59..5974875ef7551708cc9a13f7ebad038e0f813b11 100644 (file)
--- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformer.java
+++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformer.java
@@ -29,6 +29,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.transform.dom.DOMResult;
 import javax.xml.transform.dom.DOMSource;
@@ -311,13 +312,19 @@ public class NetconfMessageTransformer implements MessageTransformer<NetconfMess
 
     private NormalizedNode<?, ?> parseResult(final NetconfMessage message,
             final OperationDefinition operationDefinition) {
+        final Optional<XmlElement> okResponseElement = XmlElement.fromDomDocument(message.getDocument())
+                .getOnlyChildElementWithSameNamespaceOptionally("ok");
         if (operationDefinition.getOutput().getChildNodes().isEmpty()) {
-            Preconditions.checkArgument(XmlElement.fromDomDocument(
-                message.getDocument()).getOnlyChildElementWithSameNamespaceOptionally("ok").isPresent(),
+            Preconditions.checkArgument(okResponseElement.isPresent(),
                 "Unexpected content in response of rpc: %s, %s", operationDefinition.getQName(), message);
             return null;
         } else {
-            final Element element = message.getDocument().getDocumentElement();
+            if (okResponseElement.isPresent()) {
+                LOG.debug("Received response <ok/> for RPC with defined Output");
+                return null;
+            }
+
+            Element element = message.getDocument().getDocumentElement();
             try {
                 final NormalizedNodeResult resultHolder = new NormalizedNodeResult();
                 final NormalizedNodeStreamWriter writer = ImmutableNormalizedNodeStreamWriter.from(resultHolder);

diff --git a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformerTest.java b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformerTest.java
index 7850cc735b89f5f655240b757241428de4373945..bde0a48e505e5663e3e8b73b8bda6c3299eb6c19 100644 (file)
--- a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformerTest.java
+++ b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformerTest.java
@@ -8,6 +8,7 @@
 package org.opendaylight.netconf.sal.connect.netconf.schema.mapping;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
@@ -102,6 +103,8 @@ public class NetconfMessageTransformerTest {
 
     private static final String URN_EXAMPLE_AUGMENTED_ACTION = "urn:example:augmented-action";
 
+    private static final String URN_EXAMPLE_RPCS_ACTIONS_OUTPUTS = "urn:example:rpcs-actions-outputs";
+
     private static final QName SERVER_QNAME =
             QName.create(URN_EXAMPLE_SERVER_FARM, REVISION_EXAMPLE_SERVER_FARM, "server");
     private static final QName RESET_QNAME = QName.create(SERVER_QNAME, "reset");
@@ -126,6 +129,19 @@ public class NetconfMessageTransformerTest {
     private static final SchemaPath DISABLE_INTERFACE_PATH =
             SchemaPath.create(true, DEVICE_QNAME, INTERFACE_QNAME, DISABLE_QNAME);
 
+    private static final QName CHECK_WITH_OUTPUT_QNAME =
+            QName.create(URN_EXAMPLE_RPCS_ACTIONS_OUTPUTS, "check-with-output");
+    private static final SchemaPath CHECK_WITH_OUTPUT_INTERFACE_PATH =
+            SchemaPath.create(true, DEVICE_QNAME, INTERFACE_QNAME, CHECK_WITH_OUTPUT_QNAME);
+    private static final QName CHECK_WITHOUT_OUTPUT_QNAME =
+            QName.create(URN_EXAMPLE_RPCS_ACTIONS_OUTPUTS, "check-without-output");
+    private static final SchemaPath CHECK_WITHOUT_OUTPUT_INTERFACE_PATH =
+            SchemaPath.create(true, DEVICE_QNAME, INTERFACE_QNAME, CHECK_WITHOUT_OUTPUT_QNAME);
+    private static final QName RPC_WITH_OUTPUT_QNAME =
+            QName.create(URN_EXAMPLE_RPCS_ACTIONS_OUTPUTS, "rpc-with-output");
+    private static final QName RPC_WITHOUT_OUTPUT_QNAME =
+            QName.create(URN_EXAMPLE_RPCS_ACTIONS_OUTPUTS, "rpc-without-output");
+
     private static final QName BOX_OUT_QNAME =
             QName.create(URN_EXAMPLE_SERVER_FARM, REVISION_EXAMPLE_SERVER_FARM, "box-out");
     private static final QName BOX_IN_QNAME = QName.create(BOX_OUT_QNAME, "box-in");
@@ -164,7 +180,8 @@ public class NetconfMessageTransformerTest {
 
         ACTION_SCHEMA = YangParserTestUtils.parseYangResources(NetconfMessageTransformerTest.class,
             "/schemas/example-server-farm.yang","/schemas/example-server-farm-2.yang",
-            "/schemas/conflicting-actions.yang", "/schemas/augmented-action.yang");
+            "/schemas/conflicting-actions.yang", "/schemas/augmented-action.yang",
+            "/schemas/rpcs-actions-outputs.yang");
     }
 
     @AfterClass
@@ -216,6 +233,26 @@ public class NetconfMessageTransformerTest {
         transformer.toRpcResult(new NetconfMessage(XmlUtil.readXmlToDocument(result)), toPath(NETCONF_LOCK_QNAME));
     }
 
+    @Test
+    public void testRpcEmptyBodyWithOutputDefinedSchemaResult() throws Exception {
+        final String result = "<rpc-reply xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><ok/></rpc-reply>";
+
+        DOMRpcResult domRpcResult = actionNetconfMessageTransformer
+                .toRpcResult(new NetconfMessage(XmlUtil.readXmlToDocument(result)),
+                        toPath(RPC_WITH_OUTPUT_QNAME));
+        assertNotNull(domRpcResult);
+    }
+
+    @Test
+    public void testRpcEmptyBodyWithoutOutputDefinedSchemaResult() throws Exception {
+        final String result = "<rpc-reply xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><ok/></rpc-reply>";
+
+        DOMRpcResult domRpcResult = actionNetconfMessageTransformer
+                .toRpcResult(new NetconfMessage(XmlUtil.readXmlToDocument(result)),
+                        toPath(RPC_WITHOUT_OUTPUT_QNAME));
+        assertNotNull(domRpcResult);
+    }
+
     @Test
     public void testDiscardChangesRequest() throws Exception {
         final NetconfMessage netconfMessage =
@@ -474,6 +511,8 @@ public class NetconfMessageTransformerTest {
         schemaPaths.add(XYZZY_BAR_PATH);
         schemaPaths.add(CHOICE_ACTION_PATH);
         schemaPaths.add(DISABLE_INTERFACE_PATH);
+        schemaPaths.add(CHECK_WITH_OUTPUT_INTERFACE_PATH);
+        schemaPaths.add(CHECK_WITHOUT_OUTPUT_INTERFACE_PATH);
 
         List<ActionDefinition> actions = NetconfMessageTransformer.getActions(ACTION_SCHEMA);
         assertEquals(schemaPaths.size(), actions.size());
@@ -765,6 +804,30 @@ public class NetconfMessageTransformerTest {
         assertEquals("now", leaf.getValue());
     }
 
+    @Test
+    public void toActionEmptyBodyWithOutputDefinedResultTest() throws Exception {
+        NetconfMessage message = new NetconfMessage(XmlUtil.readXmlToDocument(
+                "<rpc-reply message-id=\"101\" xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\">"
+                + "<ok/>"
+                + "</rpc-reply>"));
+        DOMActionResult actionResult =
+                actionNetconfMessageTransformer.toActionResult(CHECK_WITH_OUTPUT_INTERFACE_PATH, message);
+        assertNotNull(actionResult);
+        assertFalse(actionResult.getOutput().isPresent());
+    }
+
+    @Test
+    public void toActionEmptyBodyWithoutOutputDefinedResultTest() throws Exception {
+        NetconfMessage message = new NetconfMessage(XmlUtil.readXmlToDocument(
+                "<rpc-reply message-id=\"101\" xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\">"
+                + "<ok/>"
+                + "</rpc-reply>"));
+        DOMActionResult actionResult =
+                actionNetconfMessageTransformer.toActionResult(CHECK_WITHOUT_OUTPUT_INTERFACE_PATH, message);
+        assertNotNull(actionResult);
+        assertFalse(actionResult.getOutput().isPresent());
+    }
+
     private static void checkAction(final QName actionQname, final Node action , final String inputLocalName,
             final String inputNodeName, final String inputValue) {
         checkNode(action, actionQname.getLocalName(), actionQname.getLocalName(),

diff --git a/netconf/sal-netconf-connector/src/test/resources/schemas/rpcs-actions-outputs.yang b/netconf/sal-netconf-connector/src/test/resources/schemas/rpcs-actions-outputs.yang
new file mode 100644 (file)
index 0000000..3b61d86
--- /dev/null
+++ b/netconf/sal-netconf-connector/src/test/resources/schemas/rpcs-actions-outputs.yang
@@ -0,0 +1,30 @@
+module rpcs-actions-outputs {
+  yang-version 1.1;
+  namespace "urn:example:rpcs-actions-outputs";
+  prefix "rao";
+
+  import example-server-farm { prefix sfarm; revision-date 2018-08-07; }
+
+  augment "/sfarm:device/sfarm:interface" {
+    action check-with-output {
+      output {
+        leaf not-mandatory-message {
+          type string;
+        }
+      }
+    }
+    action check-without-output {
+    }
+  }
+
+  rpc rpc-with-output {
+    output {
+      leaf not-mandatory-message {
+        type string;
+      }
+    }
+  }
+
+  rpc rpc-without-output {
+  }
+}
\ No newline at end of file