Skip to content

Commit 88cf8ac

Browse files
committed
Fix windows empty line in logging capture (#52162)
This commit fixes another edge case in handling windows newlines in our capture of stdout/stderr to log4j. The case is that the \r appears at the beginning of the buffer when flushing, which would unintentionally be emitted as an empty string. This commit skips the flush if only a \r was found. closes #51838
1 parent 6b60085 commit 88cf8ac

File tree

2 files changed

+13
-4
lines changed

2 files changed

+13
-4
lines changed

server/src/main/java/org/elasticsearch/common/logging/LoggingOutputStream.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ public void flush() {
9696
// windows case: remove the first part of newlines there too
9797
--used;
9898
}
99+
if (used == 0) {
100+
// only windows \r was in the buffer
101+
return;
102+
}
99103
log(new String(buffer.bytes, 0, used, StandardCharsets.UTF_8));
100104
if (buffer.bytes.length != DEFAULT_BUFFER_LENGTH) {
101105
threadLocal.set(new Buffer()); // reset size

server/src/test/java/org/elasticsearch/common/logging/LoggingOutputStreamTests.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,15 @@ public void createStream() throws Exception {
5858
printStream = new PrintStream(loggingStream, false, StandardCharsets.UTF_8.name());
5959
}
6060

61-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/51838")
62-
public void testEmptyLine() {
63-
printStream.println("");
61+
public void testEmptyLineUnix() {
62+
printStream.print("\n");
63+
assertTrue(loggingStream.lines.isEmpty());
64+
printStream.flush();
65+
assertTrue(loggingStream.lines.isEmpty());
66+
}
67+
68+
public void testEmptyLineWindows() {
69+
printStream.print("\r\n");
6470
assertTrue(loggingStream.lines.isEmpty());
6571
printStream.flush();
6672
assertTrue(loggingStream.lines.isEmpty());
@@ -96,7 +102,6 @@ public void testBufferExtension() {
96102
assertThat(loggingStream.threadLocal.get().bytes.length, equalTo(DEFAULT_BUFFER_LENGTH));
97103
}
98104

99-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/51838")
100105
public void testMaxBuffer() {
101106
String longStr = randomAlphaOfLength(MAX_BUFFER_LENGTH);
102107
String extraLongStr = longStr + "OVERFLOW";

0 commit comments

Comments
 (0)