From dbee51767a931517234cca36aadaf583b6e5c936 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 10 Feb 2020 10:38:54 -0800 Subject: [PATCH] Fix windows empty line in logging capture 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 --- .../common/logging/LoggingOutputStream.java | 4 ++++ .../common/logging/LoggingOutputStreamTests.java | 13 +++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/logging/LoggingOutputStream.java b/server/src/main/java/org/elasticsearch/common/logging/LoggingOutputStream.java index b5679ada5a28f..54af0a1ccfdf0 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/LoggingOutputStream.java +++ b/server/src/main/java/org/elasticsearch/common/logging/LoggingOutputStream.java @@ -96,6 +96,10 @@ public void flush() { // windows case: remove the first part of newlines there too --used; } + if (used == 0) { + // only windows \r was in the buffer + return; + } log(new String(buffer.bytes, 0, used, StandardCharsets.UTF_8)); if (buffer.bytes.length != DEFAULT_BUFFER_LENGTH) { threadLocal.set(new Buffer()); // reset size diff --git a/server/src/test/java/org/elasticsearch/common/logging/LoggingOutputStreamTests.java b/server/src/test/java/org/elasticsearch/common/logging/LoggingOutputStreamTests.java index 826eb40bfd440..aa1d8669a8e3b 100644 --- a/server/src/test/java/org/elasticsearch/common/logging/LoggingOutputStreamTests.java +++ b/server/src/test/java/org/elasticsearch/common/logging/LoggingOutputStreamTests.java @@ -58,9 +58,15 @@ public void createStream() { printStream = new PrintStream(loggingStream, false, StandardCharsets.UTF_8); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/51838") - public void testEmptyLine() { - printStream.println(""); + public void testEmptyLineUnix() { + printStream.print("\n"); + assertTrue(loggingStream.lines.isEmpty()); + printStream.flush(); + assertTrue(loggingStream.lines.isEmpty()); + } + + public void testEmptyLineWindows() { + printStream.print("\r\n"); assertTrue(loggingStream.lines.isEmpty()); printStream.flush(); assertTrue(loggingStream.lines.isEmpty()); @@ -96,7 +102,6 @@ public void testBufferExtension() { assertThat(loggingStream.threadLocal.get().bytes.length, equalTo(DEFAULT_BUFFER_LENGTH)); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/51838") public void testMaxBuffer() { String longStr = randomAlphaOfLength(MAX_BUFFER_LENGTH); String extraLongStr = longStr + "OVERFLOW";