From dde43c77da09194d6ccf1e7ba77f5fb15b27b967 Mon Sep 17 00:00:00 2001 From: Kim Seon Woo Date: Sat, 16 Sep 2023 12:24:53 +0900 Subject: [PATCH 1/5] Add maskHeaders, maskRequestHeaders, maskResponseHeaders which can be used for formatting logs --- .../logging/AbstractLogFormatterBuilder.java | 97 +++++++++++++++++++ .../common/logging/JsonLogFormatter.java | 29 +++++- .../logging/JsonLogFormatterBuilder.java | 19 ++++ .../common/logging/TextLogFormatter.java | 28 +++++- .../logging/TextLogFormatterBuilder.java | 18 ++++ .../common/logging/JsonLogFormatterTest.java | 52 ++++++++++ .../common/logging/TextLogFormatterTest.java | 53 ++++++++++ 7 files changed, 292 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java index fe895bd3a6b9..e5268582e609 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java @@ -18,8 +18,13 @@ import static java.util.Objects.requireNonNull; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; +import java.util.stream.Collectors; import com.linecorp.armeria.common.HttpHeaders; import com.linecorp.armeria.common.RequestContext; @@ -48,6 +53,15 @@ abstract class AbstractLogFormatterBuilder { @Nullable private BiFunction responseContentSanitizer; + @Nullable + private List maskHeaders; + + @Nullable + private List maskRequestHeaders; + + @Nullable + private List maskResponseHeaders; + /** * Sets the {@link BiFunction} to use to sanitize request headers before logging. It is common to have the * {@link BiFunction} that removes sensitive headers, like {@code Cookie}, before logging. If unset, will @@ -186,6 +200,54 @@ public AbstractLogFormatterBuilder responseContentSanitizer( return responseContentSanitizer; } + /** + * Sets the {@link List} to use to mask request and response headers before logging. + */ + public AbstractLogFormatterBuilder maskHeaders(String... headers) { + this.maskHeaders = Arrays.stream(headers).map(String::toLowerCase).collect(Collectors.toList()); + return this; + } + + /** + * Returns the {@link List} to use to mask request and response headers before logging. + */ + @Nullable + final List maskHeaders() { + return maskHeaders; + } + + /** + * Sets the {@link List} to use to mask request headers before logging. + */ + public AbstractLogFormatterBuilder maskRequestHeaders(String... headers) { + this.maskRequestHeaders = Arrays.stream(headers).map(String::toLowerCase).collect(Collectors.toList()); + return this; + } + + /** + * Returns the {@link List} to use to mask request headers before logging. + */ + @Nullable + final List maskRequestHeaders() { + return maskRequestHeaders; + } + + /** + * Sets the {@link List} to use to mask response headers before logging. + */ + public AbstractLogFormatterBuilder maskResponseHeaders(String... headers) { + this.maskResponseHeaders = Arrays.stream(headers).map(String::toLowerCase).collect(Collectors.toList()); + return this; + } + + /** + * Returns the {@link List} to use to mask response headers before logging. + */ + @Nullable + final List maskResponseHeaders() { + return maskResponseHeaders; + } + /** * Sets the {@link BiFunction} to use to sanitize request and response content before logging. It is common * to have the {@link BiFunction} that removes sensitive content, such as an GPS location query and @@ -206,4 +268,39 @@ public AbstractLogFormatterBuilder contentSanitizer( responseContentSanitizer(contentSanitizer); return this; } + + /** + * Checks whether the sanitizer and mask headers are not used together. + */ + protected void validateSanitizerAndMaskHeaders() { + if (requestHeadersSanitizer() != null) { + if (maskHeaders() != null || maskRequestHeaders() != null) { + throw new IllegalArgumentException("requestHeaderSanitizer should not be used when " + + "maskHeaders or maskRequestHeaders are used."); + } + } + + if (responseHeadersSanitizer() != null) { + if (maskHeaders() != null || maskResponseHeaders() != null) { + throw new IllegalArgumentException("responseHeaderSanitizer should not be used when " + + "maskHeaders or maskResponseHeaders are used."); + } + } + } + + /** + * Concatenates two {@link List} of headers and returns an {@link Set} of headers. + */ + protected Set concatHeaders(@Nullable List headers1, + @Nullable List headers2) { + final HashSet concatenatedHeaders = new HashSet<>(); + if (headers1 != null) { + concatenatedHeaders.addAll(headers1); + } + if (headers2 != null) { + concatenatedHeaders.addAll(headers2); + } + + return concatenatedHeaders; + } } diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java index 7286334c24a4..e9d3e6342951 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java @@ -19,6 +19,7 @@ import static java.util.Objects.requireNonNull; import java.util.List; +import java.util.Set; import java.util.function.BiFunction; import org.slf4j.Logger; @@ -30,6 +31,7 @@ import com.google.common.base.MoreObjects; import com.linecorp.armeria.common.HttpHeaders; +import com.linecorp.armeria.common.HttpHeadersBuilder; import com.linecorp.armeria.common.RequestContext; import com.linecorp.armeria.common.SerializationFormat; import com.linecorp.armeria.common.annotation.Nullable; @@ -44,6 +46,8 @@ final class JsonLogFormatter implements LogFormatter { private static final Logger logger = LoggerFactory.getLogger(JsonLogFormatter.class); + private static final String MASK = "****"; + static final LogFormatter DEFAULT_INSTANCE = new JsonLogFormatterBuilder().build(); private final BiFunction @@ -64,6 +68,10 @@ final class JsonLogFormatter implements LogFormatter { private final BiFunction responseContentSanitizer; + private final Set maskRequestHeaders; + + private final Set maskResponseHeaders; + private final ObjectMapper objectMapper; JsonLogFormatter( @@ -77,6 +85,8 @@ final class JsonLogFormatter implements LogFormatter { ? extends @Nullable JsonNode> responseTrailersSanitizer, BiFunction requestContentSanitizer, BiFunction responseContentSanitizer, + Set maskRequestHeaders, + Set maskResponseHeaders, ObjectMapper objectMapper) { this.requestHeadersSanitizer = requestHeadersSanitizer; this.responseHeadersSanitizer = responseHeadersSanitizer; @@ -84,6 +94,8 @@ final class JsonLogFormatter implements LogFormatter { this.responseTrailersSanitizer = responseTrailersSanitizer; this.requestContentSanitizer = requestContentSanitizer; this.responseContentSanitizer = responseContentSanitizer; + this.maskRequestHeaders = maskRequestHeaders; + this.maskResponseHeaders = maskResponseHeaders; this.objectMapper = objectMapper; } @@ -108,7 +120,9 @@ public String formatRequest(RequestOnlyLog log) { final JsonNode sanitizedHeaders; if (RequestLogProperty.REQUEST_HEADERS.isAvailable(flags)) { - sanitizedHeaders = requestHeadersSanitizer.apply(ctx, log.requestHeaders()); + sanitizedHeaders = + maskRequestHeaders.isEmpty() ? requestHeadersSanitizer.apply(ctx, log.requestHeaders()) + : applyMasking(log.requestHeaders(), maskRequestHeaders); } else { sanitizedHeaders = null; } @@ -207,7 +221,11 @@ public String formatResponse(RequestLog log) { final JsonNode sanitizedHeaders; if (RequestLogProperty.RESPONSE_HEADERS.isAvailable(flags)) { - sanitizedHeaders = responseHeadersSanitizer.apply(ctx, log.responseHeaders()); + sanitizedHeaders = + maskResponseHeaders.isEmpty() ? responseHeadersSanitizer.apply(ctx, + log.responseHeaders()) + : applyMasking(log.responseHeaders(), + maskResponseHeaders); } else { sanitizedHeaders = null; } @@ -278,6 +296,13 @@ public String formatResponse(RequestLog log) { } } + private JsonNode applyMasking(HttpHeaders headers, Set maskHeaders) { + final HttpHeadersBuilder builder = HttpHeaders.builder(); + headers.forEach( + (name, value) -> builder.add(name, maskHeaders.contains(name.toString()) ? MASK : value)); + return objectMapper.valueToTree(builder.build()); + } + @Override public String toString() { return MoreObjects.toStringHelper(this) diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java index 5c1a177bea91..8cf40f34c7c8 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java @@ -102,6 +102,21 @@ public JsonLogFormatterBuilder contentSanitizer( return (JsonLogFormatterBuilder) super.contentSanitizer(contentSanitizer); } + @Override + public JsonLogFormatterBuilder maskHeaders(String... headers) { + return (JsonLogFormatterBuilder) super.maskHeaders(headers); + } + + @Override + public JsonLogFormatterBuilder maskRequestHeaders(String... headers) { + return (JsonLogFormatterBuilder) super.maskRequestHeaders(headers); + } + + @Override + public JsonLogFormatterBuilder maskResponseHeaders(String... headers) { + return (JsonLogFormatterBuilder) super.maskResponseHeaders(headers); + } + /** * Returns a newly-created JSON {@link LogFormatter} based on the properties of this builder. */ @@ -112,6 +127,8 @@ public LogFormatter build() { defaultSanitizer(objectMapper); final BiFunction defaultContentSanitizer = defaultSanitizer(objectMapper); + + validateSanitizerAndMaskHeaders(); return new JsonLogFormatter( firstNonNull(requestHeadersSanitizer(), defaultHeadersSanitizer), firstNonNull(responseHeadersSanitizer(), defaultHeadersSanitizer), @@ -119,6 +136,8 @@ public LogFormatter build() { firstNonNull(responseTrailersSanitizer(), defaultHeadersSanitizer), firstNonNull(requestContentSanitizer(), defaultContentSanitizer), firstNonNull(responseContentSanitizer(), defaultContentSanitizer), + concatHeaders(maskRequestHeaders(), maskHeaders()), + concatHeaders(maskResponseHeaders(), maskHeaders()), objectMapper); } diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java index be19207c8a3f..db819ee5f15c 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java @@ -19,11 +19,13 @@ import static java.util.Objects.requireNonNull; import java.util.List; +import java.util.Set; import java.util.function.BiFunction; import com.google.common.base.MoreObjects; import com.linecorp.armeria.common.HttpHeaders; +import com.linecorp.armeria.common.HttpHeadersBuilder; import com.linecorp.armeria.common.RequestContext; import com.linecorp.armeria.common.SerializationFormat; import com.linecorp.armeria.common.annotation.Nullable; @@ -39,6 +41,8 @@ final class TextLogFormatter implements LogFormatter { static final LogFormatter DEFAULT_INSTANCE = new TextLogFormatterBuilder().build(); + private static final String MASK = "****"; + private final BiFunction requestHeadersSanitizer; @@ -57,6 +61,10 @@ final class TextLogFormatter implements LogFormatter { private final BiFunction responseContentSanitizer; + private final Set maskRequestHeaders; + + private final Set maskResponseHeaders; + private final boolean includeContext; TextLogFormatter( @@ -72,6 +80,8 @@ final class TextLogFormatter implements LogFormatter { ? extends @Nullable String> requestContentSanitizer, BiFunction responseContentSanitizer, + Set maskRequestHeaders, + Set maskResponseHeaders, boolean includeContext) { this.requestHeadersSanitizer = requestHeadersSanitizer; this.responseHeadersSanitizer = responseHeadersSanitizer; @@ -79,6 +89,8 @@ final class TextLogFormatter implements LogFormatter { this.responseTrailersSanitizer = responseTrailersSanitizer; this.requestContentSanitizer = requestContentSanitizer; this.responseContentSanitizer = responseContentSanitizer; + this.maskRequestHeaders = maskRequestHeaders; + this.maskResponseHeaders = maskResponseHeaders; this.includeContext = includeContext; } @@ -106,7 +118,9 @@ public String formatRequest(RequestOnlyLog log) { final String sanitizedHeaders; if (RequestLogProperty.REQUEST_HEADERS.isAvailable(flags)) { - sanitizedHeaders = requestHeadersSanitizer.apply(ctx, log.requestHeaders()); + sanitizedHeaders = + maskRequestHeaders.isEmpty() ? requestHeadersSanitizer.apply(ctx, log.requestHeaders()) + : applyMasking(log.requestHeaders(), maskRequestHeaders); } else { sanitizedHeaders = null; } @@ -221,7 +235,9 @@ public String formatResponse(RequestLog log) { final String sanitizedHeaders; if (RequestLogProperty.RESPONSE_HEADERS.isAvailable(flags)) { - sanitizedHeaders = responseHeadersSanitizer.apply(ctx, log.responseHeaders()); + sanitizedHeaders = + maskResponseHeaders.isEmpty() ? responseHeadersSanitizer.apply(ctx, log.responseHeaders()) + : applyMasking(log.responseHeaders(), maskResponseHeaders); } else { sanitizedHeaders = null; } @@ -306,6 +322,14 @@ public String formatResponse(RequestLog log) { } } + private String applyMasking(HttpHeaders headers, Set maskHeaders) { + final HttpHeadersBuilder builder = HttpHeaders.builder(); + headers.forEach( + (name, value) -> builder.add(name, maskHeaders.contains(name.toString()) ? MASK : value) + ); + return builder.build().toString(); + } + @Override public String toString() { return MoreObjects.toStringHelper(this) diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java index 1ffef0547720..018d37ca3c78 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java @@ -88,6 +88,21 @@ public TextLogFormatterBuilder contentSanitizer( return (TextLogFormatterBuilder) super.contentSanitizer(contentSanitizer); } + @Override + public TextLogFormatterBuilder maskHeaders(String... headers) { + return (TextLogFormatterBuilder) super.maskHeaders(headers); + } + + @Override + public TextLogFormatterBuilder maskRequestHeaders(String... headers) { + return (TextLogFormatterBuilder) super.maskRequestHeaders(headers); + } + + @Override + public TextLogFormatterBuilder maskResponseHeaders(String... headers) { + return (TextLogFormatterBuilder) super.maskResponseHeaders(headers); + } + /** * Sets whether to include stringified {@link RequestContext} in the result of * {@link LogFormatter#formatRequest(RequestOnlyLog)} and {@link LogFormatter#formatResponse(RequestLog)}. @@ -102,6 +117,7 @@ public TextLogFormatterBuilder includeContext(boolean includeContext) { * Returns a newly-created text {@link LogFormatter} based on the properties of this builder. */ public LogFormatter build() { + validateSanitizerAndMaskHeaders(); return new TextLogFormatter( firstNonNull(requestHeadersSanitizer(), defaultSanitizer()), firstNonNull(responseHeadersSanitizer(), defaultSanitizer()), @@ -109,6 +125,8 @@ public LogFormatter build() { firstNonNull(responseTrailersSanitizer(), defaultSanitizer()), firstNonNull(requestContentSanitizer(), defaultSanitizer()), firstNonNull(responseContentSanitizer(), defaultSanitizer()), + concatHeaders(maskRequestHeaders(), maskHeaders()), + concatHeaders(maskResponseHeaders(), maskHeaders()), includeContext); } diff --git a/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java b/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java index 4c1303e04925..5f6e9a0b9ccd 100644 --- a/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java +++ b/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java @@ -18,12 +18,18 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpRequest; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.ResponseHeaders; import com.linecorp.armeria.server.ServiceRequestContext; class JsonLogFormatterTest { @@ -53,4 +59,50 @@ void formatResponse() { .matches("^\\{\"type\":\"response\",\"startTime\":\".+\",\"length\":\".+\"," + "\"duration\":\".+\",\"totalDuration\":\".+\",\"headers\":\\{\".+\"}}$"); } + + @Test + void maskRequestHeaders() { + final LogFormatter logFormatter = LogFormatter.builderForJson() + .maskHeaders("Cookie") + .maskRequestHeaders("Authorization") + .build(); + final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", + "Cookie", "Armeria=awesome", + "Authorization", "authorization")); + final ServiceRequestContext ctx = ServiceRequestContext.of(req); + final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); + log.endRequest(); + final String requestLog = logFormatter.formatRequest(log); + + final Matcher matcher1 = Pattern.compile("\"cookie\":\"(.*?)\"").matcher(requestLog); + assertThat(matcher1.find()).isTrue(); + assertThat(matcher1.group(1)).isEqualTo("****"); + + final Matcher matcher2 = Pattern.compile("\"authorization\":\"(.*?)\"").matcher(requestLog); + assertThat(matcher2.find()).isTrue(); + assertThat(matcher2.group(1)).isEqualTo("****"); + } + + @Test + void maskResponseHeaders() { + final LogFormatter logFormatter = LogFormatter.builderForJson() + .maskHeaders("Content-Type") + .maskResponseHeaders("Set-Cookie") + .build(); + final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello")); + final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); + log.responseHeaders(ResponseHeaders.of(HttpStatus.OK, + "Content-Type", "text/html", + "Set-Cookie", "Armeria=awesome")); + log.endResponse(); + final String responseLog = logFormatter.formatResponse(log); + System.out.println(responseLog); + final Matcher matcher1 = Pattern.compile("\"content-type\":\"(.*?)\"").matcher(responseLog); + assertThat(matcher1.find()).isTrue(); + assertThat(matcher1.group(1)).isEqualTo("****"); + + final Matcher matcher2 = Pattern.compile("\"set-cookie\":\"(.*?)\"").matcher(responseLog); + assertThat(matcher2.find()).isTrue(); + assertThat(matcher2.group(1)).isEqualTo("****"); + } } diff --git a/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java b/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java index 0dbc36122b49..5193fadcd64c 100644 --- a/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java +++ b/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java @@ -18,11 +18,18 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpRequest; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.armeria.common.ResponseHeaders; import com.linecorp.armeria.server.ServiceRequestContext; class TextLogFormatterTest { @@ -60,4 +67,50 @@ void formatResponse(boolean containContext) { assertThat(responseLog).matches(regex); } } + + @Test + void maskRequestHeaders() { + final LogFormatter logFormatter = LogFormatter.builderForText() + .maskHeaders("Cookie") + .maskRequestHeaders("Authorization") + .build(); + final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", + "Cookie", "Armeria=awesome", + "Authorization", "authorization")); + final ServiceRequestContext ctx = ServiceRequestContext.of(req); + final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); + log.endRequest(); + final String requestLog = logFormatter.formatRequest(log); + System.out.println(requestLog); + final Matcher matcher1 = Pattern.compile("cookie=(.*?)[,\\]]").matcher(requestLog); + assertThat(matcher1.find()).isTrue(); + assertThat(matcher1.group(1)).isEqualTo("****"); + + final Matcher matcher2 = Pattern.compile("authorization=(.*?)[,\\]]").matcher(requestLog); + assertThat(matcher2.find()).isTrue(); + assertThat(matcher2.group(1)).isEqualTo("****"); + } + + @Test + void maskResponseHeaders() { + final LogFormatter logFormatter = LogFormatter.builderForText() + .maskHeaders("Content-Type") + .maskResponseHeaders("Set-Cookie") + .build(); + final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello")); + final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); + log.responseHeaders(ResponseHeaders.of(HttpStatus.OK, + "Content-Type", "text/html", + "Set-Cookie", "Armeria=awesome")); + log.endResponse(); + final String responseLog = logFormatter.formatResponse(log); + System.out.println(responseLog); + final Matcher matcher1 = Pattern.compile("content-type=(.*?)[,\\]]").matcher(responseLog); + assertThat(matcher1.find()).isTrue(); + assertThat(matcher1.group(1)).isEqualTo("****"); + + final Matcher matcher2 = Pattern.compile("set-cookie=(.*?)[,\\]]").matcher(responseLog); + assertThat(matcher2.find()).isTrue(); + assertThat(matcher2.group(1)).isEqualTo("****"); + } } From 30627b6dfbbe70b9808a498a14732c6adfade7c5 Mon Sep 17 00:00:00 2001 From: Kim Seon Woo Date: Thu, 5 Oct 2023 01:38:31 +0900 Subject: [PATCH 2/5] Add HeadersSanitizer, HeadersSanitizerBuilder and it's implementations based on Text and Json --- .../AbstractHeadersSanitizerBuilder.java | 72 ++++++++++++++++ .../armeria/common/HeadersSanitizer.java | 59 +++++++++++++ .../armeria/common/JsonHeadersSanitizer.java | 56 +++++++++++++ .../common/JsonHeadersSanitizerBuilder.java | 84 +++++++++++++++++++ .../armeria/common/TextHeadersSanitizer.java | 54 ++++++++++++ .../common/TextHeadersSanitizerBuilder.java | 62 ++++++++++++++ .../logging/AbstractLogFormatterBuilder.java | 79 +++-------------- 7 files changed, 397 insertions(+), 69 deletions(-) create mode 100644 core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java create mode 100644 core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java create mode 100644 core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java create mode 100644 core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java create mode 100644 core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java create mode 100644 core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java diff --git a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java new file mode 100644 index 000000000000..a798c67e7b22 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java @@ -0,0 +1,72 @@ +/* + * Copyright 2023 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package com.linecorp.armeria.common; + +import static java.util.Objects.requireNonNull; + +import java.util.Arrays; +import java.util.Set; +import java.util.function.BiFunction; +import java.util.stream.Collectors; + +import com.linecorp.armeria.common.annotation.Nullable; + +/** + * A skeletal builder implementation for {@link HeadersSanitizer}. + */ +abstract class AbstractHeadersSanitizerBuilder { + + @Nullable + private BiFunction headersSanitizer; + + @Nullable + private Set headersMask; + + /** + * Sets the {@link BiFunction} to use to sanitize headers before logging. It is common to have the + * {@link BiFunction} that removes sensitive headers, like Cookie, before logging. + */ + public AbstractHeadersSanitizerBuilder headersSanitizer( + BiFunction headersSanitizer) { + this.headersSanitizer = requireNonNull(headersSanitizer, "headersSanitizer"); + return this; + } + + /** + * Returns the {@link BiFunction} to use to sanitize headers before logging. + */ + @Nullable + final BiFunction headersSanitizer() { + return headersSanitizer; + } + + /** + * Sets the {@link Set} to use to mask headers before logging. + */ + public AbstractHeadersSanitizerBuilder headersMask(String... headers) { + headersMask = Arrays.stream(headers).map(String::toLowerCase).collect(Collectors.toSet()); + return this; + } + + /** + * Returns the {@link Set} to use to mask headers before logging. + */ + @Nullable + final Set headersMask() { + return headersMask; + } +} diff --git a/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java b/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java new file mode 100644 index 000000000000..1eff2b8221b0 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java @@ -0,0 +1,59 @@ +/* + * Copyright 2023 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package com.linecorp.armeria.common; + +import com.fasterxml.jackson.databind.JsonNode; + +/** + * A sanitizer that sanitizes {@link HttpHeaders}. + */ +@FunctionalInterface +public interface HeadersSanitizer { + + /** + * Returns the default text {@link HeadersSanitizer}. + */ + static HeadersSanitizer ofText() { + return TextHeadersSanitizer.DEFAULT_INSTANCE; + } + + /** + * Returns a newly created {@link TextHeadersSanitizerBuilder}. + */ + static TextHeadersSanitizerBuilder builderForText() { + return new TextHeadersSanitizerBuilder(); + } + + /** + * Returns the default json {@link HeadersSanitizer}. + */ + static HeadersSanitizer ofJson() { + return JsonHeadersSanitizer.DEFAULT_INSTANCE; + } + + /** + * Returns a newly created {@link JsonHeadersSanitizerBuilder}. + */ + static JsonHeadersSanitizerBuilder builderForJson() { + return new JsonHeadersSanitizerBuilder(); + } + + /** + * Returns the sanitized {@link HttpHeaders}. + */ + T sanitizeHeaders(RequestContext ctx, HttpHeaders headers); +} diff --git a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java new file mode 100644 index 000000000000..f63dc63fec33 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java @@ -0,0 +1,56 @@ +/* + * Copyright 2023 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package com.linecorp.armeria.common; + +import java.util.Set; +import java.util.function.BiFunction; + +import com.fasterxml.jackson.databind.JsonNode; + +import com.linecorp.armeria.common.annotation.Nullable; + +/** + * A sanitizer that sanitizes {@link HttpHeaders} and returns {@link JsonNode}. + */ +public class JsonHeadersSanitizer implements HeadersSanitizer { + + static final HeadersSanitizer DEFAULT_INSTANCE = new JsonHeadersSanitizerBuilder().build(); + + private static final String MASK = "****"; + + private final BiFunction headersSanitizer; + + private final Set headersMask; + + JsonHeadersSanitizer( + BiFunction + headersSanitizer, + Set headersMask) { + this.headersSanitizer = headersSanitizer; + this.headersMask = headersMask; + } + + @Override + public JsonNode sanitizeHeaders(RequestContext ctx, HttpHeaders headers) { + final HttpHeadersBuilder builder = headers.toBuilder(); + headers.forEach( + (name, value) -> builder.set(name, headersMask.contains(name.toString()) ? MASK : value)); + + return headersSanitizer.apply(ctx, builder.build()); + } +} diff --git a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java new file mode 100644 index 000000000000..1a859666bea7 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java @@ -0,0 +1,84 @@ +/* + * Copyright 2023 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package com.linecorp.armeria.common; + +import static com.google.common.base.MoreObjects.firstNonNull; +import static java.util.Objects.requireNonNull; + +import java.util.HashSet; +import java.util.Set; +import java.util.function.BiFunction; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + +import com.linecorp.armeria.common.annotation.Nullable; +import com.linecorp.armeria.internal.common.JacksonUtil; + +/** + * A builder implementation for {@link JsonHeadersSanitizer}. + */ +public class JsonHeadersSanitizerBuilder extends AbstractHeadersSanitizerBuilder { + @Nullable + private ObjectMapper objectMapper; + + /** + * Sets the {@link ObjectMapper} that will be used to convert an object into a JSON format message. + */ + public JsonHeadersSanitizerBuilder objectMapper(ObjectMapper objectMapper) { + this.objectMapper = requireNonNull(objectMapper, "objectMapper"); + return this; + } + + /** + * Sets the {@link BiFunction} to use to sanitize headers before logging. + */ + @Override + public JsonHeadersSanitizerBuilder headersSanitizer( + BiFunction + headersSanitizer) { + return (JsonHeadersSanitizerBuilder) super.headersSanitizer(headersSanitizer); + } + + /** + * Sets the {@link Set} to use to mask headers before logging. + */ + @Override + public JsonHeadersSanitizerBuilder headersMask(String... headers) { + return (JsonHeadersSanitizerBuilder) super.headersMask(headers); + } + + /** + * Returns a newly-created JSON {@link HeadersSanitizer} based on the properties of this builder. + */ + public JsonHeadersSanitizer build() { + final ObjectMapper objectMapper = this.objectMapper != null ? + this.objectMapper : JacksonUtil.newDefaultObjectMapper(); + + return new JsonHeadersSanitizer(firstNonNull(headersSanitizer(), defaultSanitizer(objectMapper)), + firstNonNull(headersMask(), defaultMask())); + } + + private static BiFunction + defaultSanitizer(ObjectMapper objectMapper) { + return (requestContext, obj) -> objectMapper.valueToTree(obj); + } + + private static Set defaultMask() { + return new HashSet<>(); + } +} diff --git a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java new file mode 100644 index 000000000000..140e85c440f8 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java @@ -0,0 +1,54 @@ +/* + * Copyright 2023 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package com.linecorp.armeria.common; + +import java.util.Set; +import java.util.function.BiFunction; + +import com.linecorp.armeria.common.annotation.Nullable; + +/** + * A sanitizer that sanitizes {@link HttpHeaders} and returns {@link String}. + */ +public class TextHeadersSanitizer implements HeadersSanitizer { + + static final HeadersSanitizer DEFAULT_INSTANCE = new TextHeadersSanitizerBuilder().build(); + + private static final String MASK = "****"; + + private final BiFunction headersSanitizer; + + private final Set headersMask; + + TextHeadersSanitizer( + BiFunction + headersSanitizer, + Set headersMask) { + this.headersSanitizer = headersSanitizer; + this.headersMask = headersMask; + } + + @Override + public String sanitizeHeaders(RequestContext ctx, HttpHeaders headers) { + final HttpHeadersBuilder builder = headers.toBuilder(); + headers.forEach( + (name, value) -> builder.set(name, headersMask.contains(name.toString()) ? MASK : value) + ); + return headersSanitizer.apply(ctx, builder.build()); + } +} diff --git a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java new file mode 100644 index 000000000000..24f998c28383 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java @@ -0,0 +1,62 @@ +/* + * Copyright 2023 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package com.linecorp.armeria.common; + +import static com.google.common.base.MoreObjects.firstNonNull; + +import java.util.HashSet; +import java.util.Set; +import java.util.function.BiFunction; + +/** + * A builder implementation for {@link TextHeadersSanitizer}. + */ +public class TextHeadersSanitizerBuilder extends AbstractHeadersSanitizerBuilder { + + /** + * Sets the {@link BiFunction} to use to sanitize headers before logging. + */ + @Override + public TextHeadersSanitizerBuilder headersSanitizer( + BiFunction headersSanitizer) { + return (TextHeadersSanitizerBuilder) super.headersSanitizer(headersSanitizer); + } + + /** + * Sets the {@link Set} to use to mask headers before logging. + */ + @Override + public TextHeadersSanitizerBuilder headersMask(String... headers) { + return (TextHeadersSanitizerBuilder) super.headersMask(headers); + } + + /** + * Returns a newly-created text {@link HeadersSanitizer} based on the properties of this builder. + */ + public TextHeadersSanitizer build() { + return new TextHeadersSanitizer(firstNonNull(headersSanitizer(), defaultSanitizer()), + firstNonNull(headersMask(), defaultMask())); + } + + private static BiFunction defaultSanitizer() { + return (requestContext, object) -> object.toString(); + } + + private static Set defaultMask() { + return new HashSet<>(); + } +} diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java index e5268582e609..03d7c8bb7a69 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java @@ -18,13 +18,8 @@ import static java.util.Objects.requireNonNull; -import java.util.Arrays; -import java.util.HashSet; -import java.util.List; -import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; -import java.util.stream.Collectors; import com.linecorp.armeria.common.HttpHeaders; import com.linecorp.armeria.common.RequestContext; @@ -54,13 +49,10 @@ abstract class AbstractLogFormatterBuilder { private BiFunction responseContentSanitizer; @Nullable - private List maskHeaders; + private String[] maskRequestHeaders; @Nullable - private List maskRequestHeaders; - - @Nullable - private List maskResponseHeaders; + private String[] maskResponseHeaders; /** * Sets the {@link BiFunction} to use to sanitize request headers before logging. It is common to have the @@ -201,50 +193,34 @@ public AbstractLogFormatterBuilder responseContentSanitizer( } /** - * Sets the {@link List} to use to mask request and response headers before logging. - */ - public AbstractLogFormatterBuilder maskHeaders(String... headers) { - this.maskHeaders = Arrays.stream(headers).map(String::toLowerCase).collect(Collectors.toList()); - return this; - } - - /** - * Returns the {@link List} to use to mask request and response headers before logging. - */ - @Nullable - final List maskHeaders() { - return maskHeaders; - } - - /** - * Sets the {@link List} to use to mask request headers before logging. + * Sets an array of {@link String} to use to mask request headers before logging. */ public AbstractLogFormatterBuilder maskRequestHeaders(String... headers) { - this.maskRequestHeaders = Arrays.stream(headers).map(String::toLowerCase).collect(Collectors.toList()); + this.maskRequestHeaders = headers; return this; } /** - * Returns the {@link List} to use to mask request headers before logging. + * Returns an array of {@link String} to use to mask request headers before logging. */ @Nullable - final List maskRequestHeaders() { + final String[] maskRequestHeaders() { return maskRequestHeaders; } /** - * Sets the {@link List} to use to mask response headers before logging. + * Sets an array of {@link String} to use to mask response headers before logging. */ public AbstractLogFormatterBuilder maskResponseHeaders(String... headers) { - this.maskResponseHeaders = Arrays.stream(headers).map(String::toLowerCase).collect(Collectors.toList()); + this.maskResponseHeaders = headers; return this; } /** - * Returns the {@link List} to use to mask response headers before logging. + * Returns an array of {@link String} to use to mask response headers before logging. */ @Nullable - final List maskResponseHeaders() { + final String[] maskResponseHeaders() { return maskResponseHeaders; } @@ -268,39 +244,4 @@ public AbstractLogFormatterBuilder contentSanitizer( responseContentSanitizer(contentSanitizer); return this; } - - /** - * Checks whether the sanitizer and mask headers are not used together. - */ - protected void validateSanitizerAndMaskHeaders() { - if (requestHeadersSanitizer() != null) { - if (maskHeaders() != null || maskRequestHeaders() != null) { - throw new IllegalArgumentException("requestHeaderSanitizer should not be used when " + - "maskHeaders or maskRequestHeaders are used."); - } - } - - if (responseHeadersSanitizer() != null) { - if (maskHeaders() != null || maskResponseHeaders() != null) { - throw new IllegalArgumentException("responseHeaderSanitizer should not be used when " + - "maskHeaders or maskResponseHeaders are used."); - } - } - } - - /** - * Concatenates two {@link List} of headers and returns an {@link Set} of headers. - */ - protected Set concatHeaders(@Nullable List headers1, - @Nullable List headers2) { - final HashSet concatenatedHeaders = new HashSet<>(); - if (headers1 != null) { - concatenatedHeaders.addAll(headers1); - } - if (headers2 != null) { - concatenatedHeaders.addAll(headers2); - } - - return concatenatedHeaders; - } } From 6e5df41ac8a92ccd45573c63ce3d8b46e15bd2d2 Mon Sep 17 00:00:00 2001 From: Kim Seon Woo Date: Thu, 5 Oct 2023 01:38:42 +0900 Subject: [PATCH 3/5] Modify LogFormatter implementations to use HeadersSanitizer --- .../common/logging/JsonLogFormatter.java | 42 ++++--------------- .../logging/JsonLogFormatterBuilder.java | 30 ++++++++----- .../common/logging/TextLogFormatter.java | 41 ++++-------------- .../logging/TextLogFormatterBuilder.java | 30 ++++++++----- .../common/logging/JsonLogFormatterTest.java | 19 +++++++-- .../common/logging/TextLogFormatterTest.java | 17 +++++--- 6 files changed, 82 insertions(+), 97 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java index e9d3e6342951..c701d333087a 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java @@ -19,7 +19,6 @@ import static java.util.Objects.requireNonNull; import java.util.List; -import java.util.Set; import java.util.function.BiFunction; import org.slf4j.Logger; @@ -31,7 +30,7 @@ import com.google.common.base.MoreObjects; import com.linecorp.armeria.common.HttpHeaders; -import com.linecorp.armeria.common.HttpHeadersBuilder; +import com.linecorp.armeria.common.JsonHeadersSanitizer; import com.linecorp.armeria.common.RequestContext; import com.linecorp.armeria.common.SerializationFormat; import com.linecorp.armeria.common.annotation.Nullable; @@ -46,15 +45,11 @@ final class JsonLogFormatter implements LogFormatter { private static final Logger logger = LoggerFactory.getLogger(JsonLogFormatter.class); - private static final String MASK = "****"; - static final LogFormatter DEFAULT_INSTANCE = new JsonLogFormatterBuilder().build(); - private final BiFunction - requestHeadersSanitizer; + private final JsonHeadersSanitizer requestHeadersSanitizer; - private final BiFunction - responseHeadersSanitizer; + private final JsonHeadersSanitizer responseHeadersSanitizer; private final BiFunction requestTrailersSanitizer; @@ -68,25 +63,17 @@ final class JsonLogFormatter implements LogFormatter { private final BiFunction responseContentSanitizer; - private final Set maskRequestHeaders; - - private final Set maskResponseHeaders; - private final ObjectMapper objectMapper; JsonLogFormatter( - BiFunction requestHeadersSanitizer, - BiFunction responseHeadersSanitizer, + JsonHeadersSanitizer requestHeadersSanitizer, + JsonHeadersSanitizer responseHeadersSanitizer, BiFunction requestTrailersSanitizer, BiFunction responseTrailersSanitizer, BiFunction requestContentSanitizer, BiFunction responseContentSanitizer, - Set maskRequestHeaders, - Set maskResponseHeaders, ObjectMapper objectMapper) { this.requestHeadersSanitizer = requestHeadersSanitizer; this.responseHeadersSanitizer = responseHeadersSanitizer; @@ -94,8 +81,6 @@ final class JsonLogFormatter implements LogFormatter { this.responseTrailersSanitizer = responseTrailersSanitizer; this.requestContentSanitizer = requestContentSanitizer; this.responseContentSanitizer = responseContentSanitizer; - this.maskRequestHeaders = maskRequestHeaders; - this.maskResponseHeaders = maskResponseHeaders; this.objectMapper = objectMapper; } @@ -120,9 +105,7 @@ public String formatRequest(RequestOnlyLog log) { final JsonNode sanitizedHeaders; if (RequestLogProperty.REQUEST_HEADERS.isAvailable(flags)) { - sanitizedHeaders = - maskRequestHeaders.isEmpty() ? requestHeadersSanitizer.apply(ctx, log.requestHeaders()) - : applyMasking(log.requestHeaders(), maskRequestHeaders); + sanitizedHeaders = requestHeadersSanitizer.sanitizeHeaders(ctx, log.requestHeaders()); } else { sanitizedHeaders = null; } @@ -221,11 +204,7 @@ public String formatResponse(RequestLog log) { final JsonNode sanitizedHeaders; if (RequestLogProperty.RESPONSE_HEADERS.isAvailable(flags)) { - sanitizedHeaders = - maskResponseHeaders.isEmpty() ? responseHeadersSanitizer.apply(ctx, - log.responseHeaders()) - : applyMasking(log.responseHeaders(), - maskResponseHeaders); + sanitizedHeaders = responseHeadersSanitizer.sanitizeHeaders(ctx, log.responseHeaders()); } else { sanitizedHeaders = null; } @@ -296,13 +275,6 @@ public String formatResponse(RequestLog log) { } } - private JsonNode applyMasking(HttpHeaders headers, Set maskHeaders) { - final HttpHeadersBuilder builder = HttpHeaders.builder(); - headers.forEach( - (name, value) -> builder.add(name, maskHeaders.contains(name.toString()) ? MASK : value)); - return objectMapper.valueToTree(builder.build()); - } - @Override public String toString() { return MoreObjects.toStringHelper(this) diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java index 8cf40f34c7c8..3cf322cf8abb 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java @@ -24,7 +24,9 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.linecorp.armeria.common.HeadersSanitizer; import com.linecorp.armeria.common.HttpHeaders; +import com.linecorp.armeria.common.JsonHeadersSanitizerBuilder; import com.linecorp.armeria.common.RequestContext; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.annotation.UnstableApi; @@ -102,11 +104,6 @@ public JsonLogFormatterBuilder contentSanitizer( return (JsonLogFormatterBuilder) super.contentSanitizer(contentSanitizer); } - @Override - public JsonLogFormatterBuilder maskHeaders(String... headers) { - return (JsonLogFormatterBuilder) super.maskHeaders(headers); - } - @Override public JsonLogFormatterBuilder maskRequestHeaders(String... headers) { return (JsonLogFormatterBuilder) super.maskRequestHeaders(headers); @@ -128,16 +125,29 @@ public LogFormatter build() { final BiFunction defaultContentSanitizer = defaultSanitizer(objectMapper); - validateSanitizerAndMaskHeaders(); + final JsonHeadersSanitizerBuilder requestHeadersSanitizerBuilder = HeadersSanitizer.builderForJson(); + if (requestHeadersSanitizer() != null) { + requestHeadersSanitizerBuilder.headersSanitizer(requestHeadersSanitizer()); + } + if (maskRequestHeaders() != null) { + requestHeadersSanitizerBuilder.headersMask(maskRequestHeaders()); + } + + final JsonHeadersSanitizerBuilder responseHeadersSanitizerBuilder = HeadersSanitizer.builderForJson(); + if (responseHeadersSanitizer() != null) { + responseHeadersSanitizerBuilder.headersSanitizer(responseHeadersSanitizer()); + } + if (maskResponseHeaders() != null) { + responseHeadersSanitizerBuilder.headersMask(maskResponseHeaders()); + } + return new JsonLogFormatter( - firstNonNull(requestHeadersSanitizer(), defaultHeadersSanitizer), - firstNonNull(responseHeadersSanitizer(), defaultHeadersSanitizer), + requestHeadersSanitizerBuilder.build(), + responseHeadersSanitizerBuilder.build(), firstNonNull(requestTrailersSanitizer(), defaultHeadersSanitizer), firstNonNull(responseTrailersSanitizer(), defaultHeadersSanitizer), firstNonNull(requestContentSanitizer(), defaultContentSanitizer), firstNonNull(responseContentSanitizer(), defaultContentSanitizer), - concatHeaders(maskRequestHeaders(), maskHeaders()), - concatHeaders(maskResponseHeaders(), maskHeaders()), objectMapper); } diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java index db819ee5f15c..4f233c9b5b65 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java @@ -19,15 +19,14 @@ import static java.util.Objects.requireNonNull; import java.util.List; -import java.util.Set; import java.util.function.BiFunction; import com.google.common.base.MoreObjects; import com.linecorp.armeria.common.HttpHeaders; -import com.linecorp.armeria.common.HttpHeadersBuilder; import com.linecorp.armeria.common.RequestContext; import com.linecorp.armeria.common.SerializationFormat; +import com.linecorp.armeria.common.TextHeadersSanitizer; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.annotation.UnstableApi; import com.linecorp.armeria.common.util.TextFormatter; @@ -41,13 +40,9 @@ final class TextLogFormatter implements LogFormatter { static final LogFormatter DEFAULT_INSTANCE = new TextLogFormatterBuilder().build(); - private static final String MASK = "****"; + private final TextHeadersSanitizer requestHeadersSanitizer; - private final BiFunction requestHeadersSanitizer; - - private final BiFunction responseHeadersSanitizer; + private final TextHeadersSanitizer responseHeadersSanitizer; private final BiFunction requestTrailersSanitizer; @@ -61,17 +56,11 @@ final class TextLogFormatter implements LogFormatter { private final BiFunction responseContentSanitizer; - private final Set maskRequestHeaders; - - private final Set maskResponseHeaders; - private final boolean includeContext; TextLogFormatter( - BiFunction requestHeadersSanitizer, - BiFunction responseHeadersSanitizer, + TextHeadersSanitizer requestHeadersSanitizer, + TextHeadersSanitizer responseHeadersSanitizer, BiFunction requestTrailersSanitizer, BiFunction requestContentSanitizer, BiFunction responseContentSanitizer, - Set maskRequestHeaders, - Set maskResponseHeaders, boolean includeContext) { this.requestHeadersSanitizer = requestHeadersSanitizer; this.responseHeadersSanitizer = responseHeadersSanitizer; @@ -89,8 +76,6 @@ final class TextLogFormatter implements LogFormatter { this.responseTrailersSanitizer = responseTrailersSanitizer; this.requestContentSanitizer = requestContentSanitizer; this.responseContentSanitizer = responseContentSanitizer; - this.maskRequestHeaders = maskRequestHeaders; - this.maskResponseHeaders = maskResponseHeaders; this.includeContext = includeContext; } @@ -118,9 +103,7 @@ public String formatRequest(RequestOnlyLog log) { final String sanitizedHeaders; if (RequestLogProperty.REQUEST_HEADERS.isAvailable(flags)) { - sanitizedHeaders = - maskRequestHeaders.isEmpty() ? requestHeadersSanitizer.apply(ctx, log.requestHeaders()) - : applyMasking(log.requestHeaders(), maskRequestHeaders); + sanitizedHeaders = requestHeadersSanitizer.sanitizeHeaders(ctx, log.requestHeaders()); } else { sanitizedHeaders = null; } @@ -235,9 +218,7 @@ public String formatResponse(RequestLog log) { final String sanitizedHeaders; if (RequestLogProperty.RESPONSE_HEADERS.isAvailable(flags)) { - sanitizedHeaders = - maskResponseHeaders.isEmpty() ? responseHeadersSanitizer.apply(ctx, log.responseHeaders()) - : applyMasking(log.responseHeaders(), maskResponseHeaders); + sanitizedHeaders = responseHeadersSanitizer.sanitizeHeaders(ctx, log.responseHeaders()); } else { sanitizedHeaders = null; } @@ -322,14 +303,6 @@ public String formatResponse(RequestLog log) { } } - private String applyMasking(HttpHeaders headers, Set maskHeaders) { - final HttpHeadersBuilder builder = HttpHeaders.builder(); - headers.forEach( - (name, value) -> builder.add(name, maskHeaders.contains(name.toString()) ? MASK : value) - ); - return builder.build().toString(); - } - @Override public String toString() { return MoreObjects.toStringHelper(this) diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java index 018d37ca3c78..6b3475f2ab27 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java @@ -20,8 +20,10 @@ import java.util.function.BiFunction; +import com.linecorp.armeria.common.HeadersSanitizer; import com.linecorp.armeria.common.HttpHeaders; import com.linecorp.armeria.common.RequestContext; +import com.linecorp.armeria.common.TextHeadersSanitizerBuilder; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.annotation.UnstableApi; @@ -88,11 +90,6 @@ public TextLogFormatterBuilder contentSanitizer( return (TextLogFormatterBuilder) super.contentSanitizer(contentSanitizer); } - @Override - public TextLogFormatterBuilder maskHeaders(String... headers) { - return (TextLogFormatterBuilder) super.maskHeaders(headers); - } - @Override public TextLogFormatterBuilder maskRequestHeaders(String... headers) { return (TextLogFormatterBuilder) super.maskRequestHeaders(headers); @@ -117,16 +114,29 @@ public TextLogFormatterBuilder includeContext(boolean includeContext) { * Returns a newly-created text {@link LogFormatter} based on the properties of this builder. */ public LogFormatter build() { - validateSanitizerAndMaskHeaders(); + final TextHeadersSanitizerBuilder requestHeadersSanitizerBuilder = HeadersSanitizer.builderForText(); + if (requestHeadersSanitizer() != null) { + requestHeadersSanitizerBuilder.headersSanitizer(requestHeadersSanitizer()); + } + if (maskRequestHeaders() != null) { + requestHeadersSanitizerBuilder.headersMask(maskRequestHeaders()); + } + + final TextHeadersSanitizerBuilder responseHeadersSanitizerBuilder = HeadersSanitizer.builderForText(); + if (responseHeadersSanitizer() != null) { + responseHeadersSanitizerBuilder.headersSanitizer(responseHeadersSanitizer()); + } + if (maskResponseHeaders() != null) { + responseHeadersSanitizerBuilder.headersMask(maskResponseHeaders()); + } + return new TextLogFormatter( - firstNonNull(requestHeadersSanitizer(), defaultSanitizer()), - firstNonNull(responseHeadersSanitizer(), defaultSanitizer()), + requestHeadersSanitizerBuilder.build(), + responseHeadersSanitizerBuilder.build(), firstNonNull(requestTrailersSanitizer(), defaultSanitizer()), firstNonNull(responseTrailersSanitizer(), defaultSanitizer()), firstNonNull(requestContentSanitizer(), defaultSanitizer()), firstNonNull(responseContentSanitizer(), defaultSanitizer()), - concatHeaders(maskRequestHeaders(), maskHeaders()), - concatHeaders(maskResponseHeaders(), maskHeaders()), includeContext); } diff --git a/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java b/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java index 5f6e9a0b9ccd..02f5dfaffa30 100644 --- a/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java +++ b/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java @@ -25,15 +25,20 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import com.fasterxml.jackson.databind.ObjectMapper; + import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpRequest; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.ResponseHeaders; +import com.linecorp.armeria.internal.common.JacksonUtil; import com.linecorp.armeria.server.ServiceRequestContext; class JsonLogFormatterTest { + private final ObjectMapper objectMapper = JacksonUtil.newDefaultObjectMapper(); + @ParameterizedTest @CsvSource({ "true", "false" }) void formatRequest() { @@ -63,7 +68,11 @@ void formatResponse() { @Test void maskRequestHeaders() { final LogFormatter logFormatter = LogFormatter.builderForJson() - .maskHeaders("Cookie") + .requestHeadersSanitizer( + (ctx, headers) -> objectMapper.valueToTree( + headers.toBuilder() + .set("Cookie", "****") + .build())) .maskRequestHeaders("Authorization") .build(); final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", @@ -86,8 +95,12 @@ void maskRequestHeaders() { @Test void maskResponseHeaders() { final LogFormatter logFormatter = LogFormatter.builderForJson() - .maskHeaders("Content-Type") - .maskResponseHeaders("Set-Cookie") + .responseHeadersSanitizer( + (ctx, headers) -> objectMapper.valueToTree( + headers.toBuilder() + .set("Set-Cookie", "****") + .build())) + .maskResponseHeaders("Content-Type", "Set-Cookie") .build(); final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello")); final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); diff --git a/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java b/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java index 5193fadcd64c..a90f5909ba99 100644 --- a/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java +++ b/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java @@ -71,7 +71,11 @@ void formatResponse(boolean containContext) { @Test void maskRequestHeaders() { final LogFormatter logFormatter = LogFormatter.builderForText() - .maskHeaders("Cookie") + .requestHeadersSanitizer( + (ctx, headers) -> headers.toBuilder() + .set("Cookie", "****") + .build() + .toString()) .maskRequestHeaders("Authorization") .build(); final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", @@ -81,7 +85,6 @@ void maskRequestHeaders() { final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); log.endRequest(); final String requestLog = logFormatter.formatRequest(log); - System.out.println(requestLog); final Matcher matcher1 = Pattern.compile("cookie=(.*?)[,\\]]").matcher(requestLog); assertThat(matcher1.find()).isTrue(); assertThat(matcher1.group(1)).isEqualTo("****"); @@ -94,8 +97,13 @@ void maskRequestHeaders() { @Test void maskResponseHeaders() { final LogFormatter logFormatter = LogFormatter.builderForText() - .maskHeaders("Content-Type") - .maskResponseHeaders("Set-Cookie") + .responseHeadersSanitizer( + (ctx, headers) -> headers.toBuilder() + .set("Set-Cookie", + "****") + .build() + .toString()) + .maskResponseHeaders("Content-Type") .build(); final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello")); final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); @@ -104,7 +112,6 @@ void maskResponseHeaders() { "Set-Cookie", "Armeria=awesome")); log.endResponse(); final String responseLog = logFormatter.formatResponse(log); - System.out.println(responseLog); final Matcher matcher1 = Pattern.compile("content-type=(.*?)[,\\]]").matcher(responseLog); assertThat(matcher1.find()).isTrue(); assertThat(matcher1.group(1)).isEqualTo("****"); From d3bc0015ecca78d4b6844bae443cd677e003c111 Mon Sep 17 00:00:00 2001 From: Kim Seon Woo Date: Thu, 5 Oct 2023 09:33:02 +0900 Subject: [PATCH 4/5] Update tests --- .../common/logging/JsonLogFormatterTest.java | 16 +++++++++++++--- .../common/logging/TextLogFormatterTest.java | 15 +++++++++++++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java b/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java index 02f5dfaffa30..8f838b06a7d0 100644 --- a/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java +++ b/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java @@ -77,7 +77,9 @@ void maskRequestHeaders() { .build(); final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", "Cookie", "Armeria=awesome", - "Authorization", "authorization")); + "Authorization", "authorization", + "Cache-Control", "no-cache")); + final ServiceRequestContext ctx = ServiceRequestContext.of(req); final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); log.endRequest(); @@ -90,6 +92,10 @@ void maskRequestHeaders() { final Matcher matcher2 = Pattern.compile("\"authorization\":\"(.*?)\"").matcher(requestLog); assertThat(matcher2.find()).isTrue(); assertThat(matcher2.group(1)).isEqualTo("****"); + + final Matcher matcher3 = Pattern.compile("\"cache-control\":\"(.*?)\"").matcher(requestLog); + assertThat(matcher3.find()).isTrue(); + assertThat(matcher3.group(1)).isEqualTo("no-cache"); } @Test @@ -106,10 +112,10 @@ void maskResponseHeaders() { final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); log.responseHeaders(ResponseHeaders.of(HttpStatus.OK, "Content-Type", "text/html", - "Set-Cookie", "Armeria=awesome")); + "Set-Cookie", "Armeria=awesome", + "Cache-Control", "no-cache")); log.endResponse(); final String responseLog = logFormatter.formatResponse(log); - System.out.println(responseLog); final Matcher matcher1 = Pattern.compile("\"content-type\":\"(.*?)\"").matcher(responseLog); assertThat(matcher1.find()).isTrue(); assertThat(matcher1.group(1)).isEqualTo("****"); @@ -117,5 +123,9 @@ void maskResponseHeaders() { final Matcher matcher2 = Pattern.compile("\"set-cookie\":\"(.*?)\"").matcher(responseLog); assertThat(matcher2.find()).isTrue(); assertThat(matcher2.group(1)).isEqualTo("****"); + + final Matcher matcher3 = Pattern.compile("\"cache-control\":\"(.*?)\"").matcher(responseLog); + assertThat(matcher3.find()).isTrue(); + assertThat(matcher3.group(1)).isEqualTo("no-cache"); } } diff --git a/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java b/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java index a90f5909ba99..b127057d85e2 100644 --- a/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java +++ b/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java @@ -80,7 +80,9 @@ void maskRequestHeaders() { .build(); final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", "Cookie", "Armeria=awesome", - "Authorization", "authorization")); + "Authorization", "authorization", + "Cache-Control", "no-cache")); + final ServiceRequestContext ctx = ServiceRequestContext.of(req); final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); log.endRequest(); @@ -92,6 +94,10 @@ void maskRequestHeaders() { final Matcher matcher2 = Pattern.compile("authorization=(.*?)[,\\]]").matcher(requestLog); assertThat(matcher2.find()).isTrue(); assertThat(matcher2.group(1)).isEqualTo("****"); + + final Matcher matcher3 = Pattern.compile("cache-control=(.*?)[,\\]]").matcher(requestLog); + assertThat(matcher3.find()).isTrue(); + assertThat(matcher3.group(1)).isEqualTo("no-cache"); } @Test @@ -109,7 +115,8 @@ void maskResponseHeaders() { final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); log.responseHeaders(ResponseHeaders.of(HttpStatus.OK, "Content-Type", "text/html", - "Set-Cookie", "Armeria=awesome")); + "Set-Cookie", "Armeria=awesome", + "Cache-Control", "no-cache")); log.endResponse(); final String responseLog = logFormatter.formatResponse(log); final Matcher matcher1 = Pattern.compile("content-type=(.*?)[,\\]]").matcher(responseLog); @@ -119,5 +126,9 @@ void maskResponseHeaders() { final Matcher matcher2 = Pattern.compile("set-cookie=(.*?)[,\\]]").matcher(responseLog); assertThat(matcher2.find()).isTrue(); assertThat(matcher2.group(1)).isEqualTo("****"); + + final Matcher matcher3 = Pattern.compile("cache-control=(.*?)[,\\]]").matcher(responseLog); + assertThat(matcher3.find()).isTrue(); + assertThat(matcher3.group(1)).isEqualTo("no-cache"); } } From 1c94801fc146ce825ba283cfc6c7f04a36d2aa07 Mon Sep 17 00:00:00 2001 From: Kim Seon Woo Date: Tue, 31 Oct 2023 22:35:52 +0900 Subject: [PATCH 5/5] Update tests --- core/build.gradle | 4 ++ .../AbstractHeadersSanitizerBuilder.java | 18 ++++++--- .../armeria/common/HeadersSanitizer.java | 11 +++--- .../armeria/common/JsonHeadersSanitizer.java | 9 ++++- .../common/JsonHeadersSanitizerBuilder.java | 17 +++++---- .../armeria/common/TextHeadersSanitizer.java | 11 ++++-- .../common/TextHeadersSanitizerBuilder.java | 17 +++++---- .../logging/AbstractLogFormatterBuilder.java | 38 ------------------- .../logging/JsonLogFormatterBuilder.java | 16 -------- .../logging/TextLogFormatterBuilder.java | 16 -------- .../common/logging/JsonLogFormatterTest.java | 14 +++++-- .../common/logging/TextLogFormatterTest.java | 14 ++++++- 12 files changed, 80 insertions(+), 105 deletions(-) diff --git a/core/build.gradle b/core/build.gradle index 1a09b8467061..eb894d016ac2 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -281,3 +281,7 @@ class PublicSuffixesTask extends DefaultTask { } task publicSuffixes(type: PublicSuffixesTask) + +test { + testLogging.showStandardStreams = true +} diff --git a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java index a798c67e7b22..d15469be7b79 100644 --- a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java @@ -18,10 +18,10 @@ import static java.util.Objects.requireNonNull; -import java.util.Arrays; import java.util.Set; import java.util.function.BiFunction; -import java.util.stream.Collectors; + +import com.google.common.collect.ImmutableSet; import com.linecorp.armeria.common.annotation.Nullable; @@ -33,8 +33,7 @@ abstract class AbstractHeadersSanitizerBuilder { @Nullable private BiFunction headersSanitizer; - @Nullable - private Set headersMask; + private Set headersMask = ImmutableSet.of(); /** * Sets the {@link BiFunction} to use to sanitize headers before logging. It is common to have the @@ -58,14 +57,21 @@ public AbstractHeadersSanitizerBuilder headersSanitizer( * Sets the {@link Set} to use to mask headers before logging. */ public AbstractHeadersSanitizerBuilder headersMask(String... headers) { - headersMask = Arrays.stream(headers).map(String::toLowerCase).collect(Collectors.toSet()); + headersMask = ImmutableSet.copyOf(requireNonNull(headers, "headers")); + return this; + } + + /** + * Sets the {@link Set} to use to mask headers before logging. + */ + public AbstractHeadersSanitizerBuilder headersMask(Iterable headers) { + headersMask = ImmutableSet.copyOf(requireNonNull(headers, "headers")); return this; } /** * Returns the {@link Set} to use to mask headers before logging. */ - @Nullable final Set headersMask() { return headersMask; } diff --git a/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java b/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java index 1eff2b8221b0..e909c403f0de 100644 --- a/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java @@ -16,19 +16,20 @@ package com.linecorp.armeria.common; +import java.util.function.BiFunction; + import com.fasterxml.jackson.databind.JsonNode; /** * A sanitizer that sanitizes {@link HttpHeaders}. */ -@FunctionalInterface -public interface HeadersSanitizer { +public interface HeadersSanitizer extends BiFunction { /** * Returns the default text {@link HeadersSanitizer}. */ static HeadersSanitizer ofText() { - return TextHeadersSanitizer.DEFAULT_INSTANCE; + return TextHeadersSanitizer.INSTANCE; } /** @@ -42,7 +43,7 @@ static TextHeadersSanitizerBuilder builderForText() { * Returns the default json {@link HeadersSanitizer}. */ static HeadersSanitizer ofJson() { - return JsonHeadersSanitizer.DEFAULT_INSTANCE; + return JsonHeadersSanitizer.INSTANCE; } /** @@ -55,5 +56,5 @@ static JsonHeadersSanitizerBuilder builderForJson() { /** * Returns the sanitized {@link HttpHeaders}. */ - T sanitizeHeaders(RequestContext ctx, HttpHeaders headers); + T sanitizeHeaders(RequestContext requestContext, HttpHeaders headers); } diff --git a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java index f63dc63fec33..e826c1fc3df2 100644 --- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java @@ -26,9 +26,9 @@ /** * A sanitizer that sanitizes {@link HttpHeaders} and returns {@link JsonNode}. */ -public class JsonHeadersSanitizer implements HeadersSanitizer { +public final class JsonHeadersSanitizer implements HeadersSanitizer { - static final HeadersSanitizer DEFAULT_INSTANCE = new JsonHeadersSanitizerBuilder().build(); + static final HeadersSanitizer INSTANCE = new JsonHeadersSanitizerBuilder().build(); private static final String MASK = "****"; @@ -53,4 +53,9 @@ public JsonNode sanitizeHeaders(RequestContext ctx, HttpHeaders headers) { return headersSanitizer.apply(ctx, builder.build()); } + + @Override + public JsonNode apply(RequestContext requestContext, HttpHeaders headers) { + return null; + } } diff --git a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java index 1a859666bea7..4a20594730ec 100644 --- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java @@ -19,7 +19,6 @@ import static com.google.common.base.MoreObjects.firstNonNull; import static java.util.Objects.requireNonNull; -import java.util.HashSet; import java.util.Set; import java.util.function.BiFunction; @@ -32,7 +31,7 @@ /** * A builder implementation for {@link JsonHeadersSanitizer}. */ -public class JsonHeadersSanitizerBuilder extends AbstractHeadersSanitizerBuilder { +public final class JsonHeadersSanitizerBuilder extends AbstractHeadersSanitizerBuilder { @Nullable private ObjectMapper objectMapper; @@ -62,6 +61,14 @@ public JsonHeadersSanitizerBuilder headersMask(String... headers) { return (JsonHeadersSanitizerBuilder) super.headersMask(headers); } + /** + * Sets the {@link Set} to use to mask headers before logging. + */ + @Override + public JsonHeadersSanitizerBuilder headersMask(Iterable headers) { + return (JsonHeadersSanitizerBuilder) super.headersMask(headers); + } + /** * Returns a newly-created JSON {@link HeadersSanitizer} based on the properties of this builder. */ @@ -70,15 +77,11 @@ public JsonHeadersSanitizer build() { this.objectMapper : JacksonUtil.newDefaultObjectMapper(); return new JsonHeadersSanitizer(firstNonNull(headersSanitizer(), defaultSanitizer(objectMapper)), - firstNonNull(headersMask(), defaultMask())); + headersMask()); } private static BiFunction defaultSanitizer(ObjectMapper objectMapper) { return (requestContext, obj) -> objectMapper.valueToTree(obj); } - - private static Set defaultMask() { - return new HashSet<>(); - } } diff --git a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java index 140e85c440f8..14f1351be79b 100644 --- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java @@ -24,9 +24,9 @@ /** * A sanitizer that sanitizes {@link HttpHeaders} and returns {@link String}. */ -public class TextHeadersSanitizer implements HeadersSanitizer { +public final class TextHeadersSanitizer implements HeadersSanitizer { - static final HeadersSanitizer DEFAULT_INSTANCE = new TextHeadersSanitizerBuilder().build(); + static final HeadersSanitizer INSTANCE = new TextHeadersSanitizerBuilder().build(); private static final String MASK = "****"; @@ -44,11 +44,16 @@ public class TextHeadersSanitizer implements HeadersSanitizer { } @Override - public String sanitizeHeaders(RequestContext ctx, HttpHeaders headers) { + public String apply(RequestContext ctx, HttpHeaders headers) { final HttpHeadersBuilder builder = headers.toBuilder(); headers.forEach( (name, value) -> builder.set(name, headersMask.contains(name.toString()) ? MASK : value) ); return headersSanitizer.apply(ctx, builder.build()); } + + @Override + public String sanitizeHeaders(RequestContext requestContext, HttpHeaders headers) { + return null; + } } diff --git a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java index 24f998c28383..bce02e5ef51b 100644 --- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java @@ -18,14 +18,13 @@ import static com.google.common.base.MoreObjects.firstNonNull; -import java.util.HashSet; import java.util.Set; import java.util.function.BiFunction; /** * A builder implementation for {@link TextHeadersSanitizer}. */ -public class TextHeadersSanitizerBuilder extends AbstractHeadersSanitizerBuilder { +public final class TextHeadersSanitizerBuilder extends AbstractHeadersSanitizerBuilder { /** * Sets the {@link BiFunction} to use to sanitize headers before logging. @@ -44,19 +43,23 @@ public TextHeadersSanitizerBuilder headersMask(String... headers) { return (TextHeadersSanitizerBuilder) super.headersMask(headers); } + /** + * Sets the {@link Set} to use to mask headers before logging. + */ + @Override + public TextHeadersSanitizerBuilder headersMask(Iterable headers) { + return (TextHeadersSanitizerBuilder) super.headersMask(headers); + } + /** * Returns a newly-created text {@link HeadersSanitizer} based on the properties of this builder. */ public TextHeadersSanitizer build() { return new TextHeadersSanitizer(firstNonNull(headersSanitizer(), defaultSanitizer()), - firstNonNull(headersMask(), defaultMask())); + headersMask()); } private static BiFunction defaultSanitizer() { return (requestContext, object) -> object.toString(); } - - private static Set defaultMask() { - return new HashSet<>(); - } } diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java index 03d7c8bb7a69..fe895bd3a6b9 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractLogFormatterBuilder.java @@ -48,12 +48,6 @@ abstract class AbstractLogFormatterBuilder { @Nullable private BiFunction responseContentSanitizer; - @Nullable - private String[] maskRequestHeaders; - - @Nullable - private String[] maskResponseHeaders; - /** * Sets the {@link BiFunction} to use to sanitize request headers before logging. It is common to have the * {@link BiFunction} that removes sensitive headers, like {@code Cookie}, before logging. If unset, will @@ -192,38 +186,6 @@ public AbstractLogFormatterBuilder responseContentSanitizer( return responseContentSanitizer; } - /** - * Sets an array of {@link String} to use to mask request headers before logging. - */ - public AbstractLogFormatterBuilder maskRequestHeaders(String... headers) { - this.maskRequestHeaders = headers; - return this; - } - - /** - * Returns an array of {@link String} to use to mask request headers before logging. - */ - @Nullable - final String[] maskRequestHeaders() { - return maskRequestHeaders; - } - - /** - * Sets an array of {@link String} to use to mask response headers before logging. - */ - public AbstractLogFormatterBuilder maskResponseHeaders(String... headers) { - this.maskResponseHeaders = headers; - return this; - } - - /** - * Returns an array of {@link String} to use to mask response headers before logging. - */ - @Nullable - final String[] maskResponseHeaders() { - return maskResponseHeaders; - } - /** * Sets the {@link BiFunction} to use to sanitize request and response content before logging. It is common * to have the {@link BiFunction} that removes sensitive content, such as an GPS location query and diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java index 3cf322cf8abb..e4fd96e7dc93 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java @@ -104,16 +104,6 @@ public JsonLogFormatterBuilder contentSanitizer( return (JsonLogFormatterBuilder) super.contentSanitizer(contentSanitizer); } - @Override - public JsonLogFormatterBuilder maskRequestHeaders(String... headers) { - return (JsonLogFormatterBuilder) super.maskRequestHeaders(headers); - } - - @Override - public JsonLogFormatterBuilder maskResponseHeaders(String... headers) { - return (JsonLogFormatterBuilder) super.maskResponseHeaders(headers); - } - /** * Returns a newly-created JSON {@link LogFormatter} based on the properties of this builder. */ @@ -129,17 +119,11 @@ public LogFormatter build() { if (requestHeadersSanitizer() != null) { requestHeadersSanitizerBuilder.headersSanitizer(requestHeadersSanitizer()); } - if (maskRequestHeaders() != null) { - requestHeadersSanitizerBuilder.headersMask(maskRequestHeaders()); - } final JsonHeadersSanitizerBuilder responseHeadersSanitizerBuilder = HeadersSanitizer.builderForJson(); if (responseHeadersSanitizer() != null) { responseHeadersSanitizerBuilder.headersSanitizer(responseHeadersSanitizer()); } - if (maskResponseHeaders() != null) { - responseHeadersSanitizerBuilder.headersMask(maskResponseHeaders()); - } return new JsonLogFormatter( requestHeadersSanitizerBuilder.build(), diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java index 6b3475f2ab27..9cced46a70ea 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java @@ -90,16 +90,6 @@ public TextLogFormatterBuilder contentSanitizer( return (TextLogFormatterBuilder) super.contentSanitizer(contentSanitizer); } - @Override - public TextLogFormatterBuilder maskRequestHeaders(String... headers) { - return (TextLogFormatterBuilder) super.maskRequestHeaders(headers); - } - - @Override - public TextLogFormatterBuilder maskResponseHeaders(String... headers) { - return (TextLogFormatterBuilder) super.maskResponseHeaders(headers); - } - /** * Sets whether to include stringified {@link RequestContext} in the result of * {@link LogFormatter#formatRequest(RequestOnlyLog)} and {@link LogFormatter#formatResponse(RequestLog)}. @@ -118,17 +108,11 @@ public LogFormatter build() { if (requestHeadersSanitizer() != null) { requestHeadersSanitizerBuilder.headersSanitizer(requestHeadersSanitizer()); } - if (maskRequestHeaders() != null) { - requestHeadersSanitizerBuilder.headersMask(maskRequestHeaders()); - } final TextHeadersSanitizerBuilder responseHeadersSanitizerBuilder = HeadersSanitizer.builderForText(); if (responseHeadersSanitizer() != null) { responseHeadersSanitizerBuilder.headersSanitizer(responseHeadersSanitizer()); } - if (maskResponseHeaders() != null) { - responseHeadersSanitizerBuilder.headersMask(maskResponseHeaders()); - } return new TextLogFormatter( requestHeadersSanitizerBuilder.build(), diff --git a/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java b/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java index 8f838b06a7d0..114bf62edf07 100644 --- a/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java +++ b/core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java @@ -73,7 +73,11 @@ void maskRequestHeaders() { headers.toBuilder() .set("Cookie", "****") .build())) - .maskRequestHeaders("Authorization") + .headersSanitizer( + (ctx, headers) -> objectMapper.valueToTree( + headers.toBuilder() + .set("Authorization", "****") + .build())) .build(); final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", "Cookie", "Armeria=awesome", @@ -104,9 +108,13 @@ void maskResponseHeaders() { .responseHeadersSanitizer( (ctx, headers) -> objectMapper.valueToTree( headers.toBuilder() - .set("Set-Cookie", "****") + .set("Content-Type", "****") + .build())) + .headersSanitizer( + (ctx, headers) -> objectMapper.valueToTree( + headers.toBuilder() + .set("Authorization", "****") .build())) - .maskResponseHeaders("Content-Type", "Set-Cookie") .build(); final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello")); final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); diff --git a/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java b/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java index b127057d85e2..8d12e956619c 100644 --- a/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java +++ b/core/src/test/java/com/linecorp/armeria/common/logging/TextLogFormatterTest.java @@ -76,7 +76,12 @@ void maskRequestHeaders() { .set("Cookie", "****") .build() .toString()) - .maskRequestHeaders("Authorization") + .headersSanitizer( + (ctx, headers) -> headers.toBuilder() + .set("Authorization", + "****") + .build() + .toString()) .build(); final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", "Cookie", "Armeria=awesome", @@ -104,12 +109,17 @@ void maskRequestHeaders() { void maskResponseHeaders() { final LogFormatter logFormatter = LogFormatter.builderForText() .responseHeadersSanitizer( + (ctx, headers) -> headers.toBuilder() + .set("Content-Type", + "****") + .build() + .toString()) + .headersSanitizer( (ctx, headers) -> headers.toBuilder() .set("Set-Cookie", "****") .build() .toString()) - .maskResponseHeaders("Content-Type") .build(); final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello")); final DefaultRequestLog log = (DefaultRequestLog) ctx.log();