github-advanced-security[bot] commented on code in PR #18696:
URL: https://github.com/apache/druid/pull/18696#discussion_r2462654317
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -747,21 +768,30 @@
}
/**
- * Deletes a directory and logs about it. This method should only be called
under the lock of a {@link #segmentLocks}
+ * Performs an atomic move to a sibling {@link #DROP_PATH} directory, and
then deletes the directory and logs about
+ * it. This method should only be called under the lock of a {@link
#segmentLocks}.
*/
- private static void deleteCacheEntryDirectory(final File path)
+ private static void atomicMoveAndDeleteCacheEntryDirectory(final File path)
{
- log.info("Deleting directory[%s]", path);
+ final File parent = path.getParentFile();
+ final File tempLocation = new File(parent, DROP_PATH);
try {
- FileUtils.deleteDirectory(path);
+ if (!tempLocation.exists()) {
+ FileUtils.mkdirp(tempLocation);
+ }
+ final File tempPath = new File(tempLocation, path.getName());
+ log.debug("moving[%s] to temp location[%s]", path, tempLocation);
+ Files.move(path.toPath(), tempPath.toPath(),
StandardCopyOption.ATOMIC_MOVE);
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/druid/security/code-scanning/10526)
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -747,21 +768,30 @@
}
/**
- * Deletes a directory and logs about it. This method should only be called
under the lock of a {@link #segmentLocks}
+ * Performs an atomic move to a sibling {@link #DROP_PATH} directory, and
then deletes the directory and logs about
+ * it. This method should only be called under the lock of a {@link
#segmentLocks}.
*/
- private static void deleteCacheEntryDirectory(final File path)
+ private static void atomicMoveAndDeleteCacheEntryDirectory(final File path)
{
- log.info("Deleting directory[%s]", path);
+ final File parent = path.getParentFile();
+ final File tempLocation = new File(parent, DROP_PATH);
try {
- FileUtils.deleteDirectory(path);
+ if (!tempLocation.exists()) {
+ FileUtils.mkdirp(tempLocation);
+ }
+ final File tempPath = new File(tempLocation, path.getName());
+ log.debug("moving[%s] to temp location[%s]", path, tempLocation);
+ Files.move(path.toPath(), tempPath.toPath(),
StandardCopyOption.ATOMIC_MOVE);
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/druid/security/code-scanning/10525)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
