diff --git a/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java b/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java index 9b6fb8d1c..4a7067540 100644 --- a/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java +++ b/powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/internal/LambdaJsonEncoderTest.java @@ -127,7 +127,8 @@ void shouldLogArgumentsAsJsonWhenUsingRawJson() { assertThat(contentOf(logFile)) .contains("\"input\":{\"awsRegion\":\"eu-west-1\",\"body\":\"plop\",\"eventSource\":\"eb\",\"messageAttributes\":{\"keyAttribute\":{\"stringListValues\":[\"val1\",\"val2\",\"val3\"]}},\"messageId\":\"1212abcd\"}") .contains("\"message\":\"1212abcd\"") - .contains("\"message\":\"Message body = plop and id = \"1212abcd\"\""); + // Should auto-escape double quotes around id + .contains("\"message\":\"Message body = plop and id = \\\"1212abcd\\\"\""); // Reserved keys should be ignored PowertoolsLoggedFields.stringValues().stream().forEach(reservedKey -> { assertThat(contentOf(logFile)).doesNotContain("\"" + reservedKey + "\":\"shouldBeIgnored\""); @@ -158,7 +159,8 @@ void shouldLogArgumentsAsJsonWhenUsingKeyValue() { assertThat(contentOf(logFile)) .contains("\"input\":{\"awsRegion\":\"eu-west-1\",\"body\":\"plop\",\"eventSource\":\"eb\",\"messageAttributes\":{\"keyAttribute\":{\"stringListValues\":[\"val1\",\"val2\",\"val3\"]}},\"messageId\":\"1212abcd\"}") .contains("\"message\":\"1212abcd\"") - .contains("\"message\":\"Message body = plop and id = \"1212abcd\"\""); + // Should auto-escape double quotes around id + .contains("\"message\":\"Message body = plop and id = \\\"1212abcd\\\"\""); // Reserved keys should be ignored PowertoolsLoggedFields.stringValues().stream().forEach(reservedKey -> { assertThat(contentOf(logFile)).doesNotContain("\"" + reservedKey + "\":\"shouldBeIgnored\""); @@ -295,7 +297,8 @@ void shouldLogEventAsStringForStreamHandler() throws IOException { File logFile = new File("target/logfile.json"); assertThat(contentOf(logFile)) .contains("\"message\":\"Handler Event\"") - .contains("\"event\":\"{\"key\":\"value\"}\""); // logged as String for StreamHandler + // logged as String for StreamHandler (should auto-escape double-quotes to avoid breaking JSON format) + .contains("\"event\":\"{\\\"key\\\":\\\"value\\\"}\""); } @Test diff --git a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/internal/JsonSerializer.java b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/internal/JsonSerializer.java index 0b4359825..82bc76a38 100644 --- a/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/internal/JsonSerializer.java +++ b/powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/internal/JsonSerializer.java @@ -53,7 +53,7 @@ public JsonSerializer(StringBuilder builder) { } this.builder = builder; } - + public void writeStartArray() { builder.append('['); } @@ -84,7 +84,8 @@ public void writeString(String text) { if (text == null) { writeNull(); } else { - builder.append("\"").append(text).append("\""); + // Escape double quotes to avoid breaking JSON format + builder.append("\"").append(text.replace("\"", "\\\"")).append("\""); } } @@ -314,7 +315,7 @@ public void writeMap(final Map map) { writeNull(); } else { writeStartObject(); - for (Iterator> entries = map.entrySet().iterator(); entries.hasNext(); ) { + for (Iterator> entries = map.entrySet().iterator(); entries.hasNext();) { Map.Entry entry = entries.next(); writeObjectField(String.valueOf(entry.getKey()), entry.getValue()); if (entries.hasNext()) { @@ -326,16 +327,16 @@ public void writeMap(final Map map) { } public void writeObject(Object value) { - + // null if (value == null) { writeNull(); - } - + } + else if (value instanceof String) { writeString((String) value); - } - + } + // number & boolean else if (value instanceof Number) { Number n = (Number) value; @@ -364,8 +365,8 @@ else if (value instanceof Number) { writeBoolean((Boolean) value); } else if (value instanceof AtomicBoolean) { writeBoolean(((AtomicBoolean) value).get()); - } - + } + // list & collection else if (value instanceof List) { final List list = (List) value; @@ -373,15 +374,14 @@ else if (value instanceof List) { } else if (value instanceof Collection) { final Collection collection = (Collection) value; writeArray(collection); - } - + } + // map else if (value instanceof Map) { final Map map = (Map) value; writeMap(map); } - // arrays else if (value instanceof char[]) { final char[] charValues = (char[]) value; @@ -411,7 +411,7 @@ else if (value instanceof char[]) { final Object[] values = (Object[]) value; writeArray(values); } - + else if (value instanceof JsonNode) { JsonNode node = (JsonNode) value; @@ -462,7 +462,7 @@ else if (value instanceof JsonNode) { case OBJECT: case POJO: writeStartObject(); - for (Iterator> entries = node.fields(); entries.hasNext(); ) { + for (Iterator> entries = node.fields(); entries.hasNext();) { Map.Entry entry = entries.next(); writeObjectField(entry.getKey(), entry.getValue()); if (entries.hasNext()) { @@ -475,7 +475,7 @@ else if (value instanceof JsonNode) { case ARRAY: ArrayNode arrayNode = (ArrayNode) node; writeStartArray(); - for (Iterator elements = arrayNode.elements(); elements.hasNext(); ) { + for (Iterator elements = arrayNode.elements(); elements.hasNext();) { writeObject(elements.next()); if (elements.hasNext()) { builder.append(','); @@ -558,7 +558,7 @@ public void writeTree(TreeNode rootNode) { writeNull(); } else if (rootNode instanceof TextNode) { writeString(((TextNode) rootNode).asText()); - } else if (rootNode instanceof BooleanNode) { + } else if (rootNode instanceof BooleanNode) { writeBoolean(((BooleanNode) rootNode).asBoolean()); } else if (rootNode instanceof NumericNode) { NumericNode numericNode = (NumericNode) rootNode; @@ -595,5 +595,3 @@ public void close() { // nothing to do } } - -