Skip to content

Commit 1eb0c68

Browse files
ivicacclaude
ivicac
and
claude
committed
4446 Fix FTP component PR review comments
- Stream file downloads via PipedInputStream instead of buffering entire file in memory - Fix isDirectory() to use changeWorkingDirectory for non-existent paths - Check makeDirectory() return value and throw ProviderException on failure - Return null for missing modifiedAt instead of empty string - Load known_hosts for SFTP host key verification, fall back to PromiscuousVerifier - Remove hardcoded port default; auto-select 21 (FTP) or 22 (SFTP) based on protocol Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 08d526e commit 1eb0c68

6 files changed

Lines changed: 91 additions & 40 deletions

File tree

‎server/libs/modules/components/ftp/src/main/java/com/bytechef/component/ftp/action/FtpDownloadFileAction.java‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@
2828
import com.bytechef.component.definition.Parameters;
2929
import com.bytechef.component.exception.ProviderException;
3030
import com.bytechef.component.ftp.util.RemoteFileClient;
31-
import java.io.ByteArrayInputStream;
32-
import java.io.ByteArrayOutputStream;
3331
import java.io.IOException;
32+
import java.io.PipedInputStream;
33+
import java.io.PipedOutputStream;
3434

3535
/**
3636
* @author Ivica Cardic
@@ -59,12 +59,28 @@ protected static FileEntry perform(
5959
String remotePath = inputParameters.getRequiredString(PATH);
6060
String filename = remotePath.substring(remotePath.lastIndexOf('/') + 1);
6161

62-
try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) {
63-
remoteFileClient.retrieveFile(remotePath, outputStream);
62+
try (PipedInputStream pipedInputStream = new PipedInputStream();
63+
PipedOutputStream pipedOutputStream = new PipedOutputStream(pipedInputStream)) {
6464

65-
try (ByteArrayInputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray())) {
66-
return context.file(file -> file.storeContent(filename, inputStream));
67-
}
65+
Thread writerThread = new Thread(() -> {
66+
try {
67+
remoteFileClient.retrieveFile(remotePath, pipedOutputStream);
68+
} catch (IOException ioException) {
69+
throw new ProviderException(
70+
"Failed to download file: " + ioException.getMessage(), ioException);
71+
} finally {
72+
try {
73+
pipedOutputStream.close();
74+
} catch (IOException ioException) {
75+
throw new ProviderException(
76+
"Failed to close pipe: " + ioException.getMessage(), ioException);
77+
}
78+
}
79+
});
80+
81+
writerThread.start();
82+
83+
return context.file(file -> file.storeContent(filename, pipedInputStream));
6884
}
6985
} catch (IOException ioException) {
7086
throw new ProviderException("Failed to download file: " + ioException.getMessage(), ioException);

‎server/libs/modules/components/ftp/src/main/java/com/bytechef/component/ftp/action/FtpListAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ private static void listFiles(
106106
fileInfo.put("type", type);
107107
fileInfo.put("size", file.size());
108108
fileInfo.put("modifiedAt", file.modifiedAt() != null ? file.modifiedAt()
109-
.toString() : "");
109+
.toString() : null);
110110

111111
results.add(fileInfo);
112112

‎server/libs/modules/components/ftp/src/main/java/com/bytechef/component/ftp/connection/FtpConnection.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ public class FtpConnection {
4646
.required(true),
4747
integer(PORT)
4848
.label("Port")
49-
.description("The port number of the FTP server.")
50-
.defaultValue(21)
51-
.required(true),
49+
.description(
50+
"The port number of the server. Defaults to 21 for FTP and 22 for SFTP if not specified.")
51+
.required(false),
5252
string(USERNAME)
5353
.label("Username")
5454
.description("The username for authentication.")

‎server/libs/modules/components/ftp/src/main/java/com/bytechef/component/ftp/util/FtpRemoteFileClient.java‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,22 +131,57 @@ public void createDirectoryTree(String path) throws IOException {
131131
String directoryPath = currentPath.toString();
132132

133133
if (!ftpClient.changeWorkingDirectory(directoryPath)) {
134-
ftpClient.makeDirectory(directoryPath);
134+
boolean created = ftpClient.makeDirectory(directoryPath);
135+
136+
if (!created) {
137+
throw new ProviderException(
138+
"Failed to create remote directory '" + directoryPath + "': " +
139+
ftpClient.getReplyString());
140+
}
141+
142+
if (!ftpClient.changeWorkingDirectory(directoryPath)) {
143+
throw new ProviderException(
144+
"Created remote directory but cannot access '" + directoryPath + "': " +
145+
ftpClient.getReplyString());
146+
}
135147
}
136148
}
137149

138150
ftpClient.changeWorkingDirectory("/");
139151
}
140152

141153
@Override
154+
@SuppressWarnings("PMD.EmptyCatchBlock")
142155
public boolean isDirectory(String path) throws IOException {
143156
FTPFile[] files = ftpClient.listFiles(path);
144157

145-
if (files.length == 1 && files[0].isFile()) {
146-
return false;
158+
if (files.length == 1) {
159+
FTPFile file = files[0];
160+
161+
if (file.isDirectory()) {
162+
return true;
163+
}
164+
165+
if (file.isFile()) {
166+
return false;
167+
}
168+
} else if (files.length > 1) {
169+
return true;
147170
}
148171

149-
return true;
172+
String currentDir = ftpClient.printWorkingDirectory();
173+
174+
try {
175+
return ftpClient.changeWorkingDirectory(path);
176+
} finally {
177+
if (currentDir != null) {
178+
try {
179+
ftpClient.changeWorkingDirectory(currentDir);
180+
} catch (IOException ioException) {
181+
// Best-effort restoration of working directory
182+
}
183+
}
184+
}
150185
}
151186

152187
@Override

‎server/libs/modules/components/ftp/src/main/java/com/bytechef/component/ftp/util/RemoteFileClient.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,12 @@ private static RemoteFileClient createSftpClient(Parameters connectionParameters
109109
try {
110110
SSHClient sshClient = new SSHClient();
111111

112-
sshClient.addHostKeyVerifier(new PromiscuousVerifier());
112+
try {
113+
sshClient.loadKnownHosts();
114+
} catch (IOException ioException) {
115+
sshClient.addHostKeyVerifier(new PromiscuousVerifier());
116+
}
117+
113118
sshClient.connect(host, port);
114119
sshClient.authPassword(username, password);
115120

‎server/libs/modules/components/ftp/src/test/resources/definition/ftp_v1.json‎

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,6 @@
304304
"type": "BOOLEAN"
305305
} ],
306306
"resumePerform": null,
307-
"suspendPerform": null,
308307
"title": "Upload File",
309308
"workflowNodeDescription": null
310309
}, {
@@ -552,7 +551,6 @@
552551
"type": "STRING"
553552
} ],
554553
"resumePerform": null,
555-
"suspendPerform": null,
556554
"title": "Download File",
557555
"workflowNodeDescription": null
558556
}, {
@@ -912,7 +910,6 @@
912910
"type": "BOOLEAN"
913911
} ],
914912
"resumePerform": null,
915-
"suspendPerform": null,
916913
"title": "List Directory",
917914
"workflowNodeDescription": null
918915
}, {
@@ -996,8 +993,8 @@
996993
},
997994
"placeholder": null,
998995
"sampleOutput": {
999-
"deletedPath": "/uploads/old-file.pdf",
1000-
"success": true
996+
"success": true,
997+
"deletedPath": "/uploads/old-file.pdf"
1001998
}
1002999
},
10031000
"outputSchema": {
@@ -1067,8 +1064,8 @@
10671064
"type": "OBJECT"
10681065
},
10691066
"sampleOutput": {
1070-
"deletedPath": "/uploads/old-file.pdf",
1071-
"success": true
1067+
"success": true,
1068+
"deletedPath": "/uploads/old-file.pdf"
10721069
}
10731070
},
10741071
"perform": { },
@@ -1120,7 +1117,6 @@
11201117
"type": "BOOLEAN"
11211118
} ],
11221119
"resumePerform": null,
1123-
"suspendPerform": null,
11241120
"title": "Delete",
11251121
"workflowNodeDescription": null
11261122
}, {
@@ -1225,9 +1221,9 @@
12251221
},
12261222
"placeholder": null,
12271223
"sampleOutput": {
1224+
"newPath": "/archive/new-name.pdf",
12281225
"success": true,
1229-
"oldPath": "/uploads/old-name.pdf",
1230-
"newPath": "/archive/new-name.pdf"
1226+
"oldPath": "/uploads/old-name.pdf"
12311227
}
12321228
},
12331229
"outputSchema": {
@@ -1318,9 +1314,9 @@
13181314
"type": "OBJECT"
13191315
},
13201316
"sampleOutput": {
1317+
"newPath": "/archive/new-name.pdf",
13211318
"success": true,
1322-
"oldPath": "/uploads/old-name.pdf",
1323-
"newPath": "/archive/new-name.pdf"
1319+
"oldPath": "/uploads/old-name.pdf"
13241320
}
13251321
},
13261322
"perform": { },
@@ -1393,7 +1389,6 @@
13931389
"type": "BOOLEAN"
13941390
} ],
13951391
"resumePerform": null,
1396-
"suspendPerform": null,
13971392
"title": "Rename/Move",
13981393
"workflowNodeDescription": null
13991394
} ],
@@ -2386,8 +2381,8 @@
23862381
},
23872382
"placeholder": null,
23882383
"sampleOutput": {
2389-
"deletedPath": "/uploads/old-file.pdf",
2390-
"success": true
2384+
"success": true,
2385+
"deletedPath": "/uploads/old-file.pdf"
23912386
}
23922387
},
23932388
"outputSchema": {
@@ -2457,8 +2452,8 @@
24572452
"type": "OBJECT"
24582453
},
24592454
"sampleOutput": {
2460-
"deletedPath": "/uploads/old-file.pdf",
2461-
"success": true
2455+
"success": true,
2456+
"deletedPath": "/uploads/old-file.pdf"
24622457
}
24632458
},
24642459
"processErrorResponse": null,
@@ -2614,9 +2609,9 @@
26142609
},
26152610
"placeholder": null,
26162611
"sampleOutput": {
2612+
"newPath": "/archive/new-name.pdf",
26172613
"success": true,
2618-
"oldPath": "/uploads/old-name.pdf",
2619-
"newPath": "/archive/new-name.pdf"
2614+
"oldPath": "/uploads/old-name.pdf"
26202615
}
26212616
},
26222617
"outputSchema": {
@@ -2707,9 +2702,9 @@
27072702
"type": "OBJECT"
27082703
},
27092704
"sampleOutput": {
2705+
"newPath": "/archive/new-name.pdf",
27102706
"success": true,
2711-
"oldPath": "/uploads/old-name.pdf",
2712-
"newPath": "/archive/new-name.pdf"
2707+
"oldPath": "/uploads/old-name.pdf"
27132708
}
27142709
},
27152710
"processErrorResponse": null,
@@ -2835,8 +2830,8 @@
28352830
}, {
28362831
"advancedOption": null,
28372832
"controlType": "INTEGER",
2838-
"defaultValue": 21,
2839-
"description": "The port number of the FTP server.",
2833+
"defaultValue": null,
2834+
"description": "The port number of the server. Defaults to 21 for FTP and 22 for SFTP if not specified.",
28402835
"displayCondition": null,
28412836
"exampleValue": null,
28422837
"expressionEnabled": null,
@@ -2849,7 +2844,7 @@
28492844
"options": null,
28502845
"optionsDataSource": null,
28512846
"placeholder": null,
2852-
"required": true,
2847+
"required": false,
28532848
"type": "INTEGER"
28542849
}, {
28552850
"advancedOption": null,

0 commit comments

Comments
 (0)