Skip to content

Commit 1aed83b

Browse files
authored
Stop producing text content preview for non-text types with charset (#6485)
Motivation: When no `ContentPreviewer` is selected to produce a content preview, `TextContentPreviewer` acts as a fallback not only when the content type is textual but also when a charset is specified. For example, a request with a `Content-Type` header of `image/png; charset=utf-8` will still be recorded because the charset is specified. Modifications: - Stop producing text content preview for non-text types with charset. Result: - **Breaking change**: non-textual content with charset is no longer recorded by the `ContentPreviewer` fallback.
1 parent b8a29b0 commit 1aed83b

File tree

7 files changed

+6
-14
lines changed

7 files changed

+6
-14
lines changed

core/src/main/java/com/linecorp/armeria/client/logging/ContentPreviewingClient.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ public final class ContentPreviewingClient extends SimpleDecoratingHttpClient {
6868
* {@link RequestHeaders} or {@link ResponseHeaders} meets any of the following conditions:
6969
* <ul>
7070
* <li>when it matches {@code text/*} or {@code application/x-www-form-urlencoded}</li>
71-
* <li>when its charset has been specified</li>
7271
* <li>when its subtype is {@code "xml"} or {@code "json"}</li>
7372
* <li>when its subtype ends with {@code "+xml"} or {@code "+json"}</li>
7473
* </ul>
@@ -86,7 +85,6 @@ public static Function<? super HttpClient, ContentPreviewingClient> newDecorator
8685
* {@link RequestHeaders} or {@link ResponseHeaders} meets any of the following conditions:
8786
* <ul>
8887
* <li>when it matches {@code text/*} or {@code application/x-www-form-urlencoded}</li>
89-
* <li>when its charset has been specified</li>
9088
* <li>when its subtype is {@code "xml"} or {@code "json"}</li>
9189
* <li>when its subtype ends with {@code "+xml"} or {@code "+json"}</li>
9290
* </ul>

core/src/main/java/com/linecorp/armeria/common/logging/ContentPreviewerFactory.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ static ContentPreviewerFactoryBuilder builder() {
4040
* any of the following conditions:
4141
* <ul>
4242
* <li>when it matches {@code text/*} or {@code application/x-www-form-urlencoded}</li>
43-
* <li>when its charset has been specified</li>
4443
* <li>when its subtype is {@code "xml"} or {@code "json"}</li>
4544
* <li>when its subtype ends with {@code "+xml"} or {@code "+json"}</li>
4645
* </ul>
@@ -58,7 +57,6 @@ static ContentPreviewerFactory text(int maxLength) {
5857
* any of the following conditions:
5958
* <ul>
6059
* <li>when it matches {@code text/*} or {@code application/x-www-form-urlencoded}</li>
61-
* <li>when its charset has been specified</li>
6260
* <li>when its subtype is {@code "xml"} or {@code "json"}</li>
6361
* <li>when its subtype ends with {@code "+xml"} or {@code "+json"}</li>
6462
* </ul>

core/src/main/java/com/linecorp/armeria/common/logging/DefaultContentPreviewFactory.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,6 @@ private ContentPreviewer contentPreviewer(RequestContext ctx, HttpHeaders header
7575

7676
final MediaType contentType = headers.contentType();
7777
if (contentType != null) {
78-
final Charset charset = contentType.charset();
79-
if (charset != null) {
80-
return new TextContentPreviewer(maxLength, charset);
81-
}
82-
8378
if ("text".equals(contentType.type()) ||
8479
textSubTypes.contains(contentType.subtype()) ||
8580
textSubTypeSuffixes.stream().anyMatch(contentType.subtype()::endsWith) ||

core/src/main/java/com/linecorp/armeria/server/logging/ContentPreviewingService.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ public final class ContentPreviewingService extends SimpleDecoratingHttpService
6969
* {@link RequestHeaders} or {@link ResponseHeaders} meets any of the following conditions:
7070
* <ul>
7171
* <li>when it matches {@code text/*} or {@code application/x-www-form-urlencoded}</li>
72-
* <li>when its charset has been specified</li>
7372
* <li>when its subtype is {@code "xml"} or {@code "json"}</li>
7473
* <li>when its subtype ends with {@code "+xml"} or {@code "+json"}</li>
7574
* </ul>
@@ -87,7 +86,6 @@ public static Function<? super HttpService, ContentPreviewingService> newDecorat
8786
* {@link RequestHeaders} or {@link ResponseHeaders} meets any of the following conditions:
8887
* <ul>
8988
* <li>when it matches {@code text/*} or {@code application/x-www-form-urlencoded}</li>
90-
* <li>when its charset has been specified</li>
9189
* <li>when its subtype is {@code "xml"} or {@code "json"}</li>
9290
* <li>when its subtype ends with {@code "+xml"} or {@code "+json"}</li>
9391
* </ul>

core/src/test/java/com/linecorp/armeria/common/logging/ContentPreviewerFactoryTest.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,16 @@ void testOfText() {
7777
contentPreviewer = factory.requestContentPreviewer(
7878
ctx, RequestHeaders.of(HttpMethod.POST, "/",
7979
HttpHeaderNames.CONTENT_TYPE, "my/type; charset=UTF-8"));
80-
checkProduced(contentPreviewer);
80+
assertThat(contentPreviewer.produce()).isNull();
8181

8282
contentPreviewer = factory.requestContentPreviewer(ctx, reqHeaders(MediaType.BASIC_AUDIO));
8383
contentPreviewer.onData(HttpData.ofUtf8("hello!"));
8484
assertThat(contentPreviewer.produce()).isNull();
85+
86+
contentPreviewer = factory.requestContentPreviewer(
87+
ctx, reqHeaders(MediaType.PNG.withCharset(StandardCharsets.UTF_8)));
88+
contentPreviewer.onData(HttpData.ofUtf8("hello!"));
89+
assertThat(contentPreviewer.produce()).isNull();
8590
}
8691

8792
private static void checkProduced(ContentPreviewer contentPreviewer) {

site-new/docs/advanced/structured-logging.mdx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,6 @@ cb.decorator(ContentPreviewingClient.newDecorator(100));
295295
Note that the above decorators enable the previews only for textual content
296296
which meets one of the following cases:
297297
- when its type matches `text/*` or `application/x-www-form-urlencoded`.
298-
- when its charset has been specified. e.g. application/json; charset=utf-8.
299298
- when its subtype is `xml` or `json`. e.g. application/xml, application/json.
300299
- when its subtype ends with `+xml` or `+json`. e.g. application/atom+xml, application/hal+json
301300

site/src/pages/docs/advanced-structured-logging.mdx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,6 @@ cb.decorator(ContentPreviewingClient.newDecorator(100));
356356
Note that the above decorators enable the previews only for textual content
357357
which meets one of the following cases:
358358
- when its type matches `text/*` or `application/x-www-form-urlencoded`.
359-
- when its charset has been specified. e.g. application/json; charset=utf-8.
360359
- when its subtype is `xml` or `json`. e.g. application/xml, application/json.
361360
- when its subtype ends with `+xml` or `+json`. e.g. application/atom+xml, application/hal+json
362361

0 commit comments

Comments
 (0)