From 3c163cbf79fb0be8b593dfa494aa7b33eccf3af6 Mon Sep 17 00:00:00 2001 From: Kim Seon Woo Date: Wed, 6 Dec 2023 23:44:06 +0900 Subject: [PATCH 01/16] Add HeadersSanitizer to allow users to easily mask headers --- .../AbstractHeadersSanitizerBuilder.java | 72 +++++++++++++++++ .../armeria/common/HeadersSanitizer.java | 54 +++++++++++++ .../armeria/common/JsonHeadersSanitizer.java | 59 ++++++++++++++ .../common/JsonHeadersSanitizerBuilder.java | 79 +++++++++++++++++++ .../armeria/common/TextHeadersSanitizer.java | 66 ++++++++++++++++ .../common/TextHeadersSanitizerBuilder.java | 57 +++++++++++++ .../common/logging/JsonLogFormatterTest.java | 79 +++++++++++++++++++ .../common/logging/TextLogFormatterTest.java | 75 ++++++++++++++++++ 8 files changed, 541 insertions(+) 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 00000000000..20fe21bc2ae --- /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.Set; +import java.util.function.Function; + +import com.google.common.collect.ImmutableSet; + +/** + * A skeletal builder implementation for {@link HeadersSanitizer}. + */ +abstract class AbstractHeadersSanitizerBuilder { + + private Set maskHeaders = ImmutableSet.of(); + + private Function mask = (header) -> "****"; + + /** + * Sets the {@link Set} which includes headers to mask before logging. + */ + public AbstractHeadersSanitizerBuilder maskHeaders(String... headers) { + maskHeaders = ImmutableSet.copyOf(requireNonNull(headers, "headers")); + return this; + } + + /** + * Sets the {@link Set} which includes headers to mask before logging. + */ + public AbstractHeadersSanitizerBuilder maskHeaders(Iterable headers) { + maskHeaders = ImmutableSet.copyOf(requireNonNull(headers, "headers")); + return this; + } + + /** + * Returns the {@link Set} which includes headers to mask before logging. + */ + final Set maskHeaders() { + return maskHeaders; + } + + /** + * Returns the {@link Function} to use to mask headers before logging. + */ + final Function mask() { + return mask; + } + + /** + * Sets the {@link Function} to use to mask headers before logging. + */ + public AbstractHeadersSanitizerBuilder mask(Function mask) { + this.mask = requireNonNull(mask, "mask"); + return this; + } +} 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 00000000000..299edc6b81d --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.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.function.BiFunction; + +import com.fasterxml.jackson.databind.JsonNode; + +/** + * A sanitizer that sanitizes {@link HttpHeaders}. + */ +public interface HeadersSanitizer extends BiFunction { + /** + * Returns the default text {@link HeadersSanitizer}. + */ + static HeadersSanitizer ofText() { + return TextHeadersSanitizer.INSTANCE; + } + + /** + * Returns a newly created {@link TextHeadersSanitizerBuilder}. + */ + static TextHeadersSanitizerBuilder builderForText() { + return new TextHeadersSanitizerBuilder(); + } + + /** + * Returns the default json {@link HeadersSanitizer}. + */ + static HeadersSanitizer ofJson() { + return JsonHeadersSanitizer.INSTANCE; + } + + /** + * Returns a newly created {@link JsonHeadersSanitizerBuilder}. + */ + static JsonHeadersSanitizerBuilder builderForJson() { + return new JsonHeadersSanitizerBuilder(); + } +} 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 00000000000..feaf289f611 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.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 java.util.Map; +import java.util.Set; +import java.util.function.Function; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; + +import io.netty.util.AsciiString; + +/** + * A sanitizer that sanitizes {@link HttpHeaders} and returns {@link JsonNode}. + */ +public final class JsonHeadersSanitizer implements HeadersSanitizer { + + static final HeadersSanitizer INSTANCE = new JsonHeadersSanitizerBuilder().build(); + private final Set maskHeaders; + private final Function mask; + private final ObjectMapper objectMapper; + + JsonHeadersSanitizer(Set maskHeaders, Function mask, ObjectMapper objectMapper) { + this.maskHeaders = maskHeaders; + this.mask = mask; + this.objectMapper = objectMapper; + } + + @Override + public JsonNode apply(RequestContext requestContext, HttpHeaders headers) { + final ObjectNode result = objectMapper.createObjectNode(); + for (Map.Entry e : headers) { + final String header = e.getKey().toString(); + if (maskHeaders.contains(header)) { + result.put(header, mask.apply(e.getValue())); + } else { + result.put(header, e.getValue()); + } + } + + return result; + } +} 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 00000000000..e5df8101624 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java @@ -0,0 +1,79 @@ +/* + * 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.Set; +import java.util.function.Function; + +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 final class JsonHeadersSanitizerBuilder extends AbstractHeadersSanitizerBuilder { + + @Nullable + private ObjectMapper objectMapper; + + /** + * Sets the {@link Set} which includes headers to mask before logging. + */ + @Override + public JsonHeadersSanitizerBuilder maskHeaders(String... headers) { + return (JsonHeadersSanitizerBuilder) super.maskHeaders(headers); + } + + /** + * Sets the {@link Set} which includes headers to mask before logging. + */ + @Override + public JsonHeadersSanitizerBuilder maskHeaders(Iterable headers) { + return (JsonHeadersSanitizerBuilder) super.maskHeaders(headers); + } + + /** + * Sets the {@link Function} to use to mask headers before logging. + */ + @Override + public JsonHeadersSanitizerBuilder mask(Function mask) { + return (JsonHeadersSanitizerBuilder) super.mask(mask); + } + + /** + * 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; + } + + /** + * 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(maskHeaders(), mask(), objectMapper); + } +} 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 00000000000..33f1849fa60 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java @@ -0,0 +1,66 @@ +/* + * 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.Map; +import java.util.Set; +import java.util.function.Function; + +import io.netty.util.AsciiString; + +/** + * A sanitizer that sanitizes {@link HttpHeaders} and returns {@link String}. + */ +public final class TextHeadersSanitizer implements HeadersSanitizer { + + static final HeadersSanitizer INSTANCE = new TextHeadersSanitizerBuilder().build(); + + private final Set maskHeaders; + + private final Function mask; + + TextHeadersSanitizer(Set maskHeaders, Function mask) { + this.maskHeaders = maskHeaders; + this.mask = mask; + } + + @Override + public String apply(RequestContext ctx, HttpHeaders headers) { + if (headers.isEmpty()) { + return headers.isEndOfStream() ? "[EOS]" : "[]"; + } + + final StringBuilder sb = new StringBuilder(); + if (headers.isEndOfStream()) { + sb.append("[EOS], "); + } else { + sb.append('['); + } + + for (Map.Entry e : headers) { + final String header = e.getKey().toString(); + if (maskHeaders.contains(header)) { + sb.append(header).append('=').append(mask.apply(e.getValue())).append(", "); + } else { + sb.append(header).append('=').append(e.getValue()).append(", "); + } + } + + sb.setCharAt(sb.length() - 2, ']'); + return sb.substring(0, sb.length() - 1); + } +} 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 00000000000..4db01f375cd --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java @@ -0,0 +1,57 @@ +/* + * 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.Function; + +/** + * A builder implementation for {@link TextHeadersSanitizer}. + */ +public final class TextHeadersSanitizerBuilder extends AbstractHeadersSanitizerBuilder { + + /** + * Sets the {@link Set} which includes headers to mask before logging. + */ + @Override + public TextHeadersSanitizerBuilder maskHeaders(String... headers) { + return (TextHeadersSanitizerBuilder) super.maskHeaders(headers); + } + + /** + * Sets the {@link Set} which includes headers to mask before logging. + */ + @Override + public TextHeadersSanitizerBuilder maskHeaders(Iterable headers) { + return (TextHeadersSanitizerBuilder) super.maskHeaders(headers); + } + + /** + * Sets the {@link Function} to use to mask headers before logging. + */ + @Override + public TextHeadersSanitizerBuilder mask(Function mask) { + return (TextHeadersSanitizerBuilder) super.mask(mask); + } + + /** + * Returns a newly-created text {@link HeadersSanitizer} based on the properties of this builder. + */ + public TextHeadersSanitizer build() { + return new TextHeadersSanitizer(maskHeaders(), mask()); + } +} 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 4c1303e0492..2b61e0a86a7 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,16 +18,29 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.function.Function; +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.fasterxml.jackson.databind.ObjectMapper; + +import com.linecorp.armeria.common.HeadersSanitizer; 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() { @@ -53,4 +66,70 @@ void formatResponse() { .matches("^\\{\"type\":\"response\",\"startTime\":\".+\",\"length\":\".+\"," + "\"duration\":\".+\",\"totalDuration\":\".+\",\"headers\":\\{\".+\"}}$"); } + + @Test + void maskRequestHeaders() { + final Function maskingFunction = (header) -> "****armeria****"; + final LogFormatter logFormatter = LogFormatter.builderForJson() + .requestHeadersSanitizer( + HeadersSanitizer.builderForJson() + .maskHeaders("cookie", + "authorization") + .mask(maskingFunction) + .build()) + .build(); + final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", + "Cookie", "Armeria=awesome", + "Authorization", "Basic XXX==", + "Cache-Control", "no-cache")); + + 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(maskingFunction.apply("Armeria=awesome")); + + final Matcher matcher2 = Pattern.compile("\"authorization\":\"(.*?)\"").matcher(requestLog); + assertThat(matcher2.find()).isTrue(); + assertThat(matcher2.group(1)).isEqualTo(maskingFunction.apply("Basic XXX==")); + + final Matcher matcher3 = Pattern.compile("\"cache-control\":\"(.*?)\"").matcher(requestLog); + assertThat(matcher3.find()).isTrue(); + assertThat(matcher3.group(1)).isEqualTo("no-cache"); + } + + @Test + void maskResponseHeaders() { + final Function maskingFunction = (header) -> "****armeria****"; + final LogFormatter logFormatter = LogFormatter.builderForJson() + .responseHeadersSanitizer( + HeadersSanitizer.builderForJson() + .maskHeaders("content-type", + "set-cookie") + .mask(maskingFunction) + .build()) + .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", + "Cache-Control", "no-cache")); + log.endResponse(); + final String responseLog = logFormatter.formatResponse(log); + final Matcher matcher1 = Pattern.compile("\"content-type\":\"(.*?)\"").matcher(responseLog); + assertThat(matcher1.find()).isTrue(); + assertThat(matcher1.group(1)).isEqualTo(maskingFunction.apply("text/html")); + + final Matcher matcher2 = Pattern.compile("\"set-cookie\":\"(.*?)\"").matcher(responseLog); + assertThat(matcher2.find()).isTrue(); + assertThat(matcher2.group(1)).isEqualTo(maskingFunction.apply("Armeria=awesome")); + + 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 0dbc36122b4..35e1e7c6a0f 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,20 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.function.Function; +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.HeadersSanitizer; 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 +69,70 @@ void formatResponse(boolean containContext) { assertThat(responseLog).matches(regex); } } + + @Test + void maskRequestHeaders() { + final Function maskingFunction = (header) -> "****armeria****"; + final LogFormatter logFormatter = LogFormatter.builderForText() + .requestHeadersSanitizer( + HeadersSanitizer.builderForText() + .maskHeaders("cookie", + "authorization") + .mask(maskingFunction) + .build()) + .build(); + final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", + "Cookie", "Armeria=awesome", + "Authorization", "Basic XXX==", + "Cache-Control", "no-cache")); + + 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(maskingFunction.apply("Armeria=awesome")); + + final Matcher matcher2 = Pattern.compile("authorization=(.*?)[,\\]]").matcher(requestLog); + assertThat(matcher2.find()).isTrue(); + assertThat(matcher2.group(1)).isEqualTo(maskingFunction.apply("Basic XXX==")); + + final Matcher matcher3 = Pattern.compile("cache-control=(.*?)[,\\]]").matcher(requestLog); + assertThat(matcher3.find()).isTrue(); + assertThat(matcher3.group(1)).isEqualTo("no-cache"); + } + + @Test + void maskResponseHeaders() { + final Function maskingFunction = (header) -> "****armeria****"; + final LogFormatter logFormatter = LogFormatter.builderForText() + .responseHeadersSanitizer( + HeadersSanitizer.builderForText() + .maskHeaders("content-type", + "set-cookie") + .mask(maskingFunction) + .build()) + .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", + "Cache-Control", "no-cache")); + log.endResponse(); + final String responseLog = logFormatter.formatResponse(log); + final Matcher matcher1 = Pattern.compile("content-type=(.*?)[,\\]]").matcher(responseLog); + assertThat(matcher1.find()).isTrue(); + assertThat(matcher1.group(1)).isEqualTo(maskingFunction.apply("text/html")); + + final Matcher matcher2 = Pattern.compile("set-cookie=(.*?)[,\\]]").matcher(responseLog); + assertThat(matcher2.find()).isTrue(); + assertThat(matcher2.group(1)).isEqualTo(maskingFunction.apply("Armeria=awesome")); + + final Matcher matcher3 = Pattern.compile("cache-control=(.*?)[,\\]]").matcher(responseLog); + assertThat(matcher3.find()).isTrue(); + assertThat(matcher3.group(1)).isEqualTo("no-cache"); + } } From a52c45edd85a2d8a157a19761d2b6b7d191beea8 Mon Sep 17 00:00:00 2001 From: Kim Seon Woo Date: Wed, 6 Dec 2023 23:52:40 +0900 Subject: [PATCH 02/16] Nit - fix comments - change method order --- .../common/AbstractHeadersSanitizerBuilder.java | 14 +++++++------- .../common/JsonHeadersSanitizerBuilder.java | 4 ++-- .../common/TextHeadersSanitizerBuilder.java | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) 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 20fe21bc2ae..ead1830f494 100644 --- a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java @@ -55,13 +55,6 @@ final Set maskHeaders() { return maskHeaders; } - /** - * Returns the {@link Function} to use to mask headers before logging. - */ - final Function mask() { - return mask; - } - /** * Sets the {@link Function} to use to mask headers before logging. */ @@ -69,4 +62,11 @@ public AbstractHeadersSanitizerBuilder mask(Function mask) { this.mask = requireNonNull(mask, "mask"); return this; } + + /** + * Returns the {@link Function} to use to mask headers before logging. + */ + final Function mask() { + return mask; + } } 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 e5df8101624..d63763dc8ad 100644 --- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java @@ -60,7 +60,7 @@ public JsonHeadersSanitizerBuilder mask(Function mask) { } /** - * Sets the {@link ObjectMapper} that will be used to convert an object into a JSON format message. + * Sets the {@link ObjectMapper} that will be used to convert headers into a {@link JsonNode}. */ public JsonHeadersSanitizerBuilder objectMapper(ObjectMapper objectMapper) { this.objectMapper = requireNonNull(objectMapper, "objectMapper"); @@ -68,7 +68,7 @@ public JsonHeadersSanitizerBuilder objectMapper(ObjectMapper objectMapper) { } /** - * Returns a newly-created JSON {@link HeadersSanitizer} based on the properties of this builder. + * Returns a newly created JSON {@link HeadersSanitizer} based on the properties of this builder. */ public JsonHeadersSanitizer build() { final ObjectMapper objectMapper = this.objectMapper != 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 4db01f375cd..805ef6fbe82 100644 --- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java @@ -49,7 +49,7 @@ public TextHeadersSanitizerBuilder mask(Function mask) { } /** - * Returns a newly-created text {@link HeadersSanitizer} based on the properties of this builder. + * Returns a newly created text {@link HeadersSanitizer} based on the properties of this builder. */ public TextHeadersSanitizer build() { return new TextHeadersSanitizer(maskHeaders(), mask()); From 0904f65995a9811925f2ec34b1fb114f58dde191 Mon Sep 17 00:00:00 2001 From: Kim Seon Woo Date: Sun, 7 Jan 2024 10:42:10 +0900 Subject: [PATCH 03/16] Apply comments - Fix naming - Mask sensitive headers by default --- .../AbstractHeadersSanitizerBuilder.java | 35 +++--- .../armeria/common/JsonHeadersSanitizer.java | 36 ++++-- .../common/JsonHeadersSanitizerBuilder.java | 17 +-- .../armeria/common/TextHeadersSanitizer.java | 35 ++++-- .../common/TextHeadersSanitizerBuilder.java | 16 +-- .../logging/JsonLogFormatterBuilder.java | 5 +- .../logging/TextLogFormatterBuilder.java | 5 +- .../common/logging/JsonLogFormatterTest.java | 104 +++++++++++++----- .../common/logging/TextLogFormatterTest.java | 80 +++++++++++++- 9 files changed, 245 insertions(+), 88 deletions(-) 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 ead1830f494..1269878ebe6 100644 --- a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java @@ -18,55 +18,64 @@ import static java.util.Objects.requireNonNull; +import java.util.Arrays; +import java.util.HashSet; import java.util.Set; import java.util.function.Function; -import com.google.common.collect.ImmutableSet; - /** * A skeletal builder implementation for {@link HeadersSanitizer}. */ abstract class AbstractHeadersSanitizerBuilder { - private Set maskHeaders = ImmutableSet.of(); + private final Set maskingHeaders = new HashSet<>(); private Function mask = (header) -> "****"; /** * Sets the {@link Set} which includes headers to mask before logging. */ - public AbstractHeadersSanitizerBuilder maskHeaders(String... headers) { - maskHeaders = ImmutableSet.copyOf(requireNonNull(headers, "headers")); + public AbstractHeadersSanitizerBuilder maskingHeaders(CharSequence... headers) { + requireNonNull(headers, "headers"); + Arrays.stream(headers).map(header -> header.toString().toLowerCase()).forEach(maskingHeaders::add); return this; } /** * Sets the {@link Set} which includes headers to mask before logging. */ - public AbstractHeadersSanitizerBuilder maskHeaders(Iterable headers) { - maskHeaders = ImmutableSet.copyOf(requireNonNull(headers, "headers")); + public AbstractHeadersSanitizerBuilder maskingHeaders(Iterable headers) { + requireNonNull(headers, "headers"); + headers.forEach(header -> maskingHeaders.add(header.toString().toLowerCase())); return this; } /** * Returns the {@link Set} which includes headers to mask before logging. */ - final Set maskHeaders() { - return maskHeaders; + final Set maskingHeaders() { + return maskingHeaders; } /** - * Sets the {@link Function} to use to mask headers before logging. + * Sets the {@link Function} to use to maskFunction headers before logging. */ - public AbstractHeadersSanitizerBuilder mask(Function mask) { - this.mask = requireNonNull(mask, "mask"); + public AbstractHeadersSanitizerBuilder maskingFunction(Function maskingFunction) { + this.mask = requireNonNull(maskingFunction, "maskingFunction"); return this; } /** * Returns the {@link Function} to use to mask headers before logging. */ - final Function mask() { + final Function maskingFunction() { return mask; } + + protected final Set defaultMaskingHeaders() { + final HashSet defaultMaskingHeaders = new HashSet<>(); + defaultMaskingHeaders.add(HttpHeaderNames.AUTHORIZATION.toLowerCase().toString()); + defaultMaskingHeaders.add(HttpHeaderNames.SET_COOKIE.toLowerCase().toString()); + return defaultMaskingHeaders; + } } 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 feaf289f611..419cd683888 100644 --- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java @@ -16,7 +16,11 @@ package com.linecorp.armeria.common; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.function.Function; @@ -32,26 +36,34 @@ public final class JsonHeadersSanitizer implements HeadersSanitizer { static final HeadersSanitizer INSTANCE = new JsonHeadersSanitizerBuilder().build(); - private final Set maskHeaders; - private final Function mask; + private final Set maskingHeaders; + private final Function maskingFunction; private final ObjectMapper objectMapper; - JsonHeadersSanitizer(Set maskHeaders, Function mask, ObjectMapper objectMapper) { - this.maskHeaders = maskHeaders; - this.mask = mask; + JsonHeadersSanitizer(Set maskingHeaders, Function maskingFunction, + ObjectMapper objectMapper) { + this.maskingHeaders = maskingHeaders; + this.maskingFunction = maskingFunction; this.objectMapper = objectMapper; } @Override public JsonNode apply(RequestContext requestContext, HttpHeaders headers) { final ObjectNode result = objectMapper.createObjectNode(); - for (Map.Entry e : headers) { - final String header = e.getKey().toString(); - if (maskHeaders.contains(header)) { - result.put(header, mask.apply(e.getValue())); - } else { - result.put(header, e.getValue()); - } + final Map> headersWithValuesAsList = new HashMap<>(); + for (Map.Entry entry : headers) { + final String header = entry.getKey().toString().toLowerCase(); + final String value = maskingHeaders.contains(header) ? maskingFunction.apply(entry.getValue()) + : entry.getValue(); + headersWithValuesAsList.computeIfAbsent(header, k -> new ArrayList<>()).add(value); + } + + final Set>> entries = headersWithValuesAsList.entrySet(); + for (Map.Entry> entry : entries) { + final String header = entry.getKey(); + final List values = entry.getValue(); + + result.put(header, values.size() > 1 ? values.toString() : values.get(0)); } return result; 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 d63763dc8ad..69c0c75bb4a 100644 --- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java @@ -39,24 +39,24 @@ public final class JsonHeadersSanitizerBuilder extends AbstractHeadersSanitizerB * Sets the {@link Set} which includes headers to mask before logging. */ @Override - public JsonHeadersSanitizerBuilder maskHeaders(String... headers) { - return (JsonHeadersSanitizerBuilder) super.maskHeaders(headers); + public JsonHeadersSanitizerBuilder maskingHeaders(CharSequence... headers) { + return (JsonHeadersSanitizerBuilder) super.maskingHeaders(headers); } /** * Sets the {@link Set} which includes headers to mask before logging. */ @Override - public JsonHeadersSanitizerBuilder maskHeaders(Iterable headers) { - return (JsonHeadersSanitizerBuilder) super.maskHeaders(headers); + public JsonHeadersSanitizerBuilder maskingHeaders(Iterable headers) { + return (JsonHeadersSanitizerBuilder) super.maskingHeaders(headers); } /** * Sets the {@link Function} to use to mask headers before logging. */ @Override - public JsonHeadersSanitizerBuilder mask(Function mask) { - return (JsonHeadersSanitizerBuilder) super.mask(mask); + public JsonHeadersSanitizerBuilder maskingFunction(Function maskingFunction) { + return (JsonHeadersSanitizerBuilder) super.maskingFunction(maskingFunction); } /** @@ -74,6 +74,9 @@ public JsonHeadersSanitizer build() { final ObjectMapper objectMapper = this.objectMapper != null ? this.objectMapper : JacksonUtil.newDefaultObjectMapper(); - return new JsonHeadersSanitizer(maskHeaders(), mask(), objectMapper); + final Set maskingHeaders = maskingHeaders(); + return new JsonHeadersSanitizer( + !maskingHeaders.isEmpty() ? maskingHeaders : defaultMaskingHeaders(), maskingFunction(), + objectMapper); } } 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 33f1849fa60..2dbff4585e9 100644 --- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java @@ -16,6 +16,9 @@ package com.linecorp.armeria.common; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Function; @@ -29,13 +32,13 @@ public final class TextHeadersSanitizer implements HeadersSanitizer { static final HeadersSanitizer INSTANCE = new TextHeadersSanitizerBuilder().build(); - private final Set maskHeaders; + private final Set maskingHeaders; - private final Function mask; + private final Function maskingFunction; - TextHeadersSanitizer(Set maskHeaders, Function mask) { - this.maskHeaders = maskHeaders; - this.mask = mask; + TextHeadersSanitizer(Set maskingHeaders, Function maskingFunction) { + this.maskingHeaders = maskingHeaders; + this.maskingFunction = maskingFunction; } @Override @@ -51,13 +54,21 @@ public String apply(RequestContext ctx, HttpHeaders headers) { sb.append('['); } - for (Map.Entry e : headers) { - final String header = e.getKey().toString(); - if (maskHeaders.contains(header)) { - sb.append(header).append('=').append(mask.apply(e.getValue())).append(", "); - } else { - sb.append(header).append('=').append(e.getValue()).append(", "); - } + final Map> headersWithValuesAsList = new HashMap<>(); + for (Map.Entry entry : headers) { + final String header = entry.getKey().toString().toLowerCase(); + final String value = maskingHeaders.contains(header) ? maskingFunction.apply(entry.getValue()) + : entry.getValue(); + headersWithValuesAsList.computeIfAbsent(header, k -> new ArrayList<>()).add(value); + } + + final Set>> entries = headersWithValuesAsList.entrySet(); + for (Map.Entry> entry : entries) { + final String header = entry.getKey(); + final List values = entry.getValue(); + + sb.append(header).append('=') + .append(values.size() > 1 ? values.toString() : values.get(0)).append(", "); } sb.setCharAt(sb.length() - 2, ']'); 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 805ef6fbe82..240837e5324 100644 --- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java @@ -28,30 +28,32 @@ public final class TextHeadersSanitizerBuilder extends AbstractHeadersSanitizerB * Sets the {@link Set} which includes headers to mask before logging. */ @Override - public TextHeadersSanitizerBuilder maskHeaders(String... headers) { - return (TextHeadersSanitizerBuilder) super.maskHeaders(headers); + public TextHeadersSanitizerBuilder maskingHeaders(CharSequence... headers) { + return (TextHeadersSanitizerBuilder) super.maskingHeaders(headers); } /** * Sets the {@link Set} which includes headers to mask before logging. */ @Override - public TextHeadersSanitizerBuilder maskHeaders(Iterable headers) { - return (TextHeadersSanitizerBuilder) super.maskHeaders(headers); + public TextHeadersSanitizerBuilder maskingHeaders(Iterable headers) { + return (TextHeadersSanitizerBuilder) super.maskingHeaders(headers); } /** * Sets the {@link Function} to use to mask headers before logging. */ @Override - public TextHeadersSanitizerBuilder mask(Function mask) { - return (TextHeadersSanitizerBuilder) super.mask(mask); + public TextHeadersSanitizerBuilder maskingFunction(Function maskingFunction) { + return (TextHeadersSanitizerBuilder) super.maskingFunction(maskingFunction); } /** * Returns a newly created text {@link HeadersSanitizer} based on the properties of this builder. */ public TextHeadersSanitizer build() { - return new TextHeadersSanitizer(maskHeaders(), mask()); + final Set maskingHeaders = maskingHeaders(); + return new TextHeadersSanitizer(!maskingHeaders.isEmpty() ? maskingHeaders : defaultMaskingHeaders(), + maskingFunction()); } } 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 5c1a177bea9..db14cc740ae 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,6 +24,7 @@ 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.RequestContext; import com.linecorp.armeria.common.annotation.Nullable; @@ -113,8 +114,8 @@ public LogFormatter build() { final BiFunction defaultContentSanitizer = defaultSanitizer(objectMapper); return new JsonLogFormatter( - firstNonNull(requestHeadersSanitizer(), defaultHeadersSanitizer), - firstNonNull(responseHeadersSanitizer(), defaultHeadersSanitizer), + firstNonNull(requestHeadersSanitizer(), HeadersSanitizer.ofJson()), + firstNonNull(responseHeadersSanitizer(), HeadersSanitizer.ofJson()), firstNonNull(requestTrailersSanitizer(), defaultHeadersSanitizer), firstNonNull(responseTrailersSanitizer(), defaultHeadersSanitizer), firstNonNull(requestContentSanitizer(), defaultContentSanitizer), 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 1ffef054772..310399be100 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,6 +20,7 @@ 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.annotation.Nullable; @@ -103,8 +104,8 @@ public TextLogFormatterBuilder includeContext(boolean includeContext) { */ public LogFormatter build() { return new TextLogFormatter( - firstNonNull(requestHeadersSanitizer(), defaultSanitizer()), - firstNonNull(responseHeadersSanitizer(), defaultSanitizer()), + firstNonNull(requestHeadersSanitizer(), HeadersSanitizer.ofText()), + firstNonNull(responseHeadersSanitizer(), HeadersSanitizer.ofText()), firstNonNull(requestTrailersSanitizer(), defaultSanitizer()), firstNonNull(responseTrailersSanitizer(), defaultSanitizer()), firstNonNull(requestContentSanitizer(), defaultSanitizer()), 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 2b61e0a86a7..4d757b483b2 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 @@ -26,21 +26,16 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; -import com.fasterxml.jackson.databind.ObjectMapper; - import com.linecorp.armeria.common.HeadersSanitizer; 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() { @@ -67,20 +62,59 @@ void formatResponse() { "\"duration\":\".+\",\"totalDuration\":\".+\",\"headers\":\\{\".+\"}}$"); } + @Test + void maskSensitiveHeadersByDefault() { + final LogFormatter logFormatter = LogFormatter.builderForJson() + .responseHeadersSanitizer( + HeadersSanitizer.builderForJson().build()) + .build(); + final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello")); + final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); + log.responseHeaders(ResponseHeaders.of(HttpStatus.OK, "Set-Cookie", "Armeria=awesome")); + log.endResponse(); + + final String responseLog = logFormatter.formatResponse(log); + final Matcher matcher1 = Pattern.compile("\"set-cookie\":\"(.*?)\"").matcher(responseLog); + assertThat(matcher1.find()).isTrue(); + assertThat(matcher1.group(1)).isEqualTo("****"); + } + + @Test + void defaultMaskingHeadersShouldBeOverridable() { + final LogFormatter logFormatter = LogFormatter.builderForJson() + .responseHeadersSanitizer( + HeadersSanitizer.builderForJson() + .maskingHeaders("Cache-Control") + .build()) + .build(); + final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello")); + final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); + log.responseHeaders(ResponseHeaders.of(HttpStatus.OK, "Set-Cookie", "armeria=fun", + "Cache-Control", "no-cache")); + log.endResponse(); + + final String responseLog = logFormatter.formatResponse(log); + final Matcher matcher1 = Pattern.compile("\"set-cookie\":\"(.*?)\"").matcher(responseLog); + assertThat(matcher1.find()).isTrue(); + assertThat(matcher1.group(1)).isEqualTo("armeria=fun"); + + final Matcher matcher2 = Pattern.compile("\"cache-control\":\"(.*?)\"").matcher(responseLog); + assertThat(matcher2.find()).isTrue(); + assertThat(matcher2.group(1)).isEqualTo("****"); + } + @Test void maskRequestHeaders() { final Function maskingFunction = (header) -> "****armeria****"; final LogFormatter logFormatter = LogFormatter.builderForJson() .requestHeadersSanitizer( HeadersSanitizer.builderForJson() - .maskHeaders("cookie", - "authorization") - .mask(maskingFunction) + .maskingHeaders("accept") + .maskingFunction(maskingFunction) .build()) .build(); final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", - "Cookie", "Armeria=awesome", - "Authorization", "Basic XXX==", + "Accept", "text/html", "Cache-Control", "no-cache")); final ServiceRequestContext ctx = ServiceRequestContext.of(req); @@ -88,17 +122,13 @@ void maskRequestHeaders() { log.endRequest(); final String requestLog = logFormatter.formatRequest(log); - final Matcher matcher1 = Pattern.compile("\"cookie\":\"(.*?)\"").matcher(requestLog); + final Matcher matcher1 = Pattern.compile("\"accept\":\"(.*?)\"").matcher(requestLog); assertThat(matcher1.find()).isTrue(); - assertThat(matcher1.group(1)).isEqualTo(maskingFunction.apply("Armeria=awesome")); + assertThat(matcher1.group(1)).isEqualTo(maskingFunction.apply("text/html")); - final Matcher matcher2 = Pattern.compile("\"authorization\":\"(.*?)\"").matcher(requestLog); + final Matcher matcher2 = Pattern.compile("\"cache-control\":\"(.*?)\"").matcher(requestLog); assertThat(matcher2.find()).isTrue(); - assertThat(matcher2.group(1)).isEqualTo(maskingFunction.apply("Basic XXX==")); - - final Matcher matcher3 = Pattern.compile("\"cache-control\":\"(.*?)\"").matcher(requestLog); - assertThat(matcher3.find()).isTrue(); - assertThat(matcher3.group(1)).isEqualTo("no-cache"); + assertThat(matcher2.group(1)).isEqualTo("no-cache"); } @Test @@ -107,16 +137,14 @@ void maskResponseHeaders() { final LogFormatter logFormatter = LogFormatter.builderForJson() .responseHeadersSanitizer( HeadersSanitizer.builderForJson() - .maskHeaders("content-type", - "set-cookie") - .mask(maskingFunction) + .maskingHeaders("content-type") + .maskingFunction(maskingFunction) .build()) .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", "Cache-Control", "no-cache")); log.endResponse(); final String responseLog = logFormatter.formatResponse(log); @@ -124,12 +152,34 @@ void maskResponseHeaders() { assertThat(matcher1.find()).isTrue(); assertThat(matcher1.group(1)).isEqualTo(maskingFunction.apply("text/html")); - final Matcher matcher2 = Pattern.compile("\"set-cookie\":\"(.*?)\"").matcher(responseLog); + final Matcher matcher2 = Pattern.compile("\"cache-control\":\"(.*?)\"").matcher(responseLog); assertThat(matcher2.find()).isTrue(); - assertThat(matcher2.group(1)).isEqualTo(maskingFunction.apply("Armeria=awesome")); + assertThat(matcher2.group(1)).isEqualTo("no-cache"); + } + + @Test + void maskRequestHeadersWithDuplicateHeaderName() { + final Function maskingFunction = (header) -> "****armeria****"; + final LogFormatter logFormatter = LogFormatter.builderForJson() + .requestHeadersSanitizer( + HeadersSanitizer.builderForJson() + .maskingHeaders("accept-encoding") + .maskingHeaders("content-type") + .maskingFunction(maskingFunction) + .build()) + .build(); + final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", + "Accept-Encoding", "gzip", + "Accept-Encoding", "deflate")); - final Matcher matcher3 = Pattern.compile("\"cache-control\":\"(.*?)\"").matcher(responseLog); - assertThat(matcher3.find()).isTrue(); - assertThat(matcher3.group(1)).isEqualTo("no-cache"); + 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("\"accept-encoding\":\"(.*?)\"").matcher(requestLog); + assertThat(matcher1.find()).isTrue(); + assertThat(matcher1.group(1)).isEqualTo( + "[" + maskingFunction.apply("gzip") + ", " + maskingFunction.apply("deflate") + "]"); } } 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 35e1e7c6a0f..215f3697d19 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 @@ -70,15 +70,57 @@ void formatResponse(boolean containContext) { } } + @Test + void maskSensitiveHeadersByDefault() { + final LogFormatter logFormatter = LogFormatter.builderForText() + .responseHeadersSanitizer( + HeadersSanitizer.builderForText().build() + ) + .build(); + final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello")); + final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); + log.responseHeaders(ResponseHeaders.of(HttpStatus.OK, "Set-Cookie", "Armeria=awesome")); + log.endResponse(); + + final String responseLog = logFormatter.formatResponse(log); + final Matcher matcher = Pattern.compile("set-cookie=(.*?)[,\\]]").matcher(responseLog); + assertThat(matcher.find()).isTrue(); + assertThat(matcher.group(1)).isEqualTo("****"); + } + + @Test + void defaultMaskingHeadersShouldBeOverridable() { + final LogFormatter logFormatter = LogFormatter.builderForText() + .responseHeadersSanitizer( + HeadersSanitizer.builderForText() + .maskingHeaders("Cache-Control") + .build()) + .build(); + final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello")); + final DefaultRequestLog log = (DefaultRequestLog) ctx.log(); + log.responseHeaders(ResponseHeaders.of(HttpStatus.OK, "Set-Cookie", "armeria=fun", + "Cache-Control", "no-cache")); + log.endResponse(); + + final String responseLog = logFormatter.formatResponse(log); + final Matcher matcher1 = Pattern.compile("set-cookie=(.*?)[,\\]]").matcher(responseLog); + assertThat(matcher1.find()).isTrue(); + assertThat(matcher1.group(1)).isEqualTo("armeria=fun"); + + final Matcher matcher2 = Pattern.compile("cache-control=(.*?)[,\\]]").matcher(responseLog); + assertThat(matcher2.find()).isTrue(); + assertThat(matcher2.group(1)).isEqualTo("****"); + } + @Test void maskRequestHeaders() { final Function maskingFunction = (header) -> "****armeria****"; final LogFormatter logFormatter = LogFormatter.builderForText() .requestHeadersSanitizer( HeadersSanitizer.builderForText() - .maskHeaders("cookie", - "authorization") - .mask(maskingFunction) + .maskingHeaders("cookie", + "authorization") + .maskingFunction(maskingFunction) .build()) .build(); final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", @@ -110,9 +152,9 @@ void maskResponseHeaders() { final LogFormatter logFormatter = LogFormatter.builderForText() .responseHeadersSanitizer( HeadersSanitizer.builderForText() - .maskHeaders("content-type", - "set-cookie") - .mask(maskingFunction) + .maskingHeaders("content-type", + "set-cookie") + .maskingFunction(maskingFunction) .build()) .build(); final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello")); @@ -135,4 +177,30 @@ void maskResponseHeaders() { assertThat(matcher3.find()).isTrue(); assertThat(matcher3.group(1)).isEqualTo("no-cache"); } + + @Test + void maskRequestHeadersWithDuplicateHeaderName() { + final Function maskingFunction = (header) -> "****armeria****"; + final LogFormatter logFormatter = LogFormatter.builderForText() + .requestHeadersSanitizer( + HeadersSanitizer.builderForText() + .maskingHeaders("accept-encoding") + .maskingHeaders("content-type") + .maskingFunction(maskingFunction) + .build()) + .build(); + final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", + "Accept-Encoding", "gzip", + "Accept-Encoding", "deflate")); + + 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("accept-encoding=\\[(.*?)]").matcher(requestLog); + assertThat(matcher1.find()).isTrue(); + assertThat(matcher1.group(1)).isEqualTo( + maskingFunction.apply("gzip") + ", " + maskingFunction.apply("deflate")); + } } From b3dfd7d08a35a15df0894f77a8cb9de3ea8a5d87 Mon Sep 17 00:00:00 2001 From: Kim Seon Woo Date: Sun, 7 Jan 2024 11:30:50 +0900 Subject: [PATCH 04/16] Fix HashMap to LinkedHashMap to maintain order of headers --- .../com/linecorp/armeria/common/JsonHeadersSanitizer.java | 3 ++- .../com/linecorp/armeria/common/TextHeadersSanitizer.java | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) 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 419cd683888..09094a6b6fe 100644 --- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -50,7 +51,7 @@ public final class JsonHeadersSanitizer implements HeadersSanitizer { @Override public JsonNode apply(RequestContext requestContext, HttpHeaders headers) { final ObjectNode result = objectMapper.createObjectNode(); - final Map> headersWithValuesAsList = new HashMap<>(); + final Map> headersWithValuesAsList = new LinkedHashMap<>(); for (Map.Entry entry : headers) { final String header = entry.getKey().toString().toLowerCase(); final String value = maskingHeaders.contains(header) ? maskingFunction.apply(entry.getValue()) 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 2dbff4585e9..aa8434e78a5 100644 --- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java @@ -18,12 +18,14 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Function; import io.netty.util.AsciiString; +import scala.collection.generic.Sorted; /** * A sanitizer that sanitizes {@link HttpHeaders} and returns {@link String}. @@ -54,7 +56,7 @@ public String apply(RequestContext ctx, HttpHeaders headers) { sb.append('['); } - final Map> headersWithValuesAsList = new HashMap<>(); + final Map> headersWithValuesAsList = new LinkedHashMap<>(); for (Map.Entry entry : headers) { final String header = entry.getKey().toString().toLowerCase(); final String value = maskingHeaders.contains(header) ? maskingFunction.apply(entry.getValue()) From b4cb62bcef75f5e3c2b6c4058bc73a029f465168 Mon Sep 17 00:00:00 2001 From: songmw725 Date: Wed, 24 Jan 2024 18:18:49 +0900 Subject: [PATCH 05/16] Fix style issues --- .../AbstractHeadersSanitizerBuilder.java | 40 +++++++++---------- .../armeria/common/HeadersSanitizer.java | 1 + .../armeria/common/JsonHeadersSanitizer.java | 30 +++----------- .../common/JsonHeadersSanitizerBuilder.java | 20 ++-------- .../armeria/common/TextHeadersSanitizer.java | 24 ++++++----- .../common/TextHeadersSanitizerBuilder.java | 18 ++------- 6 files changed, 47 insertions(+), 86 deletions(-) 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 1269878ebe6..bc8b1f66248 100644 --- a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java @@ -18,50 +18,55 @@ import static java.util.Objects.requireNonNull; -import java.util.Arrays; import java.util.HashSet; import java.util.Set; import java.util.function.Function; +import com.google.common.base.Ascii; +import com.google.common.collect.ImmutableSet; + /** * A skeletal builder implementation for {@link HeadersSanitizer}. */ -abstract class AbstractHeadersSanitizerBuilder { +public abstract class AbstractHeadersSanitizerBuilder { + + private static final Set DEFAULT_MASKING_HEADERS = + ImmutableSet.of(Ascii.toLowerCase(HttpHeaderNames.AUTHORIZATION), + Ascii.toLowerCase(HttpHeaderNames.SET_COOKIE)); private final Set maskingHeaders = new HashSet<>(); - private Function mask = (header) -> "****"; + private Function maskingFunction = header -> "****"; /** - * Sets the {@link Set} which includes headers to mask before logging. + * Sets the headers to mask before logging. */ public AbstractHeadersSanitizerBuilder maskingHeaders(CharSequence... headers) { requireNonNull(headers, "headers"); - Arrays.stream(headers).map(header -> header.toString().toLowerCase()).forEach(maskingHeaders::add); - return this; + return maskingHeaders(ImmutableSet.copyOf(headers)); } /** - * Sets the {@link Set} which includes headers to mask before logging. + * Sets the headers to mask before logging. */ public AbstractHeadersSanitizerBuilder maskingHeaders(Iterable headers) { requireNonNull(headers, "headers"); - headers.forEach(header -> maskingHeaders.add(header.toString().toLowerCase())); + headers.forEach(header -> maskingHeaders.add(Ascii.toLowerCase(header))); return this; } - /** - * Returns the {@link Set} which includes headers to mask before logging. - */ final Set maskingHeaders() { - return maskingHeaders; + if (!maskingHeaders.isEmpty()) { + return ImmutableSet.copyOf(maskingHeaders); + } + return DEFAULT_MASKING_HEADERS; } /** * Sets the {@link Function} to use to maskFunction headers before logging. */ public AbstractHeadersSanitizerBuilder maskingFunction(Function maskingFunction) { - this.mask = requireNonNull(maskingFunction, "maskingFunction"); + this.maskingFunction = requireNonNull(maskingFunction, "maskingFunction"); return this; } @@ -69,13 +74,6 @@ public AbstractHeadersSanitizerBuilder maskingFunction(Function maskingFunction() { - return mask; - } - - protected final Set defaultMaskingHeaders() { - final HashSet defaultMaskingHeaders = new HashSet<>(); - defaultMaskingHeaders.add(HttpHeaderNames.AUTHORIZATION.toLowerCase().toString()); - defaultMaskingHeaders.add(HttpHeaderNames.SET_COOKIE.toLowerCase().toString()); - return defaultMaskingHeaders; + return maskingFunction; } } 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 299edc6b81d..1656a6100ef 100644 --- a/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java @@ -23,6 +23,7 @@ /** * A sanitizer that sanitizes {@link HttpHeaders}. */ +@FunctionalInterface public interface HeadersSanitizer extends BiFunction { /** * Returns the default text {@link HeadersSanitizer}. 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 09094a6b6fe..2102bf62f8e 100644 --- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java @@ -16,12 +16,8 @@ package com.linecorp.armeria.common; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Map.Entry; +import static com.linecorp.armeria.common.TextHeadersSanitizer.maskHeaders; + import java.util.Set; import java.util.function.Function; @@ -29,12 +25,10 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; -import io.netty.util.AsciiString; - /** * A sanitizer that sanitizes {@link HttpHeaders} and returns {@link JsonNode}. */ -public final class JsonHeadersSanitizer implements HeadersSanitizer { +final class JsonHeadersSanitizer implements HeadersSanitizer { static final HeadersSanitizer INSTANCE = new JsonHeadersSanitizerBuilder().build(); private final Set maskingHeaders; @@ -51,21 +45,9 @@ public final class JsonHeadersSanitizer implements HeadersSanitizer { @Override public JsonNode apply(RequestContext requestContext, HttpHeaders headers) { final ObjectNode result = objectMapper.createObjectNode(); - final Map> headersWithValuesAsList = new LinkedHashMap<>(); - for (Map.Entry entry : headers) { - final String header = entry.getKey().toString().toLowerCase(); - final String value = maskingHeaders.contains(header) ? maskingFunction.apply(entry.getValue()) - : entry.getValue(); - headersWithValuesAsList.computeIfAbsent(header, k -> new ArrayList<>()).add(value); - } - - final Set>> entries = headersWithValuesAsList.entrySet(); - for (Map.Entry> entry : entries) { - final String header = entry.getKey(); - final List values = entry.getValue(); - - result.put(header, values.size() > 1 ? values.toString() : values.get(0)); - } + maskHeaders(headers, maskingHeaders, maskingFunction, + (header, values) -> result.put(header, values.size() > 1 ? + values.toString() : values.get(0))); return result; } 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 69c0c75bb4a..bdee7fc66e7 100644 --- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java @@ -18,7 +18,6 @@ import static java.util.Objects.requireNonNull; -import java.util.Set; import java.util.function.Function; import com.fasterxml.jackson.databind.JsonNode; @@ -28,32 +27,23 @@ import com.linecorp.armeria.internal.common.JacksonUtil; /** - * A builder implementation for {@link JsonHeadersSanitizer}. + * A builder implementation for JSON {@link HeadersSanitizer}. */ public final class JsonHeadersSanitizerBuilder extends AbstractHeadersSanitizerBuilder { @Nullable private ObjectMapper objectMapper; - /** - * Sets the {@link Set} which includes headers to mask before logging. - */ @Override public JsonHeadersSanitizerBuilder maskingHeaders(CharSequence... headers) { return (JsonHeadersSanitizerBuilder) super.maskingHeaders(headers); } - /** - * Sets the {@link Set} which includes headers to mask before logging. - */ @Override public JsonHeadersSanitizerBuilder maskingHeaders(Iterable headers) { return (JsonHeadersSanitizerBuilder) super.maskingHeaders(headers); } - /** - * Sets the {@link Function} to use to mask headers before logging. - */ @Override public JsonHeadersSanitizerBuilder maskingFunction(Function maskingFunction) { return (JsonHeadersSanitizerBuilder) super.maskingFunction(maskingFunction); @@ -70,13 +60,9 @@ public JsonHeadersSanitizerBuilder objectMapper(ObjectMapper objectMapper) { /** * Returns a newly created JSON {@link HeadersSanitizer} based on the properties of this builder. */ - public JsonHeadersSanitizer build() { + public HeadersSanitizer build() { final ObjectMapper objectMapper = this.objectMapper != null ? this.objectMapper : JacksonUtil.newDefaultObjectMapper(); - - final Set maskingHeaders = maskingHeaders(); - return new JsonHeadersSanitizer( - !maskingHeaders.isEmpty() ? maskingHeaders : defaultMaskingHeaders(), maskingFunction(), - objectMapper); + return new JsonHeadersSanitizer(maskingHeaders(), maskingFunction(), objectMapper); } } 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 aa8434e78a5..fbff9674568 100644 --- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java @@ -17,20 +17,19 @@ package com.linecorp.armeria.common; import java.util.ArrayList; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.BiConsumer; import java.util.function.Function; import io.netty.util.AsciiString; -import scala.collection.generic.Sorted; /** * A sanitizer that sanitizes {@link HttpHeaders} and returns {@link String}. */ -public final class TextHeadersSanitizer implements HeadersSanitizer { +final class TextHeadersSanitizer implements HeadersSanitizer { static final HeadersSanitizer INSTANCE = new TextHeadersSanitizerBuilder().build(); @@ -56,6 +55,18 @@ public String apply(RequestContext ctx, HttpHeaders headers) { sb.append('['); } + maskHeaders(headers, maskingHeaders, maskingFunction, + (header, values) -> sb.append(header).append('=') + .append(values.size() > 1 ? + values.toString() : values.get(0)).append(", ")); + + sb.setCharAt(sb.length() - 2, ']'); + return sb.substring(0, sb.length() - 1); + } + + static void maskHeaders( + HttpHeaders headers, Set maskingHeaders, Function maskingFunction, + final BiConsumer> consumer) { final Map> headersWithValuesAsList = new LinkedHashMap<>(); for (Map.Entry entry : headers) { final String header = entry.getKey().toString().toLowerCase(); @@ -68,12 +79,7 @@ public String apply(RequestContext ctx, HttpHeaders headers) { for (Map.Entry> entry : entries) { final String header = entry.getKey(); final List values = entry.getValue(); - - sb.append(header).append('=') - .append(values.size() > 1 ? values.toString() : values.get(0)).append(", "); + consumer.accept(header, values); } - - sb.setCharAt(sb.length() - 2, ']'); - return sb.substring(0, sb.length() - 1); } } 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 240837e5324..d648577db50 100644 --- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java @@ -16,33 +16,23 @@ package com.linecorp.armeria.common; -import java.util.Set; import java.util.function.Function; /** - * A builder implementation for {@link TextHeadersSanitizer}. + * A builder implementation for Text {@link HeadersSanitizer}. */ public final class TextHeadersSanitizerBuilder extends AbstractHeadersSanitizerBuilder { - /** - * Sets the {@link Set} which includes headers to mask before logging. - */ @Override public TextHeadersSanitizerBuilder maskingHeaders(CharSequence... headers) { return (TextHeadersSanitizerBuilder) super.maskingHeaders(headers); } - /** - * Sets the {@link Set} which includes headers to mask before logging. - */ @Override public TextHeadersSanitizerBuilder maskingHeaders(Iterable headers) { return (TextHeadersSanitizerBuilder) super.maskingHeaders(headers); } - /** - * Sets the {@link Function} to use to mask headers before logging. - */ @Override public TextHeadersSanitizerBuilder maskingFunction(Function maskingFunction) { return (TextHeadersSanitizerBuilder) super.maskingFunction(maskingFunction); @@ -51,9 +41,7 @@ public TextHeadersSanitizerBuilder maskingFunction(Function mask /** * Returns a newly created text {@link HeadersSanitizer} based on the properties of this builder. */ - public TextHeadersSanitizer build() { - final Set maskingHeaders = maskingHeaders(); - return new TextHeadersSanitizer(!maskingHeaders.isEmpty() ? maskingHeaders : defaultMaskingHeaders(), - maskingFunction()); + public HeadersSanitizer build() { + return new TextHeadersSanitizer(maskingHeaders(), maskingFunction()); } } From c5b896110e4696882e03f506eb85d224c3b066d7 Mon Sep 17 00:00:00 2001 From: songmw725 Date: Wed, 24 Jan 2024 18:35:32 +0900 Subject: [PATCH 06/16] Address the comment from @ikhoon --- .../AbstractHeadersSanitizerBuilder.java | 14 +++++++------- .../armeria/common/JsonHeadersSanitizer.java | 10 ++++++---- .../armeria/common/TextHeadersSanitizer.java | 18 +++++++++--------- 3 files changed, 22 insertions(+), 20 deletions(-) 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 bc8b1f66248..7c7b9ea7941 100644 --- a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java @@ -22,19 +22,19 @@ import java.util.Set; import java.util.function.Function; -import com.google.common.base.Ascii; import com.google.common.collect.ImmutableSet; +import io.netty.util.AsciiString; + /** * A skeletal builder implementation for {@link HeadersSanitizer}. */ public abstract class AbstractHeadersSanitizerBuilder { - private static final Set DEFAULT_MASKING_HEADERS = - ImmutableSet.of(Ascii.toLowerCase(HttpHeaderNames.AUTHORIZATION), - Ascii.toLowerCase(HttpHeaderNames.SET_COOKIE)); + private static final Set DEFAULT_MASKING_HEADERS = + ImmutableSet.of(HttpHeaderNames.AUTHORIZATION, HttpHeaderNames.SET_COOKIE); - private final Set maskingHeaders = new HashSet<>(); + private final Set maskingHeaders = new HashSet<>(); private Function maskingFunction = header -> "****"; @@ -51,11 +51,11 @@ public AbstractHeadersSanitizerBuilder maskingHeaders(CharSequence... headers */ public AbstractHeadersSanitizerBuilder maskingHeaders(Iterable headers) { requireNonNull(headers, "headers"); - headers.forEach(header -> maskingHeaders.add(Ascii.toLowerCase(header))); + headers.forEach(header -> maskingHeaders.add(AsciiString.of(header).toLowerCase())); return this; } - final Set maskingHeaders() { + final Set maskingHeaders() { if (!maskingHeaders.isEmpty()) { return ImmutableSet.copyOf(maskingHeaders); } 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 2102bf62f8e..4659e453fa5 100644 --- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java @@ -25,17 +25,19 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; +import io.netty.util.AsciiString; + /** * A sanitizer that sanitizes {@link HttpHeaders} and returns {@link JsonNode}. */ final class JsonHeadersSanitizer implements HeadersSanitizer { static final HeadersSanitizer INSTANCE = new JsonHeadersSanitizerBuilder().build(); - private final Set maskingHeaders; + private final Set maskingHeaders; private final Function maskingFunction; private final ObjectMapper objectMapper; - JsonHeadersSanitizer(Set maskingHeaders, Function maskingFunction, + JsonHeadersSanitizer(Set maskingHeaders, Function maskingFunction, ObjectMapper objectMapper) { this.maskingHeaders = maskingHeaders; this.maskingFunction = maskingFunction; @@ -46,8 +48,8 @@ final class JsonHeadersSanitizer implements HeadersSanitizer { public JsonNode apply(RequestContext requestContext, HttpHeaders headers) { final ObjectNode result = objectMapper.createObjectNode(); maskHeaders(headers, maskingHeaders, maskingFunction, - (header, values) -> result.put(header, values.size() > 1 ? - values.toString() : values.get(0))); + (header, values) -> result.put(header.toString(), values.size() > 1 ? + values.toString() : values.get(0))); return result; } 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 fbff9674568..242ed47e645 100644 --- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java @@ -33,11 +33,11 @@ final class TextHeadersSanitizer implements HeadersSanitizer { static final HeadersSanitizer INSTANCE = new TextHeadersSanitizerBuilder().build(); - private final Set maskingHeaders; + private final Set maskingHeaders; private final Function maskingFunction; - TextHeadersSanitizer(Set maskingHeaders, Function maskingFunction) { + TextHeadersSanitizer(Set maskingHeaders, Function maskingFunction) { this.maskingHeaders = maskingHeaders; this.maskingFunction = maskingFunction; } @@ -65,19 +65,19 @@ public String apply(RequestContext ctx, HttpHeaders headers) { } static void maskHeaders( - HttpHeaders headers, Set maskingHeaders, Function maskingFunction, - final BiConsumer> consumer) { - final Map> headersWithValuesAsList = new LinkedHashMap<>(); + HttpHeaders headers, Set maskingHeaders, Function maskingFunction, + final BiConsumer> consumer) { + final Map> headersWithValuesAsList = new LinkedHashMap<>(); for (Map.Entry entry : headers) { - final String header = entry.getKey().toString().toLowerCase(); + final AsciiString header = entry.getKey().toLowerCase(); final String value = maskingHeaders.contains(header) ? maskingFunction.apply(entry.getValue()) : entry.getValue(); headersWithValuesAsList.computeIfAbsent(header, k -> new ArrayList<>()).add(value); } - final Set>> entries = headersWithValuesAsList.entrySet(); - for (Map.Entry> entry : entries) { - final String header = entry.getKey(); + final Set>> entries = headersWithValuesAsList.entrySet(); + for (Map.Entry> entry : entries) { + final AsciiString header = entry.getKey(); final List values = entry.getValue(); consumer.accept(header, values); } From d7dbffcb2582bb3b140da31f8f03cea120ef3b58 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Mon, 29 Jan 2024 18:41:20 +0900 Subject: [PATCH 07/16] BiFunction was replaced with HeadersSanitizer --- .../armeria/common/HeadersSanitizer.java | 15 +++ .../armeria/common/JsonHeadersSanitizer.java | 2 +- .../armeria/common/TextHeadersSanitizer.java | 33 ++++--- .../logging/AbstractLogFormatterBuilder.java | 96 ++++++++++++++++--- .../common/logging/JsonLogFormatter.java | 26 ++--- .../logging/JsonLogFormatterBuilder.java | 9 +- .../common/logging/TextLogFormatter.java | 26 ++--- .../logging/TextLogFormatterBuilder.java | 8 +- 8 files changed, 147 insertions(+), 68 deletions(-) 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 1656a6100ef..4dd290b31a7 100644 --- a/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java @@ -20,6 +20,8 @@ import com.fasterxml.jackson.databind.JsonNode; +import com.linecorp.armeria.common.annotation.Nullable; + /** * A sanitizer that sanitizes {@link HttpHeaders}. */ @@ -52,4 +54,17 @@ static HeadersSanitizer ofJson() { static JsonHeadersSanitizerBuilder builderForJson() { return new JsonHeadersSanitizerBuilder(); } + + /** + * Sanitizes the specified {@link HttpHeaders}. + * If {@code null} is returned, the specified {@link HttpHeaders} will be excluded from the log. + */ + @Nullable + T sanitize(RequestContext requestContext, HttpHeaders headers); + + @Nullable + @Override + default T apply(RequestContext requestContext, HttpHeaders entries) { + return sanitize(requestContext, entries); + } } 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 4659e453fa5..9ec5f1a6b2b 100644 --- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java @@ -45,7 +45,7 @@ final class JsonHeadersSanitizer implements HeadersSanitizer { } @Override - public JsonNode apply(RequestContext requestContext, HttpHeaders headers) { + public JsonNode sanitize(RequestContext requestContext, HttpHeaders headers) { final ObjectNode result = objectMapper.createObjectNode(); maskHeaders(headers, maskingHeaders, maskingFunction, (header, values) -> result.put(header.toString(), values.size() > 1 ? 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 242ed47e645..415b2172073 100644 --- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java +++ b/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java @@ -16,14 +16,15 @@ package com.linecorp.armeria.common; -import java.util.ArrayList; -import java.util.LinkedHashMap; +import static com.google.common.collect.ImmutableList.toImmutableList; + import java.util.List; -import java.util.Map; import java.util.Set; import java.util.function.BiConsumer; import java.util.function.Function; +import com.google.common.collect.ImmutableList; + import io.netty.util.AsciiString; /** @@ -43,7 +44,7 @@ final class TextHeadersSanitizer implements HeadersSanitizer { } @Override - public String apply(RequestContext ctx, HttpHeaders headers) { + public String sanitize(RequestContext ctx, HttpHeaders headers) { if (headers.isEmpty()) { return headers.isEndOfStream() ? "[EOS]" : "[]"; } @@ -67,19 +68,17 @@ public String apply(RequestContext ctx, HttpHeaders headers) { static void maskHeaders( HttpHeaders headers, Set maskingHeaders, Function maskingFunction, final BiConsumer> consumer) { - final Map> headersWithValuesAsList = new LinkedHashMap<>(); - for (Map.Entry entry : headers) { - final AsciiString header = entry.getKey().toLowerCase(); - final String value = maskingHeaders.contains(header) ? maskingFunction.apply(entry.getValue()) - : entry.getValue(); - headersWithValuesAsList.computeIfAbsent(header, k -> new ArrayList<>()).add(value); - } - - final Set>> entries = headersWithValuesAsList.entrySet(); - for (Map.Entry> entry : entries) { - final AsciiString header = entry.getKey(); - final List values = entry.getValue(); - consumer.accept(header, values); + for (AsciiString headerName : headers.names()) { + List values = headers.getAll(headerName); + if (maskingHeaders.contains(headerName)) { + // Mask the header values. + if (values.size() == 1) { + values = ImmutableList.of(maskingFunction.apply(values.get(0))); + } else { + values = values.stream().map(maskingFunction).collect(toImmutableList()); + } + } + consumer.accept(headerName, values); } } } 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 fe895bd3a6b..08bf5bf3c39 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 @@ -21,6 +21,7 @@ import java.util.function.BiFunction; import java.util.function.Function; +import com.linecorp.armeria.common.HeadersSanitizer; import com.linecorp.armeria.common.HttpHeaders; import com.linecorp.armeria.common.RequestContext; import com.linecorp.armeria.common.annotation.Nullable; @@ -31,16 +32,16 @@ abstract class AbstractLogFormatterBuilder { @Nullable - private BiFunction requestHeadersSanitizer; + private HeadersSanitizer requestHeadersSanitizer; @Nullable - private BiFunction responseHeadersSanitizer; + private HeadersSanitizer responseHeadersSanitizer; @Nullable - private BiFunction requestTrailersSanitizer; + private HeadersSanitizer requestTrailersSanitizer; @Nullable - private BiFunction responseTrailersSanitizer; + private HeadersSanitizer responseTrailersSanitizer; @Nullable private BiFunction requestContentSanitizer; @@ -52,10 +53,26 @@ abstract class AbstractLogFormatterBuilder { * 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 * not sanitize request headers. + * + * 
{@code
+     * HeadersSanitizer headersSanitizer =
+     *   HeadersSanitizer
+     *     .builderForText()
+     *     .maskingHeaders("Authorization", "Cookie")
+     *     ...
+     *     .build();
+     *
+     *  LogFormatter
+     *    .builderForText()
+     *    .requestHeadersSanitizer(headersSanitizer)
+     *    ...
+     * }
*/ public AbstractLogFormatterBuilder requestHeadersSanitizer( BiFunction requestHeadersSanitizer) { - this.requestHeadersSanitizer = requireNonNull(requestHeadersSanitizer, "requestHeadersSanitizer"); + requireNonNull(requestHeadersSanitizer, "requestHeadersSanitizer"); + // TODO(ikhoon): Replace BiFunction with HeadersSanitizer in Armeria 2.0. + this.requestHeadersSanitizer = requestHeadersSanitizer::apply; return this; } @@ -63,7 +80,7 @@ public AbstractLogFormatterBuilder requestHeadersSanitizer( * Returns the {@link BiFunction} to use to sanitize request headers before logging. */ @Nullable - final BiFunction requestHeadersSanitizer() { + final HeadersSanitizer requestHeadersSanitizer() { return requestHeadersSanitizer; } @@ -71,10 +88,26 @@ public AbstractLogFormatterBuilder requestHeadersSanitizer( * Sets the {@link BiFunction} to use to sanitize response headers before logging. It is common to have the * {@link BiFunction} that removes sensitive headers, like {@code Set-Cookie}, before logging. If unset, * will not sanitize response headers. + * + * 
{@code
+     * HeadersSanitizer headersSanitizer =
+     *   HeadersSanitizer
+     *     .builderForText()
+     *     .maskingHeaders("Set-Cookie")
+     *     ...
+     *     .build();
+     *
+     *  LogFormatter
+     *    .builderForText()
+     *    .responseHeadersSanitizer(headersSanitizer)
+     *    ...
+     * }
*/ public AbstractLogFormatterBuilder responseHeadersSanitizer( BiFunction responseHeadersSanitizer) { - this.responseHeadersSanitizer = requireNonNull(responseHeadersSanitizer, "responseHeadersSanitizer"); + // TODO(ikhoon): Replace BiFunction with HeadersSanitizer in Armeria 2.0. + requireNonNull(responseHeadersSanitizer, "responseHeadersSanitizer"); + this.responseHeadersSanitizer = responseHeadersSanitizer::apply; return this; } @@ -82,17 +115,33 @@ public AbstractLogFormatterBuilder responseHeadersSanitizer( * Returns the {@link BiFunction} to use to sanitize response headers before logging. */ @Nullable - final BiFunction responseHeadersSanitizer() { + final HeadersSanitizer responseHeadersSanitizer() { return responseHeadersSanitizer; } /** * Sets the {@link BiFunction} to use to sanitize request trailers before logging. If unset, * will not sanitize request trailers. + * + * 
{@code
+     * HeadersSanitizer headersSanitizer =
+     *   HeadersSanitizer
+     *     .builderForText()
+     *     .maskingHeaders("...")
+     *     ...
+     *     .build();
+     *
+     *  LogFormatter
+     *    .builderForText()
+     *    .requestTrailersSanitizer(headersSanitizer)
+     *    ...
+     * }
*/ public AbstractLogFormatterBuilder requestTrailersSanitizer( BiFunction requestTrailersSanitizer) { - this.requestTrailersSanitizer = requireNonNull(requestTrailersSanitizer, "requestTrailersSanitizer"); + // TODO(ikhoon): Replace BiFunction with HeadersSanitizer in Armeria 2.0. + requireNonNull(requestTrailersSanitizer, "requestTrailersSanitizer"); + this.requestTrailersSanitizer = requestTrailersSanitizer::apply; return this; } @@ -100,17 +149,33 @@ public AbstractLogFormatterBuilder requestTrailersSanitizer( * Returns the {@link BiFunction} to use to sanitize request trailers before logging. */ @Nullable - final BiFunction requestTrailersSanitizer() { + final HeadersSanitizer requestTrailersSanitizer() { return requestTrailersSanitizer; } /** * Sets the {@link BiFunction} to use to sanitize response trailers before logging. If unset, * will not sanitize response trailers. + * + * 
{@code
+     * HeadersSanitizer headersSanitizer =
+     *   HeadersSanitizer
+     *     .builderForText()
+     *     .maskingHeaders("...")
+     *     ...
+     *     .build();
+     *
+     *  LogFormatter
+     *    .builderForText()
+     *    .responseTrailersSanitizer(headersSanitizer)
+     *    ...
+     * }
*/ public AbstractLogFormatterBuilder responseTrailersSanitizer( BiFunction responseTrailersSanitizer) { - this.responseTrailersSanitizer = requireNonNull(responseTrailersSanitizer, "responseTrailersSanitizer"); + // TODO(ikhoon): Replace BiFunction with HeadersSanitizer in Armeria 2.0. + requireNonNull(responseTrailersSanitizer, "responseTrailersSanitizer"); + this.responseTrailersSanitizer = responseTrailersSanitizer::apply; return this; } @@ -118,7 +183,7 @@ public AbstractLogFormatterBuilder responseTrailersSanitizer( * Returns the {@link Function} to use to sanitize response trailers before logging. */ @Nullable - final BiFunction responseTrailersSanitizer() { + final HeadersSanitizer responseTrailersSanitizer() { return responseTrailersSanitizer; } @@ -127,6 +192,13 @@ public AbstractLogFormatterBuilder responseTrailersSanitizer( * It is common to have the {@link BiFunction} that removes sensitive headers, like {@code "Cookie"} and * {@code "Set-Cookie"}, before logging. This method is a shortcut for: * 
{@code
+     * HeadersSanitizer headersSanitizer =
+     *   HeadersSanitizer
+     *     .builderForText()
+     *     .maskingHeaders("...")
+     *     ...
+     *     .build();
+     *
      * builder.requestHeadersSanitizer(headersSanitizer);
      * builder.requestTrailersSanitizer(headersSanitizer);
      * builder.responseHeadersSanitizer(headersSanitizer);
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 7286334c24a..ca2c821a537 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
@@ -29,7 +29,7 @@
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.base.MoreObjects;
 
-import com.linecorp.armeria.common.HttpHeaders;
+import com.linecorp.armeria.common.HeadersSanitizer;
 import com.linecorp.armeria.common.RequestContext;
 import com.linecorp.armeria.common.SerializationFormat;
 import com.linecorp.armeria.common.annotation.Nullable;
@@ -46,17 +46,13 @@ final class JsonLogFormatter implements LogFormatter {
 
     static final LogFormatter DEFAULT_INSTANCE = new JsonLogFormatterBuilder().build();
 
-    private final BiFunction
-            requestHeadersSanitizer;
+    private final HeadersSanitizer requestHeadersSanitizer;
 
-    private final BiFunction
-            responseHeadersSanitizer;
+    private final HeadersSanitizer responseHeadersSanitizer;
 
-    private final BiFunction
-            requestTrailersSanitizer;
+    private final HeadersSanitizer requestTrailersSanitizer;
 
-    private final BiFunction
-            responseTrailersSanitizer;
+    private final HeadersSanitizer responseTrailersSanitizer;
 
     private final BiFunction
             requestContentSanitizer;
@@ -67,14 +63,10 @@ final class JsonLogFormatter implements LogFormatter {
     private final ObjectMapper objectMapper;
 
     JsonLogFormatter(
-            BiFunction requestHeadersSanitizer,
-            BiFunction responseHeadersSanitizer,
-            BiFunction requestTrailersSanitizer,
-            BiFunction responseTrailersSanitizer,
+            HeadersSanitizer requestHeadersSanitizer,
+            HeadersSanitizer responseHeadersSanitizer,
+            HeadersSanitizer requestTrailersSanitizer,
+            HeadersSanitizer responseTrailersSanitizer,
             BiFunction requestContentSanitizer,
             BiFunction responseContentSanitizer,
             ObjectMapper objectMapper) {
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 db14cc740ae..8ee9e9e14dc 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
@@ -109,8 +109,8 @@ public JsonLogFormatterBuilder contentSanitizer(
     public LogFormatter build() {
         final ObjectMapper objectMapper = this.objectMapper != null ?
                                           this.objectMapper : JacksonUtil.newDefaultObjectMapper();
-        final BiFunction defaultHeadersSanitizer =
-                defaultSanitizer(objectMapper);
+        final HeadersSanitizer defaultHeadersSanitizer =
+                defaultHeadersSanitizer(objectMapper);
         final BiFunction defaultContentSanitizer =
                 defaultSanitizer(objectMapper);
         return new JsonLogFormatter(
@@ -127,4 +127,9 @@ public LogFormatter build() {
     defaultSanitizer(ObjectMapper objectMapper) {
         return (requestContext, obj) -> objectMapper.valueToTree(obj);
     }
+
+    private static HeadersSanitizer
+    defaultHeadersSanitizer(ObjectMapper objectMapper) {
+        return (requestContext, obj) -> objectMapper.valueToTree(obj);
+    }
 }
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 be19207c8a3..6c419f813a9 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
@@ -23,7 +23,7 @@
 
 import com.google.common.base.MoreObjects;
 
-import com.linecorp.armeria.common.HttpHeaders;
+import com.linecorp.armeria.common.HeadersSanitizer;
 import com.linecorp.armeria.common.RequestContext;
 import com.linecorp.armeria.common.SerializationFormat;
 import com.linecorp.armeria.common.annotation.Nullable;
@@ -39,17 +39,13 @@ final class TextLogFormatter implements LogFormatter {
 
     static final LogFormatter DEFAULT_INSTANCE = new TextLogFormatterBuilder().build();
 
-    private final BiFunction requestHeadersSanitizer;
+    private final HeadersSanitizer requestHeadersSanitizer;
 
-    private final BiFunction responseHeadersSanitizer;
+    private final HeadersSanitizer responseHeadersSanitizer;
 
-    private final BiFunction requestTrailersSanitizer;
+    private final HeadersSanitizer requestTrailersSanitizer;
 
-    private final BiFunction responseTrailersSanitizer;
+    private final HeadersSanitizer responseTrailersSanitizer;
 
     private final BiFunction requestContentSanitizer;
@@ -60,14 +56,10 @@ final class TextLogFormatter implements LogFormatter {
     private final boolean includeContext;
 
     TextLogFormatter(
-            BiFunction requestHeadersSanitizer,
-            BiFunction responseHeadersSanitizer,
-            BiFunction requestTrailersSanitizer,
-            BiFunction responseTrailersSanitizer,
+            HeadersSanitizer requestHeadersSanitizer,
+            HeadersSanitizer responseHeadersSanitizer,
+            HeadersSanitizer requestTrailersSanitizer,
+            HeadersSanitizer responseTrailersSanitizer,
             BiFunction requestContentSanitizer,
             BiFunction BiFunction defaultSanitizer() {
         return (requestContext, object) -> object.toString();
     }
+
+    private static HeadersSanitizer defaultHeadersSanitizer() {
+        return (requestContext, object) -> object.toString();
+    }
 }

From d2bd0a77930986a978ddc705335db5e6d7634abd Mon Sep 17 00:00:00 2001
From: Ikhun Um 
Date: Mon, 29 Jan 2024 19:34:15 +0900
Subject: [PATCH 08/16] Update the default sensitive headers

---
 .../armeria/common/AbstractHeadersSanitizerBuilder.java        | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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 7c7b9ea7941..7fb42fd7870 100644
--- a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java
+++ b/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java
@@ -32,7 +32,8 @@
 public abstract class AbstractHeadersSanitizerBuilder {
 
     private static final Set DEFAULT_MASKING_HEADERS =
-            ImmutableSet.of(HttpHeaderNames.AUTHORIZATION, HttpHeaderNames.SET_COOKIE);
+            ImmutableSet.of(HttpHeaderNames.AUTHORIZATION, HttpHeaderNames.COOKIE,
+                            HttpHeaderNames.SET_COOKIE, HttpHeaderNames.PROXY_AUTHORIZATION);
 
     private final Set maskingHeaders = new HashSet<>();
 

From cecbc3bf451b5576fb71ab26e23accb2330bbc87 Mon Sep 17 00:00:00 2001
From: Ikhun Um 
Date: Mon, 29 Jan 2024 19:35:12 +0900
Subject: [PATCH 09/16] add reference

---
 .../armeria/common/AbstractHeadersSanitizerBuilder.java        | 3 +++
 1 file changed, 3 insertions(+)

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 7fb42fd7870..fae67a3a4cb 100644
--- a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java
+++ b/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java
@@ -31,6 +31,9 @@
  */
 public abstract class AbstractHeadersSanitizerBuilder {
 
+    // Referenced from:
+    // - https://docs.rs/tower-http/latest/tower_http/sensitive_headers/index.html
+    // - https://techdocs.akamai.com/edge-diagnostics/reference/sensitive-headers
     private static final Set DEFAULT_MASKING_HEADERS =
             ImmutableSet.of(HttpHeaderNames.AUTHORIZATION, HttpHeaderNames.COOKIE,
                             HttpHeaderNames.SET_COOKIE, HttpHeaderNames.PROXY_AUTHORIZATION);

From c1161fc34b75b788eece510496566748274e2da4 Mon Sep 17 00:00:00 2001
From: Ikhun Um 
Date: Mon, 29 Jan 2024 19:36:16 +0900
Subject: [PATCH 10/16] final.final.final

---
 .../linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java | 1 +
 1 file changed, 1 insertion(+)

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 fae67a3a4cb..89a3e83b030 100644
--- a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java
+++ b/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java
@@ -34,6 +34,7 @@ public abstract class AbstractHeadersSanitizerBuilder {
     // Referenced from:
     // - https://docs.rs/tower-http/latest/tower_http/sensitive_headers/index.html
     // - https://techdocs.akamai.com/edge-diagnostics/reference/sensitive-headers
+    // - https://cloud.spring.io/spring-cloud-netflix/multi/multi__router_and_filter_zuul.html#_cookies_and_sensitive_headers
     private static final Set DEFAULT_MASKING_HEADERS =
             ImmutableSet.of(HttpHeaderNames.AUTHORIZATION, HttpHeaderNames.COOKIE,
                             HttpHeaderNames.SET_COOKIE, HttpHeaderNames.PROXY_AUTHORIZATION);

From 042141ac02d1fdaf5ee3d2c4eefa4cce2cd4acda Mon Sep 17 00:00:00 2001
From: Ikhun Um 
Date: Mon, 29 Jan 2024 20:38:26 +0900
Subject: [PATCH 11/16] Javadoc for the default sensitive headers

---
 .../java/com/linecorp/armeria/common/HeadersSanitizer.java    | 4 ++++
 1 file changed, 4 insertions(+)

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 4dd290b31a7..bfd3b3a9e35 100644
--- a/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java
+++ b/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java
@@ -29,6 +29,8 @@
 public interface HeadersSanitizer extends BiFunction {
     /**
      * Returns the default text {@link HeadersSanitizer}.
+     * {@link HttpHeaderNames#AUTHORIZATION}, {@link HttpHeaderNames#COOKIE}, {@link HttpHeaderNames#SET_COOKIE},
+     * and {@link HttpHeaderNames#PROXY_AUTHORIZATION} are masked.
      */
     static HeadersSanitizer ofText() {
         return TextHeadersSanitizer.INSTANCE;
@@ -43,6 +45,8 @@ static TextHeadersSanitizerBuilder builderForText() {
 
     /**
      * Returns the default json {@link HeadersSanitizer}.
+     * {@link HttpHeaderNames#AUTHORIZATION}, {@link HttpHeaderNames#COOKIE}, {@link HttpHeaderNames#SET_COOKIE},
+     * and {@link HttpHeaderNames#PROXY_AUTHORIZATION} are masked.
      */
     static HeadersSanitizer ofJson() {
         return JsonHeadersSanitizer.INSTANCE;

From b7697b5fe4618f4014ccc729abee12cf29d4a8fb Mon Sep 17 00:00:00 2001
From: Ikhun Um 
Date: Tue, 30 Jan 2024 00:39:28 +0900
Subject: [PATCH 12/16] Introduce `HeaderMaskingFunction`

- A header name is necessary to dynamically mask header values.
- Handle a null value returned by masking functions.
- Replace Function logging}/HeadersSanitizer.java       | 17 ++++---
 .../{ => logging}/JsonHeadersSanitizer.java   | 12 +++--
 .../JsonHeadersSanitizerBuilder.java          |  6 +--
 .../common/logging/JsonLogFormatter.java      |  1 -
 .../logging/JsonLogFormatterBuilder.java      |  1 -
 .../{ => logging}/TextHeadersSanitizer.java   | 33 ++++++++----
 .../TextHeadersSanitizerBuilder.java          |  6 +--
 .../common/logging/TextLogFormatter.java      |  1 -
 .../logging/TextLogFormatterBuilder.java      |  1 -
 .../common/logging/JsonLogFormatterTest.java  | 44 +++++++++++++---
 .../common/logging/TextLogFormatterTest.java  | 51 +++++++++++++++----
 14 files changed, 185 insertions(+), 56 deletions(-)
 rename core/src/main/java/com/linecorp/armeria/common/{ => logging}/AbstractHeadersSanitizerBuilder.java (77%)
 create mode 100644 core/src/main/java/com/linecorp/armeria/common/logging/HeaderMaskingFunction.java
 rename core/src/main/java/com/linecorp/armeria/common/{ => logging}/HeadersSanitizer.java (78%)
 rename core/src/main/java/com/linecorp/armeria/common/{ => logging}/JsonHeadersSanitizer.java (82%)
 rename core/src/main/java/com/linecorp/armeria/common/{ => logging}/JsonHeadersSanitizerBuilder.java (93%)
 rename core/src/main/java/com/linecorp/armeria/common/{ => logging}/TextHeadersSanitizer.java (68%)
 rename core/src/main/java/com/linecorp/armeria/common/{ => logging}/TextHeadersSanitizerBuilder.java (89%)

diff --git a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
similarity index 77%
rename from core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java
rename to core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
index 89a3e83b030..db7367928e6 100644
--- a/core/src/main/java/com/linecorp/armeria/common/AbstractHeadersSanitizerBuilder.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
@@ -14,7 +14,7 @@
  * under the License.
  */
 
-package com.linecorp.armeria.common;
+package com.linecorp.armeria.common.logging;
 
 import static java.util.Objects.requireNonNull;
 
@@ -24,6 +24,8 @@
 
 import com.google.common.collect.ImmutableSet;
 
+import com.linecorp.armeria.common.HttpHeaderNames;
+
 import io.netty.util.AsciiString;
 
 /**
@@ -41,7 +43,9 @@ public abstract class AbstractHeadersSanitizerBuilder {
 
     private final Set maskingHeaders = new HashSet<>();
 
-    private Function maskingFunction = header -> "****";
+    private HeaderMaskingFunction maskingFunction = HeaderMaskingFunction.of();
+
+    AbstractHeadersSanitizerBuilder() {}
 
     /**
      * Sets the headers to mask before logging.
@@ -69,8 +73,21 @@ final Set maskingHeaders() {
 
     /**
      * Sets the {@link Function} to use to maskFunction headers before logging.
+     * The default maskingFunction is {@link HeaderMaskingFunction#of()}
+     *
+     * 
{@code
+     * builder.maskingFunction((name, value) -> {
+     *   if (name.equals(HttpHeaderNames.AUTHORIZATION)) {
+     *      return "****";
+     *   } else if (name.equals(HttpHeaderNames.COOKIE)) {
+     *     return name.substring(0, 4) + "****";
+     *   } else {
+     *     return value;
+     *   }
+     * }
+     * }

      */
-    public AbstractHeadersSanitizerBuilder maskingFunction(Function maskingFunction) {
+    public AbstractHeadersSanitizerBuilder maskingFunction(HeaderMaskingFunction maskingFunction) {
         this.maskingFunction = requireNonNull(maskingFunction, "maskingFunction");
         return this;
     }
@@ -78,7 +95,7 @@ public AbstractHeadersSanitizerBuilder maskingFunction(Function maskingFunction() {
+    final HeaderMaskingFunction maskingFunction() {
         return maskingFunction;
     }
 }
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 08bf5bf3c39..77eba904ddd 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
@@ -21,7 +21,6 @@
 import java.util.function.BiFunction;
 import java.util.function.Function;
 
-import com.linecorp.armeria.common.HeadersSanitizer;
 import com.linecorp.armeria.common.HttpHeaders;
 import com.linecorp.armeria.common.RequestContext;
 import com.linecorp.armeria.common.annotation.Nullable;
diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/HeaderMaskingFunction.java b/core/src/main/java/com/linecorp/armeria/common/logging/HeaderMaskingFunction.java
new file mode 100644
index 00000000000..c665f2446cb
--- /dev/null
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/HeaderMaskingFunction.java
@@ -0,0 +1,42 @@
+/*
+ *  Copyright 2024 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.logging;
+
+import com.linecorp.armeria.common.annotation.Nullable;
+
+import io.netty.util.AsciiString;
+
+/**
+ * A function that masks the specified header value.
+ */
+@FunctionalInterface
+public interface HeaderMaskingFunction {
+
+    /**
+     * Returns the default {@link HeaderMaskingFunction} that masks the given value with {@code ****}.
+     */
+    static HeaderMaskingFunction of() {
+        return (name, value) -> "****";
+    }
+
+    /**
+     * Masks the specified {@code value} of the specified {@code name}.
+     * If {@code null} is returned, the specified {@code value} will be removed from the log.
+     */
+    @Nullable
+    String mask(AsciiString name, String value);
+}
diff --git a/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java b/core/src/main/java/com/linecorp/armeria/common/logging/HeadersSanitizer.java
similarity index 78%
rename from core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java
rename to core/src/main/java/com/linecorp/armeria/common/logging/HeadersSanitizer.java
index bfd3b3a9e35..4811b7fa78d 100644
--- a/core/src/main/java/com/linecorp/armeria/common/HeadersSanitizer.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/HeadersSanitizer.java
@@ -14,12 +14,15 @@
  * under the License.
  */
 
-package com.linecorp.armeria.common;
+package com.linecorp.armeria.common.logging;
 
 import java.util.function.BiFunction;
 
 import com.fasterxml.jackson.databind.JsonNode;
 
+import com.linecorp.armeria.common.HttpHeaderNames;
+import com.linecorp.armeria.common.HttpHeaders;
+import com.linecorp.armeria.common.RequestContext;
 import com.linecorp.armeria.common.annotation.Nullable;
 
 /**
@@ -28,9 +31,9 @@
 @FunctionalInterface
 public interface HeadersSanitizer extends BiFunction {
     /**
-     * Returns the default text {@link HeadersSanitizer}.
-     * {@link HttpHeaderNames#AUTHORIZATION}, {@link HttpHeaderNames#COOKIE}, {@link HttpHeaderNames#SET_COOKIE},
-     * and {@link HttpHeaderNames#PROXY_AUTHORIZATION} are masked.
+     * Returns the default text {@link HeadersSanitizer} that masks
+     * {@link HttpHeaderNames#AUTHORIZATION}, {@link HttpHeaderNames#COOKIE},
+     * {@link HttpHeaderNames#SET_COOKIE}, and {@link HttpHeaderNames#PROXY_AUTHORIZATION} with {@code ****}.
      */
     static HeadersSanitizer ofText() {
         return TextHeadersSanitizer.INSTANCE;
@@ -44,9 +47,9 @@ static TextHeadersSanitizerBuilder builderForText() {
     }
 
     /**
-     * Returns the default json {@link HeadersSanitizer}.
-     * {@link HttpHeaderNames#AUTHORIZATION}, {@link HttpHeaderNames#COOKIE}, {@link HttpHeaderNames#SET_COOKIE},
-     * and {@link HttpHeaderNames#PROXY_AUTHORIZATION} are masked.
+     * Returns the default JSON {@link HeadersSanitizer} that masks
+     * {@link HttpHeaderNames#AUTHORIZATION}, {@link HttpHeaderNames#COOKIE},
+     * {@link HttpHeaderNames#SET_COOKIE}, and {@link HttpHeaderNames#PROXY_AUTHORIZATION} with {@code ****}.
      */
     static HeadersSanitizer ofJson() {
         return JsonHeadersSanitizer.INSTANCE;
diff --git a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java b/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizer.java
similarity index 82%
rename from core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java
rename to core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizer.java
index 9ec5f1a6b2b..f9d7dab41da 100644
--- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizer.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizer.java
@@ -14,17 +14,19 @@
  * under the License.
  */
 
-package com.linecorp.armeria.common;
+package com.linecorp.armeria.common.logging;
 
-import static com.linecorp.armeria.common.TextHeadersSanitizer.maskHeaders;
+import static com.linecorp.armeria.common.logging.TextHeadersSanitizer.maskHeaders;
 
 import java.util.Set;
-import java.util.function.Function;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 
+import com.linecorp.armeria.common.HttpHeaders;
+import com.linecorp.armeria.common.RequestContext;
+
 import io.netty.util.AsciiString;
 
 /**
@@ -34,10 +36,10 @@ final class JsonHeadersSanitizer implements HeadersSanitizer {
 
     static final HeadersSanitizer INSTANCE = new JsonHeadersSanitizerBuilder().build();
     private final Set maskingHeaders;
-    private final Function maskingFunction;
+    private final HeaderMaskingFunction maskingFunction;
     private final ObjectMapper objectMapper;
 
-    JsonHeadersSanitizer(Set maskingHeaders, Function maskingFunction,
+    JsonHeadersSanitizer(Set maskingHeaders, HeaderMaskingFunction maskingFunction,
                          ObjectMapper objectMapper) {
         this.maskingHeaders = maskingHeaders;
         this.maskingFunction = maskingFunction;
diff --git a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizerBuilder.java
similarity index 93%
rename from core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java
rename to core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizerBuilder.java
index bdee7fc66e7..cd8440af1cc 100644
--- a/core/src/main/java/com/linecorp/armeria/common/JsonHeadersSanitizerBuilder.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizerBuilder.java
@@ -14,12 +14,10 @@
  * under the License.
  */
 
-package com.linecorp.armeria.common;
+package com.linecorp.armeria.common.logging;
 
 import static java.util.Objects.requireNonNull;
 
-import java.util.function.Function;
-
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 
@@ -45,7 +43,7 @@ public JsonHeadersSanitizerBuilder maskingHeaders(Iterable maskingFunction) {
+    public JsonHeadersSanitizerBuilder maskingFunction(HeaderMaskingFunction maskingFunction) {
         return (JsonHeadersSanitizerBuilder) super.maskingFunction(maskingFunction);
     }
 
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 ca2c821a537..e6a25507999 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
@@ -29,7 +29,6 @@
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.base.MoreObjects;
 
-import com.linecorp.armeria.common.HeadersSanitizer;
 import com.linecorp.armeria.common.RequestContext;
 import com.linecorp.armeria.common.SerializationFormat;
 import com.linecorp.armeria.common.annotation.Nullable;
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 8ee9e9e14dc..d3bfc634976 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,6 @@
 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.RequestContext;
 import com.linecorp.armeria.common.annotation.Nullable;
diff --git a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java b/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizer.java
similarity index 68%
rename from core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java
rename to core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizer.java
index 415b2172073..c089dffacd1 100644
--- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizer.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizer.java
@@ -14,17 +14,20 @@
  * under the License.
  */
 
-package com.linecorp.armeria.common;
+package com.linecorp.armeria.common.logging;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.BiConsumer;
-import java.util.function.Function;
 
 import com.google.common.collect.ImmutableList;
 
+import com.linecorp.armeria.common.HttpHeaders;
+import com.linecorp.armeria.common.RequestContext;
+
 import io.netty.util.AsciiString;
 
 /**
@@ -36,9 +39,10 @@ final class TextHeadersSanitizer implements HeadersSanitizer {
 
     private final Set maskingHeaders;
 
-    private final Function maskingFunction;
+    private final HeaderMaskingFunction maskingFunction;
 
-    TextHeadersSanitizer(Set maskingHeaders, Function maskingFunction) {
+    TextHeadersSanitizer(Set maskingHeaders,
+                         HeaderMaskingFunction maskingFunction) {
         this.maskingHeaders = maskingHeaders;
         this.maskingFunction = maskingFunction;
     }
@@ -66,19 +70,30 @@ public String sanitize(RequestContext ctx, HttpHeaders headers) {
     }
 
     static void maskHeaders(
-            HttpHeaders headers, Set maskingHeaders, Function maskingFunction,
-            final BiConsumer> consumer) {
+            HttpHeaders headers, Set maskingHeaders,
+            HeaderMaskingFunction maskingFunction,
+            BiConsumer> consumer) {
         for (AsciiString headerName : headers.names()) {
             List values = headers.getAll(headerName);
             if (maskingHeaders.contains(headerName)) {
                 // Mask the header values.
                 if (values.size() == 1) {
-                    values = ImmutableList.of(maskingFunction.apply(values.get(0)));
+                    final String masked = maskingFunction.mask(headerName, values.get(0));
+                    if (masked == null) {
+                        values = ImmutableList.of();
+                    } else {
+                        values = ImmutableList.of(masked);
+                    }
                 } else {
-                    values = values.stream().map(maskingFunction).collect(toImmutableList());
+                    values = values.stream()
+                                   .map(value -> maskingFunction.mask(headerName, value))
+                                   .filter(Objects::nonNull)
+                                   .collect(toImmutableList());
                 }
             }
-            consumer.accept(headerName, values);
+            if (!values.isEmpty()) {
+                consumer.accept(headerName, values);
+            }
         }
     }
 }
diff --git a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizerBuilder.java
similarity index 89%
rename from core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java
rename to core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizerBuilder.java
index d648577db50..c84353bb189 100644
--- a/core/src/main/java/com/linecorp/armeria/common/TextHeadersSanitizerBuilder.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizerBuilder.java
@@ -14,9 +14,7 @@
  * under the License.
  */
 
-package com.linecorp.armeria.common;
-
-import java.util.function.Function;
+package com.linecorp.armeria.common.logging;
 
 /**
  * A builder implementation for Text {@link HeadersSanitizer}.
@@ -34,7 +32,7 @@ public TextHeadersSanitizerBuilder maskingHeaders(Iterable maskingFunction) {
+    public TextHeadersSanitizerBuilder maskingFunction(HeaderMaskingFunction maskingFunction) {
         return (TextHeadersSanitizerBuilder) super.maskingFunction(maskingFunction);
     }
 
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 6c419f813a9..ce39d9a7f95 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
@@ -23,7 +23,6 @@
 
 import com.google.common.base.MoreObjects;
 
-import com.linecorp.armeria.common.HeadersSanitizer;
 import com.linecorp.armeria.common.RequestContext;
 import com.linecorp.armeria.common.SerializationFormat;
 import com.linecorp.armeria.common.annotation.Nullable;
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 8f7e67b45c9..e79f23fd63a 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,7 +20,6 @@
 
 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.annotation.Nullable;
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 4d757b483b2..a7f14aa1a70 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,7 +18,6 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -26,7 +25,7 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
 
-import com.linecorp.armeria.common.HeadersSanitizer;
+import com.linecorp.armeria.common.HttpHeaderNames;
 import com.linecorp.armeria.common.HttpMethod;
 import com.linecorp.armeria.common.HttpRequest;
 import com.linecorp.armeria.common.HttpStatus;
@@ -105,7 +104,7 @@ void defaultMaskingHeadersShouldBeOverridable() {
 
     @Test
     void maskRequestHeaders() {
-        final Function maskingFunction = (header) -> "****armeria****";
+        final HeaderMaskingFunction maskingFunction = (name, value) -> "****armeria****";
         final LogFormatter logFormatter = LogFormatter.builderForJson()
                                                       .requestHeadersSanitizer(
                                                               HeadersSanitizer.builderForJson()
@@ -124,7 +123,7 @@ void maskRequestHeaders() {
 
         final Matcher matcher1 = Pattern.compile("\"accept\":\"(.*?)\"").matcher(requestLog);
         assertThat(matcher1.find()).isTrue();
-        assertThat(matcher1.group(1)).isEqualTo(maskingFunction.apply("text/html"));
+        assertThat(matcher1.group(1)).isEqualTo(maskingFunction.mask(HttpHeaderNames.ACCEPT, "text/html"));
 
         final Matcher matcher2 = Pattern.compile("\"cache-control\":\"(.*?)\"").matcher(requestLog);
         assertThat(matcher2.find()).isTrue();
@@ -133,7 +132,7 @@ void maskRequestHeaders() {
 
     @Test
     void maskResponseHeaders() {
-        final Function maskingFunction = (header) -> "****armeria****";
+        final HeaderMaskingFunction maskingFunction = (name, value) -> "****armeria****";
         final LogFormatter logFormatter = LogFormatter.builderForJson()
                                                       .responseHeadersSanitizer(
                                                               HeadersSanitizer.builderForJson()
@@ -150,7 +149,8 @@ void maskResponseHeaders() {
         final String responseLog = logFormatter.formatResponse(log);
         final Matcher matcher1 = Pattern.compile("\"content-type\":\"(.*?)\"").matcher(responseLog);
         assertThat(matcher1.find()).isTrue();
-        assertThat(matcher1.group(1)).isEqualTo(maskingFunction.apply("text/html"));
+        assertThat(matcher1.group(1)).isEqualTo(
+                maskingFunction.mask(HttpHeaderNames.CONTENT_TYPE, "text/html"));
 
         final Matcher matcher2 = Pattern.compile("\"cache-control\":\"(.*?)\"").matcher(responseLog);
         assertThat(matcher2.find()).isTrue();
@@ -159,7 +159,7 @@ void maskResponseHeaders() {
 
     @Test
     void maskRequestHeadersWithDuplicateHeaderName() {
-        final Function maskingFunction = (header) -> "****armeria****";
+        final HeaderMaskingFunction maskingFunction = (name, value) -> "****armeria****";
         final LogFormatter logFormatter = LogFormatter.builderForJson()
                                                       .requestHeadersSanitizer(
                                                               HeadersSanitizer.builderForJson()
@@ -180,6 +180,34 @@ void maskRequestHeadersWithDuplicateHeaderName() {
         final Matcher matcher1 = Pattern.compile("\"accept-encoding\":\"(.*?)\"").matcher(requestLog);
         assertThat(matcher1.find()).isTrue();
         assertThat(matcher1.group(1)).isEqualTo(
-                "[" + maskingFunction.apply("gzip") + ", " + maskingFunction.apply("deflate") + "]");
+                "[" + maskingFunction.mask(HttpHeaderNames.ACCEPT_ENCODING, "gzip") + ", " +
+                maskingFunction.mask(HttpHeaderNames.ACCEPT_ENCODING, "deflate") + "]");
+    }
+
+    @Test
+    void removeSensitiveHeaders() {
+        final LogFormatter logFormatter =
+                LogFormatter.builderForJson()
+                            .responseHeadersSanitizer(
+                                    HeadersSanitizer.builderForJson()
+                                                    .maskingHeaders("set-cookie", "multiple-header")
+                                                    .maskingFunction((name, value) -> null)
+                                                    .build())
+                            .build();
+        final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello"));
+        final DefaultRequestLog log = (DefaultRequestLog) ctx.log();
+        log.responseHeaders(ResponseHeaders.of(HttpStatus.OK, HttpHeaderNames.SET_COOKIE, "armeria=fun",
+                                               "multiple-header", "armeria1", "multiple-header", "armeria2",
+                                               HttpHeaderNames.CACHE_CONTROL, "no-cache"));
+        log.endResponse();
+
+        final String responseLog = logFormatter.formatResponse(log);
+        final Matcher matcher1 = Pattern.compile("\"set-cookie\":\"(.*?)\"").matcher(responseLog);
+        assertThat(matcher1.find()).isFalse();
+        final Matcher matcher2 = Pattern.compile("\"multiple-header\":\"(.*?)\"").matcher(responseLog);
+        assertThat(matcher2.find()).isFalse();
+        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 215f3697d19..43e9b78ad23 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,7 +18,6 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -26,7 +25,7 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
 
-import com.linecorp.armeria.common.HeadersSanitizer;
+import com.linecorp.armeria.common.HttpHeaderNames;
 import com.linecorp.armeria.common.HttpMethod;
 import com.linecorp.armeria.common.HttpRequest;
 import com.linecorp.armeria.common.HttpStatus;
@@ -114,7 +113,7 @@ void defaultMaskingHeadersShouldBeOverridable() {
 
     @Test
     void maskRequestHeaders() {
-        final Function maskingFunction = (header) -> "****armeria****";
+        final HeaderMaskingFunction maskingFunction = (name, value) -> "****armeria****";
         final LogFormatter logFormatter = LogFormatter.builderForText()
                                                       .requestHeadersSanitizer(
                                                               HeadersSanitizer.builderForText()
@@ -135,11 +134,13 @@ void maskRequestHeaders() {
         System.out.println(requestLog);
         final Matcher matcher1 = Pattern.compile("cookie=(.*?)[,\\]]").matcher(requestLog);
         assertThat(matcher1.find()).isTrue();
-        assertThat(matcher1.group(1)).isEqualTo(maskingFunction.apply("Armeria=awesome"));
+        assertThat(matcher1.group(1)).isEqualTo(
+                maskingFunction.mask(HttpHeaderNames.COOKIE, "Armeria=awesome"));
 
         final Matcher matcher2 = Pattern.compile("authorization=(.*?)[,\\]]").matcher(requestLog);
         assertThat(matcher2.find()).isTrue();
-        assertThat(matcher2.group(1)).isEqualTo(maskingFunction.apply("Basic XXX=="));
+        assertThat(matcher2.group(1)).isEqualTo(
+                maskingFunction.mask(HttpHeaderNames.AUTHORIZATION, "Basic XXX=="));
 
         final Matcher matcher3 = Pattern.compile("cache-control=(.*?)[,\\]]").matcher(requestLog);
         assertThat(matcher3.find()).isTrue();
@@ -148,7 +149,7 @@ void maskRequestHeaders() {
 
     @Test
     void maskResponseHeaders() {
-        final Function maskingFunction = (header) -> "****armeria****";
+        final HeaderMaskingFunction maskingFunction = (name, value) -> "****armeria****";
         final LogFormatter logFormatter = LogFormatter.builderForText()
                                                       .responseHeadersSanitizer(
                                                               HeadersSanitizer.builderForText()
@@ -167,11 +168,13 @@ void maskResponseHeaders() {
         final String responseLog = logFormatter.formatResponse(log);
         final Matcher matcher1 = Pattern.compile("content-type=(.*?)[,\\]]").matcher(responseLog);
         assertThat(matcher1.find()).isTrue();
-        assertThat(matcher1.group(1)).isEqualTo(maskingFunction.apply("text/html"));
+        assertThat(matcher1.group(1)).isEqualTo(
+                maskingFunction.mask(HttpHeaderNames.CONTENT_TYPE, "text/html"));
 
         final Matcher matcher2 = Pattern.compile("set-cookie=(.*?)[,\\]]").matcher(responseLog);
         assertThat(matcher2.find()).isTrue();
-        assertThat(matcher2.group(1)).isEqualTo(maskingFunction.apply("Armeria=awesome"));
+        assertThat(matcher2.group(1)).isEqualTo(
+                maskingFunction.mask(HttpHeaderNames.SET_COOKIE, "Armeria=awesome"));
 
         final Matcher matcher3 = Pattern.compile("cache-control=(.*?)[,\\]]").matcher(responseLog);
         assertThat(matcher3.find()).isTrue();
@@ -180,7 +183,7 @@ void maskResponseHeaders() {
 
     @Test
     void maskRequestHeadersWithDuplicateHeaderName() {
-        final Function maskingFunction = (header) -> "****armeria****";
+        final HeaderMaskingFunction maskingFunction = (name, value) -> "****armeria****";
         final LogFormatter logFormatter = LogFormatter.builderForText()
                                                       .requestHeadersSanitizer(
                                                               HeadersSanitizer.builderForText()
@@ -201,6 +204,34 @@ void maskRequestHeadersWithDuplicateHeaderName() {
         final Matcher matcher1 = Pattern.compile("accept-encoding=\\[(.*?)]").matcher(requestLog);
         assertThat(matcher1.find()).isTrue();
         assertThat(matcher1.group(1)).isEqualTo(
-                maskingFunction.apply("gzip") + ", " + maskingFunction.apply("deflate"));
+                maskingFunction.mask(HttpHeaderNames.ACCEPT_ENCODING, "gzip") + ", " +
+                maskingFunction.mask(HttpHeaderNames.ACCEPT_ENCODING, "deflate"));
+    }
+
+    @Test
+    void removeSensitiveHeaders() {
+        final LogFormatter logFormatter =
+                LogFormatter.builderForText()
+                            .responseHeadersSanitizer(
+                                    HeadersSanitizer.builderForText()
+                                                    .maskingHeaders("set-cookie", "multiple-header")
+                                                    .maskingFunction((name, value) -> null)
+                                                    .build())
+                            .build();
+        final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello"));
+        final DefaultRequestLog log = (DefaultRequestLog) ctx.log();
+        log.responseHeaders(ResponseHeaders.of(HttpStatus.OK, HttpHeaderNames.SET_COOKIE, "armeria=fun",
+                                               "multiple-header", "armeria1", "multiple-header", "armeria2",
+                                               HttpHeaderNames.CACHE_CONTROL, "no-cache"));
+        log.endResponse();
+
+        final String responseLog = logFormatter.formatResponse(log);
+        final Matcher matcher1 = Pattern.compile("\"set-cookie\"=\"(.*?)\"").matcher(responseLog);
+        assertThat(matcher1.find()).isFalse();
+        final Matcher matcher2 = Pattern.compile("\"multiple-header\"=\"(.*?)\"").matcher(responseLog);
+        assertThat(matcher2.find()).isFalse();
+        final Matcher matcher3 = Pattern.compile("\"cache-control\"=\"(.*?)\"").matcher(responseLog);
+        assertThat(matcher3.find()).isTrue();
+        assertThat(matcher3.group(1)).isEqualTo("no-cache");
     }
 }

From ad0ab4faf0e96d0c1f6ac12ab6a6e7d23aa1d68c Mon Sep 17 00:00:00 2001
From: Ikhun Um 
Date: Tue, 30 Jan 2024 09:35:06 +0900
Subject: [PATCH 13/16] Rename maskingHeaders to sensitiveHeaders

---
 .../AbstractHeadersSanitizerBuilder.java      | 27 +++++++++++--------
 .../logging/AbstractLogFormatterBuilder.java  | 10 +++----
 .../common/logging/JsonHeadersSanitizer.java  |  8 +++---
 .../logging/JsonHeadersSanitizerBuilder.java  | 10 +++----
 .../common/logging/TextHeadersSanitizer.java  | 12 ++++-----
 .../logging/TextHeadersSanitizerBuilder.java  | 10 +++----
 .../common/logging/JsonLogFormatterTest.java  | 14 +++++-----
 .../common/logging/TextLogFormatterTest.java  | 18 ++++++-------
 8 files changed, 57 insertions(+), 52 deletions(-)

diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
index db7367928e6..e5164ec5fc7 100644
--- a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
@@ -25,6 +25,7 @@
 import com.google.common.collect.ImmutableSet;
 
 import com.linecorp.armeria.common.HttpHeaderNames;
+import com.linecorp.armeria.common.annotation.Nullable;
 
 import io.netty.util.AsciiString;
 
@@ -37,38 +38,42 @@ public abstract class AbstractHeadersSanitizerBuilder {
     // - https://docs.rs/tower-http/latest/tower_http/sensitive_headers/index.html
     // - https://techdocs.akamai.com/edge-diagnostics/reference/sensitive-headers
     // - https://cloud.spring.io/spring-cloud-netflix/multi/multi__router_and_filter_zuul.html#_cookies_and_sensitive_headers
-    private static final Set DEFAULT_MASKING_HEADERS =
+    private static final Set DEFAULT_SENSITIVE_HEADERS =
             ImmutableSet.of(HttpHeaderNames.AUTHORIZATION, HttpHeaderNames.COOKIE,
                             HttpHeaderNames.SET_COOKIE, HttpHeaderNames.PROXY_AUTHORIZATION);
 
-    private final Set maskingHeaders = new HashSet<>();
+    @Nullable
+    private Set sensitiveHeaders;
 
     private HeaderMaskingFunction maskingFunction = HeaderMaskingFunction.of();
 
     AbstractHeadersSanitizerBuilder() {}
 
     /**
-     * Sets the headers to mask before logging.
+     * Adds the headers to mask before logging.
      */
-    public AbstractHeadersSanitizerBuilder maskingHeaders(CharSequence... headers) {
+    public AbstractHeadersSanitizerBuilder sensitiveHeaders(CharSequence... headers) {
         requireNonNull(headers, "headers");
-        return maskingHeaders(ImmutableSet.copyOf(headers));
+        return sensitiveHeaders(ImmutableSet.copyOf(headers));
     }
 
     /**
      * Sets the headers to mask before logging.
      */
-    public AbstractHeadersSanitizerBuilder maskingHeaders(Iterable headers) {
+    public AbstractHeadersSanitizerBuilder sensitiveHeaders(Iterable headers) {
         requireNonNull(headers, "headers");
-        headers.forEach(header -> maskingHeaders.add(AsciiString.of(header).toLowerCase()));
+        if (sensitiveHeaders == null) {
+            sensitiveHeaders = new HashSet<>();
+        }
+        headers.forEach(header -> sensitiveHeaders.add(AsciiString.of(header).toLowerCase()));
         return this;
     }
 
-    final Set maskingHeaders() {
-        if (!maskingHeaders.isEmpty()) {
-            return ImmutableSet.copyOf(maskingHeaders);
+    final Set sensitiveHeaders() {
+        if (sensitiveHeaders != null) {
+            return ImmutableSet.copyOf(sensitiveHeaders);
         }
-        return DEFAULT_MASKING_HEADERS;
+        return DEFAULT_SENSITIVE_HEADERS;
     }
 
     /**
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 77eba904ddd..21390d65114 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
@@ -57,7 +57,7 @@ abstract class AbstractLogFormatterBuilder {
      * HeadersSanitizer headersSanitizer =
      *   HeadersSanitizer
      *     .builderForText()
-     *     .maskingHeaders("Authorization", "Cookie")
+     *     .sensitiveHeaders("Authorization", "Cookie")
      *     ...
      *     .build();
      *
@@ -92,7 +92,7 @@ final HeadersSanitizer requestHeadersSanitizer() {
      * HeadersSanitizer headersSanitizer =
      *   HeadersSanitizer
      *     .builderForText()
-     *     .maskingHeaders("Set-Cookie")
+     *     .sensitiveHeaders("Set-Cookie")
      *     ...
      *     .build();
      *
@@ -126,7 +126,7 @@ final HeadersSanitizer responseHeadersSanitizer() {
      * HeadersSanitizer headersSanitizer =
      *   HeadersSanitizer
      *     .builderForText()
-     *     .maskingHeaders("...")
+     *     .sensitiveHeaders("...")
      *     ...
      *     .build();
      *
@@ -160,7 +160,7 @@ final HeadersSanitizer requestTrailersSanitizer() {
      * HeadersSanitizer headersSanitizer =
      *   HeadersSanitizer
      *     .builderForText()
-     *     .maskingHeaders("...")
+     *     .sensitiveHeaders("...")
      *     ...
      *     .build();
      *
@@ -194,7 +194,7 @@ final HeadersSanitizer responseTrailersSanitizer() {
      * HeadersSanitizer headersSanitizer =
      *   HeadersSanitizer
      *     .builderForText()
-     *     .maskingHeaders("...")
+     *     .sensitiveHeaders("...")
      *     ...
      *     .build();
      *
diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizer.java b/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizer.java
index f9d7dab41da..c1c931eecc7 100644
--- a/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizer.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizer.java
@@ -35,13 +35,13 @@
 final class JsonHeadersSanitizer implements HeadersSanitizer {
 
     static final HeadersSanitizer INSTANCE = new JsonHeadersSanitizerBuilder().build();
-    private final Set maskingHeaders;
+    private final Set sensitiveHeaders;
     private final HeaderMaskingFunction maskingFunction;
     private final ObjectMapper objectMapper;
 
-    JsonHeadersSanitizer(Set maskingHeaders, HeaderMaskingFunction maskingFunction,
+    JsonHeadersSanitizer(Set sensitiveHeaders, HeaderMaskingFunction maskingFunction,
                          ObjectMapper objectMapper) {
-        this.maskingHeaders = maskingHeaders;
+        this.sensitiveHeaders = sensitiveHeaders;
         this.maskingFunction = maskingFunction;
         this.objectMapper = objectMapper;
     }
@@ -49,7 +49,7 @@ final class JsonHeadersSanitizer implements HeadersSanitizer {
     @Override
     public JsonNode sanitize(RequestContext requestContext, HttpHeaders headers) {
         final ObjectNode result = objectMapper.createObjectNode();
-        maskHeaders(headers, maskingHeaders, maskingFunction,
+        maskHeaders(headers, sensitiveHeaders, maskingFunction,
                     (header, values) -> result.put(header.toString(), values.size() > 1 ?
                                                                       values.toString() : values.get(0)));
 
diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizerBuilder.java
index cd8440af1cc..a86dba1d324 100644
--- a/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizerBuilder.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonHeadersSanitizerBuilder.java
@@ -33,13 +33,13 @@ public final class JsonHeadersSanitizerBuilder extends AbstractHeadersSanitizerB
     private ObjectMapper objectMapper;
 
     @Override
-    public JsonHeadersSanitizerBuilder maskingHeaders(CharSequence... headers) {
-        return (JsonHeadersSanitizerBuilder) super.maskingHeaders(headers);
+    public JsonHeadersSanitizerBuilder sensitiveHeaders(CharSequence... headers) {
+        return (JsonHeadersSanitizerBuilder) super.sensitiveHeaders(headers);
     }
 
     @Override
-    public JsonHeadersSanitizerBuilder maskingHeaders(Iterable headers) {
-        return (JsonHeadersSanitizerBuilder) super.maskingHeaders(headers);
+    public JsonHeadersSanitizerBuilder sensitiveHeaders(Iterable headers) {
+        return (JsonHeadersSanitizerBuilder) super.sensitiveHeaders(headers);
     }
 
     @Override
@@ -61,6 +61,6 @@ public JsonHeadersSanitizerBuilder objectMapper(ObjectMapper objectMapper) {
     public HeadersSanitizer build() {
         final ObjectMapper objectMapper = this.objectMapper != null ?
                                           this.objectMapper : JacksonUtil.newDefaultObjectMapper();
-        return new JsonHeadersSanitizer(maskingHeaders(), maskingFunction(), objectMapper);
+        return new JsonHeadersSanitizer(sensitiveHeaders(), maskingFunction(), objectMapper);
     }
 }
diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizer.java b/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizer.java
index c089dffacd1..befbc16731e 100644
--- a/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizer.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizer.java
@@ -37,13 +37,13 @@ final class TextHeadersSanitizer implements HeadersSanitizer {
 
     static final HeadersSanitizer INSTANCE = new TextHeadersSanitizerBuilder().build();
 
-    private final Set maskingHeaders;
+    private final Set sensitiveHeaders;
 
     private final HeaderMaskingFunction maskingFunction;
 
-    TextHeadersSanitizer(Set maskingHeaders,
+    TextHeadersSanitizer(Set sensitiveHeaders,
                          HeaderMaskingFunction maskingFunction) {
-        this.maskingHeaders = maskingHeaders;
+        this.sensitiveHeaders = sensitiveHeaders;
         this.maskingFunction = maskingFunction;
     }
 
@@ -60,7 +60,7 @@ public String sanitize(RequestContext ctx, HttpHeaders headers) {
             sb.append('[');
         }
 
-        maskHeaders(headers, maskingHeaders, maskingFunction,
+        maskHeaders(headers, sensitiveHeaders, maskingFunction,
                     (header, values) -> sb.append(header).append('=')
                                           .append(values.size() > 1 ?
                                                   values.toString() : values.get(0)).append(", "));
@@ -70,12 +70,12 @@ public String sanitize(RequestContext ctx, HttpHeaders headers) {
     }
 
     static void maskHeaders(
-            HttpHeaders headers, Set maskingHeaders,
+            HttpHeaders headers, Set sensitiveHeaders,
             HeaderMaskingFunction maskingFunction,
             BiConsumer> consumer) {
         for (AsciiString headerName : headers.names()) {
             List values = headers.getAll(headerName);
-            if (maskingHeaders.contains(headerName)) {
+            if (sensitiveHeaders.contains(headerName)) {
                 // Mask the header values.
                 if (values.size() == 1) {
                     final String masked = maskingFunction.mask(headerName, values.get(0));
diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizerBuilder.java
index c84353bb189..73db6727dbf 100644
--- a/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizerBuilder.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/TextHeadersSanitizerBuilder.java
@@ -22,13 +22,13 @@
 public final class TextHeadersSanitizerBuilder extends AbstractHeadersSanitizerBuilder {
 
     @Override
-    public TextHeadersSanitizerBuilder maskingHeaders(CharSequence... headers) {
-        return (TextHeadersSanitizerBuilder) super.maskingHeaders(headers);
+    public TextHeadersSanitizerBuilder sensitiveHeaders(CharSequence... headers) {
+        return (TextHeadersSanitizerBuilder) super.sensitiveHeaders(headers);
     }
 
     @Override
-    public TextHeadersSanitizerBuilder maskingHeaders(Iterable headers) {
-        return (TextHeadersSanitizerBuilder) super.maskingHeaders(headers);
+    public TextHeadersSanitizerBuilder sensitiveHeaders(Iterable headers) {
+        return (TextHeadersSanitizerBuilder) super.sensitiveHeaders(headers);
     }
 
     @Override
@@ -40,6 +40,6 @@ public TextHeadersSanitizerBuilder maskingFunction(HeaderMaskingFunction masking
      * Returns a newly created text {@link HeadersSanitizer} based on the properties of this builder.
      */
     public HeadersSanitizer build() {
-        return new TextHeadersSanitizer(maskingHeaders(), maskingFunction());
+        return new TextHeadersSanitizer(sensitiveHeaders(), maskingFunction());
     }
 }
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 a7f14aa1a70..519d1e34446 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
@@ -79,11 +79,11 @@ void maskSensitiveHeadersByDefault() {
     }
 
     @Test
-    void defaultMaskingHeadersShouldBeOverridable() {
+    void defaultSensitiveHeadersShouldBeOverridable() {
         final LogFormatter logFormatter = LogFormatter.builderForJson()
                                                       .responseHeadersSanitizer(
                                                               HeadersSanitizer.builderForJson()
-                                                                              .maskingHeaders("Cache-Control")
+                                                                              .sensitiveHeaders("Cache-Control")
                                                                               .build())
                                                       .build();
         final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello"));
@@ -108,7 +108,7 @@ void maskRequestHeaders() {
         final LogFormatter logFormatter = LogFormatter.builderForJson()
                                                       .requestHeadersSanitizer(
                                                               HeadersSanitizer.builderForJson()
-                                                                              .maskingHeaders("accept")
+                                                                              .sensitiveHeaders("accept")
                                                                               .maskingFunction(maskingFunction)
                                                                               .build())
                                                       .build();
@@ -136,7 +136,7 @@ void maskResponseHeaders() {
         final LogFormatter logFormatter = LogFormatter.builderForJson()
                                                       .responseHeadersSanitizer(
                                                               HeadersSanitizer.builderForJson()
-                                                                              .maskingHeaders("content-type")
+                                                                              .sensitiveHeaders("content-type")
                                                                               .maskingFunction(maskingFunction)
                                                                               .build())
                                                       .build();
@@ -163,8 +163,8 @@ void maskRequestHeadersWithDuplicateHeaderName() {
         final LogFormatter logFormatter = LogFormatter.builderForJson()
                                                       .requestHeadersSanitizer(
                                                               HeadersSanitizer.builderForJson()
-                                                                              .maskingHeaders("accept-encoding")
-                                                                              .maskingHeaders("content-type")
+                                                                              .sensitiveHeaders("accept-encoding")
+                                                                              .sensitiveHeaders("content-type")
                                                                               .maskingFunction(maskingFunction)
                                                                               .build())
                                                       .build();
@@ -190,7 +190,7 @@ void removeSensitiveHeaders() {
                 LogFormatter.builderForJson()
                             .responseHeadersSanitizer(
                                     HeadersSanitizer.builderForJson()
-                                                    .maskingHeaders("set-cookie", "multiple-header")
+                                                    .sensitiveHeaders("set-cookie", "multiple-header")
                                                     .maskingFunction((name, value) -> null)
                                                     .build())
                             .build();
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 43e9b78ad23..a5f685ade38 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
@@ -88,11 +88,11 @@ void maskSensitiveHeadersByDefault() {
     }
 
     @Test
-    void defaultMaskingHeadersShouldBeOverridable() {
+    void defaultSensitiveHeadersShouldBeOverridable() {
         final LogFormatter logFormatter = LogFormatter.builderForText()
                                                       .responseHeadersSanitizer(
                                                               HeadersSanitizer.builderForText()
-                                                                              .maskingHeaders("Cache-Control")
+                                                                              .sensitiveHeaders("Cache-Control")
                                                                               .build())
                                                       .build();
         final ServiceRequestContext ctx = ServiceRequestContext.of(HttpRequest.of(HttpMethod.GET, "/hello"));
@@ -117,8 +117,8 @@ void maskRequestHeaders() {
         final LogFormatter logFormatter = LogFormatter.builderForText()
                                                       .requestHeadersSanitizer(
                                                               HeadersSanitizer.builderForText()
-                                                                              .maskingHeaders("cookie",
-                                                                                              "authorization")
+                                                                              .sensitiveHeaders("cookie",
+                                                                                                "authorization")
                                                                               .maskingFunction(maskingFunction)
                                                                               .build())
                                                       .build();
@@ -153,8 +153,8 @@ void maskResponseHeaders() {
         final LogFormatter logFormatter = LogFormatter.builderForText()
                                                       .responseHeadersSanitizer(
                                                               HeadersSanitizer.builderForText()
-                                                                              .maskingHeaders("content-type",
-                                                                                              "set-cookie")
+                                                                              .sensitiveHeaders("content-type",
+                                                                                                "set-cookie")
                                                                               .maskingFunction(maskingFunction)
                                                                               .build())
                                                       .build();
@@ -187,8 +187,8 @@ void maskRequestHeadersWithDuplicateHeaderName() {
         final LogFormatter logFormatter = LogFormatter.builderForText()
                                                       .requestHeadersSanitizer(
                                                               HeadersSanitizer.builderForText()
-                                                                              .maskingHeaders("accept-encoding")
-                                                                              .maskingHeaders("content-type")
+                                                                              .sensitiveHeaders("accept-encoding")
+                                                                              .sensitiveHeaders("content-type")
                                                                               .maskingFunction(maskingFunction)
                                                                               .build())
                                                       .build();
@@ -214,7 +214,7 @@ void removeSensitiveHeaders() {
                 LogFormatter.builderForText()
                             .responseHeadersSanitizer(
                                     HeadersSanitizer.builderForText()
-                                                    .maskingHeaders("set-cookie", "multiple-header")
+                                                    .sensitiveHeaders("set-cookie", "multiple-header")
                                                     .maskingFunction((name, value) -> null)
                                                     .build())
                             .build();

From 669042518b5848a1c6a109abc781c3dcbf1dbedd Mon Sep 17 00:00:00 2001
From: Ikhun Um 
Date: Tue, 30 Jan 2024 10:57:10 +0900
Subject: [PATCH 14/16] Fix test failures

---
 .../common/logging/JsonLogFormatterTest.java  | 15 +++++++++------
 .../common/logging/TextLogFormatterTest.java  | 19 ++++++++++---------
 2 files changed, 19 insertions(+), 15 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 519d1e34446..965592fd936 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,6 +25,8 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
 
+import com.fasterxml.jackson.databind.JsonNode;
+
 import com.linecorp.armeria.common.HttpHeaderNames;
 import com.linecorp.armeria.common.HttpMethod;
 import com.linecorp.armeria.common.HttpRequest;
@@ -160,13 +162,14 @@ void maskResponseHeaders() {
     @Test
     void maskRequestHeadersWithDuplicateHeaderName() {
         final HeaderMaskingFunction maskingFunction = (name, value) -> "****armeria****";
+        final HeadersSanitizer headersSanitizer =
+                HeadersSanitizer.builderForJson()
+                                .sensitiveHeaders("accept-encoding")
+                                .sensitiveHeaders("content-type")
+                                .maskingFunction(maskingFunction)
+                                .build();
         final LogFormatter logFormatter = LogFormatter.builderForJson()
-                                                      .requestHeadersSanitizer(
-                                                              HeadersSanitizer.builderForJson()
-                                                                              .sensitiveHeaders("accept-encoding")
-                                                                              .sensitiveHeaders("content-type")
-                                                                              .maskingFunction(maskingFunction)
-                                                                              .build())
+                                                      .requestHeadersSanitizer(headersSanitizer)
                                                       .build();
         final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello",
                                                                  "Accept-Encoding", "gzip",
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 a5f685ade38..3eeeafff8d5 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
@@ -184,13 +184,14 @@ void maskResponseHeaders() {
     @Test
     void maskRequestHeadersWithDuplicateHeaderName() {
         final HeaderMaskingFunction maskingFunction = (name, value) -> "****armeria****";
+        final HeadersSanitizer headersSanitizer =
+                HeadersSanitizer.builderForText()
+                                .sensitiveHeaders("accept-encoding")
+                                .sensitiveHeaders("content-type")
+                                .maskingFunction(maskingFunction)
+                                .build();
         final LogFormatter logFormatter = LogFormatter.builderForText()
-                                                      .requestHeadersSanitizer(
-                                                              HeadersSanitizer.builderForText()
-                                                                              .sensitiveHeaders("accept-encoding")
-                                                                              .sensitiveHeaders("content-type")
-                                                                              .maskingFunction(maskingFunction)
-                                                                              .build())
+                                                      .requestHeadersSanitizer(headersSanitizer)
                                                       .build();
         final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello",
                                                                  "Accept-Encoding", "gzip",
@@ -226,11 +227,11 @@ void removeSensitiveHeaders() {
         log.endResponse();
 
         final String responseLog = logFormatter.formatResponse(log);
-        final Matcher matcher1 = Pattern.compile("\"set-cookie\"=\"(.*?)\"").matcher(responseLog);
+        final Matcher matcher1 = Pattern.compile("set-cookie=(.*?)[,\\]]").matcher(responseLog);
         assertThat(matcher1.find()).isFalse();
-        final Matcher matcher2 = Pattern.compile("\"multiple-header\"=\"(.*?)\"").matcher(responseLog);
+        final Matcher matcher2 = Pattern.compile("multiple-header=(.*?)[,\\]]").matcher(responseLog);
         assertThat(matcher2.find()).isFalse();
-        final Matcher matcher3 = Pattern.compile("\"cache-control\"=\"(.*?)\"").matcher(responseLog);
+        final Matcher matcher3 = Pattern.compile("cache-control=(.*?)[,\\]]").matcher(responseLog);
         assertThat(matcher3.find()).isTrue();
         assertThat(matcher3.group(1)).isEqualTo("no-cache");
     }

From c15e7bf8baf7270fbd5f7781ba0a8e5cc26a2c3a Mon Sep 17 00:00:00 2001
From: Ikhun Um 
Date: Tue, 30 Jan 2024 12:06:29 +0900
Subject: [PATCH 15/16] Remove public

---
 .../common/logging/AbstractHeadersSanitizerBuilder.java     | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
index e5164ec5fc7..2d31d04bfc5 100644
--- a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
@@ -32,7 +32,7 @@
 /**
  * A skeletal builder implementation for {@link HeadersSanitizer}.
  */
-public abstract class AbstractHeadersSanitizerBuilder {
+abstract class AbstractHeadersSanitizerBuilder {
 
     // Referenced from:
     // - https://docs.rs/tower-http/latest/tower_http/sensitive_headers/index.html
@@ -47,8 +47,6 @@ public abstract class AbstractHeadersSanitizerBuilder {
 
     private HeaderMaskingFunction maskingFunction = HeaderMaskingFunction.of();
 
-    AbstractHeadersSanitizerBuilder() {}
-
     /**
      * Adds the headers to mask before logging.
      */
@@ -83,7 +81,7 @@ final Set sensitiveHeaders() {
      * 
{@code
      * builder.maskingFunction((name, value) -> {
      *   if (name.equals(HttpHeaderNames.AUTHORIZATION)) {
-     *      return "****";
+     *     return "****";
      *   } else if (name.equals(HttpHeaderNames.COOKIE)) {
      *     return name.substring(0, 4) + "****";
      *   } else {

From a83719782d11240c88a330c0275d81538c71e641 Mon Sep 17 00:00:00 2001
From: Ikhun Um 
Date: Tue, 30 Jan 2024 12:07:52 +0900
Subject: [PATCH 16/16] Update
 core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java

---
 .../armeria/common/logging/AbstractHeadersSanitizerBuilder.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
index 2d31d04bfc5..b320d831322 100644
--- a/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
+++ b/core/src/main/java/com/linecorp/armeria/common/logging/AbstractHeadersSanitizerBuilder.java
@@ -56,7 +56,7 @@ public AbstractHeadersSanitizerBuilder sensitiveHeaders(CharSequence... heade
     }
 
     /**
-     * Sets the headers to mask before logging.
+     * Adds the headers to mask before logging.
      */
     public AbstractHeadersSanitizerBuilder sensitiveHeaders(Iterable headers) {
         requireNonNull(headers, "headers");