davsclaus commented on code in PR #24180:
URL: https://github.com/apache/camel/pull/24180#discussion_r3452210236
##########
components/camel-smb/src/main/java/org/apache/camel/component/smb/SmbOperations.java:
##########
@@ -337,12 +338,19 @@ private boolean
retrieveFileToFileInLocalWorkDirectory(String name, Exchange exc
// use relative filename in local work directory
String relativeName = file.getRelativeFilePath();
+ java.io.File localWorkDir = local;
temp = new java.io.File(local, relativeName + ".inprogress");
// create directory to local work file
local.mkdirs();
local = new java.io.File(local, relativeName);
+ // ensure the local work file stays within the local work
directory (CAMEL-23765)
+ if (endpoint.isJailStartingDirectory()) {
Review Comment:
Style (non-blocking): In the other four implementations (`FtpOperations`,
`SftpOperations`, `MinaSftpOperations`, `FilesOperations`), the jail check is
placed **before** `mkdirs()`. Here it comes **after** `local.mkdirs()` (line
343). This is safe because `mkdirs()` at that point runs on the base work
directory (before `relativeName` is appended), so no escaped path is created.
But reordering to match the other implementations would improve consistency for
future reviewers.
##########
components/camel-file/src/main/java/org/apache/camel/component/file/GenericFileHelper.java:
##########
@@ -16,16 +16,39 @@
*/
package org.apache.camel.component.file;
+import java.io.File;
import java.util.function.Supplier;
import org.apache.camel.Exchange;
import org.apache.camel.support.MessageHelper;
+import org.apache.camel.util.FileUtil;
public final class GenericFileHelper {
private GenericFileHelper() {
}
+ /**
+ * Ensures the resolved local work file stays within the configured local
work directory. The remote file name used
+ * to build the local work file path may contain {@code ../} sequences
that would otherwise resolve to a path
+ * outside the work directory.
+ *
+ * @param target the resolved local work
file (or its in-progress temp file)
Review Comment:
Minor (non-blocking): The `jailStartingDirectory` option on
`GenericFileEndpoint` is declared with `label = "producer"`, so it only appears
in the **Producer Options** table in generated component docs. This PR extends
its meaning to also govern consumer-side `localWorkDirectory` downloads. Since
the default is `true`, users only need to find the option if they want to
*disable* it (and the upgrade guide entry names it), so practical impact is
low. A follow-up to change the label to `"common"` and expand the description
would improve discoverability.
--
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]
