From 0f48e2ddbf5a00e91d72c5167e9282db113564c6 Mon Sep 17 00:00:00 2001 From: Karsten Schnitter Date: Tue, 14 Dec 2021 22:26:56 +0100 Subject: [PATCH] FIx invalid JSON messages The last version introduced a trailing comma after the fields generated by the ContextPropertyConverter, even when there were no additional fields. This lead to invalid JSON. This change fixes the problem. --- .../hcp/cf/log4j2/converter/ContextPropsConverter.java | 9 +++++---- .../sap/hcp/cf/log4j2/layout/LayoutPatternBuilder.java | 1 + .../cf/log4j2/converter/ContextPropsConverterTest.java | 8 -------- .../hcp/cf/log4j2/layout/LayoutPatternBuilderTest.java | 4 ++-- 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverter.java b/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverter.java index ebebf43b..c2a8b773 100644 --- a/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverter.java +++ b/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverter.java @@ -51,10 +51,11 @@ public void format(LogEvent event, StringBuilder toAppendTo) { addCustomFieldsFromArguments(contextData, event); int lengthBefore = toAppendTo.length(); converter.convert(toAppendTo, contextData); - // append comma only, if at least one field was added - // otherwise, there will be to commas ",," rendering the JSON invalid - if (toAppendTo.length() > lengthBefore) { - toAppendTo.append(","); + // remove comma from pattern, when no properties are added + // this is to avoid a double comma in the JSON + // Do not do this on empty messages + if (toAppendTo.length() == lengthBefore && lengthBefore > 0 && toAppendTo.charAt(lengthBefore - 1) == ',') { + toAppendTo.setLength(lengthBefore - 1); } } diff --git a/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilder.java b/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilder.java index 8bc391fa..b4550075 100644 --- a/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilder.java +++ b/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilder.java @@ -56,6 +56,7 @@ public LayoutPatternBuilder addContextProperties(List exclusions) { sb.append("%").append(ContextPropsConverter.WORD); appendParameters(asList(Boolean.toString(sendDefaultValues))); appendParameters(exclusions); + sb.append(","); return this; } diff --git a/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverterTest.java b/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverterTest.java index 88df33e9..c86f1f0f 100644 --- a/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverterTest.java +++ b/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverterTest.java @@ -9,8 +9,6 @@ import java.io.IOException; import java.util.Map; -import org.apache.logging.log4j.core.LogEvent; -import org.apache.logging.log4j.core.pattern.LogEventPatternConverter; import org.junit.Test; import org.slf4j.MDC; @@ -107,10 +105,4 @@ public void testRegisteredCustomField() throws Exception { not(hasEntry(SOME_KEY, SOME_VALUE))); } - @Override - protected String format(LogEventPatternConverter cpc, LogEvent event) { - String converted = super.format(cpc, event); - return converted.length() > 0 ? converted.substring(0, converted.length() - 1) : converted; - } - } diff --git a/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilderTest.java b/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilderTest.java index 1eaf3940..e5de172c 100644 --- a/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilderTest.java +++ b/cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilderTest.java @@ -107,7 +107,7 @@ public void applicationScenario() throws Exception { .suppressExceptions().build(); assertThat(pattern, specificPart(is( - ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{false}{excluded-field}\"#cf\":{%cf{custom-field}}%ex{0} "))); + ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{false}{excluded-field},\"#cf\":{%cf{custom-field}}%ex{0} "))); } @Test @@ -117,7 +117,7 @@ public void exceptionScenario() throws Exception { .build(); assertThat(pattern, specificPart(is( - ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{false}{excluded-field}\"#cf\":{%cf{custom-field}},\"stacktrace\":%stacktrace"))); + ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{false}{excluded-field},\"#cf\":{%cf{custom-field}},\"stacktrace\":%stacktrace"))); } private static Matcher specificPart(Matcher expected) {