-
Notifications
You must be signed in to change notification settings - Fork 749
fix: prevent mixed content in SFTP/FTP cached files #1815
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -595,24 +595,33 @@ public void run() { | |
|
|
||
| ftp.setFileType(FTP.BINARY_FILE_TYPE); | ||
|
|
||
| InputStream inputStream = ftp.retrieveFileStream(path); | ||
| if (inputStream == null) { | ||
| Log.d( | ||
| "FTP", | ||
| "FTPClient (" + ftpId + ") path: " + path + " - not found" | ||
| ); | ||
| callback.error("File not found."); | ||
| return; | ||
| // Delete existing cache file to prevent stale content | ||
| if (localFile.exists()) { | ||
| localFile.delete(); | ||
| } | ||
|
|
||
| FileOutputStream outputStream = new FileOutputStream(localFile); | ||
| byte[] buffer = new byte[1024]; | ||
| int bytesRead = -1; | ||
| while ((bytesRead = inputStream.read(buffer)) != -1) { | ||
| outputStream.write(buffer, 0, bytesRead); | ||
| try ( | ||
| InputStream inputStream = ftp.retrieveFileStream(path) | ||
| ) { | ||
| if (inputStream == null) { | ||
| Log.d( | ||
| "FTP", | ||
| "FTPClient (" + ftpId + ") path: " + path + " - not found" | ||
| ); | ||
| callback.error("File not found."); | ||
| return; | ||
| } | ||
|
Comment on lines
+606
to
+613
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When Without calling Consider checking for null before entering the try-with-resources block, or calling Prompt To Fix With AIThis is a comment left during a code review.
Path: src/plugins/ftp/src/android/com/foxdebug/ftp/Ftp.java
Line: 606:613
Comment:
When `inputStream` is null, the method returns early without calling `ftp.completePendingCommand()`. According to Apache Commons Net FTP documentation, `completePendingCommand()` must be called after using streaming methods like `retrieveFileStream()` to complete the FTP transaction, even if the stream is null.
Without calling `completePendingCommand()`, the FTP connection may be left in an inconsistent state, potentially causing subsequent FTP operations to fail or hang.
Consider checking for null before entering the try-with-resources block, or calling `completePendingCommand()` in a finally block to ensure it always executes.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| try ( | ||
| FileOutputStream outputStream = new FileOutputStream(localFile) | ||
| ) { | ||
| byte[] buffer = new byte[1024]; | ||
| int bytesRead = -1; | ||
| while ((bytesRead = inputStream.read(buffer)) != -1) { | ||
| outputStream.write(buffer, 0, bytesRead); | ||
| } | ||
| } | ||
| } | ||
| outputStream.close(); | ||
| inputStream.close(); | ||
|
|
||
| if (!ftp.completePendingCommand()) { | ||
| ftp.logout(); | ||
|
|
@@ -675,20 +684,22 @@ public void run() { | |
| ftp.setFileType(FTP.BINARY_FILE_TYPE); | ||
|
|
||
| Log.d("FTPUpload", "Destination " + remoteFilePath); | ||
| OutputStream outputStream = ftp.storeFileStream(remoteFilePath); | ||
| if (outputStream == null) { | ||
| callback.error("File not found."); | ||
| return; | ||
| } | ||
|
|
||
| InputStream inputStream = new FileInputStream(localFile); | ||
| byte[] buffer = new byte[1024]; | ||
| int bytesRead = -1; | ||
| while ((bytesRead = inputStream.read(buffer)) != -1) { | ||
| outputStream.write(buffer, 0, bytesRead); | ||
| try ( | ||
| InputStream inputStream = new FileInputStream(localFile); | ||
| OutputStream outputStream = ftp.storeFileStream(remoteFilePath) | ||
| ) { | ||
| if (outputStream == null) { | ||
| callback.error("File not found."); | ||
| return; | ||
| } | ||
|
Comment on lines
+692
to
+695
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When Without calling Consider checking for null before entering the try-with-resources block, or ensuring Prompt To Fix With AIThis is a comment left during a code review.
Path: src/plugins/ftp/src/android/com/foxdebug/ftp/Ftp.java
Line: 692:695
Comment:
When `outputStream` is null, the method returns early without calling `ftp.completePendingCommand()`. According to Apache Commons Net FTP documentation, `completePendingCommand()` must be called after using streaming methods like `storeFileStream()` to complete the FTP transaction.
Without calling `completePendingCommand()`, the FTP connection may be left in an inconsistent state, potentially causing subsequent FTP operations to fail or hang.
Consider checking for null before entering the try-with-resources block, or ensuring `completePendingCommand()` is called in a finally block.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| byte[] buffer = new byte[1024]; | ||
| int bytesRead = -1; | ||
| while ((bytesRead = inputStream.read(buffer)) != -1) { | ||
| outputStream.write(buffer, 0, bytesRead); | ||
| } | ||
| } | ||
| outputStream.close(); | ||
| inputStream.close(); | ||
|
|
||
| if (!ftp.completePendingCommand()) { | ||
| ftp.logout(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.