gnodet-bot commented on code in PR #27522:
URL: https://github.com/apache/camel/pull/27522#discussion_r4213345246
##########
components/camel-file/src/main/java/org/apache/camel/component/file/FileConsumer.java:
##########
@@ -75,6 +77,25 @@ protected Exchange createExchange(GenericFile<File> file) {
return exchange;
}
+ @Override
+ protected boolean isWithinStartingDirectory(String absoluteFilePath) {
+ // a local listing entry is a single path segment, but it can be a
symbolic link to a file or to a directory
+ // (entered with recursive) whose target lies outside the starting
directory, so compare the resolved paths
+ try {
+ Path startingDirectory = resolvedStartingDirectory;
+ if (startingDirectory == null) {
+ startingDirectory =
GenericFileHelper.resolveExistingPathSegments(getEndpoint().getFile().toPath());
Review Comment:
ℹ️ **Performance note:** `resolveExistingPathSegments` is called on every
listed file, on every poll. For directories with thousands of files, this means
one `toRealPath` traversal per file per poll. The starting directory is cached
(once per poll), but the per-file side is not. Likely acceptable for typical
workloads — just worth documenting in a comment or in the Javadoc of
`isWithinStartingDirectory` so maintainers are aware of the cost model.
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -3490,6 +3490,15 @@ Downloads to a configured `localWorkDirectory` now
resolve existing filesystem p
checking that the destination remains inside that directory. Downloads through
a symbolic link that
resolves outside the `localWorkDirectory` are rejected. Valid nested download
paths continue to work.
+=== camel-file - jailStartingDirectory resolves symbolic links
+
+With `jailStartingDirectory` enabled (the default), the file consumer and
producer now resolve symbolic links when
+checking that a file stays within the starting directory. The consumer skips a
file or (with `recursive=true`) a
+directory that is a link resolving outside of it, and the producer fails to
write through such a link or a dangling
+link. Links resolving inside the starting directory work as before. Set
`jailStartingDirectory=false` to keep following
+links to other directories (this also turns off the check of `../` in producer
file names). A skipped file is logged
Review Comment:
The phrase "also by the remote file consumers" is technically correct (the
WARN→DEBUG deduplication logic lives in `GenericFileConsumer.isValidFile` which
remote consumers inherit), but it may mislead readers into thinking remote
consumers also gained symlink resolution — they haven't, they keep the existing
lexical check. Consider a small clarification:
```suggestion
at WARN the first time and at DEBUG on later polls; this log-level change
also applies to the remote file consumers.
```
--
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]
