Skip to content

Commit

Permalink
ensure truncation body filter is always last one (#1608)
Browse files Browse the repository at this point in the history
* ensure truncation body filter is always last one

* reuse the existing truncation test

* use a new list to add the truncating body filter
  • Loading branch information
kasmarian authored Aug 11, 2023
1 parent 28e631c commit eecc6d5
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 87 deletions.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
import org.zalando.logbook.core.ResponseFilters;
import org.zalando.logbook.core.SplunkHttpLogFormatter;
import org.zalando.logbook.core.StatusAtLeastStrategy;
import org.zalando.logbook.core.TruncatingBodyFilter;
import org.zalando.logbook.core.WithoutBodyStrategy;
import org.zalando.logbook.httpclient.LogbookHttpRequestInterceptor;
import org.zalando.logbook.httpclient.LogbookHttpResponseInterceptor;
Expand All @@ -65,6 +64,8 @@
import org.zalando.logbook.servlet.SecureLogbookFilter;
import org.zalando.logbook.spring.LogbookClientHttpRequestInterceptor;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand All @@ -77,6 +78,7 @@
import static org.apiguardian.api.API.Status.STABLE;
import static org.zalando.logbook.autoconfigure.LogbookAutoConfiguration.JakartaServletFilterConfiguration.newFilter;
import static org.zalando.logbook.core.BodyFilters.defaultValue;
import static org.zalando.logbook.core.BodyFilters.truncate;
import static org.zalando.logbook.core.HeaderFilters.replaceHeaders;
import static org.zalando.logbook.core.QueryFilters.replaceQuery;

Expand Down Expand Up @@ -118,14 +120,28 @@ public Logbook logbook(
.headerFilters(headerFilters)
.queryFilters(queryFilters)
.pathFilters(pathFilters)
.bodyFilters(bodyFilters)
.bodyFilters(mergeWithTruncation(bodyFilters))
.requestFilters(requestFilters)
.responseFilters(responseFilters)
.strategy(strategy)
.sink(sink)
.build();
}

private Collection<BodyFilter> mergeWithTruncation(List<BodyFilter> bodyFilters) {
final LogbookProperties.Write write = properties.getWrite();
final int maxBodySize = write.getMaxBodySize();
if (maxBodySize < 0) {
return bodyFilters;
}

// To ensure that truncation will happen after all other body filters
final List<BodyFilter> filters = new ArrayList<>(bodyFilters);
final BodyFilter filter = truncate(maxBodySize);
filters.add(filter);
return filters;
}

private Predicate<HttpRequest> mergeWithExcludes(final Predicate<HttpRequest> predicate) {
return properties.getExclude().stream()
.map(Conditions::requestTo)
Expand Down Expand Up @@ -198,17 +214,6 @@ public BodyFilter bodyFilter() {
return defaultValue();
}

@API(status = INTERNAL)
@Bean
@ConditionalOnMissingBean(TruncatingBodyFilter.class)
@ConditionalOnProperty("logbook.write.max-body-size")
public BodyFilter truncatingBodyFilter() {
final LogbookProperties.Write write = properties.getWrite();
final int maxBodySize = write.getMaxBodySize();

return new TruncatingBodyFilter(maxBodySize);
}

@API(status = INTERNAL)
@Bean
@ConditionalOnMissingBean(JacksonJsonFieldBodyFilter.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.skyscreamer.jsonassert.JSONCompareMode;
import org.skyscreamer.jsonassert.JSONCompareResult;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.MediaType;
Expand All @@ -17,6 +18,7 @@

import java.io.IOException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -50,7 +52,9 @@ void shouldFilterRequestBody() throws IOException, JSONException {

String body = new JSONObject(message).getJSONObject("body").toString();

compareJSON(body, "{ \"first_name\": \"XXX\", \"details\": { \"last_name\": \"XXX\", \"field\":\"value\" } }", JSONCompareMode.LENIENT);
JSONCompareResult result = compareJSON(body, "{ \"first_name\": \"XXX\", \"details\": { \"last_name\": \"XXX\", \"field\":\"value\" } }", JSONCompareMode.LENIENT);

assertThat(result.passed()).isTrue();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.skyscreamer.jsonassert.JSONCompareMode;
import org.skyscreamer.jsonassert.JSONCompareResult;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.MediaType;
Expand All @@ -17,6 +18,7 @@

import java.io.IOException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -49,7 +51,8 @@ void shouldUseCustomerObfuscationReplacement() throws IOException, JSONException
final String message = captor.getValue();

String body = new JSONObject(message).getJSONObject("body").toString();
compareJSON(body, "{ \"name\": \"ZZZ\", \"details\": { \"field1\": \"value1\", \"field2\":\"value2\" } }", JSONCompareMode.LENIENT);
JSONCompareResult result = compareJSON(body, "{ \"name\": \"ZZZ\", \"details\": { \"field1\": \"value1\", \"field2\":\"value2\" } }", JSONCompareMode.LENIENT);

assertThat(result.passed()).isTrue();
}
}
Original file line number Diff line number Diff line change
@@ -1,32 +1,82 @@
package org.zalando.logbook.autoconfigure;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.zalando.logbook.BodyFilter;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.MediaType;
import org.zalando.logbook.HttpLogWriter;
import org.zalando.logbook.HttpRequest;
import org.zalando.logbook.Logbook;
import org.zalando.logbook.Precorrelation;
import org.zalando.logbook.test.MockHttpRequest;

import java.io.IOException;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.verify;

@LogbookTest(properties = "logbook.write.max-body-size = 20")
class WriteBodyMaxSizeTest {

@Autowired
@Qualifier("truncatingBodyFilter")
private BodyFilter bodyFilter;
private Logbook logbook;

@MockBean
private HttpLogWriter writer;

@BeforeEach
void setUp() {
doReturn(true).when(writer).isActive();
}

@Test
void shouldUseBodyMaxSizeFilter() {
final String body = "{\"foo\":\"someLongMessage\"}";
final String filtered = bodyFilter.filter("application/json", body);
void shouldUseBodyMaxSizeFilter() throws IOException {
final HttpRequest request = MockHttpRequest.create()
.withBodyAsString("{\"foo\":\"someLongMessage\"}")
.withContentType(MediaType.APPLICATION_JSON_VALUE);

logbook.process(request).write();

final ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
verify(writer).write(any(Precorrelation.class), captor.capture());
final String message = captor.getValue();

String body = extractBody(message);

assertThat(filtered).isEqualTo("{\"foo\":\"someLongMess...");
assertThat(body).isEqualTo("{\"foo\":\"someLongMess...}");
}

@Test
void shouldUseBodyMaxSizeOverDefaultFilter() {
final String body = "{\"open_id\":\"someLongSecret\",\"foo\":\"bar\"}";
final String filtered = bodyFilter.filter("application/json", body);
void shouldUseBodyMaxSizeOverDefaultFilter() throws IOException {
final HttpRequest request = MockHttpRequest.create()
.withBodyAsString("{\"open_id\":\"someLongSecret\",\"foo\":\"bar\"}")
.withContentType(MediaType.APPLICATION_JSON_VALUE);

assertThat(filtered).isEqualTo("{\"open_id\":\"someLong...");
logbook.process(request).write();

final ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
verify(writer).write(any(Precorrelation.class), captor.capture());
final String message = captor.getValue();

String body = extractBody(message);

assertThat(body).isEqualTo("{\"open_id\":\"XXX\",\"fo...}");
}

private static String extractBody(String message) {
Pattern pattern = Pattern.compile(".*body\":(.*)");
Matcher matcher = pattern.matcher(message);
String body = null;
if (matcher.find()) {
body = matcher.group(1);
}
return body;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
logbook:
obfuscate:
json-body-fields:
- first_name
- last_name
write:
max-body-size: 20

0 comments on commit eecc6d5

Please sign in to comment.