This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/release-2.x by this push:
new 396ec80 LOG4J2-3452: Fix race condition in FileUtils.mkdir() (#809)
396ec80 is described below
commit 396ec807aa277047c5119ce60fcc14bee7b9ffcc
Author: Stefan Vodita <[email protected]>
AuthorDate: Wed Mar 30 22:14:40 2022 +0100
LOG4J2-3452: Fix race condition in FileUtils.mkdir() (#809)
* LOG4J2-3452: Fix race condition in FileUtils.mkdir()
A race condition could appear when 2 threads were trying to create the
same directory. Both would check that the directory did not exist, then
one would create the directory and the other would throw an exception.
This was reproduced with a unit test included this commit and fixed by
using Files.createDirectories() instead of File.mkdirs(), so that the
existence check and directory creation are done as an atomic operation.
---
.../apache/logging/log4j/core/util/FileUtils.java | 22 +++++----
.../logging/log4j/core/util/FileUtilsTest.java | 55 ++++++++++++++++++++++
src/changes/changes.xml | 3 ++
3 files changed, 71 insertions(+), 9 deletions(-)
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java
index 18daaf0..c2f8671 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java
@@ -21,6 +21,7 @@ import java.io.IOException;
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 final class FileUtils {
*/
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());
}
}
diff --git
a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/FileUtilsTest.java
b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/FileUtilsTest.java
index fd1539c..be00dcf 100644
---
a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/FileUtilsTest.java
+++
b/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 class FileUtilsTest {
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());
+ }
+ }
}
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 4145154..5b1dc04 100644
--- a/src/changes/changes.xml
+++ b/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.
