diff --git a/build.gradle b/build.gradle index f33ed71..c5a8d07 100644 --- a/build.gradle +++ b/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 { diff --git a/pom.xml b/pom.xml index 9ae8483..5fc4491 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ net.juniper.netconf netconf-java - 2.2.1.0 + 2.2.1.1 jar diff --git a/src/main/java/net/juniper/netconf/element/AbstractNetconfElement.java b/src/main/java/net/juniper/netconf/element/AbstractNetconfElement.java index b41e587..a15eb23 100644 --- a/src/main/java/net/juniper/netconf/element/AbstractNetconfElement.java +++ b/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); } /** diff --git a/src/main/java/net/juniper/netconf/element/RpcReply.java b/src/main/java/net/juniper/netconf/element/RpcReply.java index 287a8dd..c0923be 100644 --- a/src/main/java/net/juniper/netconf/element/RpcReply.java +++ b/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(" T from(final String xml) throws ParserConfigurationException, IOException, SAXException, XPathExpressionException { - String cleaned = stripEomDelimiter(xml); - if (cleaned.contains(" element"); + final Element loadConfigResultsElement = requireElement( + (Element) xPath.evaluate(RpcReplyLoadConfigResults.XPATH_RPC_REPLY_LOAD_CONFIG_RESULT, document, XPathConstants.NODE), + "Missing required element"); final Element rpcReplyOkElement = (Element) xPath.evaluate(XPATH_RPC_REPLY_LOAD_CONFIG_RESULT_OK, document, XPathConstants.NODE); final List 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 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 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 getErrors() { - return new java.util.ArrayList<>((List) super.getErrors()); + return new java.util.ArrayList<>(super.getErrors()); } -} \ No newline at end of file +} diff --git a/src/test/java/net/juniper/netconf/XMLTest.java b/src/test/java/net/juniper/netconf/XMLTest.java index 3d61bcb..cba8d85 100644 --- a/src/test/java/net/juniper/netconf/XMLTest.java +++ b/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(""); + assertThat(nested.toString()) + .containsIgnoringWhitespaces(""); } @Test diff --git a/src/test/java/net/juniper/netconf/element/RpcReplyLoadConfigResultsTest.java b/src/test/java/net/juniper/netconf/element/RpcReplyLoadConfigResultsTest.java index 2a49778..d9dd0ff 100644 --- a/src/test/java/net/juniper/netconf/element/RpcReplyLoadConfigResultsTest.java +++ b/src/test/java/net/juniper/netconf/element/RpcReplyLoadConfigResultsTest.java @@ -1,9 +1,11 @@ package net.juniper.netconf.element; import org.junit.jupiter.api.Test; +import org.w3c.dom.Document; import org.xmlunit.assertj.XmlAssert; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class RpcReplyLoadConfigResultsTest { @@ -27,6 +29,9 @@ public class RpcReplyLoadConfigResultsTest { """; + private static final String LOAD_CONFIG_RESULTS_OK_NO_NAMESPACE_FRAMED = + LOAD_CONFIG_RESULTS_OK_NO_NAMESPACE + "]]>]]>"; + private static final String LOAD_CONFIG_RESULTS_ERROR_NO_NAMESPACE = "" + " + ]> + + + + + + """; + + assertThatThrownBy(() -> RpcReplyLoadConfigResults.from(withDtd)) + .isInstanceOf(Exception.class) + .hasMessageContaining("DOCTYPE"); + } + + @Test + public void willThrowWhenMessageIdIsMissingFromLoadConfigurationResultsReply() { + String withoutMessageId = """ + + + + + + """; + + assertThatThrownBy(() -> RpcReplyLoadConfigResults.from(withoutMessageId)) + .isInstanceOf(javax.xml.xpath.XPathExpressionException.class) + .hasMessageContaining("message-id"); + } + + @Test + public void willThrowWhenActionIsMissingFromLoadConfigurationResultsReply() { + String withoutAction = """ + + + + + + """; + + assertThatThrownBy(() -> RpcReplyLoadConfigResults.from(withoutAction)) + .isInstanceOf(javax.xml.xpath.XPathExpressionException.class) + .hasMessageContaining("action"); + } + @Test public void willCreateXmlOkWithoutNamespace() { @@ -247,4 +315,56 @@ public void willCreateXmlErrorWithNamespace() { .areIdentical(); } -} \ No newline at end of file + @Test + public void builderWillTreatNullErrorsAsEmptyList() { + + final RpcReplyLoadConfigResults rpcReply = RpcReplyLoadConfigResults.loadConfigResultsBuilder() + .messageId("7") + .action("set") + .errors(null) + .build(); + + assertThat(rpcReply.getErrors()) + .isEmpty(); + assertThat(rpcReply.hasErrorsOrWarnings()) + .isFalse(); + } + + @Test + public void builderWillAllowExplicitNullOriginalDocument() { + + final RpcReplyLoadConfigResults rpcReply = RpcReplyLoadConfigResults.loadConfigResultsBuilder() + .originalDocument(null) + .messageId("8") + .action("set") + .ok(true) + .build(); + + assertThat(rpcReply.getMessageId()) + .isEqualTo("8"); + assertThat(rpcReply.getAction()) + .isEqualTo("set"); + assertThat(rpcReply.isOK()) + .isTrue(); + } + + @Test + public void builderWillDefensivelyCopyOriginalDocument() throws Exception { + + final Document originalDocument = RpcReply.parseRpcReplyDocument(LOAD_CONFIG_RESULTS_OK_NO_NAMESPACE); + + final RpcReplyLoadConfigResults.Builder builder = RpcReplyLoadConfigResults.loadConfigResultsBuilder() + .originalDocument(originalDocument) + .messageId("3") + .action("set") + .ok(true); + + originalDocument.getDocumentElement().setAttribute("message-id", "999"); + + final RpcReplyLoadConfigResults rpcReply = builder.build(); + + assertThat(rpcReply.getDocument().getDocumentElement().getAttribute("message-id")) + .isEqualTo("3"); + } + +} diff --git a/src/test/java/net/juniper/netconf/element/RpcReplyTest.java b/src/test/java/net/juniper/netconf/element/RpcReplyTest.java index b1bea4e..a2fae54 100644 --- a/src/test/java/net/juniper/netconf/element/RpcReplyTest.java +++ b/src/test/java/net/juniper/netconf/element/RpcReplyTest.java @@ -1,6 +1,7 @@ package net.juniper.netconf.element; import org.junit.jupiter.api.Test; +import org.w3c.dom.Document; import org.xmlunit.assertj.XmlAssert; import java.util.Arrays; @@ -237,6 +238,23 @@ public void willCreateXmlWithErrors() { .areIdentical(); } + @Test + public void builderWillDefensivelyCopyOriginalDocument() throws Exception { + final Document originalDocument = RpcReply.parseRpcReplyDocument(RPC_REPLY_WITH_OK); + + final RpcReply.Builder builder = RpcReply.builder() + .originalDocument(originalDocument) + .messageId("5") + .ok(true); + + originalDocument.getDocumentElement().setAttribute("message-id", "999"); + + final RpcReply rpcReply = builder.build(); + + assertThat(rpcReply.getDocument().getDocumentElement().getAttribute("message-id")) + .isEqualTo("5"); + } + /** * RFC 6241 §4.3: a peer SHOULD respond with * if the incoming message is not well‑formed XML. In our client-side parser we