Federico Mariani created CAMEL-24095:
----------------------------------------
Summary: camel-file: tempFileName + fileExist=Move +
eagerDeleteTargetFile=false silently overwrites the existing target instead of
moving it (no Move branch in handleExistingTarget)
Key: CAMEL-24095
URL: https://issues.apache.org/jira/browse/CAMEL-24095
Project: Camel
Issue Type: Bug
Components: camel-ftp, camel-file
Affects Versions: 4.21.0
Reporter: Federico Mariani
Attachments: FileProducerTempFileMoveExistingNonEagerIssueTest.java
h3. Problem
{{GenericFileProducer.handleExistingTarget}} (GenericFileProducer.java:249-266)
- the non-eager exists handler used when writing via
{{tempFileName}}/{{tempPrefix}} with {{eagerDeleteTargetFile=false}} - covers
{{Ignore}}, {{Fail}} and {{Override}} but has *no Move branch*, unlike its
eager twin {{handleExistingTargetEager}} (line 235-237) which calls the
{{moveExistingFileStrategy}}. It returns false and {{renameFile(tempTarget,
target)}} proceeds: {{FileUtil.renameFile}} uses {{Files.move(...,
ATOMIC_MOVE)}} (or copy with {{REPLACE_EXISTING}} as fallback), so the
pre-existing target is overwritten and lost - never archived to the
{{moveExisting}} destination, which is the entire point of {{fileExist=Move}}.
h3. Failure scenario
{{to("file:out?tempFileName=inprogress-${file:name}&fileExist=Move&moveExisting=archive/&eagerDeleteTargetFile=false")}}
with an existing target: the old file is destroyed instead of moved to
{{archive/}}. This combination is deliberate usage - per the
{{eagerDeleteTargetFile}} javadoc, non-eager is exactly how you get "fail if a
file already exists at the move destination" semantics for the moveExisting
operation.
h3. History
CAMEL-15822 (235a0a5f3d15, 2020) was titled "fileExist=Move doesn't use
FileMoveExistingStrategy when tempFile is also configured" but patched only the
eager branch; the non-eager branch has lacked Move handling since the logic's
inception and through the 2023 helper-extraction refactor (1bcab0700cd9).
h3. Suggested fix
Add the Move branch to {{handleExistingTarget}}, mirroring
{{handleExistingTargetEager}}.
Adjacent defect in the same code path: with {{eagerDeleteTargetFile=false}},
the temp file is fully written before the exists check, and the
{{Ignore}}-return / {{Fail}}-throw / failed-rename paths all leave the temp
file orphaned on disk (unbounded accumulation with date/UUID-bearing
{{tempFileName}} expressions).
h3. Note for the fix
Existing test
{{FileProducerMoveExistingTest.testFailOnMoveExistingFileExistsEagerDeleteFalseTempFileName}}
currently *asserts the buggy behavior* (it expects the pre-existing
move-target to keep its old content, i.e. no move happened) and will need
updating together with the fix.
h3. Reproducer
Attached failing JUnit test (place in
{{core/camel-core/src/test/java/org/apache/camel/component/file/}}, where the
file component tests live). Pre-creates the target with content OLD, produces
NEW through the tempFileName+Move+non-eager endpoint, and asserts the OLD
content was archived to {{archive/}} - on current main the archive file does
not exist and OLD is destroyed.
----
_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)
