github-advanced-security[bot] commented on code in PR #18696:
URL: https://github.com/apache/druid/pull/18696#discussion_r2462262469


##########
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()) {

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/10522)



##########
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, StandardCopyOption.REPLACE_EXISTING);

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/10523)



##########
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, StandardCopyOption.REPLACE_EXISTING);

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/10524)



-- 
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]

Reply via email to