From 624fe839cba84764e7c429a6243cf3107c68e995 Mon Sep 17 00:00:00 2001 From: Lucas <16666115+EndzeitBegins@users.noreply.github.com> Date: Mon, 15 Apr 2024 20:18:15 +0200 Subject: [PATCH] Support premature termination of listing (#928) * Support premature termination of listing * Added license header + small refactor --------- Co-authored-by: Jeroen van Erp --- .../sftp/RemoteResourceFilterConverter.java | 30 ++++++++++ .../sshj/sftp/RemoteResourceSelector.java | 49 +++++++++++++++++ .../schmizz/sshj/sftp/RemoteDirectory.java | 55 +++++++++++++------ .../net/schmizz/sshj/sftp/SFTPClient.java | 17 ++++-- .../schmizz/sshj/sftp/StatefulSFTPClient.java | 22 +++++--- .../sshj/sftp/SFTPClientSpec.groovy | 55 +++++++++++++++++++ 6 files changed, 196 insertions(+), 32 deletions(-) create mode 100644 src/main/java/com/hierynomus/sshj/sftp/RemoteResourceFilterConverter.java create mode 100644 src/main/java/com/hierynomus/sshj/sftp/RemoteResourceSelector.java diff --git a/src/main/java/com/hierynomus/sshj/sftp/RemoteResourceFilterConverter.java b/src/main/java/com/hierynomus/sshj/sftp/RemoteResourceFilterConverter.java new file mode 100644 index 000000000..e73aec473 --- /dev/null +++ b/src/main/java/com/hierynomus/sshj/sftp/RemoteResourceFilterConverter.java @@ -0,0 +1,30 @@ +/* + * Copyright (C)2009 - SSHJ Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.hierynomus.sshj.sftp; + +import com.hierynomus.sshj.sftp.RemoteResourceSelector.Result; +import net.schmizz.sshj.sftp.RemoteResourceFilter; + +public class RemoteResourceFilterConverter { + + public static RemoteResourceSelector selectorFrom(RemoteResourceFilter filter) { + if (filter == null) { + return RemoteResourceSelector.ALL; + } + + return resource -> filter.accept(resource) ? Result.ACCEPT : Result.CONTINUE; + } +} diff --git a/src/main/java/com/hierynomus/sshj/sftp/RemoteResourceSelector.java b/src/main/java/com/hierynomus/sshj/sftp/RemoteResourceSelector.java new file mode 100644 index 000000000..3a9e4993d --- /dev/null +++ b/src/main/java/com/hierynomus/sshj/sftp/RemoteResourceSelector.java @@ -0,0 +1,49 @@ +/* + * Copyright (C)2009 - SSHJ Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.hierynomus.sshj.sftp; + +import net.schmizz.sshj.sftp.RemoteResourceInfo; + +public interface RemoteResourceSelector { + public static RemoteResourceSelector ALL = new RemoteResourceSelector() { + @Override + public Result select(RemoteResourceInfo resource) { + return Result.ACCEPT; + } + }; + + enum Result { + /** + * Accept the remote resource and add it to the result. + */ + ACCEPT, + + /** + * Do not add the remote resource to the result and continue with the next. + */ + CONTINUE, + + /** + * Do not add the remote resource to the result and stop further execution. + */ + BREAK; + } + + /** + * Decide whether the remote resource should be included in the result and whether execution should continue. + */ + Result select(RemoteResourceInfo resource); +} diff --git a/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java b/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java index 4f9718ed7..6e4e63757 100644 --- a/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java +++ b/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java @@ -15,6 +15,7 @@ */ package net.schmizz.sshj.sftp; +import com.hierynomus.sshj.sftp.RemoteResourceSelector; import net.schmizz.sshj.sftp.Response.StatusCode; import java.io.IOException; @@ -22,6 +23,8 @@ import java.util.List; import java.util.concurrent.TimeUnit; +import static com.hierynomus.sshj.sftp.RemoteResourceFilterConverter.selectorFrom; + public class RemoteDirectory extends RemoteResource { @@ -31,37 +34,55 @@ public RemoteDirectory(SFTPEngine requester, String path, byte[] handle) { public List scan(RemoteResourceFilter filter) throws IOException { - List rri = new LinkedList(); - // TODO: Remove GOTO! - loop: - for (; ; ) { - final Response res = requester.request(newRequest(PacketType.READDIR)) + return scan(selectorFrom(filter)); + } + + public List scan(RemoteResourceSelector selector) + throws IOException { + if (selector == null) { + selector = RemoteResourceSelector.ALL; + } + + List remoteResourceInfos = new LinkedList<>(); + + while (true) { + final Response response = requester.request(newRequest(PacketType.READDIR)) .retrieve(requester.getTimeoutMs(), TimeUnit.MILLISECONDS); - switch (res.getType()) { + switch (response.getType()) { case NAME: - final int count = res.readUInt32AsInt(); + final int count = response.readUInt32AsInt(); for (int i = 0; i < count; i++) { - final String name = res.readString(requester.sub.getRemoteCharset()); - res.readString(); // long name - IGNORED - shdve never been in the protocol - final FileAttributes attrs = res.readFileAttributes(); + final String name = response.readString(requester.sub.getRemoteCharset()); + response.readString(); // long name - IGNORED - shdve never been in the protocol + final FileAttributes attrs = response.readFileAttributes(); final PathComponents comps = requester.getPathHelper().getComponents(path, name); final RemoteResourceInfo inf = new RemoteResourceInfo(comps, attrs); - if (!(".".equals(name) || "..".equals(name)) && (filter == null || filter.accept(inf))) { - rri.add(inf); + + if (".".equals(name) || "..".equals(name)) { + continue; + } + + final RemoteResourceSelector.Result selectionResult = selector.select(inf); + switch (selectionResult) { + case ACCEPT: + remoteResourceInfos.add(inf); + break; + case CONTINUE: + continue; + case BREAK: + return remoteResourceInfos; } } break; case STATUS: - res.ensureStatusIs(StatusCode.EOF); - break loop; + response.ensureStatusIs(StatusCode.EOF); + return remoteResourceInfos; default: - throw new SFTPException("Unexpected packet: " + res.getType()); + throw new SFTPException("Unexpected packet: " + response.getType()); } } - return rri; } - } diff --git a/src/main/java/net/schmizz/sshj/sftp/SFTPClient.java b/src/main/java/net/schmizz/sshj/sftp/SFTPClient.java index af9d70650..bc94cd53d 100644 --- a/src/main/java/net/schmizz/sshj/sftp/SFTPClient.java +++ b/src/main/java/net/schmizz/sshj/sftp/SFTPClient.java @@ -15,6 +15,7 @@ */ package net.schmizz.sshj.sftp; +import com.hierynomus.sshj.sftp.RemoteResourceSelector; import net.schmizz.sshj.connection.channel.direct.SessionFactory; import net.schmizz.sshj.xfer.FilePermission; import net.schmizz.sshj.xfer.LocalDestFile; @@ -25,6 +26,8 @@ import java.io.IOException; import java.util.*; +import static com.hierynomus.sshj.sftp.RemoteResourceFilterConverter.selectorFrom; + public class SFTPClient implements Closeable { @@ -57,16 +60,18 @@ public SFTPFileTransfer getFileTransfer() { public List ls(String path) throws IOException { - return ls(path, null); + return ls(path, RemoteResourceSelector.ALL); } public List ls(String path, RemoteResourceFilter filter) throws IOException { - final RemoteDirectory dir = engine.openDir(path); - try { - return dir.scan(filter); - } finally { - dir.close(); + return ls(path, selectorFrom(filter)); + } + + public List ls(String path, RemoteResourceSelector selector) + throws IOException { + try (RemoteDirectory dir = engine.openDir(path)) { + return dir.scan(selector == null ? RemoteResourceSelector.ALL : selector); } } diff --git a/src/main/java/net/schmizz/sshj/sftp/StatefulSFTPClient.java b/src/main/java/net/schmizz/sshj/sftp/StatefulSFTPClient.java index 4f31d0e8d..872dae8e9 100644 --- a/src/main/java/net/schmizz/sshj/sftp/StatefulSFTPClient.java +++ b/src/main/java/net/schmizz/sshj/sftp/StatefulSFTPClient.java @@ -15,6 +15,7 @@ */ package net.schmizz.sshj.sftp; +import com.hierynomus.sshj.sftp.RemoteResourceSelector; import net.schmizz.sshj.connection.channel.direct.SessionFactory; import net.schmizz.sshj.xfer.LocalDestFile; import net.schmizz.sshj.xfer.LocalSourceFile; @@ -23,6 +24,8 @@ import java.util.List; import java.util.Set; +import static com.hierynomus.sshj.sftp.RemoteResourceFilterConverter.selectorFrom; + public class StatefulSFTPClient extends SFTPClient { @@ -57,7 +60,7 @@ public synchronized void cd(String dirname) public synchronized List ls() throws IOException { - return ls(cwd, null); + return ls(cwd, RemoteResourceSelector.ALL); } public synchronized List ls(RemoteResourceFilter filter) @@ -70,20 +73,21 @@ public synchronized String pwd() return super.canonicalize(cwd); } - @Override public List ls(String path) throws IOException { - return ls(path, null); + return ls(path, RemoteResourceSelector.ALL); } - @Override public List ls(String path, RemoteResourceFilter filter) throws IOException { - final RemoteDirectory dir = getSFTPEngine().openDir(cwdify(path)); - try { - return dir.scan(filter); - } finally { - dir.close(); + return ls(path, selectorFrom(filter)); + } + + @Override + public List ls(String path, RemoteResourceSelector selector) + throws IOException { + try (RemoteDirectory dir = getSFTPEngine().openDir(cwdify(path))) { + return dir.scan(selector == null ? RemoteResourceSelector.ALL : selector); } } diff --git a/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy b/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy index b3f6774c7..421ca60ad 100644 --- a/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy +++ b/src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy @@ -19,6 +19,7 @@ import com.hierynomus.sshj.test.SshServerExtension import com.hierynomus.sshj.test.util.FileUtil import net.schmizz.sshj.SSHClient import net.schmizz.sshj.sftp.FileMode +import net.schmizz.sshj.sftp.RemoteResourceInfo import net.schmizz.sshj.sftp.SFTPClient import org.junit.jupiter.api.extension.RegisterExtension import spock.lang.Specification @@ -206,6 +207,60 @@ class SFTPClientSpec extends Specification { attrs.type == FileMode.Type.DIRECTORY } + def "should support premature termination of listing"() { + given: + SSHClient sshClient = fixture.setupConnectedDefaultClient() + sshClient.authPassword("test", "test") + SFTPClient sftpClient = sshClient.newSFTPClient() + + final Path source = Files.createDirectory(temp.resolve("source")).toAbsolutePath() + final Path destination = Files.createDirectory(temp.resolve("destination")).toAbsolutePath() + final Path firstFile = Files.writeString(source.resolve("a_first.txt"), "first") + final Path secondFile = Files.writeString(source.resolve("b_second.txt"), "second") + final Path thirdFile = Files.writeString(source.resolve("c_third.txt"), "third") + final Path fourthFile = Files.writeString(source.resolve("d_fourth.txt"), "fourth") + sftpClient.put(firstFile.toString(), destination.resolve(firstFile.fileName).toString()) + sftpClient.put(secondFile.toString(), destination.resolve(secondFile.fileName).toString()) + sftpClient.put(thirdFile.toString(), destination.resolve(thirdFile.fileName).toString()) + sftpClient.put(fourthFile.toString(), destination.resolve(fourthFile.fileName).toString()) + + def filesListed = 0 + RemoteResourceInfo expectedFile = null + RemoteResourceSelector limitingSelector = new RemoteResourceSelector() { + @Override + RemoteResourceSelector.Result select(RemoteResourceInfo resource) { + filesListed += 1 + + switch(filesListed) { + case 1: + return RemoteResourceSelector.Result.CONTINUE + case 2: + expectedFile = resource + return RemoteResourceSelector.Result.ACCEPT + case 3: + return RemoteResourceSelector.Result.BREAK + default: + throw new AssertionError((Object) "Should NOT select any more resources") + } + } + } + + when: + def listingResult = sftpClient + .ls(destination.toString(), limitingSelector); + + then: + // first should be skipped by CONTINUE + listingResult.contains(expectedFile) // second should be included by ACCEPT + // third should be skipped by BREAK + // fourth should be skipped by preceding BREAK + listingResult.size() == 1 + + cleanup: + sftpClient.close() + sshClient.disconnect() + } + private void doUpload(File src, File dest) throws IOException { SSHClient sshClient = fixture.setupConnectedDefaultClient() sshClient.authPassword("test", "test")