You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by mi...@apache.org on 2016/08/30 09:27:38 UTC

[21/24] logging-log4j2 git commit: [LOG4J2-1502] CsvParameterLayout is inserting NUL character if data starts with {, (, [ or "

[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-1528
Commit: f9a5d552a9040c50adcfdd46fc2aeff67281bd76
Parents: c85a63c
Author: Gary Gregory <gg...@apache.org>
Authored: Mon Aug 29 12:41:49 2016 -0700
Committer: Gary Gregory <gg...@apache.org>
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>