From 5d1ae6fef9fbe7e1ae02ddf66b8534c9f1ca4b01 Mon Sep 17 00:00:00 2001 From: Nikita Smolenskii Date: Fri, 12 Oct 2018 09:01:35 +0200 Subject: [PATCH 1/3] add form-url-encoded body filter --- README.md | 18 +++--- .../java/org/zalando/logbook/BodyFilters.java | 40 +++++++++++--- .../org/zalando/logbook/BodyFiltersTest.java | 55 ++++++++++++++++++- 3 files changed, 96 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 78515f1a4..ab980cd03 100644 --- a/README.md +++ b/README.md @@ -167,15 +167,15 @@ e.g. *password*. Logbook supports different types of filters: -| Type | Operates on | Applies to | Default | -|---------------------|--------------------------------|------------|-----------------| -| `RawRequestFilter` | `RawHttpRequest` | request | binary/streams | -| `RawResponseFilter` | `RawHttpResponse` | response | binary/streams | -| `QueryFilter` | Query string | request | `access_token` | -| `HeaderFilter` | Header (single key-value pair) | both | `Authorization` | -| `BodyFilter` | Content-Type and body | both | n/a | -| `RequestFilter` | `HttpRequest` | request | n/a | -| `ResponseFilter` | `HttpResponse` | response | n/a | +| Type | Operates on | Applies to | Default | +|---------------------|--------------------------------|------------|-------------------------------------------------------------------| +| `RawRequestFilter` | `RawHttpRequest` | request | binary/streams | +| `RawResponseFilter` | `RawHttpResponse` | response | binary/streams | +| `QueryFilter` | Query string | request | `access_token` | +| `HeaderFilter` | Header (single key-value pair) | both | `Authorization` | +| `BodyFilter` | Content-Type and body | both | json -> `access_token`, form-url -> `client_secret` and `password`| +| `RequestFilter` | `HttpRequest` | request | n/a | +| `ResponseFilter` | `HttpResponse` | response | n/a | `QueryFilter`, `HeaderFilter` and `BodyFilter` are relatively high-level and should cover all needs in ~90% of all cases. For more complicated setups one should fallback to the low-level variants, i.e. `RawRequestFilter` and 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 c04f82d34..36f9ee946 100644 --- a/logbook-core/src/main/java/org/zalando/logbook/BodyFilters.java +++ b/logbook-core/src/main/java/org/zalando/logbook/BodyFilters.java @@ -6,6 +6,7 @@ import java.util.HashSet; import java.util.Set; import java.util.function.Predicate; +import java.util.function.UnaryOperator; import java.util.regex.Pattern; import static java.util.stream.Collectors.joining; @@ -22,7 +23,7 @@ private BodyFilters() { @API(status = MAINTAINED) public static BodyFilter defaultValue() { - return accessToken(); + return BodyFilter.merge(accessToken(), oauthRequest()); } @API(status = MAINTAINED) @@ -34,6 +35,14 @@ public static BodyFilter accessToken() { return replaceJsonStringProperty(properties, "XXX"); } + @API(status = MAINTAINED) + public static BodyFilter oauthRequest() { + final Set properties = new HashSet<>(); + properties.add("client_secret"); + properties.add("password"); + return replaceFormUrlEncodedQuery(properties, "XXX"); + } + /** * Creates a {@link BodyFilter} that replaces the properties in the json response with the replacement passed as argument. * This {@link BodyFilter} works on all levels inside the json tree and it only works with string values

@@ -52,15 +61,32 @@ public static BodyFilter accessToken() { */ @API(status = MAINTAINED) public static BodyFilter replaceJsonStringProperty(final Set properties, final String replacement) { - final String regex = properties.stream() - .map(Pattern::quote) - .collect(joining("|")); - final Predicate json = MediaTypeQuery.compile("application/json", "application/*+json"); + + final String regex = properties.stream().map(Pattern::quote).collect(joining("|")); final Pattern pattern = Pattern.compile("(\"(?:" + regex + ")\"\\s*:\\s*)\".*?\""); + final UnaryOperator delegate = body -> pattern.matcher(body).replaceAll("$1\"" + replacement + "\""); + + return (contentType, body) -> json.test(contentType) ? delegate.apply(body) : body; + } + + /** + * Creates a {@link BodyFilter} that replaces the properties in the form url encoded body with given replacement. + * + * @param properties query names properties to replace + * @param replacement String to replace the properties values + * @return BodyFilter generated + */ + @API(status = MAINTAINED) + public static BodyFilter replaceFormUrlEncodedQuery(final Set properties, final String replacement) { + final Predicate formUrlEncoded = MediaTypeQuery.compile("application/x-www-form-urlencoded"); + + final QueryFilter delegate = properties.stream() + .map(name -> QueryFilters.replaceQuery(name, replacement)) + .reduce(QueryFilter::merge) + .orElseGet(QueryFilter::none); - return (contentType, body) -> json.test(contentType) ? - pattern.matcher(body).replaceAll("$1\"" + replacement + "\"") : body; + return (contentType, body) -> formUrlEncoded.test(contentType) ? delegate.filter(body) : body; } @API(status = EXPERIMENTAL) diff --git a/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java b/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java index 7be033c14..7715ee15d 100644 --- a/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java +++ b/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java @@ -4,12 +4,13 @@ import org.junit.jupiter.api.Test; import java.util.Collections; +import java.util.function.UnaryOperator; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; -public final class BodyFiltersTest { +final class BodyFiltersTest { @Test void shouldFilterAccessTokenByDefault() { @@ -29,6 +30,24 @@ void shouldNotFilterAccessTokenInTextPlainByDefault() { assertThat(actual, is("{\"access_token\":\"secret\"}")); } + @Test + void shouldFilterClientSecretByDefault() { + final BodyFilter unit = BodyFilters.defaultValue(); + + final String actual = unit.filter("application/x-www-form-urlencoded", "client_secret=secret"); + + assertThat(actual, is("client_secret=XXX")); + } + + @Test + void shouldNotFilterClientSecretInTextPlainByDefault() { + final BodyFilter unit = BodyFilters.defaultValue(); + + final String actual = unit.filter("text/plain", "client_secret=secret"); + + assertThat(actual, is("client_secret=secret")); + } + @Test void shouldFilterNotEmptyJSONProperty() { final BodyFilter unit = BodyFilters.replaceJsonStringProperty(Collections.singleton("foo"), "XXX"); @@ -87,4 +106,38 @@ void shouldReturnXmlCompactingBodyFilter() { assertThat(bodyFilter, instanceOf(XmlCompactingBodyFilter.class)); } + + @Test + void shouldFilterFormUrlEncodedBodyIfValidRequest() { + final BodyFilter unit = BodyFilters.replaceFormUrlEncodedQuery(Collections.singleton("q"), "XXX"); + + final UnaryOperator filter = value -> unit.filter("application/x-www-form-urlencoded", value); + + assertThat(filter.apply("q=boots&sort=price&direction=asc"), is("q=XXX&sort=price&direction=asc")); + assertThat(filter.apply("sort=price&direction=asc&q=boots"), is("sort=price&direction=asc&q=XXX")); + assertThat(filter.apply("sort=price&q=boots&direction=asc"), is("sort=price&q=XXX&direction=asc")); + assertThat(filter.apply("sort=price&direction=asc"), is("sort=price&direction=asc")); + assertThat(filter.apply("q=boots"), is("q=XXX")); + assertThat(filter.apply(""), is("")); + } + + @Test + void shouldNotFilterFormUrlEncodedBodyIfNotValidContentType() { + final BodyFilter unit = BodyFilters.replaceFormUrlEncodedQuery(Collections.singleton("q"), "XXX"); + + assertThat(unit.filter("application/json", "{\"q\":\"boots\"}"), is("{\"q\":\"boots\"}")); + assertThat(unit.filter("application/xml", "boots"), is("boots")); + assertThat(unit.filter("invalid", "{\"q\":\"boots\"}"), is("{\"q\":\"boots\"}")); + assertThat(unit.filter(null, "{\"q\":\"boots\"}"), is("{\"q\":\"boots\"}")); + } + + @Test + void shouldNotFilterFormUrlEncodedBodyIfNotValidContent() { + final BodyFilter unit = BodyFilters.replaceFormUrlEncodedQuery(Collections.singleton("q"), "XXX"); + + final UnaryOperator filter = value -> unit.filter("application/x-www-form-urlencoded", value); + + assertThat(filter.apply("{\"q\":\"boots\"}"), is("{\"q\":\"boots\"}")); + assertThat(filter.apply("boots"), is("boots")); + } } From 78cd18a06128c33bd53e8ac540751bda7c985c97 Mon Sep 17 00:00:00 2001 From: Nikita Smolenskii Date: Fri, 12 Oct 2018 11:09:53 +0200 Subject: [PATCH 2/3] fix code review issues --- README.md | 18 +++++++++--------- .../java/org/zalando/logbook/BodyFilters.java | 5 +++-- .../org/zalando/logbook/BodyFiltersTest.java | 6 +++--- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index ab980cd03..79f8e5662 100644 --- a/README.md +++ b/README.md @@ -167,15 +167,15 @@ e.g. *password*. Logbook supports different types of filters: -| Type | Operates on | Applies to | Default | -|---------------------|--------------------------------|------------|-------------------------------------------------------------------| -| `RawRequestFilter` | `RawHttpRequest` | request | binary/streams | -| `RawResponseFilter` | `RawHttpResponse` | response | binary/streams | -| `QueryFilter` | Query string | request | `access_token` | -| `HeaderFilter` | Header (single key-value pair) | both | `Authorization` | -| `BodyFilter` | Content-Type and body | both | json -> `access_token`, form-url -> `client_secret` and `password`| -| `RequestFilter` | `HttpRequest` | request | n/a | -| `ResponseFilter` | `HttpResponse` | response | n/a | +| Type | Operates on | Applies to | Default | +|---------------------|--------------------------------|------------|---------------------------------------------------------------------------------------| +| `RawRequestFilter` | `RawHttpRequest` | request | binary/streams | +| `RawResponseFilter` | `RawHttpResponse` | response | binary/streams | +| `QueryFilter` | Query string | request | `access_token` | +| `HeaderFilter` | Header (single key-value pair) | both | `Authorization` | +| `BodyFilter` | Content-Type and body | both | json -> `access_token` and `refresh_token`, form-url -> `client_secret` and `password`| +| `RequestFilter` | `HttpRequest` | request | n/a | +| `ResponseFilter` | `HttpResponse` | response | n/a | `QueryFilter`, `HeaderFilter` and `BodyFilter` are relatively high-level and should cover all needs in ~90% of all cases. For more complicated setups one should fallback to the low-level variants, i.e. `RawRequestFilter` and 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 36f9ee946..5d4f113ba 100644 --- a/logbook-core/src/main/java/org/zalando/logbook/BodyFilters.java +++ b/logbook-core/src/main/java/org/zalando/logbook/BodyFilters.java @@ -30,6 +30,7 @@ public static BodyFilter defaultValue() { public static BodyFilter accessToken() { final Set properties = new HashSet<>(); properties.add("access_token"); + properties.add("refresh_token"); properties.add("open_id"); properties.add("id_token"); return replaceJsonStringProperty(properties, "XXX"); @@ -40,7 +41,7 @@ public static BodyFilter oauthRequest() { final Set properties = new HashSet<>(); properties.add("client_secret"); properties.add("password"); - return replaceFormUrlEncodedQuery(properties, "XXX"); + return replaceFormUrlEncodedProperty(properties, "XXX"); } /** @@ -78,7 +79,7 @@ public static BodyFilter replaceJsonStringProperty(final Set properties, * @return BodyFilter generated */ @API(status = MAINTAINED) - public static BodyFilter replaceFormUrlEncodedQuery(final Set properties, final String replacement) { + public static BodyFilter replaceFormUrlEncodedProperty(final Set properties, final String replacement) { final Predicate formUrlEncoded = MediaTypeQuery.compile("application/x-www-form-urlencoded"); final QueryFilter delegate = properties.stream() diff --git a/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java b/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java index 7715ee15d..50b138b59 100644 --- a/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java +++ b/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java @@ -109,7 +109,7 @@ void shouldReturnXmlCompactingBodyFilter() { @Test void shouldFilterFormUrlEncodedBodyIfValidRequest() { - final BodyFilter unit = BodyFilters.replaceFormUrlEncodedQuery(Collections.singleton("q"), "XXX"); + final BodyFilter unit = BodyFilters.replaceFormUrlEncodedProperty(Collections.singleton("q"), "XXX"); final UnaryOperator filter = value -> unit.filter("application/x-www-form-urlencoded", value); @@ -123,7 +123,7 @@ void shouldFilterFormUrlEncodedBodyIfValidRequest() { @Test void shouldNotFilterFormUrlEncodedBodyIfNotValidContentType() { - final BodyFilter unit = BodyFilters.replaceFormUrlEncodedQuery(Collections.singleton("q"), "XXX"); + final BodyFilter unit = BodyFilters.replaceFormUrlEncodedProperty(Collections.singleton("q"), "XXX"); assertThat(unit.filter("application/json", "{\"q\":\"boots\"}"), is("{\"q\":\"boots\"}")); assertThat(unit.filter("application/xml", "boots"), is("boots")); @@ -133,7 +133,7 @@ void shouldNotFilterFormUrlEncodedBodyIfNotValidContentType() { @Test void shouldNotFilterFormUrlEncodedBodyIfNotValidContent() { - final BodyFilter unit = BodyFilters.replaceFormUrlEncodedQuery(Collections.singleton("q"), "XXX"); + final BodyFilter unit = BodyFilters.replaceFormUrlEncodedProperty(Collections.singleton("q"), "XXX"); final UnaryOperator filter = value -> unit.filter("application/x-www-form-urlencoded", value); From d38cbe150f0c6560d54c4320e7d3f4e97438f01a Mon Sep 17 00:00:00 2001 From: Nikita Smolenskii Date: Fri, 12 Oct 2018 11:12:29 +0200 Subject: [PATCH 3/3] mode BodyFilters::replaceFormUrlEncodedProperty experimental --- .../src/main/java/org/zalando/logbook/BodyFilters.java | 6 +++--- .../test/java/org/zalando/logbook/BodyFiltersTest.java | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) 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 5d4f113ba..09d3578c4 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 BodyFilter.merge(accessToken(), oauthRequest()); + return accessToken(); } @API(status = MAINTAINED) @@ -36,7 +36,7 @@ public static BodyFilter accessToken() { return replaceJsonStringProperty(properties, "XXX"); } - @API(status = MAINTAINED) + @API(status = EXPERIMENTAL) public static BodyFilter oauthRequest() { final Set properties = new HashSet<>(); properties.add("client_secret"); @@ -78,7 +78,7 @@ public static BodyFilter replaceJsonStringProperty(final Set properties, * @param replacement String to replace the properties values * @return BodyFilter generated */ - @API(status = MAINTAINED) + @API(status = EXPERIMENTAL) public static BodyFilter replaceFormUrlEncodedProperty(final Set properties, final String replacement) { final Predicate formUrlEncoded = MediaTypeQuery.compile("application/x-www-form-urlencoded"); diff --git a/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java b/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java index 50b138b59..a0a63f536 100644 --- a/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java +++ b/logbook-core/src/test/java/org/zalando/logbook/BodyFiltersTest.java @@ -31,8 +31,8 @@ void shouldNotFilterAccessTokenInTextPlainByDefault() { } @Test - void shouldFilterClientSecretByDefault() { - final BodyFilter unit = BodyFilters.defaultValue(); + void shouldFilterClientSecretByOauthRequestFilter() { + final BodyFilter unit = BodyFilters.oauthRequest(); final String actual = unit.filter("application/x-www-form-urlencoded", "client_secret=secret"); @@ -40,8 +40,8 @@ void shouldFilterClientSecretByDefault() { } @Test - void shouldNotFilterClientSecretInTextPlainByDefault() { - final BodyFilter unit = BodyFilters.defaultValue(); + void shouldNotFilterClientSecretInTextPlainByOauthRequestFilter() { + final BodyFilter unit = BodyFilters.oauthRequest(); final String actual = unit.filter("text/plain", "client_secret=secret");