From e18745424d1a2c7bf53ea54b22fc02b21dcf67d4 Mon Sep 17 00:00:00 2001 From: Peter Hill Date: Fri, 24 Apr 2026 11:01:26 -0700 Subject: [PATCH 1/8] Harden NETCONF XML parsing against XXE and DTDs Configure secure XML parser settings for device sessions and parsed NETCONF elements to disable external entities, DTD loading, and external schema access. Also reject DOCTYPE declarations in rpc-reply parsing and cover the behavior with a focused reply parsing test. --- src/main/java/net/juniper/netconf/Device.java | 23 ++++++++++++++++--- .../element/AbstractNetconfElement.java | 15 ++++++++++++ .../net/juniper/netconf/element/RpcReply.java | 5 +++- .../juniper/netconf/element/RpcReplyTest.java | 18 ++++++++++++++- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/juniper/netconf/Device.java b/src/main/java/net/juniper/netconf/Device.java index 2168a14..0eed639 100644 --- a/src/main/java/net/juniper/netconf/Device.java +++ b/src/main/java/net/juniper/netconf/Device.java @@ -19,7 +19,10 @@ import org.w3c.dom.Document; import org.xml.sax.SAXException; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; @@ -361,8 +364,8 @@ private Device(Builder b) throws NetconfException { this.netconfCapabilities = b.netconfCapabilities; try { - this.xmlBuilder = javax.xml.parsers.DocumentBuilderFactory.newInstance().newDocumentBuilder(); - } catch (javax.xml.parsers.ParserConfigurationException e) { + this.xmlBuilder = createSecureDocumentBuilder(); + } catch (ParserConfigurationException e) { throw new NetconfException("Cannot create XML Parser", e); } @@ -492,6 +495,20 @@ private void loadPrivateKey() throws NetconfException { } } + private static DocumentBuilder createSecureDocumentBuilder() throws ParserConfigurationException { + DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); + documentBuilderFactory.setNamespaceAware(true); + documentBuilderFactory.setExpandEntityReferences(false); + documentBuilderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); + documentBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + documentBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + documentBuilderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + documentBuilderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + return documentBuilderFactory.newDocumentBuilder(); + } + /** * Reboot the device. * @@ -1540,4 +1557,4 @@ public String toString() { ", helloRpc='" + helloRpc + '\'' + '}'; } -} \ No newline at end of file +} diff --git a/src/main/java/net/juniper/netconf/element/AbstractNetconfElement.java b/src/main/java/net/juniper/netconf/element/AbstractNetconfElement.java index 51f7084..b41e587 100644 --- a/src/main/java/net/juniper/netconf/element/AbstractNetconfElement.java +++ b/src/main/java/net/juniper/netconf/element/AbstractNetconfElement.java @@ -4,6 +4,7 @@ import org.w3c.dom.Document; import org.w3c.dom.Element; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.OutputKeys; @@ -96,6 +97,18 @@ protected static Document createBlankDocument() { protected static DocumentBuilderFactory createDocumentBuilderFactory() { final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); documentBuilderFactory.setNamespaceAware(true); + documentBuilderFactory.setExpandEntityReferences(false); + try { + documentBuilderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); + documentBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + documentBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + documentBuilderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + documentBuilderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + } catch (ParserConfigurationException e) { + throw new IllegalStateException("Unable to configure secure XML parser", e); + } return documentBuilderFactory; } @@ -112,6 +125,8 @@ protected static DocumentBuilderFactory createDocumentBuilderFactory() { protected static String createXml(final Document document) { try { final TransformerFactory transformerFactory = TransformerFactory.newInstance(); + transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); final Transformer transformer = transformerFactory.newTransformer(); transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes"); final StringWriter stringWriter = new StringWriter(); diff --git a/src/main/java/net/juniper/netconf/element/RpcReply.java b/src/main/java/net/juniper/netconf/element/RpcReply.java index bf15051..2a4d5c5 100644 --- a/src/main/java/net/juniper/netconf/element/RpcReply.java +++ b/src/main/java/net/juniper/netconf/element/RpcReply.java @@ -333,6 +333,9 @@ public static T from(final String xml) throws ParserConfigurationException, IOException, SAXException, XPathExpressionException { String cleaned = stripEomDelimiter(xml); + if (cleaned.contains(" + ]> + + + + """; + + assertThatThrownBy(() -> RpcReply.from(withDtd)) + .isInstanceOf(Exception.class) + .hasMessageContaining("DOCTYPE"); + } + /** * RFC 6241 requires UTF‑8 encoding. Passing bytes in ISO‑8859‑1 that contain * invalid UTF‑8 sequences should also raise a parse failure. @@ -233,4 +249,4 @@ public void willParseRpcReplyWithDelimiter() throws Exception { assertThat(rpcReply.isOK()).isTrue(); assertThat(rpcReply.hasErrorsOrWarnings()).isFalse(); } -} \ No newline at end of file +} From 43156f5253e5624ef251cf1eb433fc50ec4f9a14 Mon Sep 17 00:00:00 2001 From: Peter Hill Date: Fri, 24 Apr 2026 11:02:58 -0700 Subject: [PATCH 2/8] Fix NETCONF session framing and reply correlation Switch session framing based on the peer hello, decode chunked NETCONF 1.1 replies, and return framed payloads from streaming RPC reads instead of raw transport markers. Also preserve caller-supplied rpc message-ids, generate missing message-ids for raw rpc envelopes, validate rpc-reply correlation, and ensure commitThisConfiguration always unlocks the candidate datastore on failure. --- .../net/juniper/netconf/NetconfConstants.java | 7 + .../net/juniper/netconf/NetconfSession.java | 540 ++++++++++++++++-- .../net/juniper/netconf/element/Hello.java | 2 +- .../juniper/netconf/NetconfSessionTest.java | 198 ++++++- .../net/juniper/netconf/TestConstants.java | 3 +- 5 files changed, 704 insertions(+), 46 deletions(-) diff --git a/src/main/java/net/juniper/netconf/NetconfConstants.java b/src/main/java/net/juniper/netconf/NetconfConstants.java index c664540..20c4ef6 100644 --- a/src/main/java/net/juniper/netconf/NetconfConstants.java +++ b/src/main/java/net/juniper/netconf/NetconfConstants.java @@ -47,6 +47,13 @@ public class NetconfConstants { */ public static final String URN_IETF_PARAMS_NETCONF_BASE_1_0 = "urn:ietf:params:netconf:base:1.0"; + /** + * URI form of the NETCONF Base 1.1 capability identifier. + * + * @see RFC 6242 + */ + public static final String URN_IETF_PARAMS_NETCONF_BASE_1_1 = "urn:ietf:params:netconf:base:1.1"; + /* ------------------------------------------------------------------ * Misc helpers * ------------------------------------------------------------------ */ diff --git a/src/main/java/net/juniper/netconf/NetconfSession.java b/src/main/java/net/juniper/netconf/NetconfSession.java index 5b25835..2580f4d 100644 --- a/src/main/java/net/juniper/netconf/NetconfSession.java +++ b/src/main/java/net/juniper/netconf/NetconfSession.java @@ -9,7 +9,6 @@ package net.juniper.netconf; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Charsets; import com.jcraft.jsch.Channel; import com.jcraft.jsch.JSchException; import net.juniper.netconf.element.Datastore; @@ -23,23 +22,27 @@ import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.ParserConfigurationException; import javax.xml.xpath.XPathExpressionException; +import java.io.ByteArrayOutputStream; import java.io.BufferedReader; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; -import java.io.Reader; import java.io.StringReader; import java.net.SocketTimeoutException; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * A {@code NetconfSession} is obtained by first building a @@ -88,6 +91,17 @@ public class NetconfSession { private static final String EMPTY_CONFIGURATION_TAG = ""; private static final String RUNNING_CONFIG = "running"; private static final String NETCONF_SYNTAX_ERROR_MSG_FROM_DEVICE = "netconf error: syntax error"; + private static final byte[] DEVICE_PROMPT_BYTES = NetconfConstants.DEVICE_PROMPT.getBytes(StandardCharsets.UTF_8); + private static final Pattern RPC_START_TAG_PATTERN = Pattern.compile("^]*)>", Pattern.DOTALL); + private static final Pattern MESSAGE_ID_ATTRIBUTE_PATTERN = + Pattern.compile("\\bmessage-id\\s*=\\s*(['\"])(.*?)\\1", Pattern.DOTALL); + private static final Pattern XML_DECLARATION_PATTERN = + Pattern.compile("^<\\?xml[^>]*\\?>\\s*", Pattern.DOTALL); + + private boolean useChunkedFraming; + private byte[] unreadBytes = new byte[0]; + + private record PreparedRpc(String xml, String messageId) { } NetconfSession(Channel netconfChannel, int timeout, String hello, DocumentBuilder builder) throws IOException { @@ -128,50 +142,111 @@ private void sendHello(String hello) throws IOException { @VisibleForTesting String getRpcReply(String rpc) throws IOException { - // write the rpc to the device - sendRpcRequest(rpc); + PreparedRpc preparedRpc = sendRpcRequest(rpc); + String reply; + if (startsWithChunkedFraming()) { + reply = readChunkedRpcReply(); + } else { + reply = readLegacyRpcReply(); + } + validateReplyMessageId(preparedRpc.messageId(), reply); + return reply; + } - final char[] buffer = new char[BUFFER_SIZE]; - final StringBuilder rpcReply = new StringBuilder(); + private String readLegacyRpcReply() throws IOException { + final byte[] buffer = new byte[BUFFER_SIZE]; + final ByteArrayOutputStream rpcReply = new ByteArrayOutputStream(); final long startTime = System.nanoTime(); - final Reader in = new InputStreamReader(stdInStreamFromDevice, Charsets.UTF_8); boolean timeoutNotExceeded = true; - int promptPosition; - while ((promptPosition = rpcReply.indexOf(NetconfConstants.DEVICE_PROMPT)) < 0 && - (timeoutNotExceeded = (TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTime) < commandTimeout))) { - int charsRead = in.read(buffer, 0, buffer.length); - if (charsRead < 0) throw new NetconfException("Input Stream has been closed during reading."); - rpcReply.append(buffer, 0, charsRead); + int promptPosition = -1; + while (promptPosition < 0 && + (timeoutNotExceeded = !commandTimedOut(startTime))) { + int bytesRead = readIncomingBytes(buffer); + if (bytesRead < 0) { + throw new NetconfException("Input Stream has been closed during reading."); + } + rpcReply.write(buffer, 0, bytesRead); + byte[] rpcReplyBytes = rpcReply.toByteArray(); + promptPosition = indexOf(rpcReplyBytes, DEVICE_PROMPT_BYTES); + if (promptPosition >= 0) { + int unreadStart = promptPosition + DEVICE_PROMPT_BYTES.length; + if (unreadStart < rpcReplyBytes.length) { + pushBackBytes(Arrays.copyOfRange(rpcReplyBytes, unreadStart, rpcReplyBytes.length)); + } + String reply = new String(rpcReplyBytes, 0, promptPosition, StandardCharsets.UTF_8); + log.debug("Received Netconf RPC-Reply\n{}", reply); + return reply; + } } - if (!timeoutNotExceeded) - throw new SocketTimeoutException("Command timeout limit was exceeded: " + commandTimeout); - // fixing the rpc reply by removing device prompt - log.debug("Received Netconf RPC-Reply\n{}", rpcReply); - rpcReply.setLength(promptPosition); + throw new SocketTimeoutException("Command timeout limit was exceeded: " + commandTimeout); + } - return rpcReply.toString(); + private String readChunkedRpcReply() throws IOException { + final long startTime = System.nanoTime(); + final ByteArrayOutputStream rpcReply = new ByteArrayOutputStream(); + while (!commandTimedOut(startTime)) { + String header = readChunkHeaderLine(startTime); + if (header.isEmpty()) { + continue; + } + if ("##".equals(header)) { + String reply = rpcReply.toString(StandardCharsets.UTF_8); + log.debug("Received Netconf RPC-Reply\n{}", reply); + return reply; + } + if (!header.startsWith("#")) { + throw new NetconfException("Invalid NETCONF chunked framing header: " + header); + } + int chunkSize; + try { + chunkSize = Integer.parseInt(header.substring(1)); + } catch (NumberFormatException e) { + throw new NetconfException("Invalid NETCONF chunk size: " + header, e); + } + if (chunkSize < 0) { + throw new NetconfException("Invalid NETCONF chunk size: " + header); + } + rpcReply.write(readExactBytes(chunkSize, startTime)); + } + throw new SocketTimeoutException("Command timeout limit was exceeded: " + commandTimeout); } private BufferedReader getRpcReplyRunning(String rpc) throws IOException { sendRpcRequest(rpc); return new BufferedReader( - new InputStreamReader(stdInStreamFromDevice, Charsets.UTF_8)); + new InputStreamReader(createRpcReplyInputStream(), StandardCharsets.UTF_8)); } - private void sendRpcRequest(String rpc) throws IOException { - // RFC conformance for XML type, namespaces and message ids for RPCs - messageId++; - rpc = rpc.replace("", "").trim(); - rpc = rpc.replace("", ""); - rpc = rpc.replace("", ""); - if (!rpc.contains(NetconfConstants.XML_VERSION)) { - rpc = NetconfConstants.XML_VERSION + rpc; - } - // writing the rpc to the device - log.debug("Sending Netconf RPC\n{}", rpc); - stdOutStreamToDevice.write(rpc.getBytes(Charsets.UTF_8)); + private InputStream createRpcReplyInputStream() { + return useChunkedFraming ? new ChunkedRpcReplyInputStream() : new LegacyRpcReplyInputStream(); + } + + private PreparedRpc sendRpcRequest(String rpc) throws IOException { + PreparedRpc preparedRpc = prepareRpcRequest(rpc); + log.debug("Sending Netconf RPC\n{}", preparedRpc.xml()); + stdOutStreamToDevice.write(applyTransportFraming(preparedRpc.xml())); stdOutStreamToDevice.flush(); + return preparedRpc; + } + + private PreparedRpc prepareRpcRequest(String rpc) { + String normalizedRpc = stripLegacyEndOfMessage(rpc).trim(); + String expectedMessageId = null; + if (isHelloMessage(normalizedRpc)) { + return new PreparedRpc(addXmlDeclarationIfMissing(normalizedRpc), null); + } + if (isRpcEnvelope(normalizedRpc)) { + messageId++; + expectedMessageId = extractRpcMessageId(normalizedRpc); + if (expectedMessageId == null) { + expectedMessageId = String.valueOf(messageId); + normalizedRpc = injectMessageId(normalizedRpc, expectedMessageId); + } + } + normalizedRpc = normalizedRpc.replace("", ""); + normalizedRpc = normalizedRpc.replace("", ""); + return new PreparedRpc(addXmlDeclarationIfMissing(normalizedRpc), expectedMessageId); } /** @@ -246,6 +321,7 @@ private void setHelloReply(final String reply) throws IOException { this.lastRpcReply = reply; try { this.serverHello = Hello.from(reply); + this.useChunkedFraming = serverHello.hasCapability(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1); } catch (final ParserConfigurationException | SAXException | XPathExpressionException e) { throw new NetconfException("Invalid message from server: " + reply, e); } @@ -473,13 +549,14 @@ static String fixupRpc(String rpcContent) throws IllegalArgumentException { if (rpcContent == null) { throw new IllegalArgumentException("Null RPC"); } - rpcContent = rpcContent.trim(); - if (!rpcContent.startsWith("") && !rpcContent.equals("")) { - if (rpcContent.startsWith("<")) - rpcContent = "" + rpcContent + ""; - else - rpcContent = "" + "<" + rpcContent + "/>" + ""; + String trimmedRpcContent = rpcContent.trim(); + if (isRpcEnvelope(trimmedRpcContent)) { + return rpcContent + NetconfConstants.DEVICE_PROMPT; } + if (trimmedRpcContent.startsWith("<")) + rpcContent = "" + trimmedRpcContent + ""; + else + rpcContent = "" + "<" + trimmedRpcContent + "/>" + ""; return rpcContent + NetconfConstants.DEVICE_PROMPT; } @@ -779,7 +856,12 @@ public void loadSetFile(String configFile) throws public void commitThisConfiguration(String configFile, String loadType) throws IOException, SAXException { String configuration = readConfigFile(configFile); configuration = configuration.trim(); - if (this.lockConfig()) { + if (!this.lockConfig()) { + throw new IOException("Unclean lock operation. Cannot proceed " + + "further."); + } + Throwable pendingFailure = null; + try { if (configuration.startsWith("<")) { this.loadXMLConfiguration(configuration, loadType); } else if (configuration.startsWith("set")) { @@ -788,11 +870,20 @@ public void commitThisConfiguration(String configFile, String loadType) throws I this.loadTextConfiguration(configuration, loadType); } this.commit(); - } else { - throw new IOException("Unclean lock operation. Cannot proceed " + - "further."); + } catch (Throwable t) { + pendingFailure = t; + throw t; + } finally { + try { + this.unlockConfig(); + } catch (IOException e) { + if (pendingFailure != null) { + pendingFailure.addSuppressed(e); + } else { + throw e; + } + } } - this.unlockConfig(); } /** @@ -1127,4 +1218,367 @@ public void removeAllRPCAttributes() { rpcAttrMap.clear(); rpcAttributes = null; } + + private static boolean isRpcEnvelope(String rpcContent) { + String normalizedRpcContent = stripXmlDeclaration(rpcContent.trim()); + if (!normalizedRpcContent.startsWith("' || next == '/' || Character.isWhitespace(next); + } + + private static String stripLegacyEndOfMessage(String rpc) { + String trimmed = rpc.trim(); + if (trimmed.endsWith(NetconfConstants.DEVICE_PROMPT)) { + return trimmed.substring(0, trimmed.length() - NetconfConstants.DEVICE_PROMPT.length()).trim(); + } + return trimmed; + } + + private static String stripXmlDeclaration(String xml) { + return XML_DECLARATION_PATTERN.matcher(xml).replaceFirst(""); + } + + private static String addXmlDeclarationIfMissing(String xml) { + return xml.startsWith("" + + stripXmlDeclaration(rpc).substring(startTagMatcher.end()); + return rpc.startsWith(" attribute : rpcAttrMap.entrySet()) { + if (!hasAttribute(existingAttributes, attribute.getKey())) { + attributes.append(" ") + .append(attribute.getKey()) + .append("=\"") + .append(attribute.getValue()) + .append("\""); + } + if ("xmlns".equals(attribute.getKey())) { + hasDefaultNamespace = true; + } + } + if (!hasDefaultNamespace) { + attributes.append(" xmlns=\"").append(NetconfConstants.URN_XML_NS_NETCONF_BASE_1_0).append("\""); + } + return attributes.toString(); + } + + private boolean hasAttribute(String existingAttributes, String attributeName) { + return Pattern.compile("(^|\\s)" + Pattern.quote(attributeName) + "\\s*=") + .matcher(existingAttributes) + .find(); + } + + private void validateReplyMessageId(String expectedMessageId, String reply) throws IOException { + if (expectedMessageId == null || !isRpcReply(reply.trim())) { + return; + } + try { + RpcReply rpcReply = RpcReply.from(reply); + String actualMessageId = rpcReply.getMessageId(); + if (actualMessageId != null && !expectedMessageId.equals(actualMessageId)) { + throw new NetconfException( + "Mismatched rpc-reply message-id. expected=" + expectedMessageId + ", actual=" + actualMessageId); + } + } catch (ParserConfigurationException | SAXException | XPathExpressionException e) { + throw new NetconfException("Invalid message from server: " + reply, e); + } + } + + private byte[] applyTransportFraming(String rpc) { + if (!useChunkedFraming) { + return (rpc + NetconfConstants.DEVICE_PROMPT).getBytes(StandardCharsets.UTF_8); + } + int payloadLength = rpc.getBytes(StandardCharsets.UTF_8).length; + String chunkedRpc = "\n#" + payloadLength + "\n" + rpc + "\n##\n"; + return chunkedRpc.getBytes(StandardCharsets.UTF_8); + } + + private boolean startsWithChunkedFraming() throws IOException { + int nextByte = peekIncomingByte(); + return nextByte == '\n' || nextByte == '#'; + } + + private int peekIncomingByte() throws IOException { + int nextByte = readIncomingByte(); + if (nextByte >= 0) { + pushBackBytes(new byte[] {(byte) nextByte}); + } + return nextByte; + } + + private int readIncomingBytes(byte[] buffer) throws IOException { + if (unreadBytes.length > 0) { + int bytesToCopy = Math.min(buffer.length, unreadBytes.length); + System.arraycopy(unreadBytes, 0, buffer, 0, bytesToCopy); + unreadBytes = Arrays.copyOfRange(unreadBytes, bytesToCopy, unreadBytes.length); + return bytesToCopy; + } + return stdInStreamFromDevice.read(buffer, 0, buffer.length); + } + + private int readIncomingByte() throws IOException { + if (unreadBytes.length > 0) { + int nextByte = unreadBytes[0] & 0xFF; + unreadBytes = Arrays.copyOfRange(unreadBytes, 1, unreadBytes.length); + return nextByte; + } + return stdInStreamFromDevice.read(); + } + + private void pushBackBytes(byte[] bytes) { + if (bytes.length == 0) { + return; + } + byte[] combined = new byte[bytes.length + unreadBytes.length]; + System.arraycopy(bytes, 0, combined, 0, bytes.length); + System.arraycopy(unreadBytes, 0, combined, bytes.length, unreadBytes.length); + unreadBytes = combined; + } + + private String readChunkHeaderLine(long startTime) throws IOException { + ByteArrayOutputStream lineBuffer = new ByteArrayOutputStream(); + while (!commandTimedOut(startTime)) { + int nextByte = readIncomingByte(); + if (nextByte < 0) { + throw new NetconfException("Input Stream has been closed during reading."); + } + if (nextByte == '\n') { + return lineBuffer.toString(StandardCharsets.UTF_8); + } + if (nextByte != '\r') { + lineBuffer.write(nextByte); + } + } + throw new SocketTimeoutException("Command timeout limit was exceeded: " + commandTimeout); + } + + private byte[] readExactBytes(int expectedLength, long startTime) throws IOException { + byte[] chunkBytes = new byte[expectedLength]; + int offset = 0; + while (offset < expectedLength && !commandTimedOut(startTime)) { + int bytesRead; + if (unreadBytes.length > 0) { + bytesRead = Math.min(expectedLength - offset, unreadBytes.length); + System.arraycopy(unreadBytes, 0, chunkBytes, offset, bytesRead); + unreadBytes = Arrays.copyOfRange(unreadBytes, bytesRead, unreadBytes.length); + } else { + bytesRead = stdInStreamFromDevice.read(chunkBytes, offset, expectedLength - offset); + } + if (bytesRead < 0) { + throw new NetconfException("Input Stream has been closed during reading."); + } + offset += bytesRead; + } + if (offset < expectedLength) { + throw new SocketTimeoutException("Command timeout limit was exceeded: " + commandTimeout); + } + return chunkBytes; + } + + private boolean commandTimedOut(long startTime) { + return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTime) >= commandTimeout; + } + + private abstract class RpcReplyInputStream extends InputStream { + + private final long startTime = System.nanoTime(); + private boolean finished; + private boolean closed; + + @Override + public int read() throws IOException { + if (closed) { + throw new IOException("Stream closed"); + } + if (finished) { + return -1; + } + int nextByte = readPayloadByte(); + if (nextByte < 0) { + finished = true; + } + return nextByte; + } + + @Override + public int read(byte[] buffer, int off, int len) throws IOException { + if (buffer == null) { + throw new NullPointerException("buffer"); + } + if (off < 0 || len < 0 || len > buffer.length - off) { + throw new IndexOutOfBoundsException(); + } + if (len == 0) { + return 0; + } + int firstByte = read(); + if (firstByte < 0) { + return -1; + } + buffer[off] = (byte) firstByte; + int bytesRead = 1; + while (bytesRead < len) { + int nextByte = read(); + if (nextByte < 0) { + break; + } + buffer[off + bytesRead] = (byte) nextByte; + bytesRead++; + } + return bytesRead; + } + + @Override + public void close() throws IOException { + if (closed) { + return; + } + try { + byte[] discardBuffer = new byte[BUFFER_SIZE]; + while (read(discardBuffer, 0, discardBuffer.length) != -1) { + // Drain the current reply so the session stays aligned for the next RPC. + } + } finally { + finished = true; + closed = true; + } + } + + protected abstract int readPayloadByte() throws IOException; + + protected final int readIncomingByteOrThrow() throws IOException { + if (commandTimedOut(startTime)) { + throw new SocketTimeoutException("Command timeout limit was exceeded: " + commandTimeout); + } + int nextByte = readIncomingByte(); + if (nextByte < 0) { + throw new NetconfException("Input Stream has been closed during reading."); + } + return nextByte; + } + + protected final String readChunkHeaderLine() throws IOException { + return NetconfSession.this.readChunkHeaderLine(startTime); + } + } + + private final class LegacyRpcReplyInputStream extends RpcReplyInputStream { + + private final byte[] candidatePrompt = new byte[DEVICE_PROMPT_BYTES.length]; + private int candidateLength; + + @Override + protected int readPayloadByte() throws IOException { + while (true) { + if (candidateLength > 0 && !matchesPromptPrefix()) { + return shiftCandidateByte(); + } + if (candidateLength == DEVICE_PROMPT_BYTES.length) { + candidateLength = 0; + return -1; + } + candidatePrompt[candidateLength++] = (byte) readIncomingByteOrThrow(); + } + } + + private boolean matchesPromptPrefix() { + for (int i = 0; i < candidateLength; i++) { + if (candidatePrompt[i] != DEVICE_PROMPT_BYTES[i]) { + return false; + } + } + return true; + } + + private int shiftCandidateByte() { + int nextByte = candidatePrompt[0] & 0xFF; + if (candidateLength > 1) { + System.arraycopy(candidatePrompt, 1, candidatePrompt, 0, candidateLength - 1); + } + candidateLength--; + return nextByte; + } + } + + private final class ChunkedRpcReplyInputStream extends RpcReplyInputStream { + + private int remainingChunkBytes; + + @Override + protected int readPayloadByte() throws IOException { + while (remainingChunkBytes == 0) { + String header = readChunkHeaderLine(); + if (header.isEmpty()) { + continue; + } + if ("##".equals(header)) { + return -1; + } + if (!header.startsWith("#")) { + throw new NetconfException("Invalid NETCONF chunked framing header: " + header); + } + try { + remainingChunkBytes = Integer.parseInt(header.substring(1)); + } catch (NumberFormatException e) { + throw new NetconfException("Invalid NETCONF chunk size: " + header, e); + } + if (remainingChunkBytes < 0) { + throw new NetconfException("Invalid NETCONF chunk size: " + header); + } + } + remainingChunkBytes--; + return readIncomingByteOrThrow(); + } + } + + private static int indexOf(byte[] haystack, byte[] needle) { + outer: + for (int i = 0; i <= haystack.length - needle.length; i++) { + for (int j = 0; j < needle.length; j++) { + if (haystack[i + j] != needle[j]) { + continue outer; + } + } + return i; + } + return -1; + } } diff --git a/src/main/java/net/juniper/netconf/element/Hello.java b/src/main/java/net/juniper/netconf/element/Hello.java index 2bf8927..6cf85ca 100644 --- a/src/main/java/net/juniper/netconf/element/Hello.java +++ b/src/main/java/net/juniper/netconf/element/Hello.java @@ -180,7 +180,7 @@ public Builder namespacePrefix(String namespacePrefix) { */ public Hello build() { // RFC 6241 § 8.1 — each peer MUST advertise at least the base 1.1 capability - final String BASE_11 = "urn:ietf:params:netconf:base:1.1"; + final String BASE_11 = NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1; List caps = capabilities == null ? new java.util.ArrayList<>() diff --git a/src/test/java/net/juniper/netconf/NetconfSessionTest.java b/src/test/java/net/juniper/netconf/NetconfSessionTest.java index 25730b7..9592b1c 100644 --- a/src/test/java/net/juniper/netconf/NetconfSessionTest.java +++ b/src/test/java/net/juniper/netconf/NetconfSessionTest.java @@ -16,8 +16,10 @@ import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import java.io.BufferedReader; import java.io.BufferedOutputStream; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.PipedInputStream; @@ -25,6 +27,7 @@ import java.net.SocketTimeoutException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Path; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -34,8 +37,10 @@ import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doCallRealMethod; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class NetconfSessionTest { @@ -212,6 +217,15 @@ public void fixupRpcPreservesExistingRpcTags() { .isEqualTo("fake string" + DEVICE_PROMPT); } + @Test + public void fixupRpcPreservesExistingRpcTagsWithAttributes() throws Exception { + XMLBuilder builder = new XMLBuilder(); + String rpc = builder.createNewRPC("get-interface-information", "terse").toString(); + + assertThat(NetconfSession.fixupRpc(rpc)) + .isEqualTo(rpc + DEVICE_PROMPT); + } + @Test public void fixupRpcWrapsTaggedString() { assertThat(NetconfSession.fixupRpc("")) @@ -241,6 +255,131 @@ public void instantiationFetchesHelloFromServer() throws Exception { .isTrue(); } + @Test + public void executeRpcSupportsChunkedRepliesAfterBase11Hello() throws Exception { + final String combinedMessage = createHelloMessageWithBase11() + + NetconfConstants.DEVICE_PROMPT + + toChunkedMessage(OK_RPC_REPLY); + final InputStream combinedStream = new ByteArrayInputStream(combinedMessage.getBytes(StandardCharsets.UTF_8)); + when(mockChannel.getInputStream()).thenReturn(combinedStream); + + final NetconfSession netconfSession = createNetconfSession(100); + + assertThat(netconfSession.executeRPC("").toString()) + .contains(""); + } + + @Test + public void executeRpcRunningStripsLegacyFramingAndStopsAfterSingleReply() throws Exception { + String firstReply = RpcReply.builder().ok(true).messageId("1").build().getXml(); + String secondReply = RpcReply.builder().ok(true).messageId("2").build().getXml(); + NetconfSession netconfSession = createNetconfSession( + new ByteArrayInputStream((createHelloMessage() + + NetconfConstants.DEVICE_PROMPT + + firstReply + + NetconfConstants.DEVICE_PROMPT + + secondReply + + NetconfConstants.DEVICE_PROMPT).getBytes(StandardCharsets.UTF_8)), + new ByteArrayOutputStream(), + 100 + ); + + try (BufferedReader replyReader = netconfSession.executeRPCRunning("")) { + String streamedReply = readAll(replyReader); + assertThat(streamedReply).isEqualTo(firstReply); + assertThat(streamedReply).doesNotContain(NetconfConstants.DEVICE_PROMPT); + } + + XmlAssert.assertThat(netconfSession.executeRPC("").toString()) + .and(secondReply) + .ignoreWhitespace() + .areIdentical(); + } + + @Test + public void closeOnChunkedExecuteRpcRunningDrainsCurrentReplyForNextRpc() throws Exception { + String firstReply = """ + + + alpha + beta + + + """.trim(); + String secondReply = RpcReply.builder().ok(true).messageId("2").build().getXml(); + NetconfSession netconfSession = createNetconfSession( + new ByteArrayInputStream((createHelloMessageWithBase11() + + NetconfConstants.DEVICE_PROMPT + + toChunkedMessage(firstReply) + + toChunkedMessage(secondReply)).getBytes(StandardCharsets.UTF_8)), + new ByteArrayOutputStream(), + 100 + ); + + BufferedReader replyReader = netconfSession.executeRPCRunning(""); + char[] prefix = new char[16]; + int prefixLength = replyReader.read(prefix); + assertThat(prefixLength).isPositive(); + assertThat(new String(prefix, 0, prefixLength)).startsWith("").toString()) + .and(secondReply) + .ignoreWhitespace() + .areIdentical(); + } + + @Test + public void executeRpcAddsMessageIdToAttributedRpcEnvelopeWhenMissing() throws Exception { + ByteArrayOutputStream capturedOutput = new ByteArrayOutputStream(); + String rpcReply = RpcReply.builder().ok(true).messageId("1").build().getXml(); + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream(rpcReply), + capturedOutput, + 100 + ); + + netconfSession.executeRPC(""); + + assertThat(capturedOutput.toString(StandardCharsets.UTF_8)) + .contains("" + + DEVICE_PROMPT); + } + + @Test + public void executeRpcPreservesCallerSuppliedMessageId() throws Exception { + ByteArrayOutputStream capturedOutput = new ByteArrayOutputStream(); + String rpcReply = RpcReply.builder().ok(true).messageId("77").build().getXml(); + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream(rpcReply), + capturedOutput, + 100 + ); + + netconfSession.executeRPC(""); + + assertThat(capturedOutput.toString(StandardCharsets.UTF_8)) + .contains("" + + DEVICE_PROMPT); + } + + @Test + public void executeRpcRejectsMismatchedReplyMessageId() throws Exception { + String mismatchedReply = RpcReply.builder().ok(true).messageId("999").build().getXml(); + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream(mismatchedReply), + new ByteArrayOutputStream(), + 100 + ); + + assertThatThrownBy(() -> netconfSession.executeRPC("")) + .isInstanceOf(NetconfException.class) + .hasMessageContaining("Mismatched rpc-reply message-id") + .hasMessageContaining("expected=1") + .hasMessageContaining("actual=999"); + } + @Test public void loadTextConfigurationSucceedsWithOkResponse() throws Exception { doCallRealMethod().when(mockNetconfSession) @@ -422,6 +561,21 @@ public void cancelCommitReturnsFalseOnErrorReply() throws Exception { assertThat(mockNetconfSession.cancelCommit(null)).isFalse(); } + @Test + public void commitThisConfigurationUnlocksCandidateWhenLoadFails() throws Exception { + doCallRealMethod().when(mockNetconfSession).commitThisConfiguration(anyString(), anyString()); + when(mockNetconfSession.lockConfig()).thenReturn(true); + doThrow(new LoadException("boom")).when(mockNetconfSession).loadTextConfiguration(anyString(), anyString()); + + Path configFile = Files.createTempFile("netconf-java-", ".conf"); + Files.writeString(configFile, "system { services { ftp; } }", StandardCharsets.UTF_8); + + assertThatThrownBy(() -> mockNetconfSession.commitThisConfiguration(configFile.toString(), "merge")) + .isInstanceOf(LoadException.class); + + verify(mockNetconfSession).unlockConfig(); + } + // Helper methods to reduce code duplication and improve readability private NetconfSession createNetconfSession(int commandTimeout) throws IOException { @@ -435,6 +589,32 @@ private NetconfSession createNetconfSession(int commandTimeout) throws IOExcepti return new NetconfSession(mockChannel, CONNECTION_TIMEOUT, commandTimeout, FAKE_HELLO, builder); } + private NetconfSession createNetconfSession(InputStream inputStream, + java.io.OutputStream outputStream, + int commandTimeout) throws IOException { + when(mockChannel.getInputStream()).thenReturn(inputStream); + when(mockChannel.getOutputStream()).thenReturn(outputStream); + return createNetconfSession(commandTimeout); + } + + private InputStream helloThenReplyStream(String reply) { + String combinedMessage = createHelloMessage() + + NetconfConstants.DEVICE_PROMPT + + reply + + NetconfConstants.DEVICE_PROMPT; + return new ByteArrayInputStream(combinedMessage.getBytes(StandardCharsets.UTF_8)); + } + + private String readAll(BufferedReader reader) throws IOException { + StringBuilder content = new StringBuilder(); + char[] buffer = new char[128]; + int read; + while ((read = reader.read(buffer)) != -1) { + content.append(buffer, 0, read); + } + return content.toString(); + } + private void writeDataWithDelay() throws IOException, InterruptedException { outPipe.write(FAKE_RPC_REPLY.getBytes(StandardCharsets.UTF_8)); for (int i = 0; i < 7; i++) { @@ -489,4 +669,20 @@ private String createHelloMessage() { + " 27700\n" + ""; } -} \ No newline at end of file + + private String createHelloMessageWithBase11() { + return "\n" + + " \n" + + " urn:ietf:params:netconf:base:1.0\n" + + " urn:ietf:params:netconf:base:1.1\n" + + " \n" + + " 27700\n" + + ""; + } + + private String toChunkedMessage(String payload) { + return "\n#" + payload.getBytes(StandardCharsets.UTF_8).length + "\n" + + payload + + "\n##\n"; + } +} diff --git a/src/test/java/net/juniper/netconf/TestConstants.java b/src/test/java/net/juniper/netconf/TestConstants.java index 32a95d4..3f07163 100644 --- a/src/test/java/net/juniper/netconf/TestConstants.java +++ b/src/test/java/net/juniper/netconf/TestConstants.java @@ -21,5 +21,6 @@ public class TestConstants { "urn:ietf:params:netconf:base:1.0#url?protocol=http,ftp,file\n" + "\n" + ""; - public static final String LLDP_REQUEST = ""; + public static final String LLDP_REQUEST = + ""; } From 0c54293f306eff63023d29e78116788a4bb1d543 Mon Sep 17 00:00:00 2001 From: Peter Hill Date: Fri, 24 Apr 2026 11:04:33 -0700 Subject: [PATCH 3/8] Fix nested XML path construction Make XML.addPath build each segment under the previously created node instead of attaching every segment directly to the original active element. Add a regression test for multi-segment paths so nested hierarchies are preserved correctly. --- src/main/java/net/juniper/netconf/XML.java | 6 ++++-- src/test/java/net/juniper/netconf/XMLTest.java | 13 ++++++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/juniper/netconf/XML.java b/src/main/java/net/juniper/netconf/XML.java index 1db7974..db7d074 100644 --- a/src/main/java/net/juniper/netconf/XML.java +++ b/src/main/java/net/juniper/netconf/XML.java @@ -262,10 +262,12 @@ public void addSiblings(Map map) { public XML addPath(String path) { String[] elements = path.split("/"); Preconditions.checkArgument(elements.length >= 1); + Element parent = activeElement; Element newElement = null; for (String element : elements) { newElement = ownerDoc.createElement(element); - activeElement.appendChild(newElement); + parent.appendChild(newElement); + parent = newElement; } return new XML(Objects.requireNonNull(newElement)); } @@ -560,4 +562,4 @@ public String toString() { } return str; } -} \ 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 b3cf7db..3d61bcb 100644 --- a/src/test/java/net/juniper/netconf/XMLTest.java +++ b/src/test/java/net/juniper/netconf/XMLTest.java @@ -128,6 +128,17 @@ public void GIVEN_childNodeWithText_WHEN_findValue_THEN_returnText() throws Exce assertThat(value).isEqualTo("hello"); } + @Test + public void GIVEN_multiSegmentPath_WHEN_addPath_THEN_createNestedHierarchy() throws Exception { + XMLBuilder builder = new XMLBuilder(); + XML xml = builder.createNewXML("parent"); + + xml.addPath("level1/level2/leaf"); + + assertThat(xml.toString()) + .containsIgnoringWhitespaces(""); + } + @Test public void GIVEN_parentWithTwoItems_WHEN_findNodes_item_THEN_returnBoth() throws Exception { DocumentBuilder builder = factory.newDocumentBuilder(); @@ -314,4 +325,4 @@ public void GIVEN_child_WHEN_addSiblingsMap_THEN_allAdded() throws Exception { .contains("1") .contains("2"); } -} \ No newline at end of file +} From 220688c39334ee6f1782512449ed0bcafb9bc5bd Mon Sep 17 00:00:00 2001 From: Peter Hill Date: Fri, 24 Apr 2026 13:21:23 -0700 Subject: [PATCH 4/8] Fix device session cleanup and shell command handling - clean up partial SSH state when NETCONF session creation fails - connect and release exec channels deterministically - enforce commandTimeout for shell helper reads - document the recommended Device lifecycle in the README --- README.md | 51 ++-- src/main/java/net/juniper/netconf/Device.java | 179 +++++++++++--- .../java/net/juniper/netconf/DeviceTest.java | 225 ++++++++++++++++-- 3 files changed, 381 insertions(+), 74 deletions(-) diff --git a/README.md b/README.md index 4e6211f..f703b0a 100644 --- a/README.md +++ b/README.md @@ -105,31 +105,40 @@ public class ShowInterfaces { public static void main(String args[]) throws NetconfException, ParserConfigurationException, SAXException, IOException { - //Create device - Device device = Device.builder() - .hostName("hostname") - .userName("username") - .password("password") - .connectionTimeout(2000) - .hostKeysFileName("hostKeysFileName") - .build(); - device.connect(); - - //Send RPC and receive RPC Reply as XML - XML rpc_reply = device.executeRPC("get-interface-information"); - /* OR - * device.executeRPC(""); - * OR - * device.executeRPC(""); - */ - - //Print the RPC-Reply and close the device. - System.out.println(rpc_reply); - device.close(); + try (Device device = Device.builder() + .hostName("hostname") + .userName("username") + .password("password") + .hostKeysFileName("hostKeysFileName") + .connectionTimeout(2000) + .commandTimeout(5000) + .build()) { + + // Establish the SSH transport and the default NETCONF session. + device.connect(); + + // Send RPC and receive RPC reply as XML. + XML rpcReply = device.executeRPC("get-interface-information"); + /* OR + * device.executeRPC(""); + * OR + * device.executeRPC(""); + */ + + System.out.println(rpcReply); + } } } ``` +Recommended usage: + +* Build one `Device` per target connection and use `try-with-resources` so SSH resources are released predictably. +* Call `connect()` before issuing RPCs. If `connect()` throws, no usable NETCONF session was established. +* Set `connectionTimeout` and `commandTimeout` explicitly for production use rather than relying on defaults. +* Prefer NETCONF RPC helpers (`executeRPC`, `getConfig`, `loadXMLConfiguration`, `commit`, and friends) for device operations; use shell helpers only for device-specific workflows that are not available over NETCONF. +* Shell helper reads are bounded by `commandTimeout`. If you use `runShellCommandRunning(...)`, always close the returned reader so the underlying exec channel is released. + LICENSE ======= diff --git a/src/main/java/net/juniper/netconf/Device.java b/src/main/java/net/juniper/netconf/Device.java index 0eed639..e4f5211 100644 --- a/src/main/java/net/juniper/netconf/Device.java +++ b/src/main/java/net/juniper/netconf/Device.java @@ -25,12 +25,15 @@ import javax.xml.parsers.ParserConfigurationException; import java.io.BufferedReader; import java.io.IOException; +import java.io.InterruptedIOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.net.SocketTimeoutException; import java.nio.charset.Charset; import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.concurrent.TimeUnit; /** * A Device is used to define a Netconf server. @@ -61,6 +64,7 @@ public class Device implements AutoCloseable { private static final int DEFAULT_NETCONF_PORT = 830; private static final int DEFAULT_TIMEOUT = 5000; + private static final long SHELL_READ_POLL_INTERVAL_MILLIS = 10L; private static final List DEFAULT_CLIENT_CAPABILITIES = Arrays.asList( NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0 + "#candidate", @@ -406,6 +410,9 @@ private String createHelloRPC(List capabilities) { * @throws NetconfException if there are issues communicating with the Netconf server. */ private NetconfSession createNetconfSession() throws NetconfException { + Session session = sshSession; + ChannelSubsystem channel = null; + boolean disconnectSessionOnFailure = false; if (!isConnected()) { try { if (strictHostKeyChecking) { @@ -421,31 +428,49 @@ private NetconfSession createNetconfSession() throws NetconfException { log.info("Connecting to host {} on port {}.", hostName, port); if (keyBasedAuthentication) { loadPrivateKey(); - sshSession = loginWithPrivateKey(connectionTimeout); + session = loginWithPrivateKey(connectionTimeout); } else { - sshSession = loginWithUserPass(connectionTimeout); + session = loginWithUserPass(connectionTimeout); } + disconnectSessionOnFailure = true; try { - sshSession.setTimeout(connectionTimeout); + session.setTimeout(connectionTimeout); } catch (JSchException e) { + cleanupFailedSessionInitialization(channel, session, disconnectSessionOnFailure); throw new NetconfException(String.format("Error setting session timeout: %s", e.getMessage()), e); } - if (sshSession.isConnected()) { - log.info("Connected to host {} - Timeout set to {} msecs.", hostName, sshSession.getTimeout()); + if (session.isConnected()) { + log.info("Connected to host {} - Timeout set to {} msecs.", hostName, session.getTimeout()); } else { + cleanupFailedSessionInitialization(channel, session, disconnectSessionOnFailure); throw new NetconfException("Failed to connect to host. Unknown reason"); } } try { - sshChannel = (ChannelSubsystem) sshSession.openChannel("subsystem"); - sshChannel.setSubsystem("netconf"); - return new NetconfSession(sshChannel, connectionTimeout, commandTimeout, helloRpc, xmlBuilder); + channel = (ChannelSubsystem) session.openChannel("subsystem"); + channel.setSubsystem("netconf"); + NetconfSession establishedSession = + new NetconfSession(channel, connectionTimeout, commandTimeout, helloRpc, xmlBuilder); + sshSession = session; + sshChannel = channel; + return establishedSession; } catch (JSchException | IOException e) { + cleanupFailedSessionInitialization(channel, session, disconnectSessionOnFailure); throw new NetconfException("Failed to create Netconf session:" + e.getMessage(), e); } } + private void cleanupFailedSessionInitialization(ChannelSubsystem channel, Session session, + boolean disconnectSessionOnFailure) { + if (channel != null) { + channel.disconnect(); + } + if (disconnectSessionOnFailure && session != null) { + session.disconnect(); + } + } + private Session loginWithUserPass(int timeoutMilliSeconds) throws NetconfException { try { Session session = sshClient.getSession(userName, hostName, port); @@ -566,41 +591,23 @@ public void close() { * Execute a command in shell mode. * * @param command The command to be executed in shell mode. - * @return Result of the command execution, as a String. + * @return Result of the command execution, as a String. Waiting for + * command output is bounded by {@link #getCommandTimeout()}. * @throws IOException if there are issues communicating with the Netconf server. */ public String runShellCommand(String command) throws IOException { if (!isConnected()) { return "Could not find open connection."; } - ChannelExec channel; - try { - channel = (ChannelExec) sshSession.openChannel("exec"); - } catch (JSchException e) { - throw new NetconfException(String.format("Failed to open exec session: %s", e.getMessage()), e); - } - channel.setCommand(command); - InputStream stdout; - BufferedReader bufferReader; - stdout = channel.getInputStream(); - - bufferReader = new BufferedReader(new InputStreamReader(stdout, Charset.defaultCharset())); - try { + try (BufferedReader bufferReader = openExecChannelReader(command)) { StringBuilder reply = new StringBuilder(); while (true) { - String line; - try { - line = bufferReader.readLine(); - } catch (Exception e) { - throw new NetconfException(e.getMessage(), e); - } + String line = bufferReader.readLine(); if (line == null || line.equals(NetconfConstants.EMPTY_LINE)) break; reply.append(line).append(NetconfConstants.LF); } return reply.toString(); - } finally { - bufferReader.close(); } } @@ -610,7 +617,9 @@ public String runShellCommand(String command) throws IOException { * @param command The command to be executed in shell mode. * @return Result of the command execution, as a BufferedReader. This is * useful if we want continuous stream of output, rather than wait - * for whole output till command execution completes. + * for whole output till command execution completes. Closing the returned + * reader also disconnects the underlying SSH exec channel. Each blocking + * read waits at most {@link #getCommandTimeout()} for additional output. * @throws IOException if there are issues communicating with the Netconf server. */ public BufferedReader runShellCommandRunning(String command) @@ -618,14 +627,118 @@ public BufferedReader runShellCommandRunning(String command) if (!isConnected()) { throw new IOException("Could not find open connection"); } + return openExecChannelReader(command); + } + + private BufferedReader openExecChannelReader(String command) throws IOException { ChannelExec channel; try { channel = (ChannelExec) sshSession.openChannel("exec"); } catch (JSchException e) { throw new NetconfException(String.format("Failed to open exec session: %s", e.getMessage()), e); } - InputStream stdout = channel.getInputStream(); - return new BufferedReader(new InputStreamReader(stdout, Charset.defaultCharset())); + try { + channel.setCommand(command); + InputStream stdout = channel.getInputStream(); + channel.connect(connectionTimeout); + return new BufferedReader(new InputStreamReader( + new TimeoutAwareChannelInputStream(stdout, channel, commandTimeout), + Charset.defaultCharset())) { + @Override + public void close() throws IOException { + try { + super.close(); + } finally { + channel.disconnect(); + } + } + }; + } catch (JSchException e) { + channel.disconnect(); + throw new NetconfException(String.format("Failed to start exec session: %s", e.getMessage()), e); + } catch (IOException e) { + channel.disconnect(); + throw e; + } + } + + private static final class TimeoutAwareChannelInputStream extends InputStream { + private final InputStream delegate; + private final ChannelExec channel; + private final int timeoutMillis; + + private TimeoutAwareChannelInputStream(InputStream delegate, ChannelExec channel, int timeoutMillis) { + this.delegate = delegate; + this.channel = channel; + this.timeoutMillis = timeoutMillis; + } + + @Override + public int read() throws IOException { + byte[] oneByte = new byte[1]; + int bytesRead = read(oneByte, 0, 1); + if (bytesRead < 0) { + return -1; + } + return oneByte[0] & 0xFF; + } + + @Override + public int read(byte[] buffer, int off, int len) throws IOException { + Objects.requireNonNull(buffer, "buffer"); + if (off < 0 || len < 0 || len > buffer.length - off) { + throw new IndexOutOfBoundsException(); + } + if (len == 0) { + return 0; + } + if (timeoutMillis <= 0) { + return delegate.read(buffer, off, len); + } + + long deadlineNanos = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(timeoutMillis); + while (true) { + int available = delegate.available(); + if (available > 0) { + int bytesRead = delegate.read(buffer, off, Math.min(len, available)); + if (bytesRead != 0) { + return bytesRead; + } + } else if (channel.isClosed()) { + return delegate.read(buffer, off, len); + } + + long remainingNanos = deadlineNanos - System.nanoTime(); + if (remainingNanos <= 0) { + throw new SocketTimeoutException("Command timeout limit was exceeded: " + timeoutMillis); + } + sleepForAvailableData(remainingNanos); + } + } + + @Override + public void close() throws IOException { + delegate.close(); + } + + @Override + public int available() throws IOException { + return delegate.available(); + } + + private void sleepForAvailableData(long remainingNanos) throws InterruptedIOException { + long sleepMillis = Math.min(SHELL_READ_POLL_INTERVAL_MILLIS, + Math.max(1L, TimeUnit.NANOSECONDS.toMillis(remainingNanos))); + try { + Thread.sleep(sleepMillis); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + InterruptedIOException interrupted = new InterruptedIOException( + "Interrupted while waiting for shell command output."); + interrupted.initCause(e); + throw interrupted; + } + } } /** diff --git a/src/test/java/net/juniper/netconf/DeviceTest.java b/src/test/java/net/juniper/netconf/DeviceTest.java index 505038a..cbf8e77 100644 --- a/src/test/java/net/juniper/netconf/DeviceTest.java +++ b/src/test/java/net/juniper/netconf/DeviceTest.java @@ -1,5 +1,6 @@ package net.juniper.netconf; +import com.jcraft.jsch.ChannelExec; import com.jcraft.jsch.ChannelSubsystem; import com.jcraft.jsch.HostKeyRepository; import com.jcraft.jsch.JSch; @@ -9,22 +10,26 @@ import org.junit.jupiter.api.Test; import org.xmlunit.assertj.XmlAssert; +import java.io.BufferedReader; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.net.SocketTimeoutException; import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; public class DeviceTest { @@ -117,41 +122,59 @@ public void GIVEN_deviceBuilder_THEN_buildDevice() throws NetconfException { } @Test - public void GIVEN_sshAvailableNetconfNot_THEN_closeDevice() throws Exception { + public void GIVEN_channelSetupFails_WHEN_connect_THEN_disconnectPartialSshResources() throws Exception { JSch sshClient = mock(JSch.class); Session session = mock(Session.class); HostKeyRepository hostKeyRepository = mock(HostKeyRepository.class); ChannelSubsystem channel = mock(ChannelSubsystem.class); - when(channel.isConnected()).thenReturn(false); + AtomicBoolean sessionConnected = new AtomicBoolean(false); + AtomicBoolean channelConnected = new AtomicBoolean(false); + + when(session.isConnected()).thenAnswer(invocation -> sessionConnected.get()); + doAnswer(invocation -> { + sessionConnected.set(true); + return null; + }).when(session).connect(eq(DEFAULT_TIMEOUT)); + doAnswer(invocation -> { + sessionConnected.set(false); + return null; + }).when(session).disconnect(); + + when(channel.isConnected()).thenAnswer(invocation -> channelConnected.get()); + when(channel.getInputStream()).thenReturn(new ByteArrayInputStream(new byte[0])); + when(channel.getOutputStream()).thenReturn(outputStream); + doAnswer(invocation -> { + channelConnected.set(false); + return null; + }).when(channel).disconnect(); - when(session.isConnected()).thenReturn(true); when(session.openChannel(eq(SUBSYSTEM))).thenReturn(channel); doThrow(new JSchException("failed to send channel request")).when(channel).connect(eq(DEFAULT_TIMEOUT)); when(sshClient.getSession(eq(TEST_USERNAME), eq(TEST_HOSTNAME), eq(DEFAULT_NETCONF_PORT))).thenReturn(session); when(sshClient.getHostKeyRepository()).thenReturn(hostKeyRepository); - try (Device device = Device.builder() - .sshClient(sshClient) - .hostName(TEST_HOSTNAME) - .userName(TEST_USERNAME) - .password(TEST_PASSWORD) - .strictHostKeyChecking(false) - .build()) { - device.connect(); - } catch (NetconfException e) { - // Do nothing - } + Device device = Device.builder() + .sshClient(sshClient) + .hostName(TEST_HOSTNAME) + .userName(TEST_USERNAME) + .password(TEST_PASSWORD) + .strictHostKeyChecking(false) + .build(); + + assertThatThrownBy(device::connect) + .isInstanceOf(NetconfException.class) + .hasMessageContaining("Failed to create Netconf session"); verify(channel).connect(eq(DEFAULT_TIMEOUT)); verify(channel).setSubsystem(anyString()); verify(channel).getInputStream(); verify(channel).getOutputStream(); - verify(channel).isConnected(); + verify(channel).disconnect(); verify(session).disconnect(); verify(session).openChannel(eq(SUBSYSTEM)); - verify(session, times(2)).isConnected(); + verify(session).isConnected(); verify(session).getTimeout(); verify(session).connect(eq(DEFAULT_TIMEOUT)); verify(session).setTimeout(eq(DEFAULT_TIMEOUT)); @@ -162,9 +185,136 @@ public void GIVEN_sshAvailableNetconfNot_THEN_closeDevice() throws Exception { verify(sshClient).getHostKeyRepository(); verify(sshClient).setHostKeyRepository(hostKeyRepository); - verifyNoMoreInteractions(channel); - verifyNoMoreInteractions(session); - verifyNoMoreInteractions(sshClient); + assertThat(sessionConnected.get()).isFalse(); + assertThat(channelConnected.get()).isFalse(); + assertThat(device.isConnected()).isFalse(); + } + + @Test + public void GIVEN_serverHelloInvalid_WHEN_connect_THEN_disconnectChannelAndSession() throws Exception { + JSch sshClient = mock(JSch.class); + Session session = mock(Session.class); + HostKeyRepository hostKeyRepository = mock(HostKeyRepository.class); + ChannelSubsystem channel = mock(ChannelSubsystem.class); + AtomicBoolean sessionConnected = new AtomicBoolean(false); + AtomicBoolean channelConnected = new AtomicBoolean(false); + + when(session.isConnected()).thenAnswer(invocation -> sessionConnected.get()); + doAnswer(invocation -> { + sessionConnected.set(true); + return null; + }).when(session).connect(eq(DEFAULT_TIMEOUT)); + doAnswer(invocation -> { + sessionConnected.set(false); + return null; + }).when(session).disconnect(); + + when(channel.isConnected()).thenAnswer(invocation -> channelConnected.get()); + when(channel.getInputStream()).thenReturn(new ByteArrayInputStream( + ("" + NetconfConstants.DEVICE_PROMPT).getBytes(StandardCharsets.UTF_8))); + when(channel.getOutputStream()).thenReturn(outputStream); + doAnswer(invocation -> { + channelConnected.set(true); + return null; + }).when(channel).connect(eq(DEFAULT_TIMEOUT)); + doAnswer(invocation -> { + channelConnected.set(false); + return null; + }).when(channel).disconnect(); + + when(session.openChannel(eq(SUBSYSTEM))).thenReturn(channel); + when(sshClient.getSession(eq(TEST_USERNAME), eq(TEST_HOSTNAME), eq(DEFAULT_NETCONF_PORT))).thenReturn(session); + when(sshClient.getHostKeyRepository()).thenReturn(hostKeyRepository); + + Device device = Device.builder() + .sshClient(sshClient) + .hostName(TEST_HOSTNAME) + .userName(TEST_USERNAME) + .password(TEST_PASSWORD) + .strictHostKeyChecking(false) + .build(); + + assertThatThrownBy(device::connect) + .isInstanceOf(NetconfException.class) + .hasMessageContaining("Invalid message from server"); + + verify(channel).connect(eq(DEFAULT_TIMEOUT)); + verify(channel).disconnect(); + verify(session).disconnect(); + assertThat(sessionConnected.get()).isFalse(); + assertThat(channelConnected.get()).isFalse(); + assertThat(device.isConnected()).isFalse(); + } + + @Test + public void GIVEN_connectedDevice_WHEN_runShellCommand_THEN_connectExecChannelAndDisconnectIt() throws Exception { + Device device = createConnectedDeviceForShellCommands(); + ChannelExec execChannel = mock(ChannelExec.class); + when(device.getSshSession().openChannel("exec")).thenReturn(execChannel); + when(execChannel.getInputStream()).thenReturn(new ByteArrayInputStream("show version\n".getBytes(StandardCharsets.UTF_8))); + when(execChannel.isClosed()).thenReturn(true); + + String reply = device.runShellCommand("show version"); + + assertThat(reply).isEqualTo("show version\n"); + verify(execChannel).setCommand("show version"); + verify(execChannel).connect(DEFAULT_TIMEOUT); + verify(execChannel).disconnect(); + } + + @Test + public void GIVEN_connectedDevice_WHEN_runShellCommandRunning_THEN_closeReaderDisconnectsExecChannel() throws Exception { + Device device = createConnectedDeviceForShellCommands(); + ChannelExec execChannel = mock(ChannelExec.class); + when(device.getSshSession().openChannel("exec")).thenReturn(execChannel); + when(execChannel.getInputStream()).thenReturn(new ByteArrayInputStream("stream line\n".getBytes(StandardCharsets.UTF_8))); + + BufferedReader reader = device.runShellCommandRunning("monitor interfaces"); + + assertThat(reader.readLine()).isEqualTo("stream line"); + verify(execChannel).setCommand("monitor interfaces"); + verify(execChannel).connect(DEFAULT_TIMEOUT); + verify(execChannel, times(0)).disconnect(); + + reader.close(); + + verify(execChannel).disconnect(); + } + + @Test + public void GIVEN_connectedDevice_WHEN_runShellCommandStalls_THEN_commandTimeoutApplies() throws Exception { + Device device = createConnectedDeviceForShellCommands(50); + ChannelExec execChannel = mock(ChannelExec.class); + when(device.getSshSession().openChannel("exec")).thenReturn(execChannel); + when(execChannel.getInputStream()).thenReturn(nonBlockingIdleInputStream()); + when(execChannel.isClosed()).thenReturn(false); + + assertThatThrownBy(() -> device.runShellCommand("show interfaces terse")) + .isInstanceOf(SocketTimeoutException.class) + .hasMessage("Command timeout limit was exceeded: 50"); + + verify(execChannel).connect(DEFAULT_TIMEOUT); + verify(execChannel).disconnect(); + } + + @Test + public void GIVEN_connectedDevice_WHEN_runningShellCommandStalls_THEN_readTimesOut() throws Exception { + Device device = createConnectedDeviceForShellCommands(50); + ChannelExec execChannel = mock(ChannelExec.class); + when(device.getSshSession().openChannel("exec")).thenReturn(execChannel); + when(execChannel.getInputStream()).thenReturn(nonBlockingIdleInputStream()); + when(execChannel.isClosed()).thenReturn(false); + + BufferedReader reader = device.runShellCommandRunning("monitor interfaces"); + + assertThatThrownBy(reader::readLine) + .isInstanceOf(SocketTimeoutException.class) + .hasMessage("Command timeout limit was exceeded: 50"); + + reader.close(); + + verify(execChannel).connect(DEFAULT_TIMEOUT); + verify(execChannel).disconnect(); } @Test @@ -253,4 +403,39 @@ private JSch givenConnectingSshClient() throws IOException, JSchException { .thenReturn(sshSession); return sshClient; } + + private Device createConnectedDeviceForShellCommands() throws NetconfException { + return createConnectedDeviceForShellCommands(DEFAULT_TIMEOUT); + } + + private Device createConnectedDeviceForShellCommands(int commandTimeout) throws NetconfException { + Device device = Device.builder() + .hostName(TEST_HOSTNAME) + .userName(TEST_USERNAME) + .password(TEST_PASSWORD) + .strictHostKeyChecking(false) + .commandTimeout(commandTimeout) + .build(); + Session session = mock(Session.class); + ChannelSubsystem netconfChannel = mock(ChannelSubsystem.class); + when(session.isConnected()).thenReturn(true); + when(netconfChannel.isConnected()).thenReturn(true); + device.setSshSession(session); + device.setSshChannel(netconfChannel); + return device; + } + + private InputStream nonBlockingIdleInputStream() { + return new InputStream() { + @Override + public int read() { + return -1; + } + + @Override + public int available() { + return 0; + } + }; + } } From bdd1eca568e336ce998b67d245a856ae50d9eca0 Mon Sep 17 00:00:00 2001 From: Peter Hill Date: Fri, 24 Apr 2026 13:35:05 -0700 Subject: [PATCH 5/8] Add sequential RPC alignment tests for #68 - verify back-to-back executeRPC calls stay aligned with NETCONF 1.0 framing - verify back-to-back executeRPC calls stay aligned with NETCONF 1.1 chunked framing - document the issue scope as sequential session reuse rather than concurrent RPC support --- .../juniper/netconf/NetconfSessionTest.java | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/test/java/net/juniper/netconf/NetconfSessionTest.java b/src/test/java/net/juniper/netconf/NetconfSessionTest.java index 9592b1c..89e553e 100644 --- a/src/test/java/net/juniper/netconf/NetconfSessionTest.java +++ b/src/test/java/net/juniper/netconf/NetconfSessionTest.java @@ -296,6 +296,32 @@ public void executeRpcRunningStripsLegacyFramingAndStopsAfterSingleReply() throw .areIdentical(); } + @Test + public void sequentialExecuteRpcCallsStayAlignedWithLegacyFraming() throws Exception { + String firstReply = RpcReply.builder().ok(true).messageId("1").build().getXml(); + String secondReply = RpcReply.builder().ok(true).messageId("2").build().getXml(); + NetconfSession netconfSession = createNetconfSession( + new ByteArrayInputStream((createHelloMessage() + + NetconfConstants.DEVICE_PROMPT + + firstReply + + NetconfConstants.DEVICE_PROMPT + + secondReply + + NetconfConstants.DEVICE_PROMPT).getBytes(StandardCharsets.UTF_8)), + new ByteArrayOutputStream(), + 100 + ); + + XmlAssert.assertThat(netconfSession.executeRPC("").toString()) + .and(firstReply) + .ignoreWhitespace() + .areIdentical(); + + XmlAssert.assertThat(netconfSession.executeRPC("").toString()) + .and(secondReply) + .ignoreWhitespace() + .areIdentical(); + } + @Test public void closeOnChunkedExecuteRpcRunningDrainsCurrentReplyForNextRpc() throws Exception { String firstReply = """ @@ -329,6 +355,30 @@ public void closeOnChunkedExecuteRpcRunningDrainsCurrentReplyForNextRpc() throws .areIdentical(); } + @Test + public void sequentialExecuteRpcCallsStayAlignedWithChunkedFraming() throws Exception { + String firstReply = RpcReply.builder().ok(true).messageId("1").build().getXml(); + String secondReply = RpcReply.builder().ok(true).messageId("2").build().getXml(); + NetconfSession netconfSession = createNetconfSession( + new ByteArrayInputStream((createHelloMessageWithBase11() + + NetconfConstants.DEVICE_PROMPT + + toChunkedMessage(firstReply) + + toChunkedMessage(secondReply)).getBytes(StandardCharsets.UTF_8)), + new ByteArrayOutputStream(), + 100 + ); + + XmlAssert.assertThat(netconfSession.executeRPC("").toString()) + .and(firstReply) + .ignoreWhitespace() + .areIdentical(); + + XmlAssert.assertThat(netconfSession.executeRPC("").toString()) + .and(secondReply) + .ignoreWhitespace() + .areIdentical(); + } + @Test public void executeRpcAddsMessageIdToAttributedRpcEnvelopeWhenMissing() throws Exception { ByteArrayOutputStream capturedOutput = new ByteArrayOutputStream(); From 9ca7ad50f8581c3fdfe8b4e30d7cbb9eab494d6b Mon Sep 17 00:00:00 2001 From: Peter Hill Date: Fri, 24 Apr 2026 13:39:54 -0700 Subject: [PATCH 6/8] Document NetconfSession threading expectations - clarify that a NetconfSession is intended for sequential RPC use - state that concurrent in-flight RPCs on the same session are not guaranteed safe - recommend separate sessions when applications need concurrency --- src/main/java/net/juniper/netconf/NetconfSession.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/net/juniper/netconf/NetconfSession.java b/src/main/java/net/juniper/netconf/NetconfSession.java index 2580f4d..ed184a9 100644 --- a/src/main/java/net/juniper/netconf/NetconfSession.java +++ b/src/main/java/net/juniper/netconf/NetconfSession.java @@ -58,6 +58,12 @@ * {@link #commitConfirm(long, String)}, {@link #cancelCommit(String)}). *
  • Call {@link #close()} when finished to free resources.
    • * + *

    + * A {@code NetconfSession} should be treated as a single-threaded conversation + * with the server. The library supports sequential reuse of one session across + * multiple RPCs, but does not guarantee safety for multiple concurrent + * in-flight RPCs on the same session. Use separate NETCONF sessions when + * application-level concurrency is required. */ public class NetconfSession { From 07140d6f71de802f7abd787edfb6f1966e2d0149 Mon Sep 17 00:00:00 2001 From: Peter Hill Date: Fri, 24 Apr 2026 13:54:44 -0700 Subject: [PATCH 7/8] Document compatibility and upgrade AssertJ Add a maintainer-facing compatibility matrix covering NETCONF RFC support, capability caveats, NMDA coverage, notifications gaps, and Junos-specific helpers. Also align Maven and Gradle on assertj-core 3.27.7 to address CVE-2026-24400 in AssertJ's XML pretty-printing path while keeping the existing XMLUnit-based test style. Bump version to 2.2.1.0 --- README.md | 11 ++++++ build.gradle | 6 +-- docs/compatibility.md | 92 +++++++++++++++++++++++++++++++++++++++++++ pom.xml | 4 +- 4 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 docs/compatibility.md diff --git a/README.md b/README.md index f703b0a..4227f99 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,17 @@ User may download the source code and compile it with desired JDK version. ======= +v2.2.1 +------ +* Hardened NETCONF XML parsing against XXE and DTD-based attacks +* Fixed NETCONF RPC framing and `message-id` reply correlation for sequential session reuse +* Improved SSH/NETCONF session cleanup on failed connection or session initialization +* Fixed shell exec helpers so commands are set, channels are connected, and timeout/cleanup behavior is more predictable +* Fixed nested XML path construction in the XML helper +* Documented `NetconfSession` as a sequential request/response channel rather than a concurrent in-flight RPC transport +* Added [`docs/compatibility.md`](docs/compatibility.md) with current RFC, capability, NMDA, and extension support details +* Upgraded `assertj-core` to `3.27.7` to address `CVE-2026-24400` + v2.2.0 ------ * Java 17 baseline; compiled with `--release 17` diff --git a/build.gradle b/build.gradle index bf158a1..f8a110a 100644 --- a/build.gradle +++ b/build.gradle @@ -6,7 +6,7 @@ plugins { } group = 'net.juniper.netconf' -version = '2.2.0.0' +version = '2.2.1.0' description = 'An API For NetConf client' java { @@ -26,7 +26,7 @@ dependencies { implementation 'com.google.guava:guava:32.0.1-jre' testImplementation 'org.hamcrest:hamcrest-all:1.3' - testImplementation 'org.assertj:assertj-core:3.23.1' + testImplementation 'org.assertj:assertj-core:3.27.7' testImplementation 'org.mockito:mockito-core:4.8.1' testImplementation 'commons-io:commons-io:2.14.0' testImplementation 'org.xmlunit:xmlunit-assertj:2.9.0' @@ -104,4 +104,4 @@ publishing { } } } -} \ No newline at end of file +} diff --git a/docs/compatibility.md b/docs/compatibility.md new file mode 100644 index 0000000..c0a5749 --- /dev/null +++ b/docs/compatibility.md @@ -0,0 +1,92 @@ +# Compatibility Matrix + +This document describes what `netconf-java` currently implements. It is an implementation matrix, not a blanket compliance claim. Interoperability still depends on the server's advertised capabilities and on whether the application stays within the library's supported session model. + +Mental model: today the library is a synchronous, JSch-backed NETCONF-over-SSH client. Its strongest path is one sequential RPC conversation per `NetconfSession`, with explicit timeouts and explicit cleanup. Core NETCONF 1.0 and 1.1 framing is supported, several Junos workflows are wrapped ergonomically, and the main standards gap is that optional features are not yet enforced uniformly through capability negotiation. + +## Status legend + +| Status | Meaning | +| --- | --- | +| `Supported` | Implemented and intended for normal use. | +| `Supported with caveats` | Implemented, but there are negotiation, API-surface, or interoperability caveats. | +| `Partial` | Some pieces exist, but coverage is incomplete or not first-class. | +| `Not implemented` | No dedicated support today. | + +## Core NETCONF and transport + +| Standard / feature | Status | Notes | +| --- | --- | --- | +| RFC 6242 SSH subsystem transport | `Supported` | `Device` opens an SSH subsystem channel with `subsystem=netconf` over JSch. | +| RFC 6241 `` parsing and generation | `Supported with caveats` | `Hello` parsing/building works, and `Hello.builder()` auto-adds `urn:ietf:params:netconf:base:1.1`. The library does not currently fail the session if the peers do not share a common base capability. | +| NETCONF 1.0 end-of-message framing (`]]>]]>`) | `Supported` | Legacy framing is still supported for both outbound and inbound messages. | +| NETCONF 1.1 chunked framing | `Supported` | Chunked framing is enabled when the server `` advertises `urn:ietf:params:netconf:base:1.1`. | +| NETCONF 1.0 server interoperability | `Supported with caveats` | A 1.0-only server can interoperate because the client still advertises `base:1.0` and can read/write legacy framing. | +| NETCONF 1.0-only client advertisement | `Not implemented` | The client cannot intentionally advertise only `base:1.0`; `Hello.builder()` always injects `base:1.1`. | +| RPC `message-id` generation and reply correlation | `Supported with caveats` | Missing `message-id` attributes are injected, replies are validated, and sequential same-session alignment is covered by tests. One `NetconfSession` is still a sequential conversation, not a safe multiplexed channel for concurrent in-flight RPCs. | +| `` parsing | `Supported` | `RpcReply` parses `error-type`, `error-tag`, `error-severity`, `error-path`, `error-message`, and common `error-info` fields into structured objects. | +| `` | `Supported` | `NetconfSession.close()` sends `` and disconnects the channel. | +| `` | `Supported` | `NetconfSession.killSession(String)` is implemented. | +| Secure XML parsing | `Supported` | DTDs and XXE resolution are disabled for `Device`, `Hello`, and `RpcReply` parsing paths. | + +## Standard capabilities and common operations + +| Capability / operation | Status | Notes | +| --- | --- | --- | +| `` | `Supported` | `getRunningConfigAndState(...)` issues ``. | +| `` | `Supported` | Candidate and running helpers exist. | +| `` to candidate | `Supported with caveats` | `loadXMLConfiguration(...)` and `loadTextConfiguration(...)` target `candidate`. The API does not expose `test-option`, `error-option`, or capability-gated behavior checks before use. | +| `:candidate:1.0` | `Supported with caveats` | Candidate-oriented helpers are a primary workflow, but the default client capability still uses the legacy `urn:ietf:params:netconf:base:1.0#candidate` form rather than the RFC 6241 `urn:ietf:params:netconf:capability:candidate:1.0` URN. | +| `` | `Supported` | Standard commit is implemented. | +| `:confirmed-commit:1.1` | `Supported with caveats` | `commitConfirm(seconds, persistToken)` and `cancelCommit(persistId)` exist, but the default client capability still uses the legacy `base:1.0#confirmed-commit` form instead of the RFC 6241 capability URN. | +| `:validate:1.0` | `Supported with caveats` | `validate()` is implemented against candidate, but default advertisement still uses the legacy `base:1.0#validate` URI and the call is not capability-gated at runtime. | +| `` / `` | `Partial` | Candidate lock/unlock helpers exist. There is no first-class running datastore lock helper. | +| `:writable-running:1.0` | `Partial` | Running config retrieval exists, but there is no `edit-config` helper that targets `running` and the capability is not advertised by default. | +| `:startup:1.0` | `Not implemented` | No first-class startup datastore copy/delete flows are present. | +| `:url:1.0` | `Partial` | The default client capability list advertises the legacy `base:1.0#url?protocol=http,ftp,file` form, but there is no dedicated URL-based copy/load API surface. | +| `:xpath:1.0` | `Partial` | The library can pass caller-supplied filter XML through to `` and ``, but there is no typed XPath filter API and no runtime capability check. | +| `:rollback-on-error:1.0` | `Not implemented` | No API surface for `error-option rollback-on-error`. | +| `` | `Not implemented` | No dedicated helper today. | +| `` | `Not implemented` | No dedicated helper today. | +| `` | `Not implemented` | No dedicated helper today. | +| RFC 6243 `with-defaults` | `Not implemented` | No explicit support or helper surface. | + +## NMDA and additional datastore support + +| Standard / feature | Status | Notes | +| --- | --- | --- | +| RFC 8526 `` / `ietf-netconf-nmda` | `Supported with caveats` | `getData(xpathFilter, datastore)` emits NMDA namespace-qualified requests and can target `running`, `candidate`, `startup`, `intended`, or `operational`. The call is not capability-gated; the application must know the server supports NMDA. | +| RFC 8342 datastore naming | `Partial` | `Datastore` includes `RUNNING`, `CANDIDATE`, `STARTUP`, `INTENDED`, and `OPERATIONAL`, but only `getData(...)` uses that model directly. | + +## Notifications and subscriptions + +| Standard / feature | Status | Notes | +| --- | --- | --- | +| RFC 5277 event notifications | `Not implemented` | No `create-subscription`, no notification stream reader, and no event callback surface. | +| RFC 5277 `:interleave:1.0` | `Not implemented` | No interleaving of notifications with active RPC traffic. | + +## Junos-specific and non-standard helpers + +| Feature | Status | Notes | +| --- | --- | --- | +| Junos `` helpers | `Supported` | XML, text, and set-style configuration loading helpers are present. These are vendor-specific, not portable NETCONF. | +| Junos `` | `Supported` | `commitFull()` exists and is Junos-specific. | +| Junos CLI command helper | `Supported with caveats` | `runCliCommand(...)` and `runCliCommandRunning(...)` are implemented, but they are not portable to non-Junos servers. | +| Junos configuration mode helpers | `Supported` | `openConfiguration(...)` and `closeConfiguration()` are implemented. | +| SSH exec shell helpers | `Supported with caveats` | `runShellCommand(...)` and `runShellCommandRunning(...)` now connect and clean up channels correctly, and reads are bounded by `commandTimeout`. They still do not provide a richer per-command cancellation model beyond timeout and channel close. | +| Device reboot helper | `Supported with caveats` | `reboot()` exists as a convenience helper, but it is server-specific rather than a portable standards abstraction. | + +## Interoperability caveats worth knowing + +- Default optional capability advertisement still uses legacy `urn:ietf:params:netconf:base:1.0#...` forms in `Device.DEFAULT_CLIENT_CAPABILITIES`. Many servers accept these, but strict RFC 6241 capability matching may not. +- Capability negotiation is parsed but not enforced consistently before invoking optional operations. The caller can request candidate, validate, confirmed-commit, or NMDA operations even if the server never advertised them. +- `NetconfSession` should be treated as a single sequential request/response channel. Use separate sessions for concurrent workflows. +- The SSH transport is still tightly coupled to JSch. That preserves the current JSch-based deployment model, including existing FIPS-oriented environments, but transport abstraction is future work rather than current behavior. + +## Primary implementation points + +- [`Device`](../src/main/java/net/juniper/netconf/Device.java) - SSH transport setup, client capability advertisement, device-level helpers +- [`NetconfSession`](../src/main/java/net/juniper/netconf/NetconfSession.java) - NETCONF framing, message-id handling, core RPC helpers, session lifecycle +- [`Hello`](../src/main/java/net/juniper/netconf/element/Hello.java) - `` parsing and generation +- [`RpcReply`](../src/main/java/net/juniper/netconf/element/RpcReply.java) - `` and `` parsing +- [`Datastore`](../src/main/java/net/juniper/netconf/element/Datastore.java) - datastore enum used by NMDA-style `` diff --git a/pom.xml b/pom.xml index 7eb9718..9ae8483 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ net.juniper.netconf netconf-java - 2.2.0.0 + 2.2.1.0 jar @@ -190,7 +190,7 @@ org.assertj assertj-core - 3.23.1 + 3.27.7 test From 9cfb9501759c0495c8542145346c49a07256dc90 Mon Sep 17 00:00:00 2001 From: Peter Hill Date: Fri, 24 Apr 2026 20:06:02 -0700 Subject: [PATCH 8/8] feat: negotiate NETCONF capabilities and type rpc-errors Teach sessions to negotiate the shared NETCONF base capability and gate operations against what the server actually advertises. Add structured RpcErrorException and ValidateException flows, harden rpc-error parsing for namespaced replies, and update commit/load/validate behavior to preserve server detail. Back the change with unit and integration coverage, Javadocs for the new public API, and a sanitized integration runner/docs path story. --- .gitignore | 11 +- README.md | 37 +- build.gradle | 67 +++ docs/compatibility.md | 16 +- .../net/juniper/netconf/CommitException.java | 8 +- src/main/java/net/juniper/netconf/Device.java | 41 +- .../net/juniper/netconf/LoadException.java | 14 +- .../netconf/NegotiatedCapabilities.java | 339 ++++++++++++ .../net/juniper/netconf/NetconfSession.java | 164 ++++-- .../juniper/netconf/RpcErrorException.java | 176 ++++++ .../UnsupportedCapabilityException.java | 48 ++ .../juniper/netconf/ValidateException.java | 37 ++ .../net/juniper/netconf/element/RpcReply.java | 9 +- .../java/net/juniper/netconf/DeviceTest.java | 85 ++- .../netconf/NegotiatedCapabilitiesTest.java | 121 +++++ .../juniper/netconf/NetconfSessionTest.java | 507 +++++++++++++++--- .../juniper/netconf/element/RpcReplyTest.java | 39 ++ .../netconf/integration/INTEGRATION_TESTS.md | 185 +++++-- .../integration/NetconfIntegrationTest.java | 310 ++++++++--- .../netconf/integration/RUN-CRPD-CONTAINER.md | 220 ++++++-- .../integration/run-integration-tests.sh | 66 +-- 21 files changed, 2202 insertions(+), 298 deletions(-) create mode 100644 src/main/java/net/juniper/netconf/NegotiatedCapabilities.java create mode 100644 src/main/java/net/juniper/netconf/RpcErrorException.java create mode 100644 src/main/java/net/juniper/netconf/UnsupportedCapabilityException.java create mode 100644 src/main/java/net/juniper/netconf/ValidateException.java create mode 100644 src/test/java/net/juniper/netconf/NegotiatedCapabilitiesTest.java diff --git a/.gitignore b/.gitignore index 241c1af..7755938 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ target *.iml .idea/ .gradle +.gradle-user-home/ gradle/* !gradle/wrapper/ !gradle/wrapper/gradle-wrapper.jar @@ -27,4 +28,12 @@ log-test/ .DS_Store .factorypath -settings.json \ No newline at end of file +settings.json + +# Generated integration-test container image artifacts +/src/test/resources/*-docker-*.tgz +/src/test/resources/manifest.json +/src/test/resources/repositories +/src/test/resources/[0-9a-f]*.json +/src/test/resources/[0-9a-f]*.tar +/src/test/resources/[0-9a-f]*/ diff --git a/README.md b/README.md index 4227f99..f3afc3c 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,16 @@ mvn clean package ``` (The wrapper script downloads the correct Gradle version automatically.) +To run the live NETCONF integration suite with Gradle: + +```bash +NETCONF_HOST=192.168.1.1 \ +NETCONF_USERNAME=admin \ +NETCONF_PASSWORD=secret \ +NETCONF_PORT=830 \ +./gradlew integrationTest +``` + Releases ======== Releases contain source code only. Due to changing JDK licensing, jar files are not released. @@ -47,7 +57,7 @@ User may download the source code and compile it with desired JDK version. * Instructions to build using `mvn` * Download Source Code for the required release * Compile the code and build the jar using `mvn package` - * Use the jar file from (source to netconf-java)/netconf-java/target + * Use the jar file from `./target/` * Use `mvn versions:display-dependency-updates` to identify possible target versions for dependencies ======= @@ -56,11 +66,17 @@ v2.2.1 ------ * Hardened NETCONF XML parsing against XXE and DTD-based attacks * Fixed NETCONF RPC framing and `message-id` reply correlation for sequential session reuse +* Enforced shared NETCONF base capability negotiation and derive session framing from the negotiated base version +* Capability-gated candidate, validate, and confirmed-commit operations before sending RPCs +* Added negotiated capability inspection via `Device.getNegotiatedCapabilities()` and `NetconfSession.getNegotiatedCapabilities()` +* Typed NETCONF `` replies as structured exceptions so callers can inspect server-reported error details +* Added `ValidateException` and clarified `validate()` semantics: server `rpc-error` replies throw, while warning-only or other non-`` non-error replies still return `false` * Improved SSH/NETCONF session cleanup on failed connection or session initialization * Fixed shell exec helpers so commands are set, channels are connected, and timeout/cleanup behavior is more predictable * Fixed nested XML path construction in the XML helper * Documented `NetconfSession` as a sequential request/response channel rather than a concurrent in-flight RPC transport * Added [`docs/compatibility.md`](docs/compatibility.md) with current RFC, capability, NMDA, and extension support details +* Added a dedicated Gradle `integrationTest` task that forwards NETCONF connection settings for live-server testing * Upgraded `assertj-core` to `3.27.7` to address `CVE-2026-24400` v2.2.0 @@ -107,6 +123,7 @@ SYNOPSIS import java.io.IOException; import javax.xml.parsers.ParserConfigurationException; import net.juniper.netconf.NetconfException; +import net.juniper.netconf.ValidateException; import org.xml.sax.SAXException; import net.juniper.netconf.XML; @@ -142,12 +159,30 @@ public class ShowInterfaces { } ``` +Candidate validate example: + +```Java +try { + boolean clean = device.validate(); + if (!clean) { + // Warning-only or other non-error, non- reply. + System.out.println("Validate completed without rpc-error, but did not return "); + } +} catch (ValidateException e) { + // Server returned one or more elements. + System.err.println("Validate failed: " + e.getMessage()); + e.getRpcErrors().forEach(System.err::println); +} +``` + Recommended usage: * Build one `Device` per target connection and use `try-with-resources` so SSH resources are released predictably. * Call `connect()` before issuing RPCs. If `connect()` throws, no usable NETCONF session was established. +* Inspect `getNegotiatedCapabilities()` after `connect()` if your application needs to branch on server support for candidate, validate, or confirmed-commit behavior. * Set `connectionTimeout` and `commandTimeout` explicitly for production use rather than relying on defaults. * Prefer NETCONF RPC helpers (`executeRPC`, `getConfig`, `loadXMLConfiguration`, `commit`, and friends) for device operations; use shell helpers only for device-specific workflows that are not available over NETCONF. +* Treat `ValidateException` as the server-side `rpc-error` path for `validate()`. A `false` return now means the reply was non-error but not a clean ``, typically warnings. * Shell helper reads are bounded by `commandTimeout`. If you use `runShellCommandRunning(...)`, always close the returned reader so the underlying exec channel is released. LICENSE diff --git a/build.gradle b/build.gradle index f8a110a..f33ed71 100644 --- a/build.gradle +++ b/build.gradle @@ -50,6 +50,73 @@ test { } } +def netconfPropertySpecs = [ + [name: 'netconf.host', env: 'NETCONF_HOST', required: true], + [name: 'netconf.username', env: 'NETCONF_USERNAME', required: true], + [name: 'netconf.password', env: 'NETCONF_PASSWORD', required: true], + [name: 'netconf.port', env: 'NETCONF_PORT', required: false, defaultValue: '830'], + [name: 'netconf.timeout', env: 'NETCONF_TIMEOUT', required: false, defaultValue: '30000'] +] + +def resolveNetconfProperty = { Map spec -> + if (project.hasProperty(spec.name)) { + return project.property(spec.name).toString() + } + if (System.getProperty(spec.name) != null) { + return System.getProperty(spec.name) + } + if (System.getenv(spec.env) != null) { + return System.getenv(spec.env) + } + return spec.defaultValue +} + +tasks.register('integrationTest', org.gradle.api.tasks.testing.Test) { + description = 'Runs live NETCONF integration tests against a configured endpoint.' + group = 'verification' + useJUnitPlatform() + shouldRunAfter test + testClassesDirs = sourceSets.test.output.classesDirs + classpath = sourceSets.test.runtimeClasspath + + filter { + includeTestsMatching 'net.juniper.netconf.integration.NetconfIntegrationTest' + } + + testLogging { + events "passed", "skipped", "failed" + } + + doFirst { + def resolvedProperties = [:] + def missingRequiredProperties = [] + + netconfPropertySpecs.each { spec -> + def value = resolveNetconfProperty(spec) + if (value == null || value.toString().trim().isEmpty()) { + if (spec.required) { + missingRequiredProperties << "${spec.name} (${spec.env})" + } + return + } + resolvedProperties[spec.name] = value.toString() + } + + if (!missingRequiredProperties.isEmpty()) { + throw new org.gradle.api.GradleException( + "integrationTest requires live NETCONF connection details. " + + "Provide ${missingRequiredProperties.join(', ')} via -Dnetconf.* " + + "or NETCONF_* environment variables." + ) + } + + systemProperty 'netconf.integration.enabled', 'true' + resolvedProperties.each { propertyName, propertyValue -> + systemProperty propertyName, propertyValue + } + } +} + tasks.withType(JavaCompile).configureEach { options.fork = true options.forkOptions.jvmArgs += [ diff --git a/docs/compatibility.md b/docs/compatibility.md index c0a5749..8015c3a 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -2,7 +2,7 @@ This document describes what `netconf-java` currently implements. It is an implementation matrix, not a blanket compliance claim. Interoperability still depends on the server's advertised capabilities and on whether the application stays within the library's supported session model. -Mental model: today the library is a synchronous, JSch-backed NETCONF-over-SSH client. Its strongest path is one sequential RPC conversation per `NetconfSession`, with explicit timeouts and explicit cleanup. Core NETCONF 1.0 and 1.1 framing is supported, several Junos workflows are wrapped ergonomically, and the main standards gap is that optional features are not yet enforced uniformly through capability negotiation. +Mental model: today the library is a synchronous, JSch-backed NETCONF-over-SSH client. Its strongest path is one sequential RPC conversation per `NetconfSession`, with explicit timeouts and explicit cleanup. Core NETCONF 1.0 and 1.1 framing is supported, several Junos workflows are wrapped ergonomically, and optional features are beginning to be enforced through capability negotiation, though coverage is not yet uniform across all extensions. ## Status legend @@ -18,9 +18,9 @@ Mental model: today the library is a synchronous, JSch-backed NETCONF-over-SSH c | Standard / feature | Status | Notes | | --- | --- | --- | | RFC 6242 SSH subsystem transport | `Supported` | `Device` opens an SSH subsystem channel with `subsystem=netconf` over JSch. | -| RFC 6241 `` parsing and generation | `Supported with caveats` | `Hello` parsing/building works, and `Hello.builder()` auto-adds `urn:ietf:params:netconf:base:1.1`. The library does not currently fail the session if the peers do not share a common base capability. | +| RFC 6241 `` parsing and generation | `Supported with caveats` | `Hello` parsing/building works, `Hello.builder()` auto-adds `urn:ietf:params:netconf:base:1.1`, and session establishment now fails if the peers do not share a common NETCONF base capability. The remaining caveat is that the client still cannot intentionally advertise only `base:1.0`. | | NETCONF 1.0 end-of-message framing (`]]>]]>`) | `Supported` | Legacy framing is still supported for both outbound and inbound messages. | -| NETCONF 1.1 chunked framing | `Supported` | Chunked framing is enabled when the server `` advertises `urn:ietf:params:netconf:base:1.1`. | +| NETCONF 1.1 chunked framing | `Supported` | Chunked framing is selected when both peers share `urn:ietf:params:netconf:base:1.1`. | | NETCONF 1.0 server interoperability | `Supported with caveats` | A 1.0-only server can interoperate because the client still advertises `base:1.0` and can read/write legacy framing. | | NETCONF 1.0-only client advertisement | `Not implemented` | The client cannot intentionally advertise only `base:1.0`; `Hello.builder()` always injects `base:1.1`. | | RPC `message-id` generation and reply correlation | `Supported with caveats` | Missing `message-id` attributes are injected, replies are validated, and sequential same-session alignment is covered by tests. One `NetconfSession` is still a sequential conversation, not a safe multiplexed channel for concurrent in-flight RPCs. | @@ -35,11 +35,11 @@ Mental model: today the library is a synchronous, JSch-backed NETCONF-over-SSH c | --- | --- | --- | | `` | `Supported` | `getRunningConfigAndState(...)` issues ``. | | `` | `Supported` | Candidate and running helpers exist. | -| `` to candidate | `Supported with caveats` | `loadXMLConfiguration(...)` and `loadTextConfiguration(...)` target `candidate`. The API does not expose `test-option`, `error-option`, or capability-gated behavior checks before use. | -| `:candidate:1.0` | `Supported with caveats` | Candidate-oriented helpers are a primary workflow, but the default client capability still uses the legacy `urn:ietf:params:netconf:base:1.0#candidate` form rather than the RFC 6241 `urn:ietf:params:netconf:capability:candidate:1.0` URN. | +| `` to candidate | `Supported with caveats` | `loadXMLConfiguration(...)` and `loadTextConfiguration(...)` target `candidate`, and candidate-dependent operations now fail locally when the server did not advertise candidate support. The API still does not expose `test-option` or `error-option`. | +| `:candidate:1.0` | `Supported with caveats` | Candidate-oriented helpers are a primary workflow and are now runtime-gated against the server ``, but the default client capability still uses the legacy `urn:ietf:params:netconf:base:1.0#candidate` form rather than the RFC 6241 `urn:ietf:params:netconf:capability:candidate:1.0` URN. | | `` | `Supported` | Standard commit is implemented. | -| `:confirmed-commit:1.1` | `Supported with caveats` | `commitConfirm(seconds, persistToken)` and `cancelCommit(persistId)` exist, but the default client capability still uses the legacy `base:1.0#confirmed-commit` form instead of the RFC 6241 capability URN. | -| `:validate:1.0` | `Supported with caveats` | `validate()` is implemented against candidate, but default advertisement still uses the legacy `base:1.0#validate` URI and the call is not capability-gated at runtime. | +| `:confirmed-commit:1.1` | `Supported with caveats` | `commitConfirm(seconds, persistToken)` and `cancelCommit(persistId)` are runtime-gated. Persist-based flows require modern `confirmed-commit:1.1`, while legacy confirmed-commit remains usable for same-session confirmation flows without `persist`. The default client capability advertisement still uses the legacy `base:1.0#confirmed-commit` form. | +| `:validate:1.0` | `Supported with caveats` | `validate()` is implemented against candidate and now fails locally when validate support is absent, but default advertisement still uses the legacy `base:1.0#validate` URI. | | `` / `` | `Partial` | Candidate lock/unlock helpers exist. There is no first-class running datastore lock helper. | | `:writable-running:1.0` | `Partial` | Running config retrieval exists, but there is no `edit-config` helper that targets `running` and the capability is not advertised by default. | | `:startup:1.0` | `Not implemented` | No first-class startup datastore copy/delete flows are present. | @@ -79,7 +79,7 @@ Mental model: today the library is a synchronous, JSch-backed NETCONF-over-SSH c ## Interoperability caveats worth knowing - Default optional capability advertisement still uses legacy `urn:ietf:params:netconf:base:1.0#...` forms in `Device.DEFAULT_CLIENT_CAPABILITIES`. Many servers accept these, but strict RFC 6241 capability matching may not. -- Capability negotiation is parsed but not enforced consistently before invoking optional operations. The caller can request candidate, validate, confirmed-commit, or NMDA operations even if the server never advertised them. +- Candidate, validate, and confirmed-commit flows are now capability-gated before the RPC is sent. Other optional operations are still not enforced uniformly, especially outside the classic RFC 6241 capability set. - `NetconfSession` should be treated as a single sequential request/response channel. Use separate sessions for concurrent workflows. - The SSH transport is still tightly coupled to JSch. That preserves the current JSch-based deployment model, including existing FIPS-oriented environments, but transport abstraction is future work rather than current behavior. diff --git a/src/main/java/net/juniper/netconf/CommitException.java b/src/main/java/net/juniper/netconf/CommitException.java index 5e13523..e60ea21 100644 --- a/src/main/java/net/juniper/netconf/CommitException.java +++ b/src/main/java/net/juniper/netconf/CommitException.java @@ -8,13 +8,17 @@ package net.juniper.netconf; -import java.io.IOException; +import net.juniper.netconf.element.RpcReply; /** * Describes exceptions related to commit operation */ -public class CommitException extends IOException { +public class CommitException extends RpcErrorException { CommitException(String msg) { super(msg); } + + CommitException(String msg, RpcReply rpcReply) { + super(msg, rpcReply); + } } diff --git a/src/main/java/net/juniper/netconf/Device.java b/src/main/java/net/juniper/netconf/Device.java index e4f5211..07dda16 100644 --- a/src/main/java/net/juniper/netconf/Device.java +++ b/src/main/java/net/juniper/netconf/Device.java @@ -90,6 +90,7 @@ public class Device implements AutoCloseable { private final DocumentBuilder xmlBuilder; private final List netconfCapabilities; + private final List advertisedNetconfCapabilities; private final String helloRpc; private ChannelSubsystem sshChannel; @@ -373,7 +374,9 @@ private Device(Builder b) throws NetconfException { throw new NetconfException("Cannot create XML Parser", e); } - this.helloRpc = createHelloRPC(this.netconfCapabilities); + Hello hello = createHello(this.netconfCapabilities); + this.advertisedNetconfCapabilities = hello.getCapabilities(); + this.helloRpc = createHelloRPC(hello); } @@ -385,7 +388,7 @@ private Device(Builder b) throws NetconfException { * @return List of default client capabilities. */ protected List getDefaultClientCapabilities() { - return DEFAULT_CLIENT_CAPABILITIES; + return createHello(DEFAULT_CLIENT_CAPABILITIES).getCapabilities(); } /** @@ -395,12 +398,14 @@ protected List getDefaultClientCapabilities() { * @param capabilities A list of netconf capabilities * @return the hello RPC that represents those capabilities. */ - private String createHelloRPC(List capabilities) { + private Hello createHello(List capabilities) { return Hello.builder() .capabilities(capabilities) - .build() - .getXml() - + NetconfConstants.DEVICE_PROMPT; + .build(); + } + + private String createHelloRPC(Hello hello) { + return hello.getXml() + NetconfConstants.DEVICE_PROMPT; } /** @@ -450,7 +455,8 @@ private NetconfSession createNetconfSession() throws NetconfException { channel = (ChannelSubsystem) session.openChannel("subsystem"); channel.setSubsystem("netconf"); NetconfSession establishedSession = - new NetconfSession(channel, connectionTimeout, commandTimeout, helloRpc, xmlBuilder); + new NetconfSession(channel, connectionTimeout, commandTimeout, + advertisedNetconfCapabilities, helloRpc, xmlBuilder); sshSession = session; sshChannel = channel; return establishedSession; @@ -880,6 +886,20 @@ public String getSessionId() { return this.netconfSession.getSessionId(); } + /** + * Returns the negotiated capability view for the active NETCONF session. + * + * @return immutable capability snapshot + * @throws IllegalStateException if the connection is not established + */ + public NegotiatedCapabilities getNegotiatedCapabilities() { + if (netconfSession == null) { + throw new IllegalStateException("Cannot get negotiated capabilities, you need " + + "to establish a connection first."); + } + return this.netconfSession.getNegotiatedCapabilities(); + } + /** * Check if the last RPC reply returned from Netconf server has any error. * @@ -1267,8 +1287,13 @@ public XML getRunningConfig() throws SAXException, IOException { /** * Validate the candidate configuration. * - * @return true if validation successful. + * @return {@code true} if validation completed with a clean {@code } + * reply; {@code false} for warning-only or other non-error + * non-{@code } replies * @throws java.io.IOException If there are errors communicating with the netconf server. + * @throws net.juniper.netconf.ValidateException if the server returns one + * or more {@code } + * elements * @throws org.xml.sax.SAXException If there are errors parsing the XML reply. */ public boolean validate() throws IOException, SAXException { diff --git a/src/main/java/net/juniper/netconf/LoadException.java b/src/main/java/net/juniper/netconf/LoadException.java index 253d012..1b55e9a 100644 --- a/src/main/java/net/juniper/netconf/LoadException.java +++ b/src/main/java/net/juniper/netconf/LoadException.java @@ -7,7 +7,7 @@ package net.juniper.netconf; -import java.io.IOException; +import net.juniper.netconf.element.RpcReply; /** * Exception thrown when a load RPC returns <rpc-error> or otherwise @@ -20,7 +20,7 @@ *

  • just the root cause.
    • * */ -public class LoadException extends IOException { +public class LoadException extends RpcErrorException { /** * Creates a {@code LoadException} with the supplied message. @@ -41,6 +41,16 @@ public LoadException(String message, Throwable cause) { super(message, cause); } + /** + * Creates a {@code LoadException} with a message and parsed reply. + * + * @param message description of the load failure + * @param rpcReply parsed NETCONF reply that triggered the failure + */ + public LoadException(String message, RpcReply rpcReply) { + super(message, rpcReply); + } + /** * Creates a {@code LoadException} that wraps an underlying cause. * diff --git a/src/main/java/net/juniper/netconf/NegotiatedCapabilities.java b/src/main/java/net/juniper/netconf/NegotiatedCapabilities.java new file mode 100644 index 0000000..8f62b58 --- /dev/null +++ b/src/main/java/net/juniper/netconf/NegotiatedCapabilities.java @@ -0,0 +1,339 @@ +package net.juniper.netconf; + +import java.util.List; +import java.util.Objects; + +/** + * Immutable view of the capabilities available to a NETCONF session. + *

    + * The base protocol version is negotiated from both peers' {@code } + * messages. Optional capabilities reflect what the server advertised, while + * the raw client and server capability lists are preserved for inspection. + */ +public final class NegotiatedCapabilities { + + /** + * Negotiated NETCONF base version for the session. + */ + public enum BaseVersion { + /** + * NETCONF 1.0 session using end-of-message delimiter framing. + */ + NETCONF_1_0, + /** + * NETCONF 1.1 session using RFC 6242 chunked framing. + */ + NETCONF_1_1 + } + + private static final String LEGACY_CAPABILITY_PREFIX = + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0 + "#"; + private static final String MODERN_CAPABILITY_PREFIX = + "urn:ietf:params:netconf:capability:"; + private static final String MODERN_CANDIDATE_PREFIX = MODERN_CAPABILITY_PREFIX + "candidate:"; + private static final String MODERN_VALIDATE_PREFIX = MODERN_CAPABILITY_PREFIX + "validate:"; + private static final String MODERN_CONFIRMED_COMMIT_PREFIX = MODERN_CAPABILITY_PREFIX + "confirmed-commit:"; + private static final String MODERN_WRITABLE_RUNNING_PREFIX = MODERN_CAPABILITY_PREFIX + "writable-running:"; + private static final String MODERN_STARTUP_PREFIX = MODERN_CAPABILITY_PREFIX + "startup:"; + private static final String MODERN_URL_PREFIX = MODERN_CAPABILITY_PREFIX + "url:"; + private static final String MODERN_XPATH_PREFIX = MODERN_CAPABILITY_PREFIX + "xpath:"; + private static final String MODERN_CONFIRMED_COMMIT_1_1 = + MODERN_CONFIRMED_COMMIT_PREFIX + "1.1"; + + private final List clientCapabilities; + private final List serverCapabilities; + private final BaseVersion baseVersion; + private final boolean candidate; + private final boolean validate; + private final boolean confirmedCommit; + private final boolean confirmedCommit11; + private final boolean writableRunning; + private final boolean startup; + private final boolean url; + private final boolean xpath; + + private NegotiatedCapabilities( + List clientCapabilities, + List serverCapabilities, + BaseVersion baseVersion, + boolean candidate, + boolean validate, + boolean confirmedCommit, + boolean confirmedCommit11, + boolean writableRunning, + boolean startup, + boolean url, + boolean xpath + ) { + this.clientCapabilities = List.copyOf(clientCapabilities); + this.serverCapabilities = List.copyOf(serverCapabilities); + this.baseVersion = Objects.requireNonNull(baseVersion, "baseVersion"); + this.candidate = candidate; + this.validate = validate; + this.confirmedCommit = confirmedCommit; + this.confirmedCommit11 = confirmedCommit11; + this.writableRunning = writableRunning; + this.startup = startup; + this.url = url; + this.xpath = xpath; + } + + static NegotiatedCapabilities fromCapabilities( + List clientCapabilities, + List serverCapabilities + ) throws NetconfException { + List safeClientCapabilities = + clientCapabilities == null ? List.of() : List.copyOf(clientCapabilities); + List safeServerCapabilities = + serverCapabilities == null ? List.of() : List.copyOf(serverCapabilities); + + BaseVersion negotiatedBaseVersion = + negotiateBaseVersion(safeClientCapabilities, safeServerCapabilities); + + return new NegotiatedCapabilities( + safeClientCapabilities, + safeServerCapabilities, + negotiatedBaseVersion, + hasLegacyCapabilityOrModernPrefix( + safeServerCapabilities, + LEGACY_CAPABILITY_PREFIX + "candidate", + MODERN_CANDIDATE_PREFIX + ), + hasLegacyCapabilityOrModernPrefix( + safeServerCapabilities, + LEGACY_CAPABILITY_PREFIX + "validate", + MODERN_VALIDATE_PREFIX + ), + hasLegacyCapabilityOrModernPrefix( + safeServerCapabilities, + LEGACY_CAPABILITY_PREFIX + "confirmed-commit", + MODERN_CONFIRMED_COMMIT_PREFIX + ), + safeServerCapabilities.contains(MODERN_CONFIRMED_COMMIT_1_1), + hasLegacyCapabilityOrModernPrefix( + safeServerCapabilities, + LEGACY_CAPABILITY_PREFIX + "writable-running", + MODERN_WRITABLE_RUNNING_PREFIX + ), + hasLegacyCapabilityOrModernPrefix( + safeServerCapabilities, + LEGACY_CAPABILITY_PREFIX + "startup", + MODERN_STARTUP_PREFIX + ), + hasLegacyOrModernPrefix( + safeServerCapabilities, + LEGACY_CAPABILITY_PREFIX + "url", + MODERN_URL_PREFIX + ), + hasLegacyCapabilityOrModernPrefix( + safeServerCapabilities, + LEGACY_CAPABILITY_PREFIX + "xpath", + MODERN_XPATH_PREFIX + ) + ); + } + + private static BaseVersion negotiateBaseVersion( + List clientCapabilities, + List serverCapabilities + ) throws NetconfException { + boolean clientSupportsBase11 = + clientCapabilities.contains(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1); + boolean serverSupportsBase11 = + serverCapabilities.contains(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1); + if (clientSupportsBase11 && serverSupportsBase11) { + return BaseVersion.NETCONF_1_1; + } + + boolean clientSupportsBase10 = + clientCapabilities.contains(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0); + boolean serverSupportsBase10 = + serverCapabilities.contains(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0); + if (clientSupportsBase10 && serverSupportsBase10) { + return BaseVersion.NETCONF_1_0; + } + + throw new NetconfException( + "Client and server do not share a common NETCONF base capability. client=" + + clientCapabilities + ", server=" + serverCapabilities + ); + } + + private static boolean hasLegacyCapabilityOrModernPrefix( + List capabilities, + String legacyCapability, + String modernPrefix + ) { + return capabilities.stream() + .anyMatch(capability -> capability.equals(legacyCapability) + || capability.startsWith(modernPrefix)); + } + + private static boolean hasLegacyOrModernPrefix( + List capabilities, + String legacyPrefix, + String modernPrefix + ) { + return capabilities.stream() + .anyMatch(capability -> capability.startsWith(legacyPrefix) + || capability.startsWith(modernPrefix)); + } + + /** + * Returns the negotiated NETCONF base version. + * + * @return negotiated base version shared by the client and server + */ + public BaseVersion getBaseVersion() { + return baseVersion; + } + + /** + * Indicates whether the session should use NETCONF 1.1 chunked framing. + * + * @return {@code true} for NETCONF 1.1 sessions, {@code false} for 1.0 + */ + public boolean usesChunkedFraming() { + return baseVersion == BaseVersion.NETCONF_1_1; + } + + /** + * Indicates whether the server advertised candidate datastore support. + * + * @return {@code true} if candidate operations are supported + */ + public boolean supportsCandidate() { + return candidate; + } + + /** + * Indicates whether the server advertised validate support. + * + * @return {@code true} if {@code } is supported + */ + public boolean supportsValidate() { + return validate; + } + + /** + * Indicates whether the server advertised confirmed-commit support. + * + * @return {@code true} if confirmed commit is supported + */ + public boolean supportsConfirmedCommit() { + return confirmedCommit; + } + + /** + * Indicates whether the server advertised confirmed-commit 1.1 support. + * + * @return {@code true} if persist/persist-id confirmed commit is supported + */ + public boolean supportsConfirmedCommit11() { + return confirmedCommit11; + } + + /** + * Indicates whether the server advertised writable-running support. + * + * @return {@code true} if direct running datastore edits are supported + */ + public boolean supportsWritableRunning() { + return writableRunning; + } + + /** + * Indicates whether the server advertised startup datastore support. + * + * @return {@code true} if startup datastore operations are supported + */ + public boolean supportsStartup() { + return startup; + } + + /** + * Indicates whether the server advertised URL capability support. + * + * @return {@code true} if URL-based operations are supported + */ + public boolean supportsUrl() { + return url; + } + + /** + * Indicates whether the server advertised XPath filtering support. + * + * @return {@code true} if XPath filters are supported + */ + public boolean supportsXPath() { + return xpath; + } + + /** + * Returns the client capability list used during the hello exchange. + * + * @return immutable list of client-advertised capabilities + */ + public List getClientCapabilities() { + return clientCapabilities; + } + + /** + * Returns the capability list advertised by the server hello. + * + * @return immutable list of server-advertised capabilities + */ + public List getServerCapabilities() { + return serverCapabilities; + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof NegotiatedCapabilities that)) { + return false; + } + return candidate == that.candidate + && validate == that.validate + && confirmedCommit == that.confirmedCommit + && confirmedCommit11 == that.confirmedCommit11 + && writableRunning == that.writableRunning + && startup == that.startup + && url == that.url + && xpath == that.xpath + && clientCapabilities.equals(that.clientCapabilities) + && serverCapabilities.equals(that.serverCapabilities) + && baseVersion == that.baseVersion; + } + + @Override + public int hashCode() { + return Objects.hash( + clientCapabilities, + serverCapabilities, + baseVersion, + candidate, + validate, + confirmedCommit, + confirmedCommit11, + writableRunning, + startup, + url, + xpath + ); + } + + @Override + public String toString() { + return "NegotiatedCapabilities{" + + "baseVersion=" + baseVersion + + ", candidate=" + candidate + + ", validate=" + validate + + ", confirmedCommit=" + confirmedCommit + + ", confirmedCommit11=" + confirmedCommit11 + + ", writableRunning=" + writableRunning + + ", startup=" + startup + + ", url=" + url + + ", xpath=" + xpath + + '}'; + } +} diff --git a/src/main/java/net/juniper/netconf/NetconfSession.java b/src/main/java/net/juniper/netconf/NetconfSession.java index ed184a9..fa1bcff 100644 --- a/src/main/java/net/juniper/netconf/NetconfSession.java +++ b/src/main/java/net/juniper/netconf/NetconfSession.java @@ -104,19 +104,33 @@ public class NetconfSession { private static final Pattern XML_DECLARATION_PATTERN = Pattern.compile("^<\\?xml[^>]*\\?>\\s*", Pattern.DOTALL); + private final List clientCapabilities; private boolean useChunkedFraming; + private NegotiatedCapabilities negotiatedCapabilities; private byte[] unreadBytes = new byte[0]; private record PreparedRpc(String xml, String messageId) { } NetconfSession(Channel netconfChannel, int timeout, String hello, DocumentBuilder builder) throws IOException { - this(netconfChannel, timeout, timeout, hello, builder); + this(netconfChannel, timeout, timeout, defaultClientCapabilities(), hello, builder); } NetconfSession(Channel netconfChannel, int connectionTimeout, int commandTimeout, String hello, DocumentBuilder builder) throws IOException { + this(netconfChannel, connectionTimeout, commandTimeout, defaultClientCapabilities(), hello, builder); + } + + NetconfSession(Channel netconfChannel, int timeout, List clientCapabilities, + String hello, DocumentBuilder builder) throws IOException { + this(netconfChannel, timeout, timeout, clientCapabilities, hello, builder); + } + + NetconfSession(Channel netconfChannel, int connectionTimeout, int commandTimeout, + List clientCapabilities, + String hello, + DocumentBuilder builder) throws IOException { stdInStreamFromDevice = netconfChannel.getInputStream(); stdOutStreamToDevice = netconfChannel.getOutputStream(); @@ -129,10 +143,18 @@ private record PreparedRpc(String xml, String messageId) { } this.netconfChannel = netconfChannel; this.commandTimeout = commandTimeout; this.builder = builder; + this.clientCapabilities = clientCapabilities == null ? List.of() : List.copyOf(clientCapabilities); sendHello(hello); } + private static List defaultClientCapabilities() { + return List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1 + ); + } + private XML convertToXML(String xml) throws SAXException, IOException { if (xml.contains(NETCONF_SYNTAX_ERROR_MSG_FROM_DEVICE)) { throw new NetconfException(String.format("Netconf server detected an error: %s", xml)); @@ -150,7 +172,7 @@ private void sendHello(String hello) throws IOException { String getRpcReply(String rpc) throws IOException { PreparedRpc preparedRpc = sendRpcRequest(rpc); String reply; - if (startsWithChunkedFraming()) { + if (useChunkedFraming) { reply = readChunkedRpcReply(); } else { reply = readLegacyRpcReply(); @@ -290,6 +312,7 @@ public String getRpcAttributes() { * @throws java.io.IOException If there are issues reading the config file. */ public void loadXMLConfiguration(String configuration, String loadType) throws IOException, SAXException { + requireCandidateCapability("loadXMLConfiguration"); validateLoadType(loadType); configuration = configuration.trim(); if (!configuration.startsWith(" message from server: " + reply, e); } @@ -358,6 +386,7 @@ private void setLastRpcReply(final String reply) throws IOException { * @throws java.io.IOException If there are issues reading the config file. */ public void loadTextConfiguration(String configuration, String loadType) throws IOException, SAXException { + requireCandidateCapability("loadTextConfiguration"); String rpc = "" + "" + "" + @@ -381,11 +410,15 @@ public void loadTextConfiguration(String configuration, String loadType) throws } if (hasError() || !isOK()) { - throw new LoadException("Load operation returned error"); + throw new LoadException( + RpcErrorException.buildMessage("Load operation", lastRpcReplyObject), + lastRpcReplyObject + ); } } private String getConfig(String configTree) throws IOException { + requireCandidateCapability("getCandidateConfig"); String rpc = "" + "" + @@ -486,18 +519,27 @@ public Hello getServerHello() { return serverHello; } + /** + * Returns the negotiated capability view for this session. + * + * @return immutable capability snapshot + */ + public NegotiatedCapabilities getNegotiatedCapabilities() { + return negotiatedCapabilities; + } + /** * Sends a raw RPC string, waits for the {@code <rpc-reply>}, and converts the * response to an {@link XML} object. *

    * If the server includes any {@code <rpc-error>} elements, the call - * now throws a {@link NetconfException}. This makes error handling + * throws a {@link RpcErrorException}. This makes error handling * symmetrical with other high‑level helpers (e.g. {@code load*()}, * {@code commit()}). * * @param rpcContent the RPC payload (with or without <rpc> wrapper) * @return parsed {@link XML} representation of the reply - * @throws NetconfException if the reply contains one or more + * @throws RpcErrorException if the reply contains one or more * {@code <rpc-error>} elements * @throws SAXException if the reply cannot be parsed as XML * @throws IOException on transport errors @@ -507,7 +549,10 @@ public XML executeRPC(String rpcContent) throws SAXException, IOException { setLastRpcReply(rpcReply); if (hasError()) { - throw new NetconfException("RPC returned error: " + rpcReply); + throw new RpcErrorException( + RpcErrorException.buildMessage("RPC execution", lastRpcReplyObject), + lastRpcReplyObject + ); } return convertToXML(rpcReply); } @@ -677,6 +722,7 @@ public boolean isOK() { * @throws java.io.IOException If there are issues communicating with the netconf server. */ public boolean lockConfig() throws IOException, SAXException { + requireCandidateCapability("lockConfig"); String rpc = "" + "" + "" + @@ -696,6 +742,7 @@ public boolean lockConfig() throws IOException, SAXException { * @throws java.io.IOException If there are issues communicating with the netconf server. */ public boolean unlockConfig() throws IOException { + requireCandidateCapability("unlockConfig"); String rpc = "" + "" + "" + @@ -749,6 +796,7 @@ public boolean killSession(String sessionId) throws IOException, SAXException { * @throws java.io.IOException If there are issues reading the config file. */ public void loadSetConfiguration(String configuration) throws IOException { + requireCandidateCapability("loadSetConfiguration"); String rpc = "" + "" + "" + @@ -757,8 +805,12 @@ public void loadSetConfiguration(String configuration) throws IOException { "" + ""; setLastRpcReply(getRpcReply(rpc)); - if (hasError() || !isOK()) - throw new LoadException("Load operation returned error"); + if (hasError() || !isOK()) { + throw new LoadException( + RpcErrorException.buildMessage("Load operation", lastRpcReplyObject), + lastRpcReplyObject + ); + } } /** @@ -772,6 +824,7 @@ public void loadSetConfiguration(String configuration) throws IOException { * @throws java.io.IOException If there are issues reading the config file. */ public void loadXMLFile(String configFile, String loadType) throws IOException, SAXException { + requireCandidateCapability("loadXMLFile"); validateLoadType(loadType); loadXMLConfiguration(readConfigFile(configFile), loadType); } @@ -816,6 +869,7 @@ private String readConfigFile(String configFile) throws IOException { * @throws java.io.IOException If there are issues reading the config file. */ public void loadTextFile(String configFile, String loadType) throws IOException, SAXException { + requireCandidateCapability("loadTextFile"); validateLoadType(loadType); loadTextConfiguration(readConfigFile(configFile), loadType); } @@ -831,6 +885,7 @@ public void loadTextFile(String configFile, String loadType) throws IOException, */ public void loadSetFile(String configFile) throws IOException { + requireCandidateCapability("loadSetFile"); loadSetConfiguration(readConfigFile(configFile)); } @@ -860,6 +915,7 @@ public void loadSetFile(String configFile) throws * @throws org.xml.sax.SAXException if there are errors parsing the XML reply. */ public void commitThisConfiguration(String configFile, String loadType) throws IOException, SAXException { + requireCandidateCapability("commitThisConfiguration"); String configuration = readConfigFile(configFile); configuration = configuration.trim(); if (!this.lockConfig()) { @@ -899,13 +955,18 @@ public void commitThisConfiguration(String configFile, String loadType) throws I * @throws org.xml.sax.SAXException If there are errors parsing the XML reply. */ public void commit() throws IOException, SAXException { + requireCandidateCapability("commit"); String rpc = "" + "" + "" + NetconfConstants.DEVICE_PROMPT; setLastRpcReply(getRpcReply(rpc)); - if (hasError() || !isOK()) - throw new CommitException("Commit operation returned error."); + if (hasError() || !isOK()) { + throw new CommitException( + RpcErrorException.buildMessage("Commit operation", lastRpcReplyObject), + lastRpcReplyObject + ); + } } /** @@ -945,6 +1006,8 @@ public void commitConfirm(long seconds) throws IOException { * @throws IOException if communication with the device fails */ public void commitConfirm(long seconds, String persistToken) throws IOException { + requireCandidateCapability("commitConfirm"); + requireConfirmedCommitCapability("commitConfirm", persistToken != null); StringBuilder rpc = new StringBuilder(); rpc.append(""); if (seconds > 0) { @@ -957,7 +1020,10 @@ public void commitConfirm(long seconds, String persistToken) throws IOException setLastRpcReply(getRpcReply(rpc.toString())); if (hasError() || !isOK()) { - throw new CommitException("Confirmed-commit operation returned error."); + throw new CommitException( + RpcErrorException.buildMessage("Confirmed-commit operation", lastRpcReplyObject), + lastRpcReplyObject + ); } } @@ -972,6 +1038,8 @@ public void commitConfirm(long seconds, String persistToken) throws IOException * @throws SAXException if the reply cannot be parsed */ public boolean cancelCommit(String persistId) throws IOException, SAXException { + requireCandidateCapability("cancelCommit"); + requireConfirmedCommitCapability("cancelCommit", persistId != null); StringBuilder rpc = new StringBuilder(); rpc.append(""); if (persistId != null) { @@ -991,6 +1059,7 @@ public boolean cancelCommit(String persistId) throws IOException, SAXException { * @throws java.io.IOException If there are errors communicating with the netconf server. */ public void commitFull() throws CommitException, IOException { + requireCandidateCapability("commitFull"); String rpc = "" + "" + "" + @@ -998,8 +1067,12 @@ public void commitFull() throws CommitException, IOException { "" + NetconfConstants.DEVICE_PROMPT; setLastRpcReply(getRpcReply(rpc)); - if (hasError() || !isOK()) - throw new CommitException("Commit operation returned error."); + if (hasError() || !isOK()) { + throw new CommitException( + RpcErrorException.buildMessage("Commit operation", lastRpcReplyObject), + lastRpcReplyObject + ); + } } @@ -1058,10 +1131,16 @@ public XML getRunningConfig() throws SAXException, IOException { /** * Validate the candidate configuration. * - * @return true if validation successful. + * @return {@code true} if validation completed with a clean {@code } + * reply; {@code false} for warning-only or other non-error + * non-{@code } replies * @throws java.io.IOException If there are errors communicating with the netconf server. + * @throws ValidateException if the server returns one or more + * {@code } elements */ public boolean validate() throws IOException { + requireCandidateCapability("validate"); + requireValidateCapability("validate"); String rpc = "" + "" + @@ -1072,7 +1151,13 @@ public boolean validate() throws IOException { "" + NetconfConstants.DEVICE_PROMPT; setLastRpcReply(getRpcReply(rpc)); - return !hasError() && isOK(); + if (hasError()) { + throw new ValidateException( + RpcErrorException.buildMessage("Validate operation", lastRpcReplyObject), + lastRpcReplyObject + ); + } + return isOK(); } /** @@ -1337,19 +1422,6 @@ private byte[] applyTransportFraming(String rpc) { return chunkedRpc.getBytes(StandardCharsets.UTF_8); } - private boolean startsWithChunkedFraming() throws IOException { - int nextByte = peekIncomingByte(); - return nextByte == '\n' || nextByte == '#'; - } - - private int peekIncomingByte() throws IOException { - int nextByte = readIncomingByte(); - if (nextByte >= 0) { - pushBackBytes(new byte[] {(byte) nextByte}); - } - return nextByte; - } - private int readIncomingBytes(byte[] buffer) throws IOException { if (unreadBytes.length > 0) { int bytesToCopy = Math.min(buffer.length, unreadBytes.length); @@ -1423,6 +1495,38 @@ private boolean commandTimedOut(long startTime) { return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTime) >= commandTimeout; } + private NegotiatedCapabilities requireNegotiatedCapabilities() { + if (negotiatedCapabilities == null) { + throw new IllegalStateException("NETCONF session capabilities have not been negotiated"); + } + return negotiatedCapabilities; + } + + private void requireCandidateCapability(String operation) throws UnsupportedCapabilityException { + requireCapability(requireNegotiatedCapabilities().supportsCandidate(), "candidate", operation); + } + + private void requireValidateCapability(String operation) throws UnsupportedCapabilityException { + requireCapability(requireNegotiatedCapabilities().supportsValidate(), "validate", operation); + } + + private void requireConfirmedCommitCapability(String operation, boolean requiresPersistSupport) + throws UnsupportedCapabilityException { + NegotiatedCapabilities capabilities = requireNegotiatedCapabilities(); + requireCapability( + requiresPersistSupport ? capabilities.supportsConfirmedCommit11() : capabilities.supportsConfirmedCommit(), + requiresPersistSupport ? "confirmed-commit:1.1" : "confirmed-commit", + operation + ); + } + + private void requireCapability(boolean supported, String capability, String operation) + throws UnsupportedCapabilityException { + if (!supported) { + throw new UnsupportedCapabilityException(operation, capability); + } + } + private abstract class RpcReplyInputStream extends InputStream { private final long startTime = System.nanoTime(); diff --git a/src/main/java/net/juniper/netconf/RpcErrorException.java b/src/main/java/net/juniper/netconf/RpcErrorException.java new file mode 100644 index 0000000..c280121 --- /dev/null +++ b/src/main/java/net/juniper/netconf/RpcErrorException.java @@ -0,0 +1,176 @@ +/* + Copyright (c) 2013 Juniper Networks, Inc. + All Rights Reserved + + Use is subject to license terms. + +*/ + +package net.juniper.netconf; + +import net.juniper.netconf.element.RpcError; +import net.juniper.netconf.element.RpcReply; + +import java.util.List; +import java.util.StringJoiner; + +/** + * Exception thrown when a NETCONF RPC returns one or more {@code } + * elements. + */ +public class RpcErrorException extends NetconfException { + + /** + * Parsed NETCONF reply associated with the exception, when available. + */ + private final RpcReply rpcReply; + + /** + * Creates a new exception with the provided message. + * + * @param message human-readable description of the failure + */ + public RpcErrorException(String message) { + this(message, null, null); + } + + /** + * Creates a new exception with the provided message and root cause. + * + * @param message human-readable description of the failure + * @param cause root cause for the failure + */ + public RpcErrorException(String message, Throwable cause) { + this(message, null, cause); + } + + /** + * Creates a new exception that wraps a root cause. + * + * @param cause root cause for the failure + */ + public RpcErrorException(Throwable cause) { + this(cause == null ? null : cause.getMessage(), null, cause); + } + + /** + * Creates a new exception with the provided message and parsed RPC reply. + * + * @param message human-readable description of the failure + * @param rpcReply parsed NETCONF reply that triggered the exception + */ + public RpcErrorException(String message, RpcReply rpcReply) { + this(message, rpcReply, null); + } + + /** + * Creates a new exception with a message, parsed reply, and root cause. + * + * @param message human-readable description of the failure + * @param rpcReply parsed NETCONF reply that triggered the exception + * @param cause root cause for the failure + */ + public RpcErrorException(String message, RpcReply rpcReply, Throwable cause) { + super(message, cause); + this.rpcReply = rpcReply; + } + + /** + * Returns the parsed NETCONF {@code rpc-reply}, if available. + * + * @return parsed reply or {@code null} when unavailable + */ + public RpcReply getRpcReply() { + return rpcReply; + } + + /** + * Returns the parsed NETCONF {@code rpc-error} elements from the reply. + * + *

    The returned list may contain both error- and warning-severity + * {@link RpcError} objects because both are represented as + * {@code } elements on the wire.

    + * + * @return immutable snapshot of parsed rpc-error elements + */ + public List getRpcErrors() { + return rpcReply == null ? List.of() : List.copyOf(rpcReply.getErrors()); + } + + static String buildMessage(String operation, RpcReply rpcReply) { + String normalizedOperation = operation == null || operation.isBlank() + ? "NETCONF operation" + : operation; + + if (rpcReply == null) { + return normalizedOperation + " returned an error reply."; + } + + List rpcErrors = rpcReply.getErrors(); + if (rpcErrors.isEmpty()) { + return normalizedOperation + " returned a non-ok rpc-reply without rpc-error details."; + } + + String issueLabel; + if (rpcReply.hasErrors() && rpcReply.hasWarnings()) { + issueLabel = rpcErrors.size() == 1 ? "rpc issue" : "rpc issues"; + } else if (rpcReply.hasErrors()) { + issueLabel = rpcErrors.size() == 1 ? "rpc-error" : "rpc-errors"; + } else { + issueLabel = rpcErrors.size() == 1 ? "rpc-warning" : "rpc-warnings"; + } + + StringJoiner summaries = new StringJoiner("; "); + for (RpcError rpcError : rpcErrors) { + summaries.add(formatRpcError(rpcError)); + } + + return normalizedOperation + " returned " + rpcErrors.size() + " " + issueLabel + + ": " + summaries; + } + + private static String formatRpcError(RpcError rpcError) { + StringJoiner descriptors = new StringJoiner(", ", "[", "]"); + + if (rpcError.errorSeverity() != null) { + descriptors.add(rpcError.errorSeverity().getTextContent()); + } + if (rpcError.errorTag() != null) { + descriptors.add(rpcError.errorTag().getTextContent()); + } + if (rpcError.errorType() != null) { + descriptors.add(rpcError.errorType().getTextContent()); + } + + StringBuilder summary = new StringBuilder(descriptors.toString()); + String detail = firstNonBlank(rpcError.errorMessage(), rpcError.errorPath()); + if (detail != null) { + summary.append(' ').append(detail); + } + + if (rpcError.errorInfo() != null) { + appendNamedDetail(summary, "bad-element", rpcError.errorInfo().getBadElement()); + appendNamedDetail(summary, "bad-attribute", rpcError.errorInfo().getBadAttribute()); + appendNamedDetail(summary, "bad-namespace", rpcError.errorInfo().getBadNamespace()); + appendNamedDetail(summary, "session-id", rpcError.errorInfo().getSessionId()); + } + + return summary.toString(); + } + + private static void appendNamedDetail(StringBuilder summary, String name, String value) { + if (value == null || value.isBlank()) { + return; + } + summary.append(" (").append(name).append('=').append(value).append(')'); + } + + private static String firstNonBlank(String... values) { + for (String value : values) { + if (value != null && !value.isBlank()) { + return value; + } + } + return null; + } +} diff --git a/src/main/java/net/juniper/netconf/UnsupportedCapabilityException.java b/src/main/java/net/juniper/netconf/UnsupportedCapabilityException.java new file mode 100644 index 0000000..80bc773 --- /dev/null +++ b/src/main/java/net/juniper/netconf/UnsupportedCapabilityException.java @@ -0,0 +1,48 @@ +package net.juniper.netconf; + +/** + * Raised when an operation requires a NETCONF capability the server did not + * advertise in its {@code }. + */ +public class UnsupportedCapabilityException extends NetconfException { + + /** + * Operation blocked because the server did not advertise the required capability. + */ + private final String operation; + /** + * Capability required for the attempted operation. + */ + private final String requiredCapability; + + /** + * Creates an exception describing the missing capability. + * + * @param operation operation that was attempted + * @param requiredCapability capability required to perform it + */ + public UnsupportedCapabilityException(String operation, String requiredCapability) { + super("NETCONF server does not advertise capability '" + requiredCapability + + "' required for operation '" + operation + "'"); + this.operation = operation; + this.requiredCapability = requiredCapability; + } + + /** + * Returns the operation that was blocked by capability negotiation. + * + * @return operation name used in the exception message + */ + public String getOperation() { + return operation; + } + + /** + * Returns the capability required for the attempted operation. + * + * @return NETCONF capability URI or capability label + */ + public String getRequiredCapability() { + return requiredCapability; + } +} diff --git a/src/main/java/net/juniper/netconf/ValidateException.java b/src/main/java/net/juniper/netconf/ValidateException.java new file mode 100644 index 0000000..bd39f94 --- /dev/null +++ b/src/main/java/net/juniper/netconf/ValidateException.java @@ -0,0 +1,37 @@ +/* + Copyright (c) 2013 Juniper Networks, Inc. + All Rights Reserved + + Use is subject to license terms. + +*/ + +package net.juniper.netconf; + +import net.juniper.netconf.element.RpcReply; + +/** + * Exception thrown when a NETCONF {@code } RPC returns + * one or more {@code } elements. + */ +public class ValidateException extends RpcErrorException { + + /** + * Creates a {@code ValidateException} with the supplied message. + * + * @param message description of the validation failure + */ + public ValidateException(String message) { + super(message); + } + + /** + * Creates a {@code ValidateException} with the supplied message and reply. + * + * @param message description of the validation failure + * @param rpcReply parsed NETCONF reply + */ + public ValidateException(String message, RpcReply rpcReply) { + super(message, rpcReply); + } +} diff --git a/src/main/java/net/juniper/netconf/element/RpcReply.java b/src/main/java/net/juniper/netconf/element/RpcReply.java index 2a4d5c5..287a8dd 100644 --- a/src/main/java/net/juniper/netconf/element/RpcReply.java +++ b/src/main/java/net/juniper/netconf/element/RpcReply.java @@ -33,8 +33,9 @@ public class RpcReply extends AbstractNetconfElement { /** XPath for the <ok> element inside an <rpc-reply>. */ private static final String XPATH_RPC_REPLY_OK = XPATH_RPC_REPLY + getXpathFor("ok"); - /** XPath for any <rpc-error> elements inside the reply. */ - private static final String XPATH_RPC_REPLY_ERROR = XPATH_RPC_REPLY + getXpathFor("rpc-error"); + /** XPath for any NETCONF <rpc-error> descendants inside the reply. */ + private static final String XPATH_RPC_REPLY_ERROR = + XPATH_RPC_REPLY + "//*[namespace-uri()='urn:ietf:params:xml:ns:netconf:base:1.0' and local-name()='rpc-error']"; /** XPath for the <error-type> child of an <rpc-error>. */ private static final String XPATH_RPC_REPLY_ERROR_TYPE = getXpathFor("error-type"); @@ -472,9 +473,9 @@ public RpcReply( this.namespacePrefix = namespacePrefix; this.messageId = messageId; this.ok = ok; - this.errors = errors; + this.errors = errors == null ? List.of() : List.copyOf(errors); this.sessionId = sessionId; - this.capabilities = capabilities; + this.capabilities = capabilities == null ? List.of() : List.copyOf(capabilities); } /** diff --git a/src/test/java/net/juniper/netconf/DeviceTest.java b/src/test/java/net/juniper/netconf/DeviceTest.java index cbf8e77..3d8762c 100644 --- a/src/test/java/net/juniper/netconf/DeviceTest.java +++ b/src/test/java/net/juniper/netconf/DeviceTest.java @@ -384,6 +384,89 @@ public void GIVEN_newDevice_WHEN_connect_THEN_sendHelloWithCustomCapabilities() .areIdentical(); } + @Test + public void GIVEN_connectedDevice_WHEN_getNegotiatedCapabilities_THEN_returnSessionCapabilityView() throws Exception { + Device device = Device.builder() + .sshClient(givenConnectingSshClient()) + .hostName(TEST_HOSTNAME) + .userName(TEST_USERNAME) + .password(TEST_PASSWORD) + .strictHostKeyChecking(false) + .build(); + + device.connect(); + + NegotiatedCapabilities capabilities = device.getNegotiatedCapabilities(); + + assertThat(capabilities.getBaseVersion()).isEqualTo(NegotiatedCapabilities.BaseVersion.NETCONF_1_1); + assertThat(capabilities.usesChunkedFraming()).isTrue(); + assertThat(capabilities.getServerCapabilities()) + .contains(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1); + } + + @Test + public void GIVEN_noSharedBaseCapability_WHEN_connect_THEN_disconnectChannelAndSession() throws Exception { + JSch sshClient = mock(JSch.class); + Session session = mock(Session.class); + HostKeyRepository hostKeyRepository = mock(HostKeyRepository.class); + ChannelSubsystem channel = mock(ChannelSubsystem.class); + AtomicBoolean sessionConnected = new AtomicBoolean(false); + AtomicBoolean channelConnected = new AtomicBoolean(false); + + when(session.isConnected()).thenAnswer(invocation -> sessionConnected.get()); + doAnswer(invocation -> { + sessionConnected.set(true); + return null; + }).when(session).connect(eq(DEFAULT_TIMEOUT)); + doAnswer(invocation -> { + sessionConnected.set(false); + return null; + }).when(session).disconnect(); + + when(channel.isConnected()).thenAnswer(invocation -> channelConnected.get()); + when(channel.getInputStream()).thenReturn(new ByteArrayInputStream( + (""" + + + urn:ietf:params:netconf:base:1.0 + + 27700 + """ + NetconfConstants.DEVICE_PROMPT).getBytes(StandardCharsets.UTF_8))); + when(channel.getOutputStream()).thenReturn(outputStream); + doAnswer(invocation -> { + channelConnected.set(true); + return null; + }).when(channel).connect(eq(DEFAULT_TIMEOUT)); + doAnswer(invocation -> { + channelConnected.set(false); + return null; + }).when(channel).disconnect(); + + when(session.openChannel(eq(SUBSYSTEM))).thenReturn(channel); + when(sshClient.getSession(eq(TEST_USERNAME), eq(TEST_HOSTNAME), eq(DEFAULT_NETCONF_PORT))).thenReturn(session); + when(sshClient.getHostKeyRepository()).thenReturn(hostKeyRepository); + + Device device = Device.builder() + .sshClient(sshClient) + .hostName(TEST_HOSTNAME) + .userName(TEST_USERNAME) + .password(TEST_PASSWORD) + .netconfCapabilities(Collections.singletonList(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1)) + .strictHostKeyChecking(false) + .build(); + + assertThatThrownBy(device::connect) + .isInstanceOf(NetconfException.class) + .hasMessageContaining("do not share a common NETCONF base capability"); + + verify(channel).disconnect(); + verify(session).disconnect(); + assertThat(sessionConnected.get()).isFalse(); + assertThat(channelConnected.get()).isFalse(); + assertThat(device.isConnected()).isFalse(); + } + private JSch givenConnectingSshClient() throws IOException, JSchException { final Session sshSession = mock(Session.class); when(sshSession.isConnected()) @@ -392,7 +475,7 @@ private JSch givenConnectingSshClient() throws IOException, JSchException { when(sshChannel.getOutputStream()) .thenReturn(outputStream); final ByteArrayInputStream is = new ByteArrayInputStream( - ("" + NetconfConstants.DEVICE_PROMPT) + (HELLO_WITH_BASE_CAPABILITIES + NetconfConstants.DEVICE_PROMPT) .getBytes(StandardCharsets.UTF_8)); when(sshChannel.getInputStream()) .thenReturn(is); diff --git a/src/test/java/net/juniper/netconf/NegotiatedCapabilitiesTest.java b/src/test/java/net/juniper/netconf/NegotiatedCapabilitiesTest.java new file mode 100644 index 0000000..970f137 --- /dev/null +++ b/src/test/java/net/juniper/netconf/NegotiatedCapabilitiesTest.java @@ -0,0 +1,121 @@ +package net.juniper.netconf; + +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class NegotiatedCapabilitiesTest { + + private static final List CLIENT_CAPABILITIES = List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1 + ); + + @Test + public void normalizesLegacyAndRfcCandidateUris() throws Exception { + NegotiatedCapabilities legacyCapabilities = NegotiatedCapabilities.fromCapabilities( + CLIENT_CAPABILITIES, + List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0 + "#candidate" + ) + ); + NegotiatedCapabilities modernCapabilities = NegotiatedCapabilities.fromCapabilities( + CLIENT_CAPABILITIES, + List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + "urn:ietf:params:netconf:capability:candidate:1.0" + ) + ); + + assertThat(legacyCapabilities.supportsCandidate()).isTrue(); + assertThat(modernCapabilities.supportsCandidate()).isTrue(); + } + + @Test + public void normalizesLegacyAndRfcValidateUris() throws Exception { + NegotiatedCapabilities legacyCapabilities = NegotiatedCapabilities.fromCapabilities( + CLIENT_CAPABILITIES, + List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0 + "#validate" + ) + ); + NegotiatedCapabilities modernCapabilities = NegotiatedCapabilities.fromCapabilities( + CLIENT_CAPABILITIES, + List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + "urn:ietf:params:netconf:capability:validate:1.1" + ) + ); + + assertThat(legacyCapabilities.supportsValidate()).isTrue(); + assertThat(modernCapabilities.supportsValidate()).isTrue(); + } + + @Test + public void normalizesLegacyAndRfcConfirmedCommitUris() throws Exception { + NegotiatedCapabilities legacyCapabilities = NegotiatedCapabilities.fromCapabilities( + CLIENT_CAPABILITIES, + List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0 + "#confirmed-commit" + ) + ); + NegotiatedCapabilities modernCapabilities = NegotiatedCapabilities.fromCapabilities( + CLIENT_CAPABILITIES, + List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + "urn:ietf:params:netconf:capability:confirmed-commit:1.1" + ) + ); + + assertThat(legacyCapabilities.supportsConfirmedCommit()).isTrue(); + assertThat(legacyCapabilities.supportsConfirmedCommit11()).isFalse(); + assertThat(modernCapabilities.supportsConfirmedCommit()).isTrue(); + assertThat(modernCapabilities.supportsConfirmedCommit11()).isTrue(); + } + + @Test + public void prefersBase11WhenBothPeersSupportIt() throws Exception { + NegotiatedCapabilities capabilities = NegotiatedCapabilities.fromCapabilities( + CLIENT_CAPABILITIES, + List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1 + ) + ); + + assertThat(capabilities.getBaseVersion()) + .isEqualTo(NegotiatedCapabilities.BaseVersion.NETCONF_1_1); + assertThat(capabilities.usesChunkedFraming()).isTrue(); + } + + @Test + public void fallsBackToBase10WhenBase11IsNotShared() throws Exception { + NegotiatedCapabilities capabilities = NegotiatedCapabilities.fromCapabilities( + List.of(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0), + List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1 + ) + ); + + assertThat(capabilities.getBaseVersion()) + .isEqualTo(NegotiatedCapabilities.BaseVersion.NETCONF_1_0); + assertThat(capabilities.usesChunkedFraming()).isFalse(); + } + + @Test + public void rejectsNoCommonBaseCapability() { + assertThatThrownBy(() -> NegotiatedCapabilities.fromCapabilities( + List.of(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1), + List.of(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0) + )) + .isInstanceOf(NetconfException.class) + .hasMessageContaining("do not share a common NETCONF base capability"); + } +} diff --git a/src/test/java/net/juniper/netconf/NetconfSessionTest.java b/src/test/java/net/juniper/netconf/NetconfSessionTest.java index 89e553e..1e71e3e 100644 --- a/src/test/java/net/juniper/netconf/NetconfSessionTest.java +++ b/src/test/java/net/juniper/netconf/NetconfSessionTest.java @@ -28,6 +28,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -35,11 +36,12 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doCallRealMethod; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -68,7 +70,82 @@ public class NetconfSessionTest { " error" + " " + ""; + private static final String DETAILED_ERROR_RPC_REPLY = """ + + + protocol + operation-failed + error + syntax error + + invalid-rpc-that-should-not-exist + + + + """; + private static final String VALIDATE_ERROR_RPC_REPLY = """ + + + protocol + operation-failed + error + Error while forking commitd process + + + protocol + operation-failed + error + could not run commit script: Resource temporarily unavailable + + + """; + private static final String NAMESPACED_VALIDATE_ERROR_RPC_REPLY = """ + + + protocol + operation-failed + error + Error while forking commitd process + + + protocol + operation-failed + error + could not run commit script: Resource temporarily unavailable + + + protocol + operation-failed + error + could not start translation script handler + + + protocol + operation-failed + error + translation script failure + + + """; + private static final String WARNING_ONLY_RPC_REPLY = """ + + + warning + Could not extract delta, reading the entire configuration + + + """; private static final String NETCONF_SYNTAX_ERROR_MSG_FROM_DEVICE = "netconf error: syntax error"; + private static final List DEFAULT_CLIENT_CAPABILITIES = List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1 + ); + private static final List BASE_10_ONLY_CLIENT_CAPABILITIES = List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0 + ); + private static final List BASE_11_ONLY_CLIENT_CAPABILITIES = List.of( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1 + ); @Mock private NetconfSession mockNetconfSession; @@ -103,10 +180,13 @@ public void tearDown() throws Exception { @Test public void getCandidateConfigThrowsNetconfExceptionOnSyntaxError() throws Exception { - when(mockNetconfSession.getCandidateConfig()).thenCallRealMethod(); - when(mockNetconfSession.getRpcReply(anyString())).thenReturn(NETCONF_SYNTAX_ERROR_MSG_FROM_DEVICE); + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream(NETCONF_SYNTAX_ERROR_MSG_FROM_DEVICE), + new ByteArrayOutputStream(), + 100 + ); - assertThatThrownBy(mockNetconfSession::getCandidateConfig) + assertThatThrownBy(netconfSession::getCandidateConfig) .isInstanceOf(NetconfException.class) .hasMessage("Invalid message from server: netconf error: syntax error"); } @@ -205,6 +285,27 @@ public void executeRpcThrowsNetconfExceptionOnSyntaxError() throws Exception { .hasMessage("Invalid message from server: netconf error: syntax error"); } + @Test + public void executeRpcThrowsStructuredRpcErrorExceptionOnRpcErrorReply() throws Exception { + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream(DETAILED_ERROR_RPC_REPLY), + new ByteArrayOutputStream(), + 100 + ); + + assertThatThrownBy(() -> netconfSession.executeRPC("")) + .isInstanceOf(RpcErrorException.class) + .hasMessageContaining("syntax error") + .hasMessageContaining("bad-element=invalid-rpc-that-should-not-exist") + .satisfies(throwable -> { + RpcErrorException exception = (RpcErrorException) throwable; + assertThat(exception.getRpcReply()).isNotNull(); + assertThat(exception.getRpcErrors()).hasSize(1); + assertThat(exception.getRpcErrors().get(0).errorTag()) + .isEqualTo(RpcError.ErrorTag.OPERATION_FAILED); + }); + } + @Test public void fixupRpcWrapsStringWithoutRpcTags() { assertThat(NetconfSession.fixupRpc("fake string")) @@ -255,6 +356,41 @@ public void instantiationFetchesHelloFromServer() throws Exception { .isTrue(); } + @Test + public void sessionUsesLegacyFramingWhenNegotiatedBaseIs10() throws Exception { + ByteArrayOutputStream capturedOutput = new ByteArrayOutputStream(); + String rpcReply = RpcReply.builder().ok(true).messageId("1").build().getXml(); + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream(createHelloMessageWithBase11(), rpcReply), + capturedOutput, + BASE_10_ONLY_CLIENT_CAPABILITIES, + 100 + ); + + netconfSession.executeRPC(""); + + assertThat(netconfSession.getNegotiatedCapabilities().getBaseVersion()) + .isEqualTo(NegotiatedCapabilities.BaseVersion.NETCONF_1_0); + assertThat(netconfSession.getNegotiatedCapabilities().usesChunkedFraming()).isFalse(); + assertThat(capturedOutput.toString(StandardCharsets.UTF_8)) + .contains(NetconfConstants.DEVICE_PROMPT) + .doesNotContain("\n#"); + } + + @Test + public void sessionInitializationFailsWithoutSharedBaseCapability() { + String hello = createHelloMessageWithCapabilities(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0); + + assertThatThrownBy(() -> createNetconfSession( + helloOnlyStream(hello), + new ByteArrayOutputStream(), + BASE_11_ONLY_CLIENT_CAPABILITIES, + 100 + )) + .isInstanceOf(NetconfException.class) + .hasMessageContaining("do not share a common NETCONF base capability"); + } + @Test public void executeRpcSupportsChunkedRepliesAfterBase11Hello() throws Exception { final String combinedMessage = createHelloMessageWithBase11() @@ -269,6 +405,32 @@ public void executeRpcSupportsChunkedRepliesAfterBase11Hello() throws Exception .contains(""); } + @Test + public void getNegotiatedCapabilitiesReflectsNormalizedSessionState() throws Exception { + String hello = createHelloMessageWithCapabilities( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1, + "urn:ietf:params:netconf:capability:candidate:1.0", + "urn:ietf:params:netconf:capability:validate:1.1", + "urn:ietf:params:netconf:capability:confirmed-commit:1.1", + "urn:ietf:params:netconf:capability:writable-running:1.0" + ); + NetconfSession netconfSession = createNetconfSession( + helloOnlyStream(hello), + new ByteArrayOutputStream(), + 100 + ); + + NegotiatedCapabilities capabilities = netconfSession.getNegotiatedCapabilities(); + + assertThat(capabilities.getBaseVersion()).isEqualTo(NegotiatedCapabilities.BaseVersion.NETCONF_1_1); + assertThat(capabilities.supportsCandidate()).isTrue(); + assertThat(capabilities.supportsValidate()).isTrue(); + assertThat(capabilities.supportsConfirmedCommit()).isTrue(); + assertThat(capabilities.supportsConfirmedCommit11()).isTrue(); + assertThat(capabilities.supportsWritableRunning()).isTrue(); + } + @Test public void executeRpcRunningStripsLegacyFramingAndStopsAfterSingleReply() throws Exception { String firstReply = RpcReply.builder().ok(true).messageId("1").build().getXml(); @@ -322,6 +484,24 @@ public void sequentialExecuteRpcCallsStayAlignedWithLegacyFraming() throws Excep .areIdentical(); } + @Test + public void executeRpcUsesLegacyFramingForBase10SessionsEvenWhenReplyStartsWithNewline() throws Exception { + String firstReply = "\n" + RpcReply.builder().ok(true).messageId("1").build().getXml(); + NetconfSession netconfSession = createNetconfSession( + new ByteArrayInputStream((createHelloMessage() + + NetconfConstants.DEVICE_PROMPT + + firstReply + + NetconfConstants.DEVICE_PROMPT).getBytes(StandardCharsets.UTF_8)), + new ByteArrayOutputStream(), + 100 + ); + + XmlAssert.assertThat(netconfSession.executeRPC("").toString()) + .and(firstReply.trim()) + .ignoreWhitespace() + .areIdentical(); + } + @Test public void closeOnChunkedExecuteRpcRunningDrainsCurrentReplyForNextRpc() throws Exception { String firstReply = """ @@ -432,14 +612,13 @@ public void executeRpcRejectsMismatchedReplyMessageId() throws Exception { @Test public void loadTextConfigurationSucceedsWithOkResponse() throws Exception { - doCallRealMethod().when(mockNetconfSession) - .loadTextConfiguration(anyString(), anyString()); - when(mockNetconfSession.getRpcReply(anyString())).thenReturn(OK_RPC_REPLY); - when(mockNetconfSession.hasError()).thenReturn(false); - when(mockNetconfSession.isOK()).thenReturn(true); + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream(OK_RPC_REPLY), + new ByteArrayOutputStream(), + 100 + ); - // should complete without throwing - mockNetconfSession.loadTextConfiguration("some config", "some type"); + netconfSession.loadTextConfiguration("some config", "merge"); } @Test @@ -479,20 +658,125 @@ public void loadTextConfigurationFailsWithOkResponseButErrors() throws Exception final NetconfSession netconfSession = createNetconfSession(100); - assertThrows(LoadException.class, - () -> netconfSession.loadTextConfiguration("some config", "some type")); + assertThatThrownBy(() -> netconfSession.loadTextConfiguration("some config", "some type")) + .isInstanceOf(LoadException.class) + .hasMessageContaining("Load operation returned 1 rpc-error") + .satisfies(throwable -> { + LoadException exception = (LoadException) throwable; + assertThat(exception.getRpcReply()).isNotNull(); + assertThat(exception.getRpcErrors()).hasSize(1); + }); + } + + @Test + public void lockConfigFailsLocallyWhenServerLacksCandidateCapability() throws Exception { + ByteArrayOutputStream capturedOutput = new ByteArrayOutputStream(); + NetconfSession netconfSession = createNetconfSession( + helloOnlyStream(createHelloMessageWithCapabilities(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0)), + capturedOutput, + 100 + ); + String helloWrite = capturedOutput.toString(StandardCharsets.UTF_8); + + assertThatThrownBy(netconfSession::lockConfig) + .isInstanceOf(UnsupportedCapabilityException.class) + .hasMessageContaining("candidate") + .hasMessageContaining("lockConfig"); + + assertThat(capturedOutput.toString(StandardCharsets.UTF_8)).isEqualTo(helloWrite); + } + + @Test + public void validateFailsLocallyWhenServerLacksValidateCapability() throws Exception { + ByteArrayOutputStream capturedOutput = new ByteArrayOutputStream(); + NetconfSession netconfSession = createNetconfSession( + helloOnlyStream(createHelloMessageWithCapabilities( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0 + "#candidate" + )), + capturedOutput, + 100 + ); + String helloWrite = capturedOutput.toString(StandardCharsets.UTF_8); + + assertThatThrownBy(netconfSession::validate) + .isInstanceOf(UnsupportedCapabilityException.class) + .hasMessageContaining("validate"); + + assertThat(capturedOutput.toString(StandardCharsets.UTF_8)).isEqualTo(helloWrite); + } + + @Test + public void validateThrowsStructuredValidateExceptionOnRpcErrorReply() throws Exception { + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream(VALIDATE_ERROR_RPC_REPLY), + new ByteArrayOutputStream(), + 100 + ); + + assertThatThrownBy(netconfSession::validate) + .isInstanceOf(ValidateException.class) + .hasMessageContaining("Error while forking commitd process") + .hasMessageContaining("could not run commit script: Resource temporarily unavailable") + .satisfies(throwable -> { + ValidateException exception = (ValidateException) throwable; + assertThat(exception.getRpcReply()).isNotNull(); + assertThat(exception.getRpcErrors()).hasSize(2); + assertThat(exception.getRpcErrors()) + .allMatch(error -> error.errorSeverity() == RpcError.ErrorSeverity.ERROR); + }); + } + + @Test + public void validateThrowsStructuredValidateExceptionOnNamespacedRpcErrorReply() throws Exception { + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream(NAMESPACED_VALIDATE_ERROR_RPC_REPLY), + new ByteArrayOutputStream(), + 100 + ); + + assertThatThrownBy(netconfSession::validate) + .isInstanceOf(ValidateException.class) + .hasMessageContaining("Error while forking commitd process") + .hasMessageContaining("translation script failure") + .satisfies(throwable -> { + ValidateException exception = (ValidateException) throwable; + assertThat(exception.getRpcErrors()).hasSize(4); + assertThat(exception.getRpcErrors()) + .allMatch(error -> error.errorSeverity() == RpcError.ErrorSeverity.ERROR); + assertThat(exception.getRpcErrors()) + .extracting(RpcError::errorMessage) + .containsExactly( + "Error while forking commitd process", + "could not run commit script: Resource temporarily unavailable", + "could not start translation script handler", + "translation script failure" + ); + }); + } + + @Test + public void validateReturnsFalseForWarningOnlyReply() throws Exception { + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream(WARNING_ONLY_RPC_REPLY), + new ByteArrayOutputStream(), + 100 + ); + + assertThat(netconfSession.validate()).isFalse(); + assertThat(netconfSession.getLastRpcReplyObject().hasWarnings()).isTrue(); + assertThat(netconfSession.getLastRpcReplyObject().hasErrors()).isFalse(); } @Test public void loadXmlConfigurationSucceedsWithOkResponse() throws Exception { - doCallRealMethod().when(mockNetconfSession) - .loadXMLConfiguration(anyString(), anyString()); - when(mockNetconfSession.getRpcReply(anyString())).thenReturn(OK_RPC_REPLY); - when(mockNetconfSession.hasError()).thenReturn(false); - when(mockNetconfSession.isOK()).thenReturn(true); + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream(OK_RPC_REPLY), + new ByteArrayOutputStream(), + 100 + ); - // should complete without throwing - mockNetconfSession.loadXMLConfiguration("some config", "merge"); + netconfSession.loadXMLConfiguration("some config", "merge"); } @Test @@ -568,67 +852,129 @@ public void killSessionReturnsFalseOnErrorReply() throws Exception { @Test public void commitConfirmWithPersistCompletesSuccessfullyOnOkReply() throws Exception { - doCallRealMethod().when(mockNetconfSession).commitConfirm(600, "abc"); - when(mockNetconfSession.getRpcReply(anyString())).thenReturn(OK_RPC_REPLY); - when(mockNetconfSession.hasError()).thenReturn(false); - when(mockNetconfSession.isOK()).thenReturn(true); + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream( + createHelloMessageWithCapabilities( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + "urn:ietf:params:netconf:capability:candidate:1.0", + "urn:ietf:params:netconf:capability:confirmed-commit:1.1" + ), + OK_RPC_REPLY + ), + new ByteArrayOutputStream(), + 100 + ); - // should complete without throwing CommitException - mockNetconfSession.commitConfirm(600, "abc"); + netconfSession.commitConfirm(600, "abc"); } @Test public void commitConfirmWithPersistThrowsCommitExceptionOnErrorReply() throws Exception { - doCallRealMethod().when(mockNetconfSession).commitConfirm(600, "xyz"); - when(mockNetconfSession.getRpcReply(anyString())).thenReturn(ERROR_RPC_REPLY); - when(mockNetconfSession.hasError()).thenReturn(true); - when(mockNetconfSession.isOK()).thenReturn(false); + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream( + createHelloMessageWithCapabilities( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + "urn:ietf:params:netconf:capability:candidate:1.0", + "urn:ietf:params:netconf:capability:confirmed-commit:1.1" + ), + ERROR_RPC_REPLY + ), + new ByteArrayOutputStream(), + 100 + ); - assertThatThrownBy(() -> mockNetconfSession.commitConfirm(600, "xyz")) + assertThatThrownBy(() -> netconfSession.commitConfirm(600, "xyz")) .isInstanceOf(CommitException.class) - .hasMessage("Confirmed-commit operation returned error."); + .hasMessageContaining("Confirmed-commit operation returned") + .satisfies(throwable -> { + CommitException exception = (CommitException) throwable; + assertThat(exception.getRpcReply()).isNotNull(); + assertThat(exception.getRpcErrors()).hasSize(1); + }); + } + + @Test + public void commitConfirmWithPersistFailsLocallyWhenServerOnlyAdvertisesLegacyConfirmedCommit() throws Exception { + ByteArrayOutputStream capturedOutput = new ByteArrayOutputStream(); + NetconfSession netconfSession = createNetconfSession( + helloOnlyStream(createHelloMessage()), + capturedOutput, + 100 + ); + String helloWrite = capturedOutput.toString(StandardCharsets.UTF_8); + + assertThatThrownBy(() -> netconfSession.commitConfirm(600, "persist-token")) + .isInstanceOf(UnsupportedCapabilityException.class) + .hasMessageContaining("confirmed-commit:1.1"); + + assertThat(capturedOutput.toString(StandardCharsets.UTF_8)).isEqualTo(helloWrite); } /* ----- cancel-commit ----- */ @Test public void cancelCommitReturnsTrueOnOkReply() throws Exception { - when(mockNetconfSession.cancelCommit("abc")).thenCallRealMethod(); - when(mockNetconfSession.getRpcReply(anyString())).thenReturn(OK_RPC_REPLY); - when(mockNetconfSession.hasError()).thenReturn(false); - when(mockNetconfSession.isOK()).thenReturn(true); + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream( + createHelloMessageWithCapabilities( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + "urn:ietf:params:netconf:capability:candidate:1.0", + "urn:ietf:params:netconf:capability:confirmed-commit:1.1" + ), + OK_RPC_REPLY + ), + new ByteArrayOutputStream(), + 100 + ); - assertThat(mockNetconfSession.cancelCommit("abc")).isTrue(); + assertThat(netconfSession.cancelCommit("abc")).isTrue(); } @Test public void cancelCommitReturnsFalseOnErrorReply() throws Exception { - when(mockNetconfSession.cancelCommit(null)).thenCallRealMethod(); - when(mockNetconfSession.getRpcReply(anyString())).thenReturn(ERROR_RPC_REPLY); - when(mockNetconfSession.hasError()).thenReturn(true); - when(mockNetconfSession.isOK()).thenReturn(false); + NetconfSession netconfSession = createNetconfSession( + helloThenReplyStream( + createHelloMessageWithCapabilities( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + "urn:ietf:params:netconf:capability:candidate:1.0", + "urn:ietf:params:netconf:capability:confirmed-commit:1.1" + ), + ERROR_RPC_REPLY + ), + new ByteArrayOutputStream(), + 100 + ); - assertThat(mockNetconfSession.cancelCommit(null)).isFalse(); + assertThat(netconfSession.cancelCommit(null)).isFalse(); } @Test public void commitThisConfigurationUnlocksCandidateWhenLoadFails() throws Exception { - doCallRealMethod().when(mockNetconfSession).commitThisConfiguration(anyString(), anyString()); - when(mockNetconfSession.lockConfig()).thenReturn(true); - doThrow(new LoadException("boom")).when(mockNetconfSession).loadTextConfiguration(anyString(), anyString()); + NetconfSession netconfSession = spy(createNetconfSession( + helloOnlyStream(createHelloMessage()), + new ByteArrayOutputStream(), + 100 + )); + doReturn(true).when(netconfSession).lockConfig(); + doThrow(new LoadException("boom")).when(netconfSession).loadTextConfiguration(anyString(), anyString()); + doReturn(true).when(netconfSession).unlockConfig(); Path configFile = Files.createTempFile("netconf-java-", ".conf"); Files.writeString(configFile, "system { services { ftp; } }", StandardCharsets.UTF_8); - assertThatThrownBy(() -> mockNetconfSession.commitThisConfiguration(configFile.toString(), "merge")) + assertThatThrownBy(() -> netconfSession.commitThisConfiguration(configFile.toString(), "merge")) .isInstanceOf(LoadException.class); - verify(mockNetconfSession).unlockConfig(); + verify(netconfSession).unlockConfig(); } // Helper methods to reduce code duplication and improve readability private NetconfSession createNetconfSession(int commandTimeout) throws IOException { + return createNetconfSession(commandTimeout, DEFAULT_CLIENT_CAPABILITIES); + } + + private NetconfSession createNetconfSession(int commandTimeout, List clientCapabilities) throws IOException { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); try { builder = factory.newDocumentBuilder(); @@ -636,25 +982,41 @@ private NetconfSession createNetconfSession(int commandTimeout) throws IOExcepti throw new NetconfException(String.format("Error creating XML Parser: %s", e.getMessage())); } - return new NetconfSession(mockChannel, CONNECTION_TIMEOUT, commandTimeout, FAKE_HELLO, builder); + return new NetconfSession(mockChannel, CONNECTION_TIMEOUT, commandTimeout, clientCapabilities, FAKE_HELLO, builder); } private NetconfSession createNetconfSession(InputStream inputStream, java.io.OutputStream outputStream, int commandTimeout) throws IOException { + return createNetconfSession(inputStream, outputStream, DEFAULT_CLIENT_CAPABILITIES, commandTimeout); + } + + private NetconfSession createNetconfSession(InputStream inputStream, + java.io.OutputStream outputStream, + List clientCapabilities, + int commandTimeout) throws IOException { when(mockChannel.getInputStream()).thenReturn(inputStream); when(mockChannel.getOutputStream()).thenReturn(outputStream); - return createNetconfSession(commandTimeout); + return createNetconfSession(commandTimeout, clientCapabilities); } private InputStream helloThenReplyStream(String reply) { - String combinedMessage = createHelloMessage() + return helloThenReplyStream(createHelloMessage(), reply); + } + + private InputStream helloThenReplyStream(String helloMessage, String reply) { + String combinedMessage = helloMessage + NetconfConstants.DEVICE_PROMPT + reply + NetconfConstants.DEVICE_PROMPT; return new ByteArrayInputStream(combinedMessage.getBytes(StandardCharsets.UTF_8)); } + private InputStream helloOnlyStream(String helloMessage) { + String combinedMessage = helloMessage + NetconfConstants.DEVICE_PROMPT; + return new ByteArrayInputStream(combinedMessage.getBytes(StandardCharsets.UTF_8)); + } + private String readAll(BufferedReader reader) throws IOException { StringBuilder content = new StringBuilder(); char[] buffer = new char[128]; @@ -685,7 +1047,7 @@ private void writeDataAndClose() throws IOException, InterruptedException { } private void writeValidResponse() throws IOException, InterruptedException { - outPipe.write(FAKE_RPC_REPLY.getBytes(StandardCharsets.UTF_8)); + outPipe.write(createHelloMessage().getBytes(StandardCharsets.UTF_8)); outPipe.write(DEVICE_PROMPT_BYTE); Thread.sleep(200); outPipe.flush(); @@ -694,7 +1056,7 @@ private void writeValidResponse() throws IOException, InterruptedException { } private void writeLldpResponse(byte[] lldpResponse) throws IOException, InterruptedException { - outPipe.write(FAKE_RPC_REPLY.getBytes(StandardCharsets.UTF_8)); + outPipe.write(createHelloMessage().getBytes(StandardCharsets.UTF_8)); outPipe.write(DEVICE_PROMPT_BYTE); outPipe.flush(); Thread.sleep(800); @@ -708,26 +1070,33 @@ private void writeLldpResponse(byte[] lldpResponse) throws IOException, Interrup } private String createHelloMessage() { - return "\n" - + " \n" - + " urn:ietf:params:netconf:base:1.0\n" - + " urn:ietf:params:netconf:base:1.0#candidate\n" - + " urn:ietf:params:netconf:base:1.0#confirmed-commit\n" - + " urn:ietf:params:netconf:base:1.0#validate\n" - + " urn:ietf:params:netconf:base:1.0#url?protocol=http,ftp,file\n" - + " \n" - + " 27700\n" - + ""; + return createHelloMessageWithCapabilities( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0 + "#candidate", + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0 + "#confirmed-commit", + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0 + "#validate", + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0 + "#url?protocol=http,ftp,file" + ); } private String createHelloMessageWithBase11() { - return "\n" - + " \n" - + " urn:ietf:params:netconf:base:1.0\n" - + " urn:ietf:params:netconf:base:1.1\n" - + " \n" - + " 27700\n" - + ""; + return createHelloMessageWithCapabilities( + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0, + NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1 + ); + } + + private String createHelloMessageWithCapabilities(String... capabilities) { + StringBuilder helloBuilder = new StringBuilder(); + helloBuilder.append("\n"); + helloBuilder.append(" \n"); + for (String capability : capabilities) { + helloBuilder.append(" ").append(capability).append("\n"); + } + helloBuilder.append(" \n"); + helloBuilder.append(" 27700\n"); + helloBuilder.append(""); + return helloBuilder.toString(); } private String toChunkedMessage(String payload) { diff --git a/src/test/java/net/juniper/netconf/element/RpcReplyTest.java b/src/test/java/net/juniper/netconf/element/RpcReplyTest.java index 3e6727d..b1bea4e 100644 --- a/src/test/java/net/juniper/netconf/element/RpcReplyTest.java +++ b/src/test/java/net/juniper/netconf/element/RpcReplyTest.java @@ -42,6 +42,22 @@ public class RpcReplyTest { " Invalid IP address for interface Ethernet1/0\n" + " \n" + ""; + private static final String RPC_REPLY_WITH_NESTED_COMMIT_RESULTS_ERRORS = """ + + + + protocol + operation-failed + error + Error while forking commitd process + + + warning + Could not extract delta, reading the entire configuration + + + + """; private static final String MALFORMED_RPC_REPLY = ""; @@ -130,6 +146,29 @@ public void willParseRpcReplyWithErrors() throws Exception { .build())); } + @Test + public void willParseNestedCommitResultsErrors() throws Exception { + final RpcReply rpcReply = RpcReply.from(RPC_REPLY_WITH_NESTED_COMMIT_RESULTS_ERRORS); + + assertThat(rpcReply.getMessageId()).isEqualTo("2"); + assertThat(rpcReply.isOK()).isFalse(); + assertThat(rpcReply.hasErrorsOrWarnings()).isTrue(); + assertThat(rpcReply.hasErrors()).isTrue(); + assertThat(rpcReply.hasWarnings()).isTrue(); + assertThat(rpcReply.getErrors()) + .isEqualTo(Arrays.asList( + RpcError.builder() + .errorType(RpcError.ErrorType.PROTOCOL) + .errorTag(RpcError.ErrorTag.OPERATION_FAILED) + .errorSeverity(RpcError.ErrorSeverity.ERROR) + .errorMessage("Error while forking commitd process") + .build(), + RpcError.builder() + .errorSeverity(RpcError.ErrorSeverity.WARNING) + .errorMessage("Could not extract delta, reading the entire configuration") + .build())); + } + @Test public void willCreateXmlFromAnObject() { diff --git a/src/test/java/net/juniper/netconf/integration/INTEGRATION_TESTS.md b/src/test/java/net/juniper/netconf/integration/INTEGRATION_TESTS.md index 90f7f24..a14dfc4 100644 --- a/src/test/java/net/juniper/netconf/integration/INTEGRATION_TESTS.md +++ b/src/test/java/net/juniper/netconf/integration/INTEGRATION_TESTS.md @@ -1,53 +1,178 @@ -# NetConf Java Integration Tests +# Netconf Java Integration Tests -This directory contains integration tests for the netconf-java library that test against real network devices. +This directory contains optional integration tests that exercise the +library against a live NETCONF-over-SSH endpoint. The suite is +disabled by default and only runs when +`netconf.integration.enabled=true` is set. -## Overview +## What the suite covers -The integration tests verify: -- Basic device connection and authentication -- Server capabilities retrieval -- Configuration retrieval via get-config -- Multiple sequential connections -- Error handling and timeouts -- Device-specific RPC operations +The current integration suite verifies: -## Running the Tests +- Basic SSH and NETCONF session establishment +- Server capability retrieval after `` +- Negotiated NETCONF base-version selection from client/server + capability overlap +- Basic `` behavior against the running datastore +- Same-session sequential RPC reuse across multiple operations +- Streamed reply draining when callers close a `BufferedReader` early +- Multiple sequential connect/close cycles +- Structured `rpc-error` parsing for malformed RPCs, plus + post-error session recovery +- Capability-guarded candidate lock/validate/unlock workflow +- Connection timeout behavior against an unreachable address +- A Junos-specific `get-interface-information` smoke test -### Method 1: Using JUnit (Recommended) +Notes: + +- The suite does not perform configuration changes. +- The `get-interface-information` test is best-effort. It is useful + for Junos targets, but it may legitimately be unsupported on + non-Juniper servers. +- The suite inspects negotiated server capabilities via + `getNegotiatedCapabilities()` rather than the client's configured + hello list. + +## Prerequisites + +- Java 17 +- Maven available on `PATH` if you want to use the wrapper script +- Gradle 8+ or the checked-in `./gradlew` wrapper if you want to use + the Gradle task directly +- A reachable NETCONF server and credentials + +For a local Junos-based test target, see +[`RUN-CRPD-CONTAINER.md`](./RUN-CRPD-CONTAINER.md). That runbook +covers Docker and Podman on macOS, the minimum cRPD NETCONF config +including `rfc-compliant`, and a quick readiness check before you run +the suite. + +## Recommended: wrapper script + +The wrapper script is the easiest way to run the tests interactively. +It resolves the repository root internally, so you can invoke it from +the repo root or by absolute path from another working directory. The +wrapper currently uses Maven under the hood. + +From the repository root: ```bash -# Run with interactive prompts -mvn test -Dtest=NetconfIntegrationTest -Dnetconf.integration.enabled=true +chmod +x ./src/test/java/net/juniper/netconf/integration/run-integration-tests.sh +./src/test/java/net/juniper/netconf/integration/run-integration-tests.sh +``` + +With explicit connection details: + +```bash +./src/test/java/net/juniper/netconf/integration/run-integration-tests.sh \ + --host 192.168.1.1 \ + --username admin \ + --password secret \ + --port 830 \ + --timeout 30000 +``` + +The same values can also be provided with environment variables: + +```bash +NETCONF_HOST=192.168.1.1 \ +NETCONF_USERNAME=admin \ +NETCONF_PASSWORD=secret \ +NETCONF_PORT=830 \ +NETCONF_TIMEOUT=30000 \ +./src/test/java/net/juniper/netconf/integration/run-integration-tests.sh +``` + +The wrapper exports those variables to Maven instead of echoing +secrets on the command line. -# Run with predefined credentials +## Direct Maven invocation + +For CI or other non-interactive environments, prefer passing all +connection details explicitly: + +```bash mvn test -Dtest=NetconfIntegrationTest -Dnetconf.integration.enabled=true \ -Dnetconf.host=192.168.1.1 \ -Dnetconf.username=admin \ -Dnetconf.password=secret \ - -Dnetconf.port=830 + -Dnetconf.port=830 \ + -Dnetconf.timeout=30000 ``` -### Method 2: Using the Shell Script +If you omit required properties, the test class may prompt on stdin. +That works best from a normal terminal session; the wrapper script is +the more reliable interactive path. + +The test class also reads `NETCONF_HOST`, `NETCONF_USERNAME`, +`NETCONF_PASSWORD`, `NETCONF_PORT`, and `NETCONF_TIMEOUT` directly +from the environment. That is the better option when you do not want +credentials to appear in the shell history or Maven system-property +reports. ```bash -# Make the script executable -chmod +x run-integration-tests.sh +NETCONF_HOST=192.168.1.1 \ +NETCONF_USERNAME=admin \ +NETCONF_PASSWORD=secret \ +NETCONF_PORT=830 \ +NETCONF_TIMEOUT=30000 \ +mvn test -Dtest=NetconfIntegrationTest -Dnetconf.integration.enabled=true +``` -# Run with interactive prompts -./run-integration-tests.sh +## Direct Gradle invocation -# Run with command line arguments -./run-integration-tests.sh --host 192.168.1.1 --username admin --password secret -``` +For Gradle users, use the dedicated `integrationTest` task: -### Method 3: Manual Test Runner +```bash +./gradlew integrationTest \ + -Dnetconf.host=192.168.1.1 \ + -Dnetconf.username=admin \ + -Dnetconf.password=secret \ + -Dnetconf.port=830 \ + -Dnetconf.timeout=30000 +``` -For environments where JUnit is not available: +The same task also accepts `NETCONF_HOST`, `NETCONF_USERNAME`, +`NETCONF_PASSWORD`, `NETCONF_PORT`, and `NETCONF_TIMEOUT` from the +environment: ```bash -# Compile the manual runner -javac -cp "target/classes:target/dependency/*" src/test/java/net/juniper/netconf/integration/ManualTestRunner.java +NETCONF_HOST=192.168.1.1 \ +NETCONF_USERNAME=admin \ +NETCONF_PASSWORD=secret \ +NETCONF_PORT=830 \ +NETCONF_TIMEOUT=30000 \ +./gradlew integrationTest +``` + +Unlike the Maven wrapper flow, the Gradle task is intentionally +non-interactive. If required connection details are missing, it fails +fast with a clear error instead of trying to prompt inside the Gradle +test worker. + +## Connection properties + +The integration test class reads the following JVM system properties: + +- `netconf.integration.enabled=true` +- `netconf.host` +- `netconf.username` +- `netconf.password` +- `netconf.port` (default `830`) +- `netconf.timeout` (default `30000`) + +## Operational notes -# Run the manual tests -java -cp "target/classes:target/dependency/*:src/test/java" net.juniper.netconf.integration. \ No newline at end of file +- The tests build `Device` instances with + `strictHostKeyChecking(false)` for convenience. That is acceptable + for disposable local test targets, but it should not be copied into + production usage. +- The suite assumes the target speaks NETCONF over SSH and that the + supplied account has enough privilege for basic `` and + vendor RPCs. +- For local containerized targets such as cRPD, pass the published + host-side NETCONF port, for example `8830`, rather than the + container's internal port `830`. +- If you use the cRPD container workflow, downloaded image archives + and extracted Docker image artifacts under `src/test/resources/` are + treated as local test assets and are ignored by git. diff --git a/src/test/java/net/juniper/netconf/integration/NetconfIntegrationTest.java b/src/test/java/net/juniper/netconf/integration/NetconfIntegrationTest.java index 252937f..29593bd 100644 --- a/src/test/java/net/juniper/netconf/integration/NetconfIntegrationTest.java +++ b/src/test/java/net/juniper/netconf/integration/NetconfIntegrationTest.java @@ -2,16 +2,26 @@ import net.juniper.netconf.Device; import net.juniper.netconf.NetconfException; +import net.juniper.netconf.NetconfConstants; +import net.juniper.netconf.NetconfSession; +import net.juniper.netconf.NegotiatedCapabilities; +import net.juniper.netconf.RpcErrorException; +import net.juniper.netconf.ValidateException; import net.juniper.netconf.XML; +import net.juniper.netconf.element.RpcError; +import net.juniper.netconf.element.RpcReply; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.condition.EnabledIfSystemProperty; +import java.io.BufferedReader; import java.io.Console; import java.io.IOException; import java.util.Scanner; import java.nio.charset.StandardCharsets; +import java.lang.reflect.Field; import static org.junit.jupiter.api.Assertions.*; @@ -19,11 +29,12 @@ * Integration tests for netconf-java library against real network devices. * * To run these tests: - * mvn test -Dtest=NetconfIntegrationTest -Dnetconf.integration.enabled=true - * - * Or run with specific device details: * mvn test -Dtest=NetconfIntegrationTest -Dnetconf.integration.enabled=true \ * -Dnetconf.host=192.168.1.1 -Dnetconf.username=admin -Dnetconf.password=secret + * + * Or provide the same values with environment variables: + * NETCONF_HOST=192.168.1.1 NETCONF_USERNAME=admin NETCONF_PASSWORD=secret \ + * mvn test -Dtest=NetconfIntegrationTest -Dnetconf.integration.enabled=true */ @EnabledIfSystemProperty(named = "netconf.integration.enabled", matches = "true") public class NetconfIntegrationTest { @@ -36,17 +47,17 @@ public class NetconfIntegrationTest { @BeforeAll static void setupCredentials() { - // Try to get credentials from system properties first - hostname = System.getProperty("netconf.host"); - username = System.getProperty("netconf.username"); - password = System.getProperty("netconf.password"); + // Prefer explicit JVM properties, then fall back to environment variables. + hostname = getConfigValue("netconf.host", "NETCONF_HOST"); + username = getConfigValue("netconf.username", "NETCONF_USERNAME"); + password = getConfigValue("netconf.password", "NETCONF_PASSWORD"); - String portStr = System.getProperty("netconf.port"); + String portStr = getConfigValue("netconf.port", "NETCONF_PORT"); if (portStr != null) { port = Integer.parseInt(portStr); } - String timeoutStr = System.getProperty("netconf.timeout"); + String timeoutStr = getConfigValue("netconf.timeout", "NETCONF_TIMEOUT"); if (timeoutStr != null) { timeout = Integer.parseInt(timeoutStr); } @@ -97,12 +108,20 @@ static void setupCredentials() { assertNotNull(password, "Password must be provided"); } - @Test - @DisplayName("Test device connection and basic capabilities") - void testDeviceConnection() throws NetconfException { - System.out.println("Testing device connection..."); + private static String getConfigValue(String propertyName, String environmentName) { + String propertyValue = System.getProperty(propertyName); + if (propertyValue != null && !propertyValue.trim().isEmpty()) { + return propertyValue; + } + String environmentValue = System.getenv(environmentName); + if (environmentValue != null && !environmentValue.trim().isEmpty()) { + return environmentValue; + } + return null; + } - Device device = Device.builder() + private static Device newDevice() throws NetconfException { + return Device.builder() .hostName(hostname) .userName(username) .password(password) @@ -110,18 +129,44 @@ void testDeviceConnection() throws NetconfException { .connectionTimeout(timeout) .strictHostKeyChecking(false) .build(); + } + + private static RpcReply getLastRpcReplyObject(Device device) { + try { + Field sessionField = Device.class.getDeclaredField("netconfSession"); + sessionField.setAccessible(true); + NetconfSession session = (NetconfSession) sessionField.get(device); + assertNotNull(session, "Netconf session should be present after connect"); + RpcReply reply = session.getLastRpcReplyObject(); + assertNotNull(reply, "Last RPC reply should be captured"); + return reply; + } catch (ReflectiveOperationException e) { + throw new AssertionError("Failed to inspect Device netconfSession for integration assertions", e); + } + } + + @Test + @DisplayName("Test device connection and basic capabilities") + void testDeviceConnection() throws NetconfException { + System.out.println("Testing device connection..."); + + Device device = newDevice(); try { device.connect(); assertTrue(device.isConnected(), "Device should be connected"); - // Test getting server capabilities - String[] capabilities = device.getNetconfCapabilities().toArray(new String[0]); + NegotiatedCapabilities negotiatedCapabilities = device.getNegotiatedCapabilities(); + assertNotNull(negotiatedCapabilities, "Negotiated capabilities should not be null"); + + // Verify the capabilities reported by the server hello. + String[] capabilities = negotiatedCapabilities.getServerCapabilities().toArray(new String[0]); assertNotNull(capabilities, "Server capabilities should not be null"); assertTrue(capabilities.length > 0, "Server should have at least one capability"); System.out.println("✓ Successfully connected to device"); System.out.println("✓ Server capabilities count: " + capabilities.length); + System.out.println("✓ Negotiated NETCONF base version: " + negotiatedCapabilities.getBaseVersion()); // Print some capabilities for debugging System.out.println("Server capabilities:"); @@ -142,18 +187,10 @@ void testDeviceConnection() throws NetconfException { @Test @DisplayName("Test basic get-config operation") - void testGetConfig() throws - org.xml.sax.SAXException, IOException { + void testGetConfig() throws org.xml.sax.SAXException, IOException, NetconfException { System.out.println("Testing get-config operation..."); - Device device = Device.builder() - .hostName(hostname) - .userName(username) - .password(password) - .port(port) - .connectionTimeout(timeout) - .strictHostKeyChecking(false) - .build(); + Device device = newDevice(); try { device.connect(); @@ -180,17 +217,10 @@ void testGetConfig() throws @Test @DisplayName("Test get operation with interface information") - void testGetInterfaceInformation() throws org.xml.sax.SAXException, IOException { + void testGetInterfaceInformation() throws org.xml.sax.SAXException, IOException, NetconfException { System.out.println("Testing get interface information..."); - Device device = Device.builder() - .hostName(hostname) - .userName(username) - .password(password) - .port(port) - .connectionTimeout(timeout) - .strictHostKeyChecking(false) - .build(); + Device device = newDevice(); try { device.connect(); @@ -275,21 +305,14 @@ void testMultipleConnections() throws NetconfException { for (int i = 1; i <= 3; i++) { System.out.println("Connection attempt " + i + "/3"); - Device device = Device.builder() - .hostName(hostname) - .userName(username) - .password(password) - .port(port) - .strictHostKeyChecking(false) - .connectionTimeout(timeout) - .build(); + Device device = newDevice(); try { device.connect(); assertTrue(device.isConnected(), "Device should be connected on attempt " + i); // Brief operation to ensure connection is working - int capabilityCount = device.getNetconfCapabilities().size(); + int capabilityCount = device.getNegotiatedCapabilities().getServerCapabilities().size(); assertTrue(capabilityCount > 0, "Should have capabilities on attempt " + i); } finally { @@ -312,33 +335,136 @@ void testMultipleConnections() throws NetconfException { } @Test - @DisplayName("Test RPC error handling") - void testRPCErrorHandling() throws NetconfException { + @DisplayName("Test negotiated base version matches server hello") + void testNegotiatedBaseVersionMatchesServerHello() throws NetconfException { + System.out.println("Testing negotiated base version..."); + + Device device = newDevice(); + + try { + device.connect(); + + NegotiatedCapabilities negotiatedCapabilities = device.getNegotiatedCapabilities(); + assertNotNull(negotiatedCapabilities, "Negotiated capabilities should not be null"); + assertNotNull(negotiatedCapabilities.getBaseVersion(), "Negotiated base version should not be null"); + + boolean serverSupportsBase11 = negotiatedCapabilities.getServerCapabilities() + .contains(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_1); + boolean serverSupportsBase10 = negotiatedCapabilities.getServerCapabilities() + .contains(NetconfConstants.URN_IETF_PARAMS_NETCONF_BASE_1_0); + + assertTrue(serverSupportsBase10 || serverSupportsBase11, + "Server should advertise at least one NETCONF base capability"); + + NegotiatedCapabilities.BaseVersion expectedBaseVersion = serverSupportsBase11 + ? NegotiatedCapabilities.BaseVersion.NETCONF_1_1 + : NegotiatedCapabilities.BaseVersion.NETCONF_1_0; + assertEquals(expectedBaseVersion, negotiatedCapabilities.getBaseVersion(), + "Negotiated base version should follow the shared hello capabilities"); + + System.out.println("✓ Negotiated base version: " + negotiatedCapabilities.getBaseVersion()); + } finally { + if (device.isConnected()) { + device.close(); + } + } + } + + @Test + @DisplayName("Test same session handles multiple sequential RPCs") + void testSequentialRpcsOnSingleSession() throws org.xml.sax.SAXException, IOException, NetconfException { + System.out.println("Testing sequential RPCs on one session..."); + + Device device = newDevice(); + + try { + device.connect(); + + XML firstReply = device.getRunningConfig(); + XML secondReply = device.getRunningConfigAndState(null); + XML thirdReply = device.getRunningConfig(); + + assertNotNull(firstReply, "First reply should not be null"); + assertNotNull(secondReply, "Second reply should not be null"); + assertNotNull(thirdReply, "Third reply should not be null"); + assertTrue(firstReply.toString().contains("rpc-reply"), "First reply should contain rpc-reply"); + assertTrue(secondReply.toString().contains("rpc-reply"), "Second reply should contain rpc-reply"); + assertTrue(thirdReply.toString().contains("rpc-reply"), "Third reply should contain rpc-reply"); + + System.out.println("✓ Same session handled sequential get-config/get/get-config RPCs"); + } finally { + if (device.isConnected()) { + device.close(); + } + } + } + + @Test + @DisplayName("Test streaming RPC close drains reply and keeps session aligned") + void testStreamingReplyCloseKeepsSessionAligned() throws org.xml.sax.SAXException, IOException, NetconfException { + System.out.println("Testing streamed RPC drain behavior..."); + + Device device = newDevice(); + + try { + device.connect(); + + char[] buffer = new char[256]; + try (BufferedReader reader = device.executeRPCRunning("")) { + int charsRead = reader.read(buffer); + assertTrue(charsRead > 0, "Streaming RPC should yield at least some response bytes"); + } + + XML followUpReply = device.getRunningConfig(); + assertNotNull(followUpReply, "Follow-up reply should not be null"); + assertTrue(followUpReply.toString().contains("rpc-reply"), + "Follow-up reply should still be parsed correctly after draining the stream"); + + System.out.println("✓ Closing streamed reply kept the session aligned"); + } finally { + if (device.isConnected()) { + device.close(); + } + } + } + + @Test + @DisplayName("Test RPC error handling returns structured rpc-error details") + void testRPCErrorHandling() throws Exception { System.out.println("Testing RPC error handling..."); - Device device = Device.builder() - .hostName(hostname) - .userName(username) - .password(password) - .port(port) - .strictHostKeyChecking(false) - .connectionTimeout(timeout) - .build(); + Device device = newDevice(); try { device.connect(); - // Send an intentionally malformed RPC - assertThrows(Exception.class, () -> device.executeRPC( - ""), - "Invalid RPC should throw an exception"); + RpcErrorException exception = assertThrows(RpcErrorException.class, () -> + device.executeRPC(""), + "Invalid RPC should throw a structured RpcErrorException"); + + assertNotNull(exception.getMessage(), "Exception message should be populated"); + assertEquals(1, exception.getRpcErrors().size(), + "Exception should expose one parsed rpc-error"); + + RpcReply rpcReply = getLastRpcReplyObject(device); + assertTrue(rpcReply.hasErrors(), "Last rpc-reply should contain an rpc-error"); + assertEquals(1, rpcReply.getErrors().size(), "Expected one rpc-error from the invalid RPC"); + + RpcError rpcError = rpcReply.getErrors().get(0); + assertEquals(RpcError.ErrorType.PROTOCOL, rpcError.errorType(), "Unexpected rpc-error type"); + assertEquals(RpcError.ErrorSeverity.ERROR, rpcError.errorSeverity(), "Unexpected rpc-error severity"); + assertEquals(RpcError.ErrorTag.OPERATION_FAILED, rpcError.errorTag(), "Unexpected rpc-error tag"); + assertNotNull(rpcError.errorInfo(), "rpc-error info should be present"); + assertEquals("invalid-rpc-that-should-not-exist", rpcError.errorInfo().getBadElement(), + "Server should identify the offending RPC element"); System.out.println("✓ RPC error handling works correctly"); + System.out.println("✓ Parsed rpc-error bad-element: " + rpcError.errorInfo().getBadElement()); - // Verify connection is still usable after error - int capabilityCount = device.getNetconfCapabilities().size(); - assertTrue(capabilityCount > 0, - "Connection should still be usable after RPC error"); + XML followUpReply = device.getRunningConfig(); + assertNotNull(followUpReply, "Connection should still support follow-up RPCs after an error"); + assertTrue(followUpReply.toString().contains("rpc-reply"), + "Follow-up reply should be a valid rpc-reply after an rpc-error"); System.out.println("✓ Connection remains usable after RPC error"); @@ -348,4 +474,62 @@ void testRPCErrorHandling() throws NetconfException { } } } -} \ No newline at end of file + + @Test + @DisplayName("Test candidate lock/validate/unlock workflow when supported") + void testCandidateLockValidateUnlockWorkflow() throws Exception { + System.out.println("Testing candidate lock/validate/unlock workflow..."); + + Device device = newDevice(); + boolean locked = false; + + try { + device.connect(); + + NegotiatedCapabilities negotiatedCapabilities = device.getNegotiatedCapabilities(); + Assumptions.assumeTrue(negotiatedCapabilities.supportsCandidate(), + "Server does not advertise candidate capability"); + Assumptions.assumeTrue(negotiatedCapabilities.supportsValidate(), + "Server does not advertise validate capability"); + + locked = device.lockConfig(); + assertTrue(locked, "Candidate datastore lock should succeed"); + + boolean validateReturnedTrue; + RpcReply validateReply; + try { + validateReturnedTrue = device.validate(); + validateReply = getLastRpcReplyObject(device); + } catch (ValidateException e) { + assertNotNull(e.getRpcReply(), "ValidateException should expose the parsed rpc-reply"); + assertFalse(e.getRpcErrors().isEmpty(), + "ValidateException should expose at least one parsed rpc-error"); + assertTrue(e.getRpcReply().hasErrors(), + "ValidateException rpc-reply should contain error-severity rpc-errors"); + System.out.println("ℹ Candidate validate returned structured rpc-errors: " + e.getRpcErrors()); + Assumptions.assumeTrue(false, + "Server validate failed in this environment: " + e.getMessage()); + return; + } + + assertTrue(validateReturnedTrue || validateReply.hasWarnings(), + "Candidate validation should either succeed cleanly or return warnings only"); + + boolean unlocked = device.unlockConfig(); + locked = false; + assertTrue(unlocked, "Candidate datastore unlock should succeed"); + + System.out.println("✓ Candidate lock/validate/unlock workflow completed"); + if (!validateReturnedTrue) { + System.out.println("ℹ Candidate validate returned warnings instead of a clean reply"); + } + } finally { + if (locked && device.isConnected()) { + assertTrue(device.unlockConfig(), "Candidate datastore unlock should succeed in cleanup"); + } + if (device.isConnected()) { + device.close(); + } + } + } +} diff --git a/src/test/java/net/juniper/netconf/integration/RUN-CRPD-CONTAINER.md b/src/test/java/net/juniper/netconf/integration/RUN-CRPD-CONTAINER.md index 68dc858..e318a9b 100644 --- a/src/test/java/net/juniper/netconf/integration/RUN-CRPD-CONTAINER.md +++ b/src/test/java/net/juniper/netconf/integration/RUN-CRPD-CONTAINER.md @@ -1,86 +1,224 @@ -# Running Juniper cRPD for integration tests +# Running Juniper cRPD for Integration Tests -This project’s integration tests need a live NETCONF endpoint. -You can spin up Juniper’s containerised Routing Protocol Daemon (**cRPD**) locally in < 1 minute. +This project's integration tests need a live NETCONF endpoint. One +easy local option is Juniper cRPD running in a container. ---- +The examples below use: -## 1 . Prerequisites +- `localhost:2222` for normal SSH and Junos CLI access +- `localhost:8830` for NETCONF-over-SSH -* **Docker + Colima** (or Docker Desktop) on macOS - ```bash - brew install colima docker docker-compose - colima start # arm64 by default; add --arch x86_64 if you need an amd64 VM - ``` +Using host port `8830` instead of `830` avoids binding a privileged +port on macOS/Linux and keeps the workflow rootless. -* **cRPD image** (free evaluation tarball from Juniper) - Place it under `src/test/resources/` as shown below. +## 1. Prerequisites ---- +- A cRPD image tarball from Juniper +- One container runtime: + - Docker Desktop or Docker + Colima + - Podman on macOS/Linux +- Java 17+ if you plan to run this repository's integration tests -## 2 . Load the image into Docker +On macOS with Docker + Colima: ```bash -docker load < src/test/resources/junos-routing-crpd-docker-23.2R1.13-arm64.tgz -# or …-amd64… if you’re running a colima --arch x86_64 VM +brew install colima docker docker-compose +colima start ``` -Verify: +On macOS with Podman: ```bash -docker images | grep crpd -# crpd 23.2R1.13 0cf5ad… 498MB +brew install podman +podman machine init +podman machine start ``` ---- +Place the cRPD image archive under `src/test/resources/`. If you +follow this workflow, the archive and extracted image artifacts are +treated as local test assets and are ignored by git. -## 3 . Start cRPD with NETCONF and SSH exposed +## 2. Load the image into your runtime + +Docker: + +```bash +docker load < src/test/resources/junos-routing-crpd-docker-amd64-23.2R1.13.tgz +docker images | grep -i crpd +``` + +Podman: + +```bash +podman load -i src/test/resources/junos-routing-crpd-docker-amd64-23.2R1.13.tgz +podman images | grep -i crpd +``` + +If you are on Apple Silicon and your cRPD image is the `amd64` build, +keep using that filename and add `--platform linux/amd64` when you +start the container. + +## 3. Start cRPD with SSH and NETCONF exposed + +Docker: ```bash +docker rm -f crpd1 docker run -d --name crpd1 --privileged \ - -p 2222:22 \ # SSH - -p 1830:830 \ # NETCONF - crpd:23.2R1.13 + --hostname crpd1 \ + -p 2222:22 \ + -p 8830:830 \ + crpd:23.2R1.13 ``` -Wait ~40 s, then configure a test user and enable NETCONF: +Podman: ```bash -docker exec -ti crpd1 cli +podman rm -f crpd1 +podman run -d --name crpd1 \ + --platform linux/amd64 \ + --privileged \ + --hostname crpd1 \ + -p 2222:22 \ + -p 8830:830 \ + localhost/crpd:23.2R1.13 +``` + +If you are not running an `amd64` image, remove `--platform +linux/amd64` from the Podman command. + +## 4. Configure the minimum Junos settings + +Wait until `docker exec -it crpd1 cli` or `podman exec -it crpd1 cli` +works, then configure a test user and enable NETCONF: + +```bash +podman exec -it crpd1 cli +# or: docker exec -it crpd1 cli -# inside Junos CLI +# inside the Junos CLI configure -set system root-authentication plain-text-password -# (enter a password, e.g. Junos123) set system login user test uid 2000 class super-user set system login user test authentication plain-text-password -# (password: test1234) +# password: test1234 set system services ssh set system services netconf ssh +set system services netconf rfc-compliant commit and-quit ``` ---- +`set system root-authentication plain-text-password` is optional. It +is only needed if you want to log in as `root` over SSH. It is not +required for this repository's NETCONF integration tests. + +The relevant committed config should look roughly like this: + +```text +system { + login { + user test { + uid 2000; + class super-user; + authentication { + encrypted-password "$"; + } + } + } + services { + ssh; + netconf { + ssh { + port 830; + } + rfc-compliant; + } + } +} +``` + +What `rfc-compliant` changes in practice on cRPD 23.2R1.13: + +- Server replies switch to RFC-shaped NETCONF XML such as + `nc:hello`, `nc:rpc-reply`, `nc:ok`, and `nc:rpc-error` +- `` replies use the newer Junos config namespace + `http://yang.juniper.net/junos/conf/root` +- The server may still advertise only NETCONF `base:1.0` in ``, even with + `rfc-compliant` enabled + +That last point matters: in this environment, `rfc-compliant` +improves XML conformance, but it does not by itself prove NETCONF 1.1 +chunked framing support. If the server does not advertise +`urn:ietf:params:netconf:base:1.1`, delimiter framing with `]]>]]>` is +still the expected behavior. + +## 5. Verify cRPD is actually ready -## 4 . Run the integration test wrapper +Container is running: + +```bash +podman ps --filter name=crpd1 +# or: docker ps --filter name=crpd1 +``` + +Port mapping looks right: + +```bash +podman port crpd1 +# or: docker port crpd1 +# expect 22/tcp -> 2222 and 830/tcp -> 8830 +``` + +Junos CLI responds: + +```bash +podman exec crpd1 cli -c "show version" +podman exec crpd1 cli -c "show configuration system services | display set" +podman exec crpd1 cli -c \ + "show configuration system login user test | display set" +# or use the same commands with `docker exec` +``` + +End-to-end NETCONF handshake works from the host: + +```bash +ssh -s -p 8830 test@localhost netconf +``` + +After you authenticate, you should see a NETCONF `` from the +server. That is a much better readiness check than only confirming +that the container exists. + +## 6. Run the integration tests + +From the repository root: ```bash ./src/test/java/net/juniper/netconf/integration/run-integration-tests.sh \ - --username test \ - --password test1234 \ - --host localhost \ - --port 1830 + --username test \ + --password test1234 \ + --host localhost \ + --port 8830 ``` -The script builds the library, spins up JUnit tests, and targets the NETCONF service you just started. +The wrapper builds the library and runs the JUnit integration suite +against the NETCONF endpoint you just started. + +If you prefer Gradle: ---- +```bash +NETCONF_HOST=localhost \ +NETCONF_USERNAME=test \ +NETCONF_PASSWORD=test1234 \ +NETCONF_PORT=8830 \ +./gradlew integrationTest +``` -### Cleaning up +## 7. Cleanup ```bash -docker rm -f crpd1 # stop & remove the container +docker rm -f crpd1 +# or: podman rm -f crpd1 ``` -That’s it! You now have a repeatable way to launch a Junos device for automated NETCONF testing. \ No newline at end of file +That gives you a repeatable local Junos target for NETCONF integration testing. diff --git a/src/test/java/net/juniper/netconf/integration/run-integration-tests.sh b/src/test/java/net/juniper/netconf/integration/run-integration-tests.sh index 9067daf..a07be8f 100755 --- a/src/test/java/net/juniper/netconf/integration/run-integration-tests.sh +++ b/src/test/java/net/juniper/netconf/integration/run-integration-tests.sh @@ -3,11 +3,14 @@ # run-integration-tests.sh # Script to run netconf-java integration tests with interactive credential prompts -set -e +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/../../../../../../.." && pwd)" echo "=== NetConf Java Integration Test Runner ===" echo "This script will run integration tests against a real network device." -echo "You will be prompted for connection details if not provided via environment." +echo "You will be prompted for connection details if not provided via arguments or environment." echo "" # Check if Maven is available @@ -17,7 +20,6 @@ if ! command -v mvn &> /dev/null; then fi # Parse command line arguments -INTERACTIVE=true SKIP_COMPILE=false while [[ $# -gt 0 ]]; do @@ -70,54 +72,42 @@ while [[ $# -gt 0 ]]; do done # Set defaults from environment if not provided via command line -NETCONF_HOST=${NETCONF_HOST:-$NETCONF_HOST} -NETCONF_USERNAME=${NETCONF_USERNAME:-$NETCONF_USERNAME} -NETCONF_PASSWORD=${NETCONF_PASSWORD:-$NETCONF_PASSWORD} -NETCONF_PORT=${NETCONF_PORT:-830} -NETCONF_TIMEOUT=${NETCONF_TIMEOUT:-30000} +NETCONF_HOST="${NETCONF_HOST:-}" +NETCONF_USERNAME="${NETCONF_USERNAME:-}" +NETCONF_PASSWORD="${NETCONF_PASSWORD:-}" +NETCONF_PORT="${NETCONF_PORT:-830}" +NETCONF_TIMEOUT="${NETCONF_TIMEOUT:-30000}" +export NETCONF_HOST NETCONF_USERNAME NETCONF_PASSWORD NETCONF_PORT NETCONF_TIMEOUT # Compile the project first (unless skipped) if [ "$SKIP_COMPILE" = false ]; then echo "Compiling project..." - mvn compile test-compile -q - if [ $? -ne 0 ]; then - echo "Error: Failed to compile project" - exit 1 - fi + ( + cd "$REPO_ROOT" + mvn compile test-compile -q + ) echo "✓ Project compiled successfully" echo "" fi # Build Maven command -MVN_CMD="mvn test -Dtest=NetconfIntegrationTest -Dnetconf.integration.enabled=true" - -# Add system properties if provided -if [ -n "$NETCONF_HOST" ]; then - MVN_CMD="$MVN_CMD -Dnetconf.host=$NETCONF_HOST" -fi - -if [ -n "$NETCONF_USERNAME" ]; then - MVN_CMD="$MVN_CMD -Dnetconf.username=$NETCONF_USERNAME" -fi - -if [ -n "$NETCONF_PASSWORD" ]; then - MVN_CMD="$MVN_CMD -Dnetconf.password=$NETCONF_PASSWORD" -fi - -if [ -n "$NETCONF_PORT" ]; then - MVN_CMD="$MVN_CMD -Dnetconf.port=$NETCONF_PORT" -fi - -if [ -n "$NETCONF_TIMEOUT" ]; then - MVN_CMD="$MVN_CMD -Dnetconf.timeout=$NETCONF_TIMEOUT" -fi +MVN_CMD=( + mvn + test + "-Dtest=NetconfIntegrationTest" + "-Dnetconf.integration.enabled=true" +) echo "Running integration tests..." -echo "Command: $MVN_CMD" +echo "Connection settings are passed via NETCONF_* environment variables." +echo "Command: mvn test -Dtest=NetconfIntegrationTest -Dnetconf.integration.enabled=true" echo "" # Execute the tests -eval $MVN_CMD +( + cd "$REPO_ROOT" + "${MVN_CMD[@]}" +) echo "" -echo "=== Integration Tests Complete ===" \ No newline at end of file +echo "=== Integration Tests Complete ==="