From 3d7c09872634c9f81d7dbf41256081e3ccf68725 Mon Sep 17 00:00:00 2001 From: Ryan Livingston Date: Fri, 9 Feb 2024 10:48:13 -0500 Subject: [PATCH 1/2] GH-427: Read initial ACK on channel open prior to direct stream upload --- .../sshd/scp/client/DefaultScpClient.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/sshd-scp/src/main/java/org/apache/sshd/scp/client/DefaultScpClient.java b/sshd-scp/src/main/java/org/apache/sshd/scp/client/DefaultScpClient.java index 7a794f41e..1708e955d 100644 --- a/sshd-scp/src/main/java/org/apache/sshd/scp/client/DefaultScpClient.java +++ b/sshd-scp/src/main/java/org/apache/sshd/scp/client/DefaultScpClient.java @@ -112,13 +112,16 @@ public void upload( String cmd = ScpClient.createSendCommand(remote, options); ClientSession session = getClientSession(); ChannelExec channel = openCommandChannel(session, cmd); - try (InputStream invOut = channel.getInvertedOut(); - OutputStream invIn = channel.getInvertedIn()) { - // NOTE: we use a mock file system since we expect no invocations for it - ScpHelper helper = new ScpHelper(session, invOut, invIn, new MockFileSystem(remote), opener, listener); - Path mockPath = new MockPath(remote); - helper.sendStream(new DefaultScpStreamResolver(name, mockPath, perms, time, size, local, cmd), - options.contains(Option.PreserveAttributes), ScpHelper.DEFAULT_SEND_BUFFER_SIZE); + try { + try (InputStream invOut = channel.getInvertedOut(); + OutputStream invIn = channel.getInvertedIn()) { + // NOTE: we use a mock file system since we expect no invocations for it + ScpHelper helper = new ScpHelper(session, invOut, invIn, new MockFileSystem(remote), opener, listener); + Path mockPath = new MockPath(remote); + helper.readAck(false); + helper.sendStream(new DefaultScpStreamResolver(name, mockPath, perms, time, size, local, cmd), + options.contains(Option.PreserveAttributes), ScpHelper.DEFAULT_SEND_BUFFER_SIZE); + } handleCommandExitStatus(cmd, channel); } finally { channel.close(false); From 61576a548abf644d0f8d0e3b9157ead9065dc23d Mon Sep 17 00:00:00 2001 From: Ryan Livingston Date: Fri, 9 Feb 2024 14:02:44 -0500 Subject: [PATCH 2/2] GH-427: Introduce ScpHelper.readAndValidateOperationAck --- .../sshd/scp/client/DefaultScpClient.java | 7 ++++--- .../org/apache/sshd/scp/common/ScpHelper.java | 21 ++++++++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/sshd-scp/src/main/java/org/apache/sshd/scp/client/DefaultScpClient.java b/sshd-scp/src/main/java/org/apache/sshd/scp/client/DefaultScpClient.java index 1708e955d..d9223d455 100644 --- a/sshd-scp/src/main/java/org/apache/sshd/scp/client/DefaultScpClient.java +++ b/sshd-scp/src/main/java/org/apache/sshd/scp/client/DefaultScpClient.java @@ -118,9 +118,10 @@ public void upload( // NOTE: we use a mock file system since we expect no invocations for it ScpHelper helper = new ScpHelper(session, invOut, invIn, new MockFileSystem(remote), opener, listener); Path mockPath = new MockPath(remote); - helper.readAck(false); - helper.sendStream(new DefaultScpStreamResolver(name, mockPath, perms, time, size, local, cmd), - options.contains(Option.PreserveAttributes), ScpHelper.DEFAULT_SEND_BUFFER_SIZE); + DefaultScpStreamResolver resolver = new DefaultScpStreamResolver(name, mockPath, perms, time, size, local, cmd); + helper.readAndValidateOperationAck(cmd, resolver); + helper.sendStream(resolver, options.contains(Option.PreserveAttributes), + ScpHelper.DEFAULT_SEND_BUFFER_SIZE); } handleCommandExitStatus(cmd, channel); } finally { diff --git a/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpHelper.java b/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpHelper.java index f6f37b917..bff83ff78 100644 --- a/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpHelper.java +++ b/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpHelper.java @@ -411,12 +411,8 @@ public String readLine(boolean canEof) throws IOException { } public void send(Collection paths, boolean recursive, boolean preserve, int bufferSize) throws IOException { - ScpAckInfo ackInfo = readAck(false); boolean debugEnabled = log.isDebugEnabled(); - if (debugEnabled) { - log.debug("send({}) ACK={}", paths, ackInfo); - } - validateOperationReadyCode("send", "Paths", ackInfo); + readAndValidateOperationAck("send", "Paths"); LinkOption[] options = IoUtils.getLinkOptions(true); for (String pattern : paths) { @@ -464,11 +460,7 @@ public void send(Collection paths, boolean recursive, boolean preserve, public void sendPaths(Collection paths, boolean recursive, boolean preserve, int bufferSize) throws IOException { - ScpAckInfo ackInfo = readAck(false); - if (log.isDebugEnabled()) { - log.debug("sendPaths({}) ACK={}", paths, ackInfo); - } - validateOperationReadyCode("sendPaths", "Paths", ackInfo); + readAndValidateOperationAck("sendPaths", "Paths"); LinkOption[] options = IoUtils.getLinkOptions(true); for (Path file : paths) { @@ -750,6 +742,15 @@ public ScpAckInfo readAck(boolean canEof) throws IOException { return ScpAckInfo.readAck(in, csIn, canEof); } + public void readAndValidateOperationAck(String cmd, Object location) throws IOException { + ScpAckInfo ackInfo = readAck(false); + boolean debugEnabled = log.isDebugEnabled(); + if (debugEnabled) { + log.debug("readAndValidateOperationAck({}) ACK={}", location, ackInfo); + } + validateOperationReadyCode(cmd, location, ackInfo); + } + @Override public String toString() { return getClass().getSimpleName() + "[" + getSession() + "]";