fix: Allow spaces in file names when using fs plugin (#152)

close #147
kestra-io · Sep 20, 2024 · e104251 · e104251
1 parent 3e75a12
commit e104251
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 2 deletions.
diff --git a/src/main/java/io/kestra/plugin/fs/vfs/VfsService.java b/src/main/java/io/kestra/plugin/fs/vfs/VfsService.java
@@ -9,6 +9,7 @@
 import org.apache.commons.vfs2.*;
 import org.apache.commons.vfs2.impl.StandardFileSystemManager;
 import org.apache.commons.vfs2.provider.AbstractFileObject;
+import org.apache.commons.vfs2.util.URIUtils;
 
 import java.io.FileOutputStream;
 import java.io.OutputStream;
@@ -210,7 +211,7 @@ public static Move.Output move(
     ) throws Exception {
         // user pass a destination without filename, we add it
         if (!isDirectory(from) && isDirectory(to)) {
-            to = to.resolve(StringUtils.stripEnd(to.getPath(), "/") + "/" + FilenameUtils.getName(from.getPath()));
+            to = to.resolve(URIUtils.encodePath(StringUtils.stripEnd(to.getPath(), "/") + "/" + FilenameUtils.getName(from.getPath())));
         }
 
         try (
@@ -222,7 +223,7 @@ public static Move.Output move(
             }
 
             if (!remote.exists()) {
-                URI pathToCreate = to.resolve("/" + FilenameUtils.getPath(to.getPath()));
+                URI pathToCreate = to.resolve("/" + URIUtils.encodePath(FilenameUtils.getPath(to.getPath())));
 
                 try (FileObject directory = fsm.resolveFile(to.toString(), fileSystemOptions)) {
                     if (!directory.exists()) {

diff --git a/src/test/java/io/kestra/plugin/fs/ftp/MoveTest.java b/src/test/java/io/kestra/plugin/fs/ftp/MoveTest.java
@@ -88,4 +88,30 @@ void moveDirectory() throws Exception {
 
         assertThat(run.getTo().getPath(), containsString(to));
     }
+
+    @Test
+    void moveFileWithSpaceToDirectoryWithSpace() throws Exception {
+        //Add space in filename
+        String from = "upload/" + IdUtils.create() + "/" + IdUtils.create() + " space .yaml";
+        String to = "upload/" + IdUtils.create() + "-move/" + IdUtils.create() + "/" + IdUtils.create() + " space2 /";
+
+        ftpUtils.upload(from);
+
+        Move task = Move.builder()
+                .id(MoveTest.class.getSimpleName())
+                .type(Move.class.getName())
+                .from(from)
+                .to(to)
+                .host("localhost")
+                .port("6621")
+                .username("guest")
+                .password("guest")
+                .build();
+
+        Move.Output run = task.run(TestsUtils.mockRunContext(runContextFactory, task, ImmutableMap.of()));
+
+        assertThat(run.getTo().getPath(), containsString(" "));
+        assertThat(run.getFrom().getPath(), containsString(" "));
+        assertThat(run.getTo().getPath(), containsString(to));
+    }
 }
diff --git a/src/test/java/io/kestra/plugin/fs/sftp/MoveTest.java b/src/test/java/io/kestra/plugin/fs/sftp/MoveTest.java
@@ -66,6 +66,31 @@ void moveFiletoDirectory() throws Exception {
         assertThat(run.getTo().getPath(), containsString(to));
     }
 
+    @Test
+    void moveFileWithSpaceToDirectoryWithSpace() throws Exception {
+        String from = "upload/" + IdUtils.create() + "/" + IdUtils.create() + " space .yaml";
+        String to = "upload/" + IdUtils.create() + "-move/" + IdUtils.create() + "/" + IdUtils.create() + " space 2 /";
+
+        sftpUtils.upload(from);
+
+        Move task = Move.builder()
+                .id(MoveTest.class.getSimpleName())
+                .type(Move.class.getName())
+                .from(from)
+                .to(to)
+                .host("localhost")
+                .port("6622")
+                .username("foo")
+                .password("pass")
+                .build();
+
+        Move.Output run = task.run(TestsUtils.mockRunContext(runContextFactory, task, ImmutableMap.of()));
+
+        assertThat(run.getTo().getPath(), containsString(" "));
+        assertThat(run.getFrom().getPath(), containsString(" "));
+        assertThat(run.getTo().getPath(), containsString(to));
+    }
+
     @Test
     void moveDirectory() throws Exception {
         String from = "upload/" + IdUtils.create() + "/" + IdUtils.create() + ".yaml";

diff --git a/src/test/java/io/kestra/plugin/fs/smb/MoveTest.java b/src/test/java/io/kestra/plugin/fs/smb/MoveTest.java
@@ -98,6 +98,44 @@ void moveFiletoDirectory() throws Exception {
         Assertions.assertDoesNotThrow(() -> fetchTo.run(TestsUtils.mockRunContext(runContextFactory, fetchTo, ImmutableMap.of())));
     }
 
+    @Test
+    void moveFileWithSpaceToDirectoryWithSpace() throws Exception {
+        String from = SmbUtils.SHARE_NAME + "/" + IdUtils.create() + "/" + IdUtils.create() + " space .yaml";
+        String to = SmbUtils.SHARE_NAME + "/" + IdUtils.create() + "-move/" + IdUtils.create() + "/ space " + IdUtils.create() + "space2/";
+
+        smbUtils.upload(from);
+
+        Move task = Move.builder()
+                .id(MoveTest.class.getSimpleName())
+                .type(Move.class.getName())
+                .from(from)
+                .to(to)
+                .host("localhost")
+                .username("alice")
+                .password("alipass")
+                .build();
+
+        Move.Output run = task.run(TestsUtils.mockRunContext(runContextFactory, task, ImmutableMap.of()));
+
+        assertThat(run.getTo().getPath(), endsWith(to + FilenameUtils.getName(from)));
+
+        Download fetchFrom = Download.builder()
+                .id(DeleteTest.class.getSimpleName())
+                .type(DeleteTest.class.getName())
+                .from(from)
+                .host("localhost")
+                .port("445")
+                .username("alice")
+                .password("alipass")
+                .build();
+        Assertions.assertThrows(FileSystemException.class, () -> fetchFrom.run(TestsUtils.mockRunContext(runContextFactory, fetchFrom, ImmutableMap.of())));
+
+        Download fetchTo = fetchFrom.toBuilder()
+                .from(to + "/" + FilenameUtils.getName(from))
+                .build();
+        Assertions.assertDoesNotThrow(() -> fetchTo.run(TestsUtils.mockRunContext(runContextFactory, fetchTo, ImmutableMap.of())));
+    }
+
     @Test
     void moveDirectory() throws Exception {
         String from = SmbUtils.SHARE_NAME + "/" + IdUtils.create() + "/" + IdUtils.create() + ".yaml";