Skip to content

Commit 752d023

Browse files
NeatGuyCodingvy
andauthored
Fix header write in RollingRandomAccessFileManager (#4008)
Co-authored-by: Volkan Yazıcı <[email protected]>
1 parent 33fff89 commit 752d023

File tree

3 files changed

+78
-1
lines changed

3 files changed

+78
-1
lines changed

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,14 @@
4141
import java.util.Set;
4242
import java.util.concurrent.locks.LockSupport;
4343
import org.apache.logging.log4j.core.config.DefaultConfiguration;
44+
import org.apache.logging.log4j.core.layout.PatternLayout;
4445
import org.apache.logging.log4j.core.util.Closer;
4546
import org.apache.logging.log4j.core.util.FileUtils;
4647
import org.apache.logging.log4j.core.util.NullOutputStream;
4748
import org.apache.logging.log4j.util.Strings;
4849
import org.junit.jupiter.api.Test;
50+
import org.junit.jupiter.params.ParameterizedTest;
51+
import org.junit.jupiter.params.provider.CsvSource;
4952

5053
/**
5154
* Tests the RollingRandomAccessFileManager class.
@@ -333,4 +336,64 @@ void testRolloverRetainsFileAttributes() throws Exception {
333336
.permissions();
334337
assertEquals(filePermissions, actualFilePermissions);
335338
}
339+
340+
@ParameterizedTest
341+
@CsvSource({
342+
"true,true",
343+
"true,false",
344+
"false,true",
345+
"false,false",
346+
})
347+
void testWriteHeaderWhetherAppendOrExists(final boolean append, final boolean fileExists) throws Exception {
348+
final File file = File.createTempFile("log4j2", "test");
349+
if (!fileExists) {
350+
file.delete(); // Ensure file doesn't exist
351+
} else {
352+
// If file exists, write some content to it so it's not empty
353+
try (FileOutputStream fos = new FileOutputStream(file)) {
354+
fos.write("EXISTING_CONTENT".getBytes());
355+
}
356+
}
357+
file.deleteOnExit();
358+
359+
final String header = "HEADER";
360+
final PatternLayout layout =
361+
PatternLayout.newBuilder().setHeader(header).build();
362+
363+
final RollingRandomAccessFileManager manager = RollingRandomAccessFileManager.getRollingRandomAccessFileManager(
364+
file.getAbsolutePath(),
365+
Strings.EMPTY,
366+
append,
367+
true,
368+
RollingRandomAccessFileManager.DEFAULT_BUFFER_SIZE,
369+
new SizeBasedTriggeringPolicy(Long.MAX_VALUE),
370+
null,
371+
null,
372+
layout,
373+
null,
374+
null,
375+
null,
376+
null);
377+
assertNotNull(manager);
378+
manager.close();
379+
380+
// Verify header was written based on logic: writeHeader = !append || !fileExistedBefore
381+
// Note: writeHeader() also checks file.length() == 0, so header is only written if file is empty
382+
assertTrue(file.exists(), "File should exist");
383+
final byte[] fileContent = Files.readAllBytes(file.toPath());
384+
final String content = new String(fileContent);
385+
final boolean expectedHeaderWritten = !append || !fileExists;
386+
if (expectedHeaderWritten) {
387+
assertTrue(
388+
content.startsWith(header),
389+
"File should start with header when append=" + append + ", fileExists=" + fileExists + ", content: "
390+
+ content);
391+
} else {
392+
// When append=true and fileExists=true, file has existing content, so header should not be written
393+
assertTrue(
394+
!content.startsWith(header),
395+
"File should not start with header when append=" + append + ", fileExists=" + fileExists
396+
+ ", content: " + content);
397+
}
398+
}
336399
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,10 @@ public static RollingRandomAccessFileManager getRollingRandomAccessFileManager(
174174
long size = 0;
175175
long time = System.currentTimeMillis();
176176
RandomAccessFile raf = null;
177+
boolean fileExistedBefore = false;
177178
if (fileName != null) {
178179
file = new File(name);
180+
fileExistedBefore = file.exists();
179181

180182
if (!append) {
181183
file.delete();
@@ -207,7 +209,7 @@ public static RollingRandomAccessFileManager getRollingRandomAccessFileManager(
207209
return null;
208210
}
209211
}
210-
final boolean writeHeader = !append || file == null || !file.exists();
212+
final boolean writeHeader = !append || file == null || !fileExistedBefore;
211213

212214
final RollingRandomAccessFileManager rrm = new RollingRandomAccessFileManager(
213215
data.getLoggerContext(),
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<entry xmlns="https://logging.apache.org/xml/ns"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="
5+
https://logging.apache.org/xml/ns
6+
https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
7+
type="fixed">
8+
<description format="asciidoc">
9+
Fix header write in `RollingRandomAccessFileManager` that was being incorrectly skipped if `append=true` and the file didn't exist before.
10+
</description>
11+
</entry>
12+

0 commit comments

Comments
 (0)