Copilot commented on code in PR #336:
URL: https://github.com/apache/logging-log4net/pull/336#discussion_r4241401479
##########
src/log4net/Appender/RollingFileAppender.cs:
##########
@@ -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);
Review Comment:
A single 1970 sample does not establish the set of numeric names a pattern
can produce. For example, `.Md` samples as `.11`, so this treats 10 as the
minimum and changes `StaticLogFileName` with a count-down limit of 10, although
`.10` is impossible and the first colliding name is `.11`. Conversely, `.yyyyM`
is ignored because its sample has five digits, but count-up backups are
unbounded and can reach the dated name (for example `.20261`), recreating the
overwrite after a restart. Determine whether an actually reachable formatted
value intersects the reachable backup indices rather than inferring this from
sample length.
This issue also appears on line 1140 of the same file.
##########
src/changelog/3.5.1/335-short-date-pattern-collides-with-backup-names.xml:
##########
@@ -0,0 +1,14 @@
+<?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 or at least five digits to keep the static
name (reported by
Review Comment:
The five-digit recommendation is unsafe for count-up configurations, whose
indices keep increasing past the retention limit. A numeric `.yyyyM` date can
still collide with backup 20261, so the release note should recommend a
nonnumeric separator instead.
##########
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 or at least five digits to keep the static name.
Review Comment:
At least five digits is not sufficient to prevent a collision because
count-up backup indices are unbounded. For example, `.yyyyM` can collide with
backup 20261. A nonnumeric separator is the safe recommendation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
