Name direct write cron files after the rollover period they cover

log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/CronTriggeringPolicyTest.java

@@ -16,13 +16,17 @@
  */
 package org.apache.logging.log4j.core.appender.rolling;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
+import java.text.SimpleDateFormat;
+import java.util.Date;
 import org.apache.logging.log4j.core.appender.RollingFileAppender;
 import org.apache.logging.log4j.core.appender.RollingRandomAccessFileAppender;
 import org.apache.logging.log4j.core.config.Configurator;
 import org.apache.logging.log4j.core.config.NullConfiguration;
 import org.apache.logging.log4j.core.layout.PatternLayout;
+import org.apache.logging.log4j.core.util.CronExpression;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -72,6 +76,30 @@ void testBuilderOnce() {
         testBuilder();
     }
 
+    /**
+     * Without a {@code fileName} the appender writes directly to the file its pattern resolves to.
+     * That file covers the whole rollover period, so it must be named after the period's start
+     * rather than after the moment the appender happened to start. Otherwise a restart later in
+     * the period opens a second file for a period that is supposed to have only one.
+     */
+    @Test
+    void testDirectWriteFileNameUsesPeriodStart() throws Exception {
+        final String schedule = "0 0 0 ? * SUN";
+        // @formatter:off
+        final RollingFileAppender raf = RollingFileAppender.newBuilder()
+                .setName("test5")
+                .setFilePattern("target/testcmd5.log-%d{yyyyMMdd}")
+                .setPolicy(CronTriggeringPolicy.createPolicy(configuration, Boolean.FALSE.toString(), schedule))
+                .setConfiguration(configuration)
+                .build();
+        // @formatter:on
+        assertNotNull(raf);
+
+        final Date periodStart = new CronExpression(schedule).getPrevFireTime(new Date());
+        final String expected = "target/testcmd5.log-" + new SimpleDateFormat("yyyyMMdd").format(periodStart);
+        assertEquals(expected, raf.getManager().getFileName());
+    }
+
     /**
      * Tests LOG4J2-1740 Add CronTriggeringPolicy programmatically leads to NPE
      */

log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/CronTriggeringPolicy.java

@@ -150,7 +150,9 @@ private static CronExpression getSchedule(final String expression) {
     private void rollover() {
         // If possible, use the time rollover was supposed to occur, not the actual time.
         final Date rollTime = future != null ? future.getFireTime() : new Date();
-        manager.rollover(cronExpression.getPrevFireTime(rollTime), lastRollDate);
+        // The file being rolled covers the period that ends at `rollTime`, so it is named after
+        // that period's start. The file replacing it opens the period beginning at `rollTime`.
+        manager.rollover(cronExpression.getPrevFireTime(rollTime), rollTime);
         if (future != null) {
             lastRollDate = future.getFireTime();
         }

log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DirectWriteRolloverStrategy.java

@@ -390,8 +390,10 @@ public String getCurrentFileName(final RollingFileManager manager) {
             final SortedMap<Integer, Path> eligibleFiles = getEligibleFiles(manager);
             final int fileIndex = eligibleFiles.size() > 0 ? (nextIndex > 0 ? nextIndex : eligibleFiles.lastKey()) : 1;
             final StringBuilder buf = new StringBuilder(255);
-            // LOG4J2-3339 - Always use the current time for new direct write files.
-            manager.getPatternProcessor().setCurrentFileTime(System.currentTimeMillis());
+            // Name the file after the start of the rollover period it belongs to, which the
+            // triggering policy records as the pattern processor's current file time. Policies
+            // that do not track a period leave that value at 0, and `formatFileName()` then
+            // falls back to the current time on its own.
             manager.getPatternProcessor().formatFileName(strSubstitutor, buf, true, fileIndex);
             final int suffixLength = suffixLength(buf.toString());
             final String name = suffixLength > 0 ? buf.substring(0, buf.length() - suffixLength) : buf.toString();

src/changelog/.2.x.x/fix_direct_write_cron_current_file_name.xml

@@ -0,0 +1,15 @@
+<?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="4227" link="https://github.com/apache/logging-log4j2/pull/4227"/>
+    <description format="asciidoc">
+        Fix the file a `CronTriggeringPolicy` appender without a `fileName` writes to. The file is now named after
+        the start of the rollover period it covers, instead of after the moment the appender started, so restarting
+        within a period reopens that period's file rather than creating another one. Note that this changes the
+        names of direct write files for such appenders.
+    </description>
+</entry>