This is an automated email from the ASF dual-hosted git repository. vy pushed a commit to branch 2.25.x in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 41e9ca55429ed7a4aef316e03985f49151d9e666 Author: Ramanathan <[email protected]> AuthorDate: Fri May 22 01:03:29 2026 +0530 Fix `createOnDemand` for Rolling File Appender (#4072) Co-authored-by: Piotr P. Karwasz <[email protected]> Co-authored-by: Volkan Yazıcı <[email protected]> --- .../RollingFileManagerCreateOnDemandTest.java | 72 +++++++ .../core/appender/rolling/RollingFileManager.java | 230 ++++++--------------- ...2006_Fix_RollingFileAppender_createOnDemand.xml | 14 ++ 3 files changed, 150 insertions(+), 166 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerCreateOnDemandTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerCreateOnDemandTest.java new file mode 100644 index 0000000000..6a2e16f457 --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerCreateOnDemandTest.java @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.logging.log4j.core.appender.rolling; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import org.apache.logging.log4j.core.config.NullConfiguration; +import org.apache.logging.log4j.core.layout.PatternLayout; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class RollingFileManagerCreateOnDemandTest { + @Test + void testCreateOnDemandDoesNotCreateDirectoryOrFile(@TempDir Path tempDir) throws Exception { + Path logDir = tempDir.resolve("onDemand"); + String logFile = logDir.resolve("test.log").toString(); + File dir = logDir.toFile(); + File file = new File(logFile); + assertFalse(dir.exists(), "Directory should not exist before logging"); + assertFalse(file.exists(), "File should not exist before logging"); + + RollingFileManager manager = RollingFileManager.getFileManager( + logFile, + logFile + ".%d{yyyy-MM-dd}", + true, + false, + NoOpTriggeringPolicy.INSTANCE, + DefaultRolloverStrategy.newBuilder().build(), + null, + PatternLayout.createDefaultLayout(), + 0, + true, + true, + null, + null, + null, + new NullConfiguration()); + assertNotNull(manager); + // Directory and file should still not exist + assertFalse(dir.exists(), "Directory should not exist after manager creation"); + assertFalse(file.exists(), "File should not exist after manager creation"); + + // Log a message + manager.writeToDestination("Hello Log4j2".getBytes(), 0, "Hello Log4j2".length()); + manager.close(); + + // Now directory and file should exist + assertTrue(dir.exists(), "Directory should exist after first log event"); + assertTrue(file.exists(), "File should exist after first log event"); + String content = new String(Files.readAllBytes(file.toPath())); + assertTrue(content.contains("Hello Log4j2")); + } +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java index 59db7fb8c2..7ec33c9f73 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java @@ -42,7 +42,6 @@ import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.appender.ConfigurationFactoryData; import org.apache.logging.log4j.core.appender.FileManager; -import org.apache.logging.log4j.core.appender.ManagerFactory; import org.apache.logging.log4j.core.appender.rolling.action.AbstractAction; import org.apache.logging.log4j.core.appender.rolling.action.Action; import org.apache.logging.log4j.core.config.Configuration; @@ -56,7 +55,6 @@ import org.apache.logging.log4j.core.util.Log4jThreadFactory; */ public class RollingFileManager extends FileManager { - private static RollingFileManagerFactory factory = new RollingFileManagerFactory(); private static final int MAX_TRIES = 3; private static final int MIN_DURATION = 100; private static final FileTime EPOCH = FileTime.fromMillis(0); @@ -296,33 +294,74 @@ public class RollingFileManager extends FileManager { final String fileOwner, final String fileGroup, final Configuration configuration) { - if (strategy instanceof DirectWriteRolloverStrategy && fileName != null) { LOGGER.error("The fileName attribute must not be specified with the DirectWriteRolloverStrategy"); return null; } - final String name = fileName == null ? pattern : fileName; + String actualName = fileName == null ? pattern : fileName; + int actualBufferSize = bufferedIO ? bufferSize : Constants.ENCODER_BYTE_BUFFER_SIZE; return narrow( RollingFileManager.class, getManager( - name, - new FactoryData( - fileName, - pattern, - append, - bufferedIO, - policy, - strategy, - advertiseURI, - layout, - bufferSize, - immediateFlush, - createOnDemand, - filePermissions, - fileOwner, - fileGroup, - configuration), - factory)); + actualName, + (name, data) -> { + long size = 0; + File file = null; + if (fileName != null) { + file = new File(fileName); + + if (!createOnDemand) { + try { + FileUtils.makeParentDirs(file); + final boolean created = file.createNewFile(); + LOGGER.trace("New file '{}' created = {}", name, created); + } catch (final IOException ioe) { + LOGGER.error("Unable to create file {}", name, ioe); + return null; + } + } + + size = append ? file.length() : 0; + } + + try { + final ByteBuffer buffer = ByteBuffer.allocate(actualBufferSize); + final OutputStream os = createOnDemand || fileName == null + ? null + : new FileOutputStream(fileName, append); + // LOG4J2-531 create file first so time has valid value. + final long initialTime = file == null || !file.exists() ? 0 : initialFileTime(file); + final boolean writeHeader = file != null && file.exists() && file.length() == 0; + + final RollingFileManager rm = new RollingFileManager( + data.getLoggerContext(), + fileName, + data.getPattern(), + os, + append, + createOnDemand, + size, + initialTime, + data.getTriggeringPolicy(), + data.getRolloverStrategy(), + advertiseURI, + layout, + filePermissions, + fileOwner, + fileGroup, + writeHeader, + buffer); + if (os != null && rm.isAttributeViewEnabled()) { + rm.defineAttributeView(file.toPath()); + } + + return rm; + } catch (final IOException ex) { + LOGGER.error("RollingFileManager ({}): {}", name, ex.getMessage(), ex); + } + return null; + }, + new FactoryData(pattern, policy, strategy, configuration))); } /** @@ -718,69 +757,28 @@ public class RollingFileManager extends FileManager { /** * Factory data. + * + * <p>This is also used by {@link RollingRandomAccessFileManager}.</p> */ - private static class FactoryData extends ConfigurationFactoryData { - private final String fileName; + static class FactoryData extends ConfigurationFactoryData { private final String pattern; - private final boolean append; - private final boolean bufferedIO; - private final int bufferSize; - private final boolean immediateFlush; - private final boolean createOnDemand; private final TriggeringPolicy policy; private final RolloverStrategy strategy; - private final String advertiseURI; - private final Layout<? extends Serializable> layout; - private final String filePermissions; - private final String fileOwner; - private final String fileGroup; /** * Creates the data for the factory. * @param pattern The pattern. - * @param append The append flag. - * @param bufferedIO The bufferedIO flag. - * @param advertiseURI - * @param layout The Layout. - * @param bufferSize the buffer size - * @param immediateFlush flush on every write or not - * @param createOnDemand true if you want to lazy-create the file (a.k.a. on-demand.) - * @param filePermissions File permissions - * @param fileOwner File owner - * @param fileGroup File group * @param configuration The configuration */ public FactoryData( - final String fileName, final String pattern, - final boolean append, - final boolean bufferedIO, final TriggeringPolicy policy, final RolloverStrategy strategy, - final String advertiseURI, - final Layout<? extends Serializable> layout, - final int bufferSize, - final boolean immediateFlush, - final boolean createOnDemand, - final String filePermissions, - final String fileOwner, - final String fileGroup, final Configuration configuration) { super(configuration); - this.fileName = fileName; this.pattern = pattern; - this.append = append; - this.bufferedIO = bufferedIO; - this.bufferSize = bufferSize; this.policy = policy; this.strategy = strategy; - this.advertiseURI = advertiseURI; - this.layout = layout; - this.immediateFlush = immediateFlush; - this.createOnDemand = createOnDemand; - this.filePermissions = filePermissions; - this.fileOwner = fileOwner; - this.fileGroup = fileGroup; } public TriggeringPolicy getTriggeringPolicy() { @@ -794,34 +792,6 @@ public class RollingFileManager extends FileManager { public String getPattern() { return pattern; } - - @Override - public String toString() { - final StringBuilder builder = new StringBuilder(); - builder.append(super.toString()); - builder.append("[pattern="); - builder.append(pattern); - builder.append(", append="); - builder.append(append); - builder.append(", bufferedIO="); - builder.append(bufferedIO); - builder.append(", bufferSize="); - builder.append(bufferSize); - builder.append(", policy="); - builder.append(policy); - builder.append(", strategy="); - builder.append(strategy); - builder.append(", advertiseURI="); - builder.append(advertiseURI); - builder.append(", layout="); - builder.append(layout); - builder.append(", filePermissions="); - builder.append(filePermissions); - builder.append(", fileOwner="); - builder.append(fileOwner); - builder.append("]"); - return builder.toString(); - } } /** @@ -838,78 +808,6 @@ public class RollingFileManager extends FileManager { setTriggeringPolicy(factoryData.getTriggeringPolicy()); } - /** - * Factory to create a RollingFileManager. - */ - private static class RollingFileManagerFactory implements ManagerFactory<RollingFileManager, FactoryData> { - - /** - * Creates a RollingFileManager. - * @param name The name of the entity to manage. - * @param data The data required to create the entity. - * @return a RollingFileManager. - */ - @Override - @SuppressFBWarnings( - value = {"PATH_TRAVERSAL_IN", "PATH_TRAVERSAL_OUT"}, - justification = "The destination file should be specified in the configuration file.") - public RollingFileManager createManager(final String name, final FactoryData data) { - long size = 0; - File file = null; - if (data.fileName != null) { - file = new File(data.fileName); - - try { - FileUtils.makeParentDirs(file); - final boolean created = data.createOnDemand ? false : file.createNewFile(); - LOGGER.trace("New file '{}' created = {}", name, created); - } catch (final IOException ioe) { - LOGGER.error("Unable to create file " + name, ioe); - return null; - } - size = data.append ? file.length() : 0; - } - - try { - final int actualSize = data.bufferedIO ? data.bufferSize : Constants.ENCODER_BYTE_BUFFER_SIZE; - final ByteBuffer buffer = ByteBuffer.wrap(new byte[actualSize]); - final OutputStream os = data.createOnDemand || data.fileName == null - ? null - : new FileOutputStream(data.fileName, data.append); - // LOG4J2-531 create file first so time has valid value. - final long initialTime = file == null || !file.exists() ? 0 : initialFileTime(file); - final boolean writeHeader = file != null && file.exists() && file.length() == 0; - - final RollingFileManager rm = new RollingFileManager( - data.getLoggerContext(), - data.fileName, - data.pattern, - os, - data.append, - data.createOnDemand, - size, - initialTime, - data.policy, - data.strategy, - data.advertiseURI, - data.layout, - data.filePermissions, - data.fileOwner, - data.fileGroup, - writeHeader, - buffer); - if (os != null && rm.isAttributeViewEnabled()) { - rm.defineAttributeView(file.toPath()); - } - - return rm; - } catch (final IOException ex) { - LOGGER.error("RollingFileManager (" + name + ") " + ex, ex); - } - return null; - } - } - static long initialFileTime(final File file) { final Path path = file.toPath(); if (Files.exists(path)) { diff --git a/src/changelog/2.25.5/LOG4J2-2006_Fix_RollingFileAppender_createOnDemand.xml b/src/changelog/2.25.5/LOG4J2-2006_Fix_RollingFileAppender_createOnDemand.xml new file mode 100644 index 0000000000..33d6f9e9fe --- /dev/null +++ b/src/changelog/2.25.5/LOG4J2-2006_Fix_RollingFileAppender_createOnDemand.xml @@ -0,0 +1,14 @@ +<?xml version="1.0" encoding="UTF-8"?> +<entry xmlns="https://logging.apache.org/xml/ns"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation=" + https://logging.apache.org/xml/ns + https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"; + type="fixed"> + <issue id="2006" link="https://github.com/apache/logging-log4j2/issues/2006"/> + <issue id="4072" link="https://github.com/apache/logging-log4j2/pull/4072"/> + <description format="asciidoc"> + Fix the `createOnDemand` behavior of `RollingFileAppender` to correctly defer file and directory creation + until the first log event, while preserving eager creation when disabled. + </description> +</entry> \ No newline at end of file
