davsclaus commented on code in PR #29855:
URL: https://github.com/apache/camel/pull/29855#discussion_r4568809716
##########
components/camel-file/src/main/java/org/apache/camel/component/file/strategy/GenericFileProcessStrategySupport.java:
##########
@@ -127,6 +128,12 @@ public void
setExclusiveReadLockStrategy(GenericFileExclusiveReadLockStrategy<T>
}
protected GenericFile<T> renameFile(GenericFileOperations<T> operations,
GenericFile<T> from, GenericFile<T> to) {
+ if (operations instanceof FileOperations fileOperations) {
+ // check before the existing target is deleted, its parent
directories are created and the file is renamed,
+ // as a symbolic link on the target path could otherwise move the
file outside the starting directory
+ fileOperations.jailMoveToStartingDirectory(new
File(to.getAbsoluteFilePath()));
Review Comment:
Thanks, reproduced and fixed in 5ffe960fe04a. In `begin`, the rename and
delete process strategies now evaluate the `preMove`, `move`/`.camel` and
`moveFailed` targets on a copy of the exchange and skip the file when one
resolves outside the starting directory. It is skipped the same way as a listed
link that resolves outside: the file is not processed and stays in the folder,
logged at WARN the first time and at DEBUG after.
`FileConsumerMoveJailStartingDirectorySymlinkTest` now asserts 0 deliveries
over at least 5 polls for `.camel`, `move`, `preMove`, `moveFailed` (failing
route), `delete`+`moveFailed`, `recursive` and `renameUsingCopy`. Only a target
that depends on what the route sets (such as an exchange property) is still
checked at move time, and the docs say that such a file is consumed again on
the next poll.
_Claude Code on behalf of davsclaus_
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc:
##########
@@ -77,6 +77,16 @@ at `DEBUG` level with the message only. Checkpoint failures
triggered by batch s
The webhook consumer still logs the body it validates at `DEBUG`, but no
longer the signature from the `X-Signature` header, nor the signature it
calculates.
The `InvalidMessageSignatureException` of a failed check no longer carries the
calculated signature: `getCalculatedSignature()` returns `null` and is
deprecated, as is the constructor that takes it.
+=== camel-file - the consumer's move, preMove and moveFailed resolve symbolic
links for jailStartingDirectory
+
+With `jailStartingDirectory` enabled (the default), the file consumer now
resolves symbolic links on the target of
+`move`, `preMove` and `moveFailed`, including the default `.camel` folder,
when that target is inside the starting
+directory. A target that resolves outside the starting directory through a
link is not used: the move fails like any
+other failed move, so the file stays where it is and the error is passed to
the exception handler. Before, the existing
Review Comment:
Agreed. The 4.24, 4.22.3 and 4.18.6 entries (f50b1024dc5e) now say that the
consumer skips the file: it is not processed and not moved, and it stays in the
folder. They also say that a deployment that links these folders elsewhere on
purpose, such as `.camel` on another volume, must set
`jailStartingDirectory=false`. The component doc says the same.
_Claude Code on behalf of davsclaus_
##########
components/camel-file/src/main/java/org/apache/camel/component/file/strategy/GenericFileProcessStrategySupport.java:
##########
@@ -127,6 +128,12 @@ public void
setExclusiveReadLockStrategy(GenericFileExclusiveReadLockStrategy<T>
}
protected GenericFile<T> renameFile(GenericFileOperations<T> operations,
GenericFile<T> from, GenericFile<T> to) {
+ if (operations instanceof FileOperations fileOperations) {
Review Comment:
Done in this PR rather than a follow-up, with the single check you
suggested. `FileOperations` now checks the path in `renameFile` (before
anything is renamed or copied, so also
`renameUsingCopy`/`copyAndDeleteOnRenameFail`), in `deleteFile`, and in
`buildDirectory` when it is about to create a missing directory. It runs before
the existing target is deleted. That covers `moveExisting`, the `tempFileName`
rename and the consumer moves. The `instanceof FileOperations` in the strategy
base class is gone. The consumer's up-front skip uses a new
`GenericFileOperations.isWithinStartingDirectory` default method that
`FileOperations` overrides. The new
`FileProducerJailStartingDirectorySymlinkMoveTest` covers both of your producer
cases.
_Claude Code on behalf of davsclaus_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
