diff --git a/logbook-core/pom.xml b/logbook-core/pom.xml index 71e2f9864..3e86e4fc2 100644 --- a/logbook-core/pom.xml +++ b/logbook-core/pom.xml @@ -22,7 +22,7 @@ com.fasterxml.jackson.core jackson-databind - true + diff --git a/logbook-core/src/main/java/org/zalando/logbook/BodyFilters.java b/logbook-core/src/main/java/org/zalando/logbook/BodyFilters.java index 09d3578c4..42296232d 100644 --- a/logbook-core/src/main/java/org/zalando/logbook/BodyFilters.java +++ b/logbook-core/src/main/java/org/zalando/logbook/BodyFilters.java @@ -23,7 +23,7 @@ private BodyFilters() { @API(status = MAINTAINED) public static BodyFilter defaultValue() { - return accessToken(); + return BodyFilter.merge(accessToken(), compactJson(new ObjectMapper())); } @API(status = MAINTAINED) diff --git a/logbook-core/src/main/java/org/zalando/logbook/JsonCompactingBodyFilter.java b/logbook-core/src/main/java/org/zalando/logbook/JsonCompactingBodyFilter.java index cb0f6489d..0440843f1 100644 --- a/logbook-core/src/main/java/org/zalando/logbook/JsonCompactingBodyFilter.java +++ b/logbook-core/src/main/java/org/zalando/logbook/JsonCompactingBodyFilter.java @@ -7,33 +7,34 @@ import java.io.IOException; import java.util.function.Predicate; - @Slf4j -class JsonCompactingBodyFilter implements BodyFilter { +final class JsonCompactingBodyFilter implements BodyFilter { + + private static final Predicate JSON = MediaTypeQuery.compile("application/json", "application/*+json"); - private final JsonCompactor jsonCompactor; private final JsonHeuristic heuristic = new JsonHeuristic(); - private final Predicate contentTypes = MediaTypeQuery.compile("application/json", "application/*+json"); + private final JsonCompactor compactor; JsonCompactingBodyFilter(final ObjectMapper objectMapper) { - jsonCompactor = new JsonCompactor(objectMapper); + this.compactor = new JsonCompactor(objectMapper); } @Override public String filter(@Nullable final String contentType, final String body) { - return contentTypes.test(contentType) && shouldCompact(body) ? compact(body) : body; + return JSON.test(contentType) && isCompactable(body) ? compact(body) : body; } - private boolean shouldCompact(final String body) { - return heuristic.isProbablyJson(body) && !jsonCompactor.isCompacted(body); + private boolean isCompactable(final String body) { + return heuristic.isProbablyJson(body) && !compactor.isCompacted(body); } private String compact(final String body) { try { - return jsonCompactor.compact(body); + return compactor.compact(body); } catch (final IOException e) { log.trace("Unable to compact body, is it a JSON?. Keep it as-is: `{}`", e.getMessage()); return body; } } + } diff --git a/logbook-core/src/main/java/org/zalando/logbook/JsonHeuristic.java b/logbook-core/src/main/java/org/zalando/logbook/JsonHeuristic.java index f252357f7..522778efb 100644 --- a/logbook-core/src/main/java/org/zalando/logbook/JsonHeuristic.java +++ b/logbook-core/src/main/java/org/zalando/logbook/JsonHeuristic.java @@ -13,6 +13,7 @@ boolean isProbablyJson(final String body) { // https://tools.ietf.org/html/rfc4627#section-2 final String trimmed = body.trim(); + // ordered by probability of occurrence in real world applications return isProbablyObject(trimmed) || isProbablyArray(trimmed) || isProbablyString(body) diff --git a/logbook-core/src/main/java/org/zalando/logbook/JsonHttpLogFormatter.java b/logbook-core/src/main/java/org/zalando/logbook/JsonHttpLogFormatter.java index ce44ddbcc..e08f14e27 100644 --- a/logbook-core/src/main/java/org/zalando/logbook/JsonHttpLogFormatter.java +++ b/logbook-core/src/main/java/org/zalando/logbook/JsonHttpLogFormatter.java @@ -5,8 +5,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import lombok.AllArgsConstructor; import org.apiguardian.api.API; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.Map; @@ -47,11 +45,9 @@ @API(status = STABLE) public final class JsonHttpLogFormatter implements PreparedHttpLogFormatter { - private static final Logger LOG = LoggerFactory.getLogger(JsonHttpLogFormatter.class); private static final Predicate JSON = MediaTypeQuery.compile("application/json", "application/*+json"); private final ObjectMapper mapper; - private final JsonCompactor compactor; private final JsonHeuristic heuristic = new JsonHeuristic(); public JsonHttpLogFormatter() { @@ -60,7 +56,6 @@ public JsonHttpLogFormatter() { public JsonHttpLogFormatter(final ObjectMapper mapper) { this.mapper = mapper; - this.compactor = new JsonCompactor(mapper); } @@ -72,7 +67,7 @@ public String format(final Map content) throws IOException { @Override public void addBody(final HttpMessage message, final Map content) throws IOException { if (isContentTypeJson(message)) { - content.put("body", tryParseBodyAsJson(message.getBodyAsString())); + content.put("body", treatPossibleJsonAsJson(message.getBodyAsString())); } else { PreparedHttpLogFormatter.super.addBody(message, content); } @@ -82,21 +77,9 @@ private boolean isContentTypeJson(final HttpMessage message) { return JSON.test(message.getContentType()); } - private Object tryParseBodyAsJson(final String body) { + private Object treatPossibleJsonAsJson(final String body) { if (heuristic.isProbablyJson(body)) { - if (compactor.isCompacted(body)) { - // any body that looks like JSON (according to our heuristic) and has no newlines would be - // incorrectly treated as JSON here which results in an invalid JSON output - // see https://github.com/zalando/logbook/issues/279 - return new JsonBody(body); - } - - try { - return new JsonBody(compactor.compact(body)); - } catch (final IOException e) { - LOG.trace("Unable to compact body, probably because it's not JSON. Embedding it as-is: [{}]", e.getMessage()); - return body; - } + return new JsonBody(body); } else { return body; } diff --git a/logbook-core/src/main/java/org/zalando/logbook/XmlCompactingBodyFilter.java b/logbook-core/src/main/java/org/zalando/logbook/XmlCompactingBodyFilter.java index ae3fe2c0e..287368678 100644 --- a/logbook-core/src/main/java/org/zalando/logbook/XmlCompactingBodyFilter.java +++ b/logbook-core/src/main/java/org/zalando/logbook/XmlCompactingBodyFilter.java @@ -24,15 +24,16 @@ import static org.zalando.fauxpas.FauxPas.throwingSupplier; @Slf4j -class XmlCompactingBodyFilter implements BodyFilter { +final class XmlCompactingBodyFilter implements BodyFilter { - private final Predicate contentTypes = MediaTypeQuery.compile("*/xml", "*/*+xml"); - private final DocumentBuilderFactory documentBuilderFactory = documentBuilderFactory(); + private static final Predicate XML = MediaTypeQuery.compile("*/xml", "*/*+xml"); + + private final DocumentBuilderFactory factory = documentBuilderFactory(); private final Transformer transformer = transformer(); @Override public String filter(@Nullable final String contentType, final String body) { - return contentTypes.test(contentType) && shouldCompact(body) ? compact(body) : body; + return XML.test(contentType) && shouldCompact(body) ? compact(body) : body; } private boolean shouldCompact(final String body) { @@ -52,7 +53,7 @@ private String compact(final String body) { } private Document parseDocument(final String body) throws Exception { - final DocumentBuilder documentBuilder = documentBuilderFactory.newDocumentBuilder(); + final DocumentBuilder documentBuilder = factory.newDocumentBuilder(); final Document document = documentBuilder.parse(new ByteArrayInputStream(body.getBytes())); removeEmptyTextNodes(document); return document; @@ -96,4 +97,5 @@ private DocumentBuilderFactory documentBuilderFactory() { return factory; }).get(); } + } diff --git a/logbook-core/src/test/java/org/zalando/logbook/JsonCompactingBodyFilterTest.java b/logbook-core/src/test/java/org/zalando/logbook/JsonCompactingBodyFilterTest.java index 645e0c657..74055c5ad 100644 --- a/logbook-core/src/test/java/org/zalando/logbook/JsonCompactingBodyFilterTest.java +++ b/logbook-core/src/test/java/org/zalando/logbook/JsonCompactingBodyFilterTest.java @@ -1,7 +1,6 @@ package org.zalando.logbook; import com.fasterxml.jackson.databind.ObjectMapper; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -9,58 +8,54 @@ class JsonCompactingBodyFilterTest { - private JsonCompactingBodyFilter bodyFilter; + private final BodyFilter unit = new JsonCompactingBodyFilter(new ObjectMapper()); /*language=JSON*/ - private final String prettifiedJson = "{\n" + + private final String pretty = "{\n" + " \"root\": {\n" + " \"child\": \"text\"\n" + " }\n" + "}"; /*language=JSON*/ - private final String minimisedJson = "{\"root\":{\"child\":\"text\"}}"; - - @BeforeEach - void setUp() { - bodyFilter = new JsonCompactingBodyFilter(new ObjectMapper()); - } + private final String compacted = "{\"root\":{\"child\":\"text\"}}"; @Test void shouldIgnoreEmptyBody() { - final String filtered = bodyFilter.filter("application/json", ""); + final String filtered = unit.filter("application/json", ""); assertThat(filtered, is("")); } @Test void shouldIgnoreInvalidContent() { final String invalidBody = "{\ninvalid}"; - final String filtered = bodyFilter.filter("application/json", invalidBody); + final String filtered = unit.filter("application/json", invalidBody); assertThat(filtered, is(invalidBody)); } @Test void shouldIgnoreInvalidContentType() { - final String filtered = bodyFilter.filter("text/plain", prettifiedJson); - assertThat(filtered, is(prettifiedJson)); + final String filtered = unit.filter("text/plain", pretty); + assertThat(filtered, is(pretty)); } @Test void shouldTransformValidJsonRequestWithSimpleContentType() { - final String filtered = bodyFilter.filter("application/json", prettifiedJson); - assertThat(filtered, is(minimisedJson)); + final String filtered = unit.filter("application/json", pretty); + assertThat(filtered, is(compacted)); } @Test void shouldTransformValidJsonRequestWithCompatibleContentType() { - final String filtered = bodyFilter.filter("application/custom+json", prettifiedJson); - assertThat(filtered, is(minimisedJson)); + final String filtered = unit.filter("application/custom+json", pretty); + assertThat(filtered, is(compacted)); } @Test void shouldSkipInvalidJsonLookingLikeAValidOne() { final String invalidJson = "{invalid}"; - final String filtered = bodyFilter.filter("application/custom+json", invalidJson); + final String filtered = unit.filter("application/custom+json", invalidJson); assertThat(filtered, is(invalidJson)); } -} \ No newline at end of file + +} diff --git a/logbook-core/src/test/java/org/zalando/logbook/JsonHttpLogFormatterTest.java b/logbook-core/src/test/java/org/zalando/logbook/JsonHttpLogFormatterTest.java index 5900c402e..d3d44c073 100644 --- a/logbook-core/src/test/java/org/zalando/logbook/JsonHttpLogFormatterTest.java +++ b/logbook-core/src/test/java/org/zalando/logbook/JsonHttpLogFormatterTest.java @@ -11,8 +11,6 @@ import static java.time.Duration.ZERO; import static java.time.Duration.ofMillis; import static java.util.Collections.singletonList; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; @@ -93,7 +91,6 @@ void shouldLogRequestWithoutContentType() throws IOException { @Test void shouldLogRequestWithoutBody() throws IOException { - final String correlationId = "ac5c3dc2-682a-11e5-83cd-10ddb1ee7671"; final HttpRequest request = MockHttpRequest.create() .withContentType("") .withBodyAsString(""); @@ -106,7 +103,6 @@ void shouldLogRequestWithoutBody() throws IOException { @Test void shouldEmbedJsonRequestBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; final HttpRequest request = MockHttpRequest.create() .withContentType("application/json") .withBodyAsString("{\"name\":\"Bob\"}"); @@ -119,7 +115,6 @@ void shouldEmbedJsonRequestBody() throws IOException { @Test void shouldNotEmbedInvalidJsonRequestBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; final HttpRequest request = MockHttpRequest.create() .withContentType("application/json") .withBodyAsString("{\"name\":\"Bob\"};"); @@ -130,22 +125,8 @@ void shouldNotEmbedInvalidJsonRequestBody() throws IOException { .assertThat("$.body", is("{\"name\":\"Bob\"};")); } - @Test - void shouldNotEmbedInvalidButProbableJsonRequestBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create() - .withContentType("application/json") - .withBodyAsString("{\"name\":\"Bob\"\n;}"); - - final String json = unit.format(new SimplePrecorrelation(systemUTC()), request); - - with(json) - .assertThat("$.body", is("{\"name\":\"Bob\"\n;}")); - } - @Test void shouldNotEmbedReplacedJsonRequestBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; final HttpRequest request = MockHttpRequest.create() .withContentType("application/json") .withBodyAsString(""); @@ -156,21 +137,8 @@ void shouldNotEmbedReplacedJsonRequestBody() throws IOException { .assertThat("$.body", is("")); } - @Test - void shouldEmbedCompactedJsonRequestBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create() - .withContentType("application/json") - .withBodyAsString("{\n \"name\": \"Bob\"\n}"); - - final String json = unit.format(new SimplePrecorrelation(systemUTC()), request); - - assertThat(json, containsString("{\"name\":\"Bob\"}")); - } - @Test void shouldEmbedCustomJsonRequestBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; final HttpRequest request = MockHttpRequest.create() .withContentType("application/custom+json") .withBodyAsString("{\"name\":\"Bob\"}"); @@ -183,7 +151,6 @@ void shouldEmbedCustomJsonRequestBody() throws IOException { @Test void shouldEmbedCustomJsonWithParametersRequestBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; final HttpRequest request = MockHttpRequest.create() .withContentType("application/custom+json; version=2") .withBodyAsString("{\"name\":\"Bob\"}"); @@ -196,7 +163,6 @@ void shouldEmbedCustomJsonWithParametersRequestBody() throws IOException { @Test void shouldNotEmbedCustomTextXmlRequestBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; final HttpRequest request = MockHttpRequest.create() .withContentType("text/xml") .withBodyAsString("{\"name\":\"Bob\"}"); @@ -209,7 +175,6 @@ void shouldNotEmbedCustomTextXmlRequestBody() throws IOException { @Test void shouldNotEmbedInvalidContentTypeRequestBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; final HttpRequest request = MockHttpRequest.create() .withContentType("x;y/z") .withBodyAsString("{\"name\":\"Bob\"}"); @@ -222,7 +187,6 @@ void shouldNotEmbedInvalidContentTypeRequestBody() throws IOException { @Test void shouldNotEmbedCustomTextJsonRequestBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; final HttpRequest request = MockHttpRequest.create() .withContentType("text/custom+json") .withBodyAsString("{\"name\":\"Bob\"}"); @@ -235,7 +199,6 @@ void shouldNotEmbedCustomTextJsonRequestBody() throws IOException { @Test void shouldNotEmbedNonJsonRequestBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; final HttpRequest request = MockHttpRequest.create() .withContentType("application/not-json") .withBodyAsString("{\"name\":\"Bob\"}"); @@ -248,7 +211,6 @@ void shouldNotEmbedNonJsonRequestBody() throws IOException { @Test void shouldEmbedEmptyJsonRequestBodyAsEmptyString() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; final HttpRequest request = MockHttpRequest.create() .withContentType("application/json"); @@ -261,7 +223,6 @@ void shouldEmbedEmptyJsonRequestBodyAsEmptyString() throws IOException { @Test void shouldLogResponse() throws IOException { final String correlationId = "53de2640-677d-11e5-bc84-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create(); final HttpResponse response = create() .withProtocolVersion("HTTP/1.0") .withOrigin(LOCAL) @@ -286,7 +247,6 @@ void shouldLogResponse() throws IOException { @Test void shouldLogResponseWithoutHeaders() throws IOException { final String correlationId = "f53ceee2-682a-11e5-a63e-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create(); final HttpResponse response = create(); final String json = unit.format(new SimpleCorrelation(correlationId, ZERO), response); @@ -299,7 +259,6 @@ void shouldLogResponseWithoutHeaders() throws IOException { @Test void shouldLogResponseWithoutBody() throws IOException { final String correlationId = "f238536c-682a-11e5-9bdd-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create(); final HttpResponse response = create() .withBodyAsString(""); @@ -312,7 +271,6 @@ void shouldLogResponseWithoutBody() throws IOException { @Test void shouldEmbedJsonResponseBodyAsIs() throws IOException { final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create(); final HttpResponse response = create() .withContentType("application/json") .withBodyAsString("{\"name\":\"Bob\"}"); @@ -323,23 +281,9 @@ void shouldEmbedJsonResponseBodyAsIs() throws IOException { .assertThat("$.body.name", is("Bob")); } - @Test - void shouldCompactEmbeddedJsonResponseBody() throws IOException { - final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create(); - final HttpResponse response = create() - .withContentType("application/json") - .withBodyAsString("{\n \"name\": \"Bob\"\n}"); - - final String json = unit.format(new SimpleCorrelation(correlationId, ZERO), response); - - assertThat(json, containsString("{\"name\":\"Bob\"}")); - } - @Test void shouldEmbedCustomJsonResponseBodyAsIs() throws IOException { final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create(); final HttpResponse response = create() .withContentType("application/custom+json") .withBodyAsString("{\"name\":\"Bob\"}"); @@ -353,7 +297,6 @@ void shouldEmbedCustomJsonResponseBodyAsIs() throws IOException { @Test void shouldNotEmbedCustomTextJsonResponseBodyAsIs() throws IOException { final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create(); final HttpResponse response = create() .withContentType("text/custom+json") .withBodyAsString("{\"name\":\"Bob\"}"); @@ -367,7 +310,6 @@ void shouldNotEmbedCustomTextJsonResponseBodyAsIs() throws IOException { @Test void shouldEmbedJsonResponseBodyAsNullIfEmpty() throws IOException { final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create(); final HttpResponse response = create() .withContentType("application/json"); @@ -380,7 +322,6 @@ void shouldEmbedJsonResponseBodyAsNullIfEmpty() throws IOException { @Test void shouldNotEmbedInvalidJsonResponseBody() throws IOException { final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create(); final HttpResponse response = create() .withContentType("application/json") .withBodyAsString("{\"name\":\"Bob\"};"); @@ -394,7 +335,6 @@ void shouldNotEmbedInvalidJsonResponseBody() throws IOException { @Test void shouldNotEmbedInvalidButProbableJsonResponseBody() throws IOException { final String correlationId = "5478b8da-6d87-11e5-a80f-10ddb1ee7671"; - final HttpRequest request = MockHttpRequest.create(); final HttpResponse response = create() .withContentType("application/json") .withBodyAsString("{\"name\":\"Bob\"\n;};");