[
https://issues.apache.org/jira/browse/CAMEL-24088?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen reassigned CAMEL-24088:
-----------------------------------
Assignee: Claus Ibsen
> camel-ftp: streamDownload treats a refused RETR as success - remote file is
> moved/deleted although it was never downloaded
> --------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-24088
> URL: https://issues.apache.org/jira/browse/CAMEL-24088
> Project: Camel
> Issue Type: Bug
> Components: camel-ftp
> Affects Versions: 4.21.0
> Reporter: Federico Mariani
> Assignee: Claus Ibsen
> Priority: Major
> Labels: code-review
> Attachments: FtpStreamDownloadRefusedRetrTest.java
>
>
> h3. Problem
> {{FTPClient.retrieveFileStream()}} returns *null* when the RETR command is
> refused (550 not-found/permission, 450 busy, 425 data-connection failure).
> {{FtpOperations.retrieveFileToStreamInBody}} (FtpOperations.java:483-487)
> ignores that contract:
> {code:java}
> if (endpoint.getConfiguration().isStreamDownload()) {
> InputStream is = client.retrieveFileStream(remoteName);
> target.setBody(is);
> exchange.getIn().setHeader(FtpConstants.REMOTE_FILE_INPUT_STREAM, is);
> result = true; // unconditional, even when is == null
> }
> {code}
> {{GenericFileConsumer.tryRetrievingFile}} treats {{true}} as success, routes
> the exchange with a *null body*, and on completion the rename/delete process
> strategy commits - moving or deleting a remote file that was never
> transferred. The in-memory path checks {{result = client.retrieveFile(...)}}
> correctly.
> h3. Failure scenario
> {{ftp://...?streamDownload=true&move=done}} (or {{delete=true}}) and the
> server transiently refuses RETR under load (425/450): the exchange completes
> "successfully" with a null body and the remote file is moved/deleted - silent
> data loss. It also makes {{ignoreFileNotFoundOrPermissionError}} dead code in
> streamDownload mode: the 550-ignore logic in
> {{FtpConsumer.ignoreCannotRetrieveFile}} is only reached when retrieve
> returns false or throws, which the stream path never does.
> h3. Related defect in the same area
> On the release side, {{releaseRetrievedFileResources}}
> (FtpOperations.java:439-450) calls {{client.completePendingCommand()}} but
> *discards the boolean result*. Commons Net contract: a {{false}} return means
> the transfer did not complete (server closed the data connection early with
> clean EOF + 426/451 reply). The route then saw a truncated stream with no
> exception and the commit still moves/deletes the remote file. Both defects
> are the same root theme - the streamDownload path never checks the client's
> success indicators - and should be fixed together.
> h3. History
> {{result = true}} has been unconditional since streamDownload was introduced;
> no CAMEL fix ever touched the null case.
> {{FtpSimpleConsumeStreamingPartialReadIT}} covers route-side read failures
> (abrupt disconnect mid-read throws), not a refused RETR nor the
> clean-EOF-negative-reply case.
> h3. Suggested fix
> {{result = is != null;}} - the FTP reply headers already set after the
> operation then drive the existing 550-ignore logic. For the release side,
> check {{completePendingCommand()}} and raise so the commit does not run.
> h3. Reproducer
> Attached failing JUnit test (place in
> {{components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/}}).
> No FTP server needed - it stubs {{FTPClient.retrieveFileStream()}} to return
> null with reply code 550 and asserts {{retrieveFile()}} returns false.
> ----
> _This issue was researched and filed by Claude Code on behalf of [~croway]
> (GitHub: Croway), as part of a deep code review of camel-file and camel-ftp.
> A failing JUnit reproducer is attached; it fails deterministically on current
> main (4.22.0-SNAPSHOT)._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
