Harden RPC reply parsing for 2.2.1.1

build.gradle

@@ -6,7 +6,7 @@ plugins {
 }
 
 group = 'net.juniper.netconf'
-version = '2.2.1.0'
+version = '2.2.1.1'
 description = 'An API For NetConf client'
 
 java {

pom.xml

@@ -7,7 +7,7 @@
 
     <groupId>net.juniper.netconf</groupId>
     <artifactId>netconf-java</artifactId>
-    <version>2.2.1.0</version>
+    <version>2.2.1.1</version>
     <packaging>jar</packaging>
 
     <properties>

src/main/java/net/juniper/netconf/element/AbstractNetconfElement.java

@@ -14,6 +14,7 @@
 import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.stream.StreamResult;
 import java.io.StringWriter;
+import java.util.Objects;
 
 import static java.lang.String.format;
 
@@ -46,8 +47,9 @@ public abstract class AbstractNetconfElement {
      * @throws NullPointerException if {@code document} is {@code null}
      */
     protected AbstractNetconfElement(final Document document) {
-        this.document = document;
-        this.xml = createXml(document);
+        final Document documentCopy = Objects.requireNonNull(copyDocument(document), "document");
+        this.document = documentCopy;
+        this.xml = createXml(documentCopy);
     }
 
     /**
@@ -57,7 +59,18 @@ protected AbstractNetconfElement(final Document document) {
      * @return a cloned {@link Document} representing this element
      */
     public Document getDocument() {
-        return (Document) document.cloneNode(true); // deep copy
+        return copyDocument(document);
+    }
+
+    /**
+     * Returns a defensive deep copy of the supplied DOM {@link Document}.
+     *
+     * @param document source document; may be {@code null}
+     * @return deep copy of {@code document}, or {@code null} when the input is
+     *         {@code null}
+     */
+    protected static Document copyDocument(final Document document) {
+        return document == null ? null : (Document) document.cloneNode(true);
     }
 
     /**

src/main/java/net/juniper/netconf/element/RpcReply.java

@@ -123,11 +123,12 @@ private Builder() { }
         /**
          * Sets the original {@link Document} this reply was parsed from.
          *
-         * @param originalDocument the source DOM {@link Document}; may be {@code null}
+         * @param originalDocument the source DOM {@link Document}; may be {@code null}.
+         *                         A defensive copy is taken immediately.
          * @return this {@code Builder} instance for method chaining
          */
         public Builder originalDocument(Document originalDocument) {
-            this.originalDocument = originalDocument;
+            this.originalDocument = copyDocument(originalDocument);
             return this;
         }
 
@@ -318,6 +319,27 @@ private static String stripEomDelimiter(String xml) {
         return xml.replaceFirst("\\Q]]>]]>\\E\\s*$", "");
     }
 
+    /**
+     * Parses a NETCONF rpc-reply document after applying the common wire-format
+     * hygiene checks shared by all rpc-reply variants.
+     *
+     * @param xml raw NETCONF XML, optionally terminated with the RFC 6242
+     *            end-of-message delimiter
+     * @return parsed DOM document
+     * @throws ParserConfigurationException if a parser cannot be configured
+     * @throws IOException if the input cannot be read
+     * @throws SAXException if the XML is not well-formed
+     */
+    protected static Document parseRpcReplyDocument(final String xml)
+        throws ParserConfigurationException, IOException, SAXException {
+        final String cleaned = stripEomDelimiter(xml);
+        if (cleaned.contains("<!DOCTYPE")) {
+            throw new IllegalArgumentException("DOCTYPE declarations are not allowed in NETCONF messages (RFC 6241 §3.2)");
+        }
+        return createDocumentBuilderFactory().newDocumentBuilder()
+            .parse(new InputSource(new StringReader(cleaned)));
+    }
+
     /**
      * Parses the given NETCONF XML string into an {@code RpcReply} (or subtype).
      *
@@ -333,17 +355,12 @@ private static String stripEomDelimiter(String xml) {
     public static <T extends AbstractNetconfElement> T from(final String xml)
         throws ParserConfigurationException, IOException, SAXException, XPathExpressionException {
 
-        String cleaned = stripEomDelimiter(xml);
-        if (cleaned.contains("<!DOCTYPE")) {
-            throw new IllegalArgumentException("DOCTYPE declarations are not allowed in NETCONF messages (RFC 6241 §3.2)");
-        }
-        final Document document = createDocumentBuilderFactory().newDocumentBuilder()
-            .parse(new InputSource(new StringReader(cleaned)));
+        final Document document = parseRpcReplyDocument(xml);
         final XPath xPath = XPathFactory.newInstance().newXPath();
 
         final Element loadConfigResultsElement = (Element) xPath.evaluate(RpcReplyLoadConfigResults.XPATH_RPC_REPLY_LOAD_CONFIG_RESULT, document, XPathConstants.NODE);
         if (loadConfigResultsElement != null) {
-            return (T) RpcReplyLoadConfigResults.from(xml);
+            return (T) RpcReplyLoadConfigResults.fromDocument(document, xPath);
         }
 
         final Element rpcReplyElement = (Element) xPath.evaluate(XPATH_RPC_REPLY, document, XPathConstants.NODE);

src/main/java/net/juniper/netconf/element/RpcReplyLoadConfigResults.java

@@ -3,7 +3,6 @@
 import net.juniper.netconf.NetconfConstants;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
-import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
 
 import javax.xml.parsers.ParserConfigurationException;
@@ -12,7 +11,6 @@
 import javax.xml.xpath.XPathExpressionException;
 import javax.xml.xpath.XPathFactory;
 import java.io.IOException;
-import java.io.StringReader;
 import java.util.List;
 import java.util.Objects;
 
@@ -41,26 +39,33 @@ public class RpcReplyLoadConfigResults extends RpcReply {
      * @throws SAXException                 if the XML is not well‑formed
      * @throws XPathExpressionException     if required nodes cannot be located
      */
+    @SuppressWarnings("unchecked")
     public static RpcReplyLoadConfigResults from(final String xml)
         throws ParserConfigurationException, IOException, SAXException, XPathExpressionException {
-
-        final Document document = createDocumentBuilderFactory().newDocumentBuilder()
-            .parse(new InputSource(new StringReader(xml)));
+        final Document document = parseRpcReplyDocument(xml);
         final XPath xPath = XPathFactory.newInstance().newXPath();
+        return fromDocument(document, xPath);
+    }
 
-        final Element rpcReplyElement = (Element) xPath.evaluate(XPATH_RPC_REPLY, document, XPathConstants.NODE);
-        final Element loadConfigResultsElement = (Element) xPath.evaluate(RpcReplyLoadConfigResults.XPATH_RPC_REPLY_LOAD_CONFIG_RESULT, document, XPathConstants.NODE);
+    static RpcReplyLoadConfigResults fromDocument(final Document document, final XPath xPath)
+        throws XPathExpressionException {
+        final Element rpcReplyElement = requireElement(
+            (Element) xPath.evaluate(XPATH_RPC_REPLY, document, XPathConstants.NODE),
+            "Missing required <rpc-reply> element");
+        final Element loadConfigResultsElement = requireElement(
+            (Element) xPath.evaluate(RpcReplyLoadConfigResults.XPATH_RPC_REPLY_LOAD_CONFIG_RESULT, document, XPathConstants.NODE),
+            "Missing required <load-configuration-results> element");
         final Element rpcReplyOkElement = (Element) xPath.evaluate(XPATH_RPC_REPLY_LOAD_CONFIG_RESULT_OK, document, XPathConstants.NODE);
         final List<RpcError> errorList = getRpcErrors(document, xPath, XPATH_RPC_REPLY_LOAD_CONFIG_RESULT_ERROR);
 
-        return RpcReplyLoadConfigResults.loadConfigResultsBuilder()
-            .originalDocument(document)
-            .namespacePrefix(null)
-            .messageId(getAttribute(rpcReplyElement, "message-id"))
-            .action(getAttribute(loadConfigResultsElement, "action"))
-            .ok(rpcReplyOkElement != null)
-            .errors(errorList)
-            .build();
+        return new RpcReplyLoadConfigResults(
+            null,
+            document,
+            requireAttribute(rpcReplyElement, "message-id", "rpc-reply"),
+            requireAttribute(loadConfigResultsElement, "action", "load-configuration-results"),
+            rpcReplyOkElement != null,
+            errorList
+        );
     }
 
     private RpcReplyLoadConfigResults(
@@ -121,12 +126,12 @@ public Builder() {
 
         /**
          * Sets the original XML Document for the reply.
-         * @param originalDocument the XML Document, must not be null
+         * @param originalDocument the XML Document, may be null. A defensive
+         *                         copy is taken immediately.
          * @return this Builder
-         * @throws NullPointerException if originalDocument is null
          */
         public Builder originalDocument(Document originalDocument) {
-            this.originalDocument = Objects.requireNonNull(originalDocument, "originalDocument must not be null");
+            this.originalDocument = copyDocument(originalDocument);
             return this;
         }
 
@@ -174,12 +179,11 @@ public Builder ok(boolean ok) {
 
         /**
          * Sets the list of errors for the reply.
-         * @param errors the list of errors, must not be null
+         * @param errors the list of errors, or null to clear them
          * @return this Builder
-         * @throws NullPointerException if errors is null
          */
         public Builder errors(List<RpcError> errors) {
-            this.errors = new java.util.ArrayList<>(Objects.requireNonNull(errors, "errors list must not be null"));
+            this.errors = errors == null ? new java.util.ArrayList<>() : new java.util.ArrayList<>(errors);
             return this;
         }
 
@@ -233,7 +237,9 @@ private static Document createDocument(
         final List<RpcError> errors) {
         final Document createdDocument = createBlankDocument();
         final Element rpcReplyElement = createdDocument.createElementNS(NetconfConstants.URN_XML_NS_NETCONF_BASE_1_0, "rpc-reply");
-        rpcReplyElement.setPrefix(namespacePrefix);
+        if (namespacePrefix != null) {
+            rpcReplyElement.setPrefix(namespacePrefix);
+        }
         rpcReplyElement.setAttribute("message-id", messageId);
         createdDocument.appendChild(rpcReplyElement);
         final Element loadConfigResultsElement = createdDocument.createElement("load-configuration-results");
@@ -242,13 +248,33 @@ private static Document createDocument(
         appendErrors(namespacePrefix, errors, createdDocument, loadConfigResultsElement);
         if (ok) {
             final Element okElement = createdDocument.createElementNS(NetconfConstants.URN_XML_NS_NETCONF_BASE_1_0, "ok");
-            okElement.setPrefix(namespacePrefix);
+            if (namespacePrefix != null) {
+                okElement.setPrefix(namespacePrefix);
+            }
             loadConfigResultsElement.appendChild(okElement);
         }
 
         return createdDocument;
     }
 
+    private static Element requireElement(final Element element, final String message)
+        throws XPathExpressionException {
+        if (element == null) {
+            throw new XPathExpressionException(message);
+        }
+        return element;
+    }
+
+    private static String requireAttribute(final Element element, final String attributeName, final String elementName)
+        throws XPathExpressionException {
+        final String attributeValue = trim(getAttribute(element, attributeName));
+        if (attributeValue == null || attributeValue.isEmpty()) {
+            throw new XPathExpressionException(
+                String.format("Missing required @%s attribute on <%s>", attributeName, elementName));
+        }
+        return attributeValue;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
@@ -276,8 +302,8 @@ public String toString() {
      *
      * @return copy of the error list
      */
-    @SuppressWarnings("unchecked") // parent class returns raw List
+    @Override
     public List<RpcError> getErrors() {
-        return new java.util.ArrayList<>((List<RpcError>) super.getErrors());
+        return new java.util.ArrayList<>(super.getErrors());
     }
-}
+}

src/test/java/net/juniper/netconf/XMLTest.java

@@ -133,10 +133,12 @@ public void GIVEN_multiSegmentPath_WHEN_addPath_THEN_createNestedHierarchy() thr
         XMLBuilder builder = new XMLBuilder();
         XML xml = builder.createNewXML("parent");
 
-        xml.addPath("level1/level2/leaf");
+        XML nested = xml.addPath("level1/level2/leaf");
 
         assertThat(xml.toString())
             .containsIgnoringWhitespaces("<parent><level1><level2><leaf/></level2></level1></parent>");
+        assertThat(nested.toString())
+            .containsIgnoringWhitespaces("<leaf/>");
     }
 
     @Test