You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rp...@apache.org on 2016/08/30 14:45:23 UTC
[44/50] 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-1010&LOG4J2-1447-injectable-contextdata&better-datastructure
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>