Skip to content

Commit

Permalink
GH-628: SFTP: Fix reading directories with trailing blanks in the name
Browse files Browse the repository at this point in the history
ValidateUtils.checkNotNullAndNotEmpty(String, ...) as a side-effect
returns the trimmed string!

Add new methods ValidateUtils.hasContent(String, ...) that don't do so,
and use that instead.
  • Loading branch information
tomaswolf committed Nov 8, 2024
1 parent 38bb2c6 commit 827e81e
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 12 deletions.
3 changes: 2 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,11 @@
## Bug Fixes

* [GH-618](https://github.com/apache/mina-sshd/issues/618) Fix reading an `OpenSshCertificate` from a `Buffer`
* [GH-628](https://github.com/apache/mina-sshd/issues/628) SFTP: fix reading directories with trailing blanks in the name

## New Features

* [GH-606](https://github.com/apache/mina-sshd/issues/606) Support ML-KEM PQC key exchange
* [GH-606](https://github.com/apache/mina-sshd/issues/606) Support ML-KEM PQC hybrid key exchanges

## Potential compatibility issues

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,27 @@ public static <T> T checkNotNull(T t, String message, Object... args) {
return t;
}

public static String hasContent(String t, String message) {
if (t == null || t.isEmpty()) {
throwIllegalArgumentException(message);
}
return t;
}

public static String hasContent(String t, String message, Object arg) {
if (t == null || t.isEmpty()) {
throwIllegalArgumentException(message, arg);
}
return t;
}

public static String hasContent(String t, String message, Object... args) {
if (t == null || t.isEmpty()) {
throwIllegalArgumentException(message, args);
}
return t;
}

public static String checkNotNullAndNotEmpty(String t, String message) {
t = checkNotNull(t, message).trim();
checkTrue(GenericUtils.length(t) > 0, message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public SftpDirEntryIterator(SftpClient client, Handle dirHandle) {
*/
public SftpDirEntryIterator(SftpClient client, String path, Handle dirHandle, boolean closeOnFinished) {
this.client = Objects.requireNonNull(client, "No SFTP client instance");
this.dirPath = ValidateUtils.checkNotNullAndNotEmpty(path, "No path");
this.dirPath = ValidateUtils.hasContent(path, "No path");
this.dirHandle = Objects.requireNonNull(dirHandle, "No directory handle");
this.closeOnFinished = closeOnFinished;
this.dirEntries = load(dirHandle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class SftpIterableDirEntry implements SftpClientHolder, Iterable<DirEntry
*/
public SftpIterableDirEntry(SftpClient client, String path) {
this.client = Objects.requireNonNull(client, "No client instance");
this.path = ValidateUtils.checkNotNullAndNotEmpty(path, "No remote path");
this.path = ValidateUtils.hasContent(path, "No remote path");
}

@Override
Expand Down
43 changes: 34 additions & 9 deletions sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,6 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

/**
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
Expand Down Expand Up @@ -1195,6 +1186,40 @@ public void writeCreateAppend(int handleSize) throws Exception {
}
}

@MethodSource("getParameters")
@ParameterizedTest(name = "FILE_HANDLE_SIZE {0}") // SSHD-1215
public void listDirWithBlank(int handleSize) throws Exception {
Assumptions.assumeFalse(OsUtils.isWin32(), "Windows does not allow trailing blanks anyway");
initSftpTest(handleSize);
Path targetPath = detectTargetFolder();
Path lclSftp = CommonTestSupportUtils.resolve(targetPath, SftpConstants.SFTP_SUBSYSTEM_NAME, getClass().getSimpleName(),
getCurrentTestName());
CommonTestSupportUtils.deleteRecursive(lclSftp);

Path parentPath = targetPath.getParent();
Path clientFolder = assertHierarchyTargetFolderExists(lclSftp.resolve("withBlank "));
String fileName = "file ";
Path clientFile = clientFolder.resolve(fileName);

byte[] foo = { 'f', 'o', 'o' };
Files.write(clientFile, foo);

String dir = CommonTestSupportUtils.resolveRelativeRemotePath(parentPath, clientFolder);

try (SftpClient sftp = createSingleSessionClient()) {
List<String> expected = new ArrayList<>();
expected.add(".");
expected.add("..");
expected.add(fileName);
List<String> actual = new ArrayList<>();
sftp.readDir(dir).iterator().forEachRemaining(e -> actual.add(e.getFilename()));
assertEquals(expected, actual, "Unexpected directory entries");
try (InputStream in = sftp.read(dir + '/' + fileName)) {
assertArrayEquals(foo, IoUtils.toByteArray(in), "Wrong file content");
}
}
}

@MethodSource("getParameters")
@ParameterizedTest(name = "FILE_HANDLE_SIZE {0}")
public void handleSize(int handleSize) throws Exception {
Expand Down

0 comments on commit 827e81e

Please sign in to comment.