From 52152f45ecffddc30b570a6907371b5a049158e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Kraus?= Date: Tue, 30 Jul 2024 11:22:47 +0200 Subject: [PATCH] Issue #7102 - WebClient should have a mode that is resilient to bad media/content types (#9040) --- .../io/helidon/common/http/MediaType.java | 40 ++++- .../integration/webclient/HeadersTest.java | 147 ++++++++++++++++++ .../helidon/webclient/NettyClientHandler.java | 5 +- .../java/io/helidon/webclient/WebClient.java | 17 +- .../webclient/WebClientConfiguration.java | 20 ++- .../WebClientResponseHeadersImpl.java | 25 ++- .../webclient/WebClientResponseImpl.java | 18 ++- 7 files changed, 260 insertions(+), 12 deletions(-) create mode 100644 tests/integration/webclient/src/test/java/io/helidon/tests/integration/webclient/HeadersTest.java diff --git a/common/http/src/main/java/io/helidon/common/http/MediaType.java b/common/http/src/main/java/io/helidon/common/http/MediaType.java index 754132f4639..4b3b87b01a0 100644 --- a/common/http/src/main/java/io/helidon/common/http/MediaType.java +++ b/common/http/src/main/java/io/helidon/common/http/MediaType.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2022 Oracle and/or its affiliates. + * Copyright (c) 2018, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -268,6 +268,12 @@ public final class MediaType implements AcceptPredicate { */ private static final CharMatcher LINEAR_WHITE_SPACE = CharMatcher.anyOf(" \t\r\n"); private static final String CHARSET_ATTRIBUTE = "charset"; + + // Relaxed media types mapping + private static final Map RELAXED_TYPES = Map.of( + "text", MediaType.TEXT_PLAIN // text -> text/plain + ); + private final String type; private final String subtype; private final Map parameters; @@ -319,13 +325,43 @@ public static MediaType create(String type, String subtype) { * @throws NullPointerException if the input is {@code null} */ public static MediaType parse(String input) { + return parse(input, true); + } + + /** + * Parses a media type from its string representation in relaxed mode. + * Predefined incomplete media types are replaced with corresponding valid media types. + * + * @param input the input string representing a media type + * @return parsed {@link MediaType} instance + * @throws IllegalArgumentException if the input is not parsable + * @throws NullPointerException if the input is {@code null} + */ + public static MediaType parseRelaxed(String input) { + return parse(input, false); + } + + /** + * Parses a media type from its string representation. + * + * @param input the input string representing a media type + * @param strictMode whether strict mode (default) shall be used + * @return parsed {@link MediaType} instance + * @throws IllegalArgumentException if the input is not parsable + * @throws NullPointerException if the input is {@code null} + */ + private static MediaType parse(String input, boolean strictMode) { Objects.requireNonNull(input, "Parameter 'input' is null!"); Tokenizer tokenizer = new Tokenizer(input); try { String type = tokenizer.consumeToken(TOKEN_MATCHER); + Map parameters = new HashMap<>(); + // Handle relaxed media type parsing when '/' character is not present + if (!strictMode && !tokenizer.hasMore() && RELAXED_TYPES.containsKey(type)) { + return RELAXED_TYPES.get(type); + } tokenizer.consumeCharacter('/'); String subtype = tokenizer.consumeToken(TOKEN_MATCHER); - Map parameters = new HashMap<>(); while (tokenizer.hasMore()) { tokenizer.consumeTokenIfPresent(LINEAR_WHITE_SPACE); tokenizer.consumeCharacter(';'); diff --git a/tests/integration/webclient/src/test/java/io/helidon/tests/integration/webclient/HeadersTest.java b/tests/integration/webclient/src/test/java/io/helidon/tests/integration/webclient/HeadersTest.java new file mode 100644 index 00000000000..36b94d6fc7f --- /dev/null +++ b/tests/integration/webclient/src/test/java/io/helidon/tests/integration/webclient/HeadersTest.java @@ -0,0 +1,147 @@ +/* + * Copyright (c) 2024 Oracle and/or its affiliates. + * + * Licensed 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 + * + * http://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 io.helidon.tests.integration.webclient; + +import java.util.Optional; +import java.util.concurrent.CompletionException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import io.helidon.common.http.Http; +import io.helidon.common.http.MediaType; +import io.helidon.webclient.WebClient; +import io.helidon.webclient.WebClientResponse; +import io.helidon.webserver.Routing; +import io.helidon.webserver.ServerRequest; +import io.helidon.webserver.ServerResponse; +import io.helidon.webserver.WebServer; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.fail; + +public class HeadersTest { + + private static final String INVALID_CONTENT_TYPE_VALUE = "invalid header value"; + private static final String INVALID_CONTENT_TYPE_TEXT = "text"; + private static final String RELAXED_CONTENT_TYPE_TEXT = "text/plain"; + + private static WebServer server; + private static WebClient client; + + @BeforeAll + static void beforeAll() throws ExecutionException, InterruptedException, TimeoutException { + server = WebServer.builder( + Routing.builder() + .get("/invalidContentType", HeadersTest::invalidContentType) + .get("/invalidTextContentType", HeadersTest::invalidTextContentType) + .build() + ) + .build(); + server.start() + .toCompletableFuture() + .get(10, TimeUnit.SECONDS); + + client = WebClient.builder() + .baseUri("http://localhost:" + server.port()) + .validateHeaders(false) + .keepAlive(true) + .build(); + } + + // HTTP service with invalid Content-Type + private static void invalidContentType(ServerRequest request, ServerResponse response) { + response.addHeader(Http.Header.CONTENT_TYPE, INVALID_CONTENT_TYPE_VALUE) + .send(); + } + + // HTTP service with Content-Type: text instead of text/plain + private static void invalidTextContentType(ServerRequest request, ServerResponse response) { + response.addHeader(Http.Header.CONTENT_TYPE, INVALID_CONTENT_TYPE_TEXT) + .send(); + } + + @AfterAll + static void afterAll() throws ExecutionException, InterruptedException, TimeoutException { + if (server != null) { + server.shutdown() + .toCompletableFuture() + .get(10, TimeUnit.SECONDS); + } + } + + // Verify that invalid content type causes an exception when being parsed + @Test + void testInvalidContentType() { + try { + client.get() + .path("/invalidContentType") + .request() + .await(10, TimeUnit.SECONDS); + fail("WebClient shall throw an exception"); + } catch (Exception ex) { + assertThat(ex, is(instanceOf(CompletionException.class))); + Throwable cause = ex.getCause(); + assertThat(cause, is(notNullValue())); + assertThat(cause, is(instanceOf(IllegalArgumentException.class))); + assertThat(cause.getMessage(), containsString(INVALID_CONTENT_TYPE_VALUE)); + } + } + + // Verify that "text" content type causes an exception when being parsed in strict mode + @Test + void testInvalidTextContentTypeStrict() { + try { + client.get() + .path("/invalidTextContentType") + .request() + .await(10, TimeUnit.SECONDS); + } catch (Exception ex) { + assertThat(ex, is(instanceOf(CompletionException.class))); + Throwable cause = ex.getCause(); + assertThat(cause, is(notNullValue())); + assertThat(cause, is(instanceOf(IllegalArgumentException.class))); + assertThat(cause.getMessage(), containsString(INVALID_CONTENT_TYPE_TEXT)); + } + } + + // Verify that "text" content type is transformed to "text/plain" in relaxed mode + @Test + void testInvalidTextContentTypeRelaxed() { + WebClient client = WebClient.builder() + .baseUri("http://localhost:" + server.port()) + .validateHeaders(false) + .keepAlive(true) + .mediaTypeParserRelaxed(true) + .build(); + WebClientResponse response = client.get() + .path("/invalidTextContentType") + .request() + .await(10, TimeUnit.SECONDS); + Optional maybeContentType = response.headers().contentType(); + assertThat(maybeContentType.isPresent(), is(true)); + assertThat(maybeContentType.get().toString(), is(RELAXED_CONTENT_TYPE_TEXT)); + } + +} diff --git a/webclient/webclient/src/main/java/io/helidon/webclient/NettyClientHandler.java b/webclient/webclient/src/main/java/io/helidon/webclient/NettyClientHandler.java index beafd36dfd4..7fa15324a2a 100644 --- a/webclient/webclient/src/main/java/io/helidon/webclient/NettyClientHandler.java +++ b/webclient/webclient/src/main/java/io/helidon/webclient/NettyClientHandler.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2023 Oracle and/or its affiliates. + * Copyright (c) 2020, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -123,7 +123,8 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws IO .status(helidonStatus(response.status())) .httpVersion(Http.Version.create(response.protocolVersion().toString())) .responseCloser(responseCloser) - .lastEndpointURI(requestConfiguration.requestURI()); + .lastEndpointURI(requestConfiguration.requestURI()) + .mediaTypeParserRelaxed(requestConfiguration.mediaTypeParserRelaxed()); HttpHeaders nettyHeaders = response.headers(); for (String name : nettyHeaders.names()) { diff --git a/webclient/webclient/src/main/java/io/helidon/webclient/WebClient.java b/webclient/webclient/src/main/java/io/helidon/webclient/WebClient.java index bfba6eaed32..b7341609741 100644 --- a/webclient/webclient/src/main/java/io/helidon/webclient/WebClient.java +++ b/webclient/webclient/src/main/java/io/helidon/webclient/WebClient.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2022 Oracle and/or its affiliates. + * Copyright (c) 2020, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -440,6 +440,21 @@ public Builder enableAutomaticCookieStore(boolean enableAutomaticCookieStore) { return this; } + /** + * Configure media type parsing mode for HTTP {@code Content-Type} header. + * Media type parsing in relaxed mode accepts incomplete Content-Type values like + * {@code "text"}, which are accepted as corresponding complete value, e.g. {@code "text/plain"}. + * Parsing in strict mode (default) expect exact {@code Content-Type} matching. + * + * @param relaxedMode value of {@code true} sets relaxed media type parsing mode, + * value of {@code false} sets strict media type parsing mode + * @return updated builder instance + */ + public Builder mediaTypeParserRelaxed(boolean relaxedMode) { + configuration.mediaTypeParserRelaxed(relaxedMode); + return this; + } + WebClientConfiguration configuration() { configuration.clientServices(services()); return configuration.build(); diff --git a/webclient/webclient/src/main/java/io/helidon/webclient/WebClientConfiguration.java b/webclient/webclient/src/main/java/io/helidon/webclient/WebClientConfiguration.java index 36f81684d00..d5db516f92e 100644 --- a/webclient/webclient/src/main/java/io/helidon/webclient/WebClientConfiguration.java +++ b/webclient/webclient/src/main/java/io/helidon/webclient/WebClientConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2023 Oracle and/or its affiliates. + * Copyright (c) 2019, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -84,6 +84,7 @@ class WebClientConfiguration { private final boolean validateHeaders; private final boolean relativeUris; private final DnsResolverType dnsResolverType; + private final boolean mediaTypeParserRelaxed; /** * Creates a new instance of client configuration. @@ -115,6 +116,7 @@ class WebClientConfiguration { this.validateHeaders = builder.validateHeaders; this.relativeUris = builder.relativeUris; this.dnsResolverType = builder.dnsResolverType; + this.mediaTypeParserRelaxed = builder.mediaTypeParserRelaxed; } /** @@ -291,6 +293,10 @@ DnsResolverType dnsResolverType() { return dnsResolverType; } + boolean mediaTypeParserRelaxed() { + return mediaTypeParserRelaxed; + } + /** * A fluent API builder for {@link WebClientConfiguration}. */ @@ -323,6 +329,7 @@ static class Builder, T extends WebClientConfiguration> private boolean validateHeaders; private boolean relativeUris; private DnsResolverType dnsResolverType; + private boolean mediaTypeParserRelaxed; @SuppressWarnings("unchecked") private B me = (B) this; @@ -635,6 +642,11 @@ B clientServices(List clientServices) { return me; } + B mediaTypeParserRelaxed(boolean relaxedMode) { + this.mediaTypeParserRelaxed = relaxedMode; + return me; + } + /** * Enable keep alive option on the connection. * @@ -699,6 +711,10 @@ public B keepAlive(boolean keepAlive) { * proxy * Proxy configuration. See {@link Proxy.Builder#config(Config)} * + * + * media-type-parser-relaxed + * Whether relaxed media type parsing mode should be used. + * * * * @param config config @@ -729,6 +745,7 @@ public B config(Config config) { config.get("dns-resolver-type").asString() .map(s -> DnsResolverType.valueOf(s.toUpperCase())) .ifPresent(this::dnsResolverType); + config.get("media-type-parser-relaxed").asBoolean().ifPresent(this::mediaTypeParserRelaxed); return me; } @@ -757,6 +774,7 @@ public B update(WebClientConfiguration configuration) { keepAlive(configuration.keepAlive); validateHeaders(configuration.validateHeaders); dnsResolverType(configuration.dnsResolverType); + mediaTypeParserRelaxed(configuration.mediaTypeParserRelaxed); configuration.cookieManager.defaultCookies().forEach(this::defaultCookie); config = configuration.config; diff --git a/webclient/webclient/src/main/java/io/helidon/webclient/WebClientResponseHeadersImpl.java b/webclient/webclient/src/main/java/io/helidon/webclient/WebClientResponseHeadersImpl.java index b23bd77ee21..c084b4a2e6b 100644 --- a/webclient/webclient/src/main/java/io/helidon/webclient/WebClientResponseHeadersImpl.java +++ b/webclient/webclient/src/main/java/io/helidon/webclient/WebClientResponseHeadersImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2022 Oracle and/or its affiliates. + * Copyright (c) 2020, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,8 +32,22 @@ */ class WebClientResponseHeadersImpl extends ReadOnlyHeaders implements WebClientResponseHeaders { - private WebClientResponseHeadersImpl(Map> headers) { + private final boolean mediaTypeParserRelaxed; + + private WebClientResponseHeadersImpl(Map> headers, boolean mediaTypeParserRelaxed) { super(headers); + this.mediaTypeParserRelaxed = mediaTypeParserRelaxed; + } + + /** + * Creates {@link WebClientResponseHeaders} instance which contains data from {@link Map}. + * + * @param headers response headers in map + * @param mediaTypeParserRelaxed whether relaxed media type parsing mode should be used + * @return response headers instance + */ + protected static WebClientResponseHeadersImpl create(Map> headers, boolean mediaTypeParserRelaxed) { + return new WebClientResponseHeadersImpl(headers, mediaTypeParserRelaxed); } /** @@ -43,7 +57,7 @@ private WebClientResponseHeadersImpl(Map> headers) { * @return response headers instance */ protected static WebClientResponseHeadersImpl create(Map> headers) { - return new WebClientResponseHeadersImpl(headers); + return new WebClientResponseHeadersImpl(headers, false); } @Override @@ -76,7 +90,10 @@ public Optional date() { @Override public Optional contentType() { - return first(Http.Header.CONTENT_TYPE).map(MediaType::parse); + return first(Http.Header.CONTENT_TYPE) + .map(mediaTypeParserRelaxed + ? MediaType::parseRelaxed + : MediaType::parse); } @Override diff --git a/webclient/webclient/src/main/java/io/helidon/webclient/WebClientResponseImpl.java b/webclient/webclient/src/main/java/io/helidon/webclient/WebClientResponseImpl.java index 7a3db937f28..a7a699e7dff 100644 --- a/webclient/webclient/src/main/java/io/helidon/webclient/WebClientResponseImpl.java +++ b/webclient/webclient/src/main/java/io/helidon/webclient/WebClientResponseImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2021 Oracle and/or its affiliates. + * Copyright (c) 2020, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -46,7 +46,7 @@ final class WebClientResponseImpl implements WebClientResponse { private final URI lastEndpointUri; private WebClientResponseImpl(Builder builder) { - headers = WebClientResponseHeadersImpl.create(builder.headers); + headers = WebClientResponseHeadersImpl.create(builder.headers, builder.mediaTypeParserRelaxed); publisher = builder.publisher; status = builder.status; version = builder.version; @@ -109,6 +109,7 @@ static class Builder implements io.helidon.common.Builder