You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by vy...@apache.org on 2020/12/02 16:15:34 UTC
[logging-log4j2] branch master updated: LOG4J2-2966 Revert the
usage of ParameterizedMessage.deepToString().
This is an automated email from the ASF dual-hosted git repository.
vy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/master by this push:
new ab8753f LOG4J2-2966 Revert the usage of ParameterizedMessage.deepToString().
ab8753f is described below
commit ab8753f631b56eb2ad958041c34b32467cdbb198
Author: Volkan Yazıcı <vo...@gmail.com>
AuthorDate: Wed Dec 2 17:07:57 2020 +0100
LOG4J2-2966 Revert the usage of ParameterizedMessage.deepToString().
There are a couple of motivations for me to revert this change, but
some particular highlights are as follows:
- Many of the type check that is performed by deepToString() is
already addressed in JsonWriter.
- Usage of an external method breaks the self-containment contract of
JsonWriter.
- deepToString() protects against recursive collections, whereas
JsonWriter doesn't. Even using deepToString() in JsonWriter
isn't enough to protect it against self-referencing collections;
all collection handling methods in JsonWriter needs to be adapted.
Hence, rather than a code base where there is partial mitigation for
this anomaly, now it is explicit that there is no built-in prevention
mechanisms. This behaviour is also in line with the Java standard
library, e.g., Arrays.toString().
---
.../json/resolver/MessageParameterResolver.java | 10 ++-----
.../json/resolver/ReadOnlyStringMapResolver.java | 6 ++--
.../template/json/resolver/TemplateResolvers.java | 3 +-
.../layout/template/json/util/JsonWriter.java | 6 ++--
.../template/json/JsonTemplateLayoutTest.java | 35 ++++++++++++++++++++--
.../asciidoc/manual/json-template-layout.adoc.vm | 16 ++++++++++
6 files changed, 59 insertions(+), 17 deletions(-)
diff --git a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/MessageParameterResolver.java b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/MessageParameterResolver.java
index 866b3d1..238f523 100644
--- a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/MessageParameterResolver.java
+++ b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/MessageParameterResolver.java
@@ -22,7 +22,6 @@ import org.apache.logging.log4j.layout.template.json.util.Recycler;
import org.apache.logging.log4j.message.Message;
import org.apache.logging.log4j.message.ParameterConsumer;
import org.apache.logging.log4j.message.ParameterVisitable;
-import org.apache.logging.log4j.message.ParameterizedMessage;
/**
* {@link Message} parameter (i.e., {@link Message#getParameters()}) resolver.
@@ -130,8 +129,7 @@ final class MessageParameterResolver implements EventResolver {
}
final Object parameter = parameters[i];
if (stringified) {
- final String stringifiedParameter =
- ParameterizedMessage.deepToString(parameter);
+ final String stringifiedParameter = String.valueOf(parameter);
jsonWriter.writeString(stringifiedParameter);
} else {
jsonWriter.writeValue(parameter);
@@ -144,8 +142,7 @@ final class MessageParameterResolver implements EventResolver {
else {
final Object parameter = parameters[index];
if (stringified) {
- final String stringifiedParameter =
- ParameterizedMessage.deepToString(parameter);
+ final String stringifiedParameter = String.valueOf(parameter);
jsonWriter.writeString(stringifiedParameter);
} else {
jsonWriter.writeValue(parameter);
@@ -206,8 +203,7 @@ final class MessageParameterResolver implements EventResolver {
// Write the value.
if (arrayNeeded || state.resolver.index == index) {
if (state.resolver.stringified) {
- final String stringifiedParameter =
- ParameterizedMessage.deepToString(parameter);
+ final String stringifiedParameter = String.valueOf(parameter);
state.jsonWriter.writeString(stringifiedParameter);
} else {
state.jsonWriter.writeValue(parameter);
diff --git a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/ReadOnlyStringMapResolver.java b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/ReadOnlyStringMapResolver.java
index 05164c9..3735017 100644
--- a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/ReadOnlyStringMapResolver.java
+++ b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/ReadOnlyStringMapResolver.java
@@ -20,7 +20,6 @@ import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.layout.template.json.util.JsonWriter;
import org.apache.logging.log4j.layout.template.json.util.Recycler;
import org.apache.logging.log4j.layout.template.json.util.RecyclerFactory;
-import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.ReadOnlyStringMap;
import org.apache.logging.log4j.util.TriConsumer;
@@ -202,8 +201,7 @@ class ReadOnlyStringMapResolver implements EventResolver {
final ReadOnlyStringMap map = mapAccessor.apply(logEvent);
final Object value = map == null ? null : map.getValue(key);
if (stringified) {
- final String valueString =
- ParameterizedMessage.deepToString(value);
+ final String valueString = String.valueOf(value);
jsonWriter.writeString(valueString);
} else {
jsonWriter.writeValue(value);
@@ -350,7 +348,7 @@ class ReadOnlyStringMapResolver implements EventResolver {
loopContext.jsonWriter.writeObjectKey(loopContext.prefixedKey);
}
if (loopContext.stringified && !(value instanceof String)) {
- final String valueString = ParameterizedMessage.deepToString(value);
+ final String valueString = String.valueOf(value);
loopContext.jsonWriter.writeString(valueString);
} else {
loopContext.jsonWriter.writeValue(value);
diff --git a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/TemplateResolvers.java b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/TemplateResolvers.java
index 1109d28..ab4dfa4 100644
--- a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/TemplateResolvers.java
+++ b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/resolver/TemplateResolvers.java
@@ -20,7 +20,6 @@ import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.layout.template.json.JsonTemplateLayout.EventTemplateAdditionalField;
import org.apache.logging.log4j.layout.template.json.util.JsonReader;
import org.apache.logging.log4j.layout.template.json.util.JsonWriter;
-import org.apache.logging.log4j.message.ParameterizedMessage;
import java.util.ArrayList;
import java.util.List;
@@ -404,7 +403,7 @@ public final class TemplateResolvers {
}
private static <V> TemplateResolver<V> ofNumber(final Number number) {
- final String numberString = ParameterizedMessage.deepToString(number);
+ final String numberString = String.valueOf(number);
return (final V ignored, final JsonWriter jsonWriter) ->
jsonWriter.writeRawString(numberString);
}
diff --git a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/util/JsonWriter.java b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/util/JsonWriter.java
index 4523452..ac35468 100644
--- a/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/util/JsonWriter.java
+++ b/log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/util/JsonWriter.java
@@ -16,7 +16,6 @@
*/
package org.apache.logging.log4j.layout.template.json.util;
-import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.BiConsumer;
import org.apache.logging.log4j.util.IndexedReadOnlyStringMap;
import org.apache.logging.log4j.util.StringBuilderFormattable;
@@ -50,6 +49,9 @@ import java.util.Objects;
* <p>
* JSON standard quoting routines are borrowed from
* <a href="https://github.com/FasterXML/jackson-core">Jackson</a>.
+ * <p>
+ * Note that this class provides no protection against recursive collections,
+ * e.g., an array where one or more elements reference to the array itself.
*/
public final class JsonWriter implements AutoCloseable, Cloneable {
@@ -216,7 +218,7 @@ public final class JsonWriter implements AutoCloseable, Cloneable {
else {
final String stringValue = value instanceof String
? (String) value
- : ParameterizedMessage.deepToString(value);
+ : String.valueOf(value);
writeString(stringValue);
}
diff --git a/log4j-layout-template-json/src/test/java/org/apache/logging/log4j/layout/template/json/JsonTemplateLayoutTest.java b/log4j-layout-template-json/src/test/java/org/apache/logging/log4j/layout/template/json/JsonTemplateLayoutTest.java
index f8d4990..5f4c8dd 100644
--- a/log4j-layout-template-json/src/test/java/org/apache/logging/log4j/layout/template/json/JsonTemplateLayoutTest.java
+++ b/log4j-layout-template-json/src/test/java/org/apache/logging/log4j/layout/template/json/JsonTemplateLayoutTest.java
@@ -2064,13 +2064,11 @@ class JsonTemplateLayoutTest {
// Create the log event.
final SimpleMessage message = new SimpleMessage("foo");
- final Level level = Level.FATAL;
final Exception thrown = new RuntimeException("bar");
final LogEvent logEvent = Log4jLogEvent
.newBuilder()
.setLoggerName(LOGGER_NAME)
.setMessage(message)
- .setLevel(level)
.setThrown(thrown)
.build();
@@ -2084,6 +2082,39 @@ class JsonTemplateLayoutTest {
}
+ @Test
+ void test_recursive_collections() {
+
+ // Create the event template.
+ final String eventTemplate = writeJson(asMap(
+ "message", asMap("$resolver", "message")));
+
+ // Create the layout.
+ final JsonTemplateLayout layout = JsonTemplateLayout
+ .newBuilder()
+ .setConfiguration(CONFIGURATION)
+ .setEventTemplate(eventTemplate)
+ .build();
+
+ // Create a recursive collection.
+ final Object[] recursiveCollection = new Object[1];
+ recursiveCollection[0] = recursiveCollection;
+
+ // Create the log event.
+ final Message message = new ObjectMessage(recursiveCollection);
+ final LogEvent logEvent = Log4jLogEvent
+ .newBuilder()
+ .setLoggerName(LOGGER_NAME)
+ .setMessage(message)
+ .build();
+
+ // Check the serialized event.
+ Assertions
+ .assertThatThrownBy(() -> layout.toSerializable(logEvent))
+ .isInstanceOf(StackOverflowError.class);
+
+ }
+
private static synchronized String writeJson(final Object value) {
final StringBuilder stringBuilder = JSON_WRITER.getStringBuilder();
stringBuilder.setLength(0);
diff --git a/src/site/asciidoc/manual/json-template-layout.adoc.vm b/src/site/asciidoc/manual/json-template-layout.adoc.vm
index c50bb0a..488d2c6 100644
--- a/src/site/asciidoc/manual/json-template-layout.adoc.vm
+++ b/src/site/asciidoc/manual/json-template-layout.adoc.vm
@@ -1221,6 +1221,22 @@ Yes, link:lookups.html[lookups] (e.g., `${dollar}{java:version}`,
`${dollar}{env:USER}`, `${dollar}{date:MM-dd-yyyy}`) are supported in string
literals of templates. Though note that they are not garbage-free.
+=== Are recursive collections supported?
+
+No. Consider a `Message` containing a recursive value as follows:
+
+[source,java]
+----
+Object[] recursiveCollection = new Object[1];
+recursiveCollection[0] = recursiveCollection;
+----
+
+While the exact exception might vary, you will most like get a
+`StackOverflowError` while trying to render `recursiveCollection` into a
+`String`. Note that this is also the default behaviour for other Java standard
+library methods, e.g., `Arrays.toString()`. Hence mind self references while
+logging.
+
[#faq-garbage-free]
=== Is `JsonTemplateLayout` garbage-free?