LOG4J2-3452: Fix race condition in FileUtils.mkdir()

log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java

@@ -21,6 +21,7 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
+import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -112,16 +113,19 @@ public static String getFileExtension(final File file) {
      */
     public static void mkdir(final File dir, final boolean createDirectoryIfNotExisting) throws IOException {
         // commons io FileUtils.forceMkdir would be useful here, we just want to omit this dependency
-        if (!dir.exists()) {
-            if (!createDirectoryIfNotExisting) {
-                throw new IOException("The directory " + dir.getAbsolutePath() + " does not exist.");
-            }
-            if (!dir.mkdirs()) {
-                throw new IOException("Could not create directory " + dir.getAbsolutePath());
-            }
+
+        if (!dir.exists() && !createDirectoryIfNotExisting) {
+            throw new IOException("The directory " + dir.getAbsolutePath() + " does not exist.");
         }
-        if (!dir.isDirectory()) {
-            throw new IOException("File " + dir + " exists and is not a directory. Unable to create directory.");
+
+        try {
+            Files.createDirectories(dir.toPath());
+        } catch (FileAlreadyExistsException e) {
+            if (!dir.isDirectory()) {
+                throw new IOException("File " + dir + " exists and is not a directory. Unable to create directory.");
+            }
+        } catch (Exception e) {
+            throw new IOException("Could not create directory " + dir.getAbsolutePath());
         }
     }
 

log4j-core/src/test/java/org/apache/logging/log4j/core/util/FileUtilsTest.java

@@ -17,10 +17,19 @@
 
 package org.apache.logging.log4j.core.util;
 
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 
 import java.io.File;
+import java.io.IOException;
 import java.net.URI;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -78,4 +87,50 @@ public void testFileFromUriWithSpacesAndPlusCharactersInName() throws Exception
         assertTrue(file.exists(), "file exists");
     }
 
+    @Nested
+    class TestMkdir {
+        @TempDir
+        File testDir;
+
+        @BeforeEach
+        public void deleteTestDir() throws IOException {
+            org.apache.commons.io.FileUtils.deleteDirectory(testDir);
+        }
+
+        @Test
+        public void testMkdirDoesntExistDontCreate() {
+            assertThrows(IOException.class, () -> FileUtils.mkdir(testDir, false));
+        }
+
+        @Test
+        public void testMkdirFileAlreadyExistsNotDir() throws IOException {
+            Files.createFile(testDir.toPath());
+            assertThrows(IOException.class, () -> FileUtils.mkdir(testDir, true));
+            Files.delete(testDir.toPath());
+        }
+
+        @Test
+        public void testMkdirConcurrent() throws InterruptedException {
+            List<Thread> threads = new ArrayList<>();
+            AtomicBoolean anyThreadThrows = new AtomicBoolean(false);
+            for (int i = 0; i < 10000; i++) {
+                threads.add(new Thread(() -> {
+                    try {
+                        FileUtils.mkdir(testDir, true);
+                    } catch (IOException e) {
+                        anyThreadThrows.set(true);
+                    }
+                }));
+            }
+
+            for (Thread t : threads) {
+                t.start();
+            }
+            for (Thread t : threads) {
+                t.join();
+            }
+
+            assertFalse(anyThreadThrows.get());
+        }
+    }
 }

src/changes/changes.xml

@@ -76,6 +76,9 @@
       <action issue="LOG4J2-3447" dev="pkarwasz" type="fix" due-to="Pooja Pandey">
         Fixes appender concurrency problems in Log4j 1.x bridge.
       </action>
+      <action issue="LOG4J2-3452" dev="stefanvodita" type="fix" due-to="Stefan Vodita">
+        Fix and test for race condition in FileUtils.mkdir().
+      </action>
       <!-- UPDATES -->
       <action issue="LOG4J2-3428" dev="ggregory" type="fix" due-to="LF-Lin">
         Update 3rd party dependencies for 2.17.3.