use a dated file name when the date pattern names files like backups

AGENTS.md

@@ -28,6 +28,10 @@ almost always be doing.
   interface or abstract class, as in `Func<ISmtpTransport> f = () => new MailKitSmtpTransport();`.
 - `x?.Method() ?? false` rather than `x is not null && x.Method()`, and merge nested guards
   into one condition.
+- In a method whose conditions are hard to follow, give their parts names: small local functions
+  such as `IsBaseFileRolledByDate()` or `HasNumberedBackups()`, even one-liners and even chained,
+  so the `if` reads as a sentence. See `RollingFileAppender.DoesDatePatternCollideWithBackupNames`.
+  In review, do not suggest inlining them to save lines. A simple guard stays inline.
 - Expression-bodied members whenever the body fits on one line, including constructors
   (`resharper_constructor_or_destructor_body = expression_body`).
 - Braces on `if`/`else` bodies even for a single statement.

src/changelog/3.5.1/335-short-date-pattern-collides-with-backup-names.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns="https://logging.apache.org/xml/ns"
+       xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="fixed">
+  <issue id="335" link="https://github.com/apache/logging-log4net/issues/335"/>
+  <description format="asciidoc">With `StaticLogFileName` a `DatePattern` such as `.dd` named dated
+  files like numbered backups, so a roll could overwrite one with the other. Such an appender is now
+  reported when activated and uses the dated file name as with `StaticLogFileName` false, so its
+  backups are numbered after the date, and an existing static file is left untouched, including
+  its numbered backups. Use a `DatePattern` with a separator, such as the default `.yyyy-MM-dd`,
+  to keep the static name (reported by @Snotface, fixed by @FreeAndNil)</description>
+</entry>

src/log4net.Tests/Appender/RollingFileAppenderShortDatePatternTest.cs

@@ -0,0 +1,191 @@
+#region Apache License
+//
+// 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.
+//
+#endregion
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using log4net.Appender;
+using log4net.Core;
+using log4net.Layout;
+using log4net.Util;
+
+using NUnit.Framework;
+
+using PeanutButter.Utils;
+
+namespace log4net.Tests.Appender;
+
+/// <summary>
+/// What <see cref="RollingFileAppender"/> does when a static name and a short date pattern name
+/// dated files like numbered backups, see #335.
+/// </summary>
+[TestFixture]
+public sealed class RollingFileAppenderShortDatePatternTest
+{
+  /// <summary>
+  /// With a static name, .dd names the file of the 10th like backup 10, so the appender is
+  /// reported and uses the dated name instead, with its backups numbered after the date.
+  /// </summary>
+  [Test]
+  [NonParallelizable]
+  public void ShortDatePatternUsesDatedFileName()
+  {
+    using AutoTempFolder folder = new();
+    string file = Path.Combine(folder.Path, "log.txt");
+    RollingFileAppender appender = CreateShortDatePatternAppender(file, ".dd", 10, -1);
+    appender.MaxFileSize = 100;
+    List<string> messages = ActivateCapturingInternalMessages(appender);
+    try
+    {
+      Assert.That(messages, Has.Exactly(1).Contains("DatePattern [.dd]"));
+      for (int i = 0; i < 5; i++)
+      {
+        appender.DoAppend(CreateEvent(new string('a', 200)));
+      }
+    }
+    finally
+    {
+      appender.Close();
+    }
+
+    string[] files = Directory.GetFiles(folder.Path);
+    Assert.That(files, Does.Not.Contain(file));
+    Assert.That(files, Does.Contain(file + ".04").And.Contain(file + ".04.1"));
+  }
+
+  /// <summary>
+  /// Only a date that a backup name can reach is a collision: counting down to 5 never writes .10,
+  /// a non-static name keeps its backups apart from the dated files, and five or more digits
+  /// never count as a backup.
+  /// </summary>
+  [TestCase(".d", true, 5, -1, true)]
+  [TestCase(".dd", true, 5, 1, true)]
+  [TestCase(".dd", true, 5, -1, false)]
+  [TestCase(".dd", true, 10, -1, true)]
+  [TestCase(".dd", true, -1, -1, true)]
+  [TestCase(".dd", false, 10, -1, false)]
+  [TestCase(".yyyyMMdd", true, 10, 1, false)]
+  [TestCase(".yyyy-MM-dd", true, 10, -1, false)]
+  [NonParallelizable]
+  public void ShortDatePatternIsReportedOnlyWhenBackupsCollide(string datePattern,
+    bool staticLogFileName, int maxSizeRollBackups, int countDirection, bool reported)
+  {
+    using AutoTempFolder folder = new();
+    string file = Path.Combine(folder.Path, "log.txt");
+    RollingFileAppender appender = CreateShortDatePatternAppender(file, datePattern,
+      maxSizeRollBackups, countDirection, staticLogFileName);
+    List<string> messages = ActivateCapturingInternalMessages(appender);
+    appender.Close();
+
+    Assert.That(messages,
+      Has.Exactly(reported ? 1 : 0).Contains("names dated files like numbered backups"));
+  }
+
+  /// <summary>
+  /// Date rolling that appends makes no numbered backups, so there is nothing to collide with.
+  /// </summary>
+  [Test]
+  [NonParallelizable]
+  public void DateRollingWithAppendIsNotReported()
+  {
+    using AutoTempFolder folder = new();
+    string file = Path.Combine(folder.Path, "log.txt");
+    RollingFileAppender appender = CreateShortDatePatternAppender(file, ".d", 10, -1,
+      rollingStyle: RollingFileAppender.RollingMode.Date);
+    List<string> messages = ActivateCapturingInternalMessages(appender);
+    appender.Close();
+
+    Assert.That(messages, Is.Empty);
+  }
+
+  /// <summary>
+  /// Date rolling without appending rolls the existing file to .1 at startup, which with .d is the
+  /// name of the file of the 1st, so the appender leaves it alone and opens the dated file.
+  /// </summary>
+  [Test]
+  [NonParallelizable]
+  public void ShortDatePatternRestartWithoutAppendKeepsExistingFile()
+  {
+    using AutoTempFolder folder = new();
+    string file = Path.Combine(folder.Path, "log.txt");
+    File.WriteAllText(file, "previous run");
+    // the same period as the mocked clock, so the start-up does not roll by date
+    File.SetLastWriteTime(file, _mockedNow);
+    RollingFileAppender appender = CreateShortDatePatternAppender(file, ".d", 3, -1,
+      rollingStyle: RollingFileAppender.RollingMode.Date, appendToFile: false);
+    List<string> messages = ActivateCapturingInternalMessages(appender);
+    appender.Close();
+
+    Assert.That(messages, Has.Exactly(1).Contains("DatePattern [.d]"));
+    Assert.That(Directory.GetFiles(folder.Path), Is.EquivalentTo(new[] { file, file + ".4" }));
+    Assert.That(File.ReadAllText(file), Is.EqualTo("previous run"));
+  }
+
+  /// <summary>
+  /// With a dated name and .d, the files of the 11th start with the name of the file of the 1st
+  /// and must not count as its backups.
+  /// </summary>
+  [Test]
+  public void DatedNameCountsOnlyItsOwnBackups()
+  {
+    RollingFileAppender appender = CreateShortDatePatternAppender("log.txt", ".d", 5, 1,
+      staticLogFileName: false);
+    appender.DateTimeStrategy = new Integration.MockDateTime(new(2026, 10, 1));
+    appender.InitializeRollBackups("log.txt",
+      ["log.txt.1", "log.txt.1.2", "log.txt.11", "log.txt.11.7"]);
+
+    Assert.That(appender.CurrentSizeRollBackups, Is.EqualTo(2));
+  }
+
+  private static readonly DateTime _mockedNow = new(2026, 10, 4, 9, 0, 0);
+
+  private static RollingFileAppender CreateShortDatePatternAppender(string file,
+    string datePattern, int maxSizeRollBackups, int countDirection, bool staticLogFileName = true,
+    RollingFileAppender.RollingMode rollingStyle = RollingFileAppender.RollingMode.Composite,
+    bool appendToFile = true) => new()
+  {
+    File = file,
+    Layout = new PatternLayout("%message%newline"),
+    RollingStyle = rollingStyle,
+    DatePattern = datePattern,
+    StaticLogFileName = staticLogFileName,
+    MaxSizeRollBackups = maxSizeRollBackups,
+    CountDirection = countDirection,
+    AppendToFile = appendToFile,
+    DateTimeStrategy = new Integration.MockDateTime(_mockedNow)
+  };
+
+  private static List<string> ActivateCapturingInternalMessages(RollingFileAppender appender)
+  {
+    List<LogLog> messages = [];
+    LogLog.ExecuteWithoutEmittingInternalMessages(() =>
+    {
+      using LogLog.LogReceivedAdapter _ = new(messages);
+      appender.ActivateOptions();
+    });
+    return messages.ConvertAll(m => m.Message);
+  }
+
+  private static LoggingEvent CreateEvent(string message) => new(new LoggingEventData
+  {
+    Level = Level.Info,
+    Message = message,
+    LoggerName = "ShortDatePattern"
+  });
+}

src/log4net/Appender/RollingFileAppender.cs

@@ -87,7 +87,9 @@ namespace log4net.Appender;
 /// <note type="caution">
 /// <para>
 /// Changing <see cref="StaticLogFileName"/> or <see cref="CountDirection"/> without clearing
-/// the log file directory of backup files will cause unexpected and unwanted side effects.  
+/// the log file directory of backup files will cause unexpected and unwanted side effects.
+/// The appender itself sets <see cref="StaticLogFileName"/> to <see langword="false"/> when the
+/// <see cref="DatePattern"/> names dated files like numbered backups, see there.
 /// </para>
 /// </note>
 /// 
@@ -271,6 +273,12 @@ public RollingFileAppender()
   /// This property determines the rollover schedule when rolling over
   /// on date.
   /// </para>
+  /// <para>
+  /// With <see cref="StaticLogFileName"/> a pattern that formats as a dot and up to four digits,
+  /// such as <c>.dd</c>, names dated files like numbered backups. Such an appender is reported
+  /// when activated and then reads <see cref="StaticLogFileName"/> as false, so the live file
+  /// is the dated one and its backups are numbered after the date.
+  /// </para>
   /// </remarks>
   public string? DatePattern { get; set; } = ".yyyy-MM-dd";
 
@@ -865,36 +873,15 @@ private void InitializeFromOneFile(string baseFile, string curFileName)
     string dir = string.IsNullOrEmpty(baseFile) ? "" : Path.GetDirectoryName(baseFile);
     string baseFileWithoutExtension = Path.Combine(dir ?? "", Path.GetFileNameWithoutExtension(baseFile));
 
-    if (curFileName.StartsWith(baseFileWithoutExtension) == false)
+    if (!IsLogFile() || IsBaseFile())
     {
-      return; // This is not a log file, so ignore
-    }
-
-    if (curFileName.Equals(baseFile, StringComparison.Ordinal))
-    {
-      return; // Base log file is not an incremented logfile (.1 or .2, etc.)
+      return;
     }
 
     // Only look for files in the current roll point
-    if (_rollDate && !StaticLogFileName)
+    if (_rollDate && !StaticLogFileName && !IsBackupOfDatedFile(Path.GetFileName(curFileName)))
     {
-      string date = DateTimeStrategy.Now.ToString(DatePattern, DateTimeFormatInfo.InvariantInfo).ToLowerInvariant();
-      // the date pattern may supply the extension, so take it from the dated name
-      string dated = Path.GetFileName(CombinePath(baseFile, date));
-      string suffix = PreserveLogFileNameExtension ? Path.GetExtension(dated) : "";
-      string prefix = dated.Substring(0, dated.Length - suffix.Length);
-      string curFileNameWithoutDir = Path.GetFileName(curFileName);
-      if (!curFileNameWithoutDir.StartsWith(prefix) || !curFileNameWithoutDir.EndsWith(suffix))
-      {
-        LogLog.Debug(_declaringType, $"Ignoring file [{curFileName}] because it is from a different date period");
-        return;
-      }
-
-      // the dated file itself has no index, and a date like .yyyyMMdd would parse as one
-      if (curFileNameWithoutDir.Length <= dated.Length)
-      {
-        return;
-      }
+      return;
     }
 
     try
@@ -939,6 +926,31 @@ private void InitializeFromOneFile(string baseFile, string curFileName)
       //when staticLogFileName == false
       LogLog.Debug(_declaringType, $"Encountered a backup file not ending in .x [{curFileName}]");
     }
+
+    bool IsLogFile() => curFileName.StartsWith(baseFileWithoutExtension);
+
+    // the base log file is not an incremented log file (.1 or .2, etc.)
+    bool IsBaseFile() => curFileName.Equals(baseFile, StringComparison.Ordinal);
+
+    bool IsBackupOfDatedFile(string fileName)
+    {
+      string date = DateTimeStrategy.Now.ToString(DatePattern, DateTimeFormatInfo.InvariantInfo)
+        .ToLowerInvariant();
+      // the date pattern may supply the extension, so take it from the dated name
+      string dated = Path.GetFileName(CombinePath(baseFile, date));
+      string suffix = PreserveLogFileNameExtension ? Path.GetExtension(dated) : "";
+      string prefix = dated.Substring(0, dated.Length - suffix.Length);
+      // the dot keeps log.txt.11 out of the scan for log.txt.1
+      if (!fileName.StartsWith(prefix + ".") || !fileName.EndsWith(suffix))
+      {
+        LogLog.Debug(_declaringType,
+          $"Ignoring file [{curFileName}] because it is from a different date period");
+        return false;
+      }
+
+      // the dated file itself has no index, and a date like .yyyyMMdd would parse as one
+      return fileName.Length > dated.Length;
+    }
   }
 
   /// <summary>
@@ -1000,6 +1012,37 @@ public void InitializeRollBackups(string baseFile, IList<string> arrayFiles)
     }
   }
 
+  /// <summary>
+  /// With a static name, a date that formats as a dot and up to four digits (<c>.dd</c> writes
+  /// <c>.10</c>) is also the name of backup 10. Counting down to a limit below the smallest such
+  /// name never reaches it.
+  /// </summary>
+  private bool DoesDatePatternCollideWithBackupNames()
+  {
+    if (DatePattern is not string datePattern || !IsBaseFileRolledByDate() || !HasNumberedBackups())
+    {
+      return false;
+    }
+
+    string sample = _sDate1970.ToString(datePattern, DateTimeFormatInfo.InvariantInfo);
+    return IsDotAndUpToFourDigits(sample) && CanBackupNumberEqualDate(sample);
+
+    bool IsBaseFileRolledByDate() => _rollDate && StaticLogFileName;
+
+    bool HasNumberedBackups() => MaxSizeRollBackups != 0 && DoesRollBySizeOrAtStart();
+
+    bool DoesRollBySizeOrAtStart() => _rollSize || !AppendToFile;
+
+    static bool IsDotAndUpToFourDigits(string sample)
+      => sample.Length is >= 2 and <= 5 && sample[0] == '.'
+        && sample.Skip(1).All(c => c is >= '0' and <= '9');
+
+    // 1970-01-01 00:00 writes every field at its narrowest, so dated names start at 10^(digits - 1)
+    bool CanBackupNumberEqualDate(string sample)
+      => CountDirection >= 0 || MaxSizeRollBackups < 0
+        || MaxSizeRollBackups >= (int)Math.Pow(10, sample.Length - 2);
+  }
+
   /// <summary>
   /// Calculates the RollPoint for the datePattern supplied.
   /// </summary>
@@ -1088,6 +1131,17 @@ public override void ActivateOptions()
       }
     }
 
+    if (DoesDatePatternCollideWithBackupNames())
+    {
+      // LogLog, not ErrorHandler: OnlyOnceErrorHandler would report nothing after this one
+      LogLog.Warn(_declaringType, $"""
+        DatePattern [{DatePattern}] of [{Name}] names dated files like numbered backups, so the
+        appender uses a dated file name as with StaticLogFileName false. Use a DatePattern with
+        a separator, such as the default .yyyy-MM-dd, to keep the static name.
+        """);
+      StaticLogFileName = false;
+    }
+
     SecurityContext ??= SecurityContextProvider.DefaultProvider.CreateSecurityContext(this);
 
     using (SecurityContext.Impersonate(this))

src/site/antora/modules/ROOT/pages/manual/configuration/appenders/rollingfileappender.adoc

@@ -88,6 +88,13 @@ For each day only the last 10 files of 1MB will be kept.
 </appender>
 ----
 
+[NOTE]
+====
+With `staticLogFileName` a `datePattern` that formats as a dot and up to four digits, such as `.dd`, names dated files like numbered backups.
+Such an appender is reported when activated and uses the dated file name as with `staticLogFileName` false, so its backups are numbered after the date.
+Use a pattern with a separator, such as the default `.yyyy-MM-dd`, to keep the static name.
+====
+
 This example show how to configure the `RollingFileAppender` to roll log files once per program execution.
 
 The `appendToFile` property is set to false to prevent the appender from overwriting the existing files.