This is an automated email from the ASF dual-hosted git repository. vy pushed a commit to branch release-2.x in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/release-2.x by this push: new b2b2068 LOG4J2-2966 Revert the usage of ParameterizedMessage.deepToString(). b2b2068 is described below commit b2b2068674664a0be28a7125de1efc154614444d Author: Volkan Yazıcı <volkan.yaz...@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 3d482fe..49ef8ca 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?