[
https://issues.apache.org/jira/browse/CAMEL-24095?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Federico Mariani updated CAMEL-24095:
-------------------------------------
Attachment: FileProducerTempFileMoveExistingNonEagerIssueTest.java
> 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-file, camel-ftp
> Affects Versions: 4.21.0
> Reporter: Federico Mariani
> Priority: Major
> Labels: code-review
> 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)
