From 88cf8ac0a84890bd51b602282455f0c1d32b43b0 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 10 Feb 2020 16:29:29 -0500 Subject: [PATCH] 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 --- .../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 b5679ada5a2..54af0a1ccfd 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 @@ class LoggingOutputStream extends OutputStream { // 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 dcc50a45b43..5fa98af1b92 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 class LoggingOutputStreamTests extends ESTestCase { printStream = new PrintStream(loggingStream, false, StandardCharsets.UTF_8.name()); } - @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 class LoggingOutputStreamTests extends ESTestCase { 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";