[LOG4J2-1502] CsvParameterLayout is inserting NUL character if data
starts with {, (, [ or "Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/f9a5d552 Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/f9a5d552 Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/f9a5d552 Branch: refs/heads/LOG4J2-1010&LOG4J2-1447-injectable-contextdata&better-datastructure Commit: f9a5d552a9040c50adcfdd46fc2aeff67281bd76 Parents: c85a63c Author: Gary Gregory <[email protected]> Authored: Mon Aug 29 12:41:49 2016 -0700 Committer: Gary Gregory <[email protected]> Committed: Mon Aug 29 12:41:49 2016 -0700 ---------------------------------------------------------------------- .../log4j/core/layout/AbstractCsvLayout.java | 10 ++-- .../CsvJsonParameterLayoutFileAppenderTest.java | 49 ++++++++++++++++---- src/changes/changes.xml | 3 ++ 3 files changed, 50 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/f9a5d552/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractCsvLayout.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractCsvLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractCsvLayout.java index 3d0d80c..ffa6ea6 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractCsvLayout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractCsvLayout.java @@ -38,13 +38,13 @@ public abstract class AbstractCsvLayout extends AbstractStringLayout { protected static CSVFormat createFormat(final String format, final Character delimiter, final Character escape, final Character quote, final QuoteMode quoteMode, final String nullString, final String recordSeparator) { CSVFormat csvFormat = CSVFormat.valueOf(format); - if (delimiter != null) { + if (isNotNul(delimiter)) { csvFormat = csvFormat.withDelimiter(delimiter); } - if (escape != null) { + if (isNotNul(escape)) { csvFormat = csvFormat.withEscape(escape); } - if (quote != null) { + if (isNotNul(quote)) { csvFormat = csvFormat.withQuote(quote); } if (quoteMode != null) { @@ -59,6 +59,10 @@ public abstract class AbstractCsvLayout extends AbstractStringLayout { return csvFormat; } + private static boolean isNotNul(final Character character) { + return character != null && character.charValue() != 0; + } + private final CSVFormat format; protected AbstractCsvLayout(final Configuration config, final Charset charset, final CSVFormat csvFormat, http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/f9a5d552/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/CsvJsonParameterLayoutFileAppenderTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/CsvJsonParameterLayoutFileAppenderTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/CsvJsonParameterLayoutFileAppenderTest.java index 27c9366..e4357b5 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/CsvJsonParameterLayoutFileAppenderTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/CsvJsonParameterLayoutFileAppenderTest.java @@ -26,13 +26,15 @@ import org.apache.logging.log4j.core.Logger; import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.junit.LoggerContextRule; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.RuleChain; import com.google.common.io.Files; +/** + * Tests https://issues.apache.org/jira/browse/LOG4J2-1502 + */ public class CsvJsonParameterLayoutFileAppenderTest { private static final String FILE_PATH = "target/CsvJsonParameterLayoutFileAppenderTest.log"; @@ -42,7 +44,7 @@ public class CsvJsonParameterLayoutFileAppenderTest { @Rule public RuleChain rule = loggerContextRule.withCleanFilesRule(FILE_PATH); - public void testNoNulCharacters(final String message) throws IOException { + public void testNoNulCharacters(final String message, final String expected) throws IOException { @SuppressWarnings("resource") final LoggerContext loggerContext = loggerContextRule.getLoggerContext(); final Logger logger = loggerContext.getLogger("com.example"); @@ -63,24 +65,53 @@ public class CsvJsonParameterLayoutFileAppenderTest { Assert.assertEquals("File contains " + count0s + " 0x00 byte at indices " + sb, 0, count0s); final List<String> readLines = Files.readLines(file, Charset.defaultCharset()); final String actual = readLines.get(0); - Assert.assertTrue(actual, actual.contains(message)); + // Assert.assertTrue(actual, actual.contains(message)); + Assert.assertEquals(actual, expected, actual); Assert.assertEquals(1, readLines.size()); } @Test - public void testNoNulCharactersABC() throws IOException { - testNoNulCharacters("ABC"); + public void testNoNulCharactersDoubleQuote() throws IOException { + // TODO This does not seem right but there is no NULs. Check Apache Commons CSV. + testNoNulCharacters("\"", "\"\"\"\""); } @Test - @Ignore("https://issues.apache.org/jira/browse/LOG4J2-1502";) public void testNoNulCharactersJson() throws IOException { - testNoNulCharacters("{\"id\":10,\"name\":\"Alice\"}"); + testNoNulCharacters("{\"id\":10,\"name\":\"Alice\"}", "\"{\"\"id\"\":10,\"\"name\"\":\"\"Alice\"\"}\""); + } + + @Test + public void testNoNulCharactersOneChar() throws IOException { + testNoNulCharacters("A", "A"); + } + + @Test + public void testNoNulCharactersOpenCurly() throws IOException { + // TODO Why is the char quoted? Check Apache Commons CSV. + testNoNulCharacters("{", "\"{\""); + } + + @Test + public void testNoNulCharactersOpenParen() throws IOException { + // TODO Why is the char quoted? Check Apache Commons CSV. + testNoNulCharacters("(", "\"(\""); + } + + @Test + public void testNoNulCharactersOpenSquare() throws IOException { + // TODO Why is the char quoted? Check Apache Commons CSV. + testNoNulCharacters("[", "\"[\""); + } + + @Test + public void testNoNulCharactersThreeChars() throws IOException { + testNoNulCharacters("ABC", "ABC"); } @Test - @Ignore("https://issues.apache.org/jira/browse/LOG4J2-1502";) public void testNoNulCharactersXml() throws IOException { - testNoNulCharacters("<test attr1='val1' attr2=\"value2\">X</test>"); + testNoNulCharacters("<test attr1='val1' attr2=\"value2\">X</test>", + "\"<test attr1='val1' attr2=\"\"value2\"\">X</test>\""); } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/f9a5d552/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index b901033..10be4c8 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -99,6 +99,9 @@ <action issue="LOG4J2-1235" dev="ggregory" type="fix" due-to="Niranjan Rao, Sascha Scholz, Aleksey Zvolinsky"> org.apache.logging.log4j.core.appender.routing.IdlePurgePolicy not working correctly. </action> + <action issue="LOG4J2-1502" dev="ggregory" type="fix" due-to="Sumit Singhal"> + CsvParameterLayout is inserting NUL character if data starts with {, (, [ or " + </action> <action issue="LOG4J2-1181" dev="mikes" type="add"> Scala API. </action>
