From e07b88c7478a75b48c0a5d415fb98040366459d3 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Mon, 8 Jul 2024 14:38:10 +0200 Subject: [PATCH] fix(rest): Ensure proper URL encoding + fix case issues with content type headers (#2826) * fix(rest): Ensure proper URL encoding + fix case issues with content type headers * fix(rest): Add error logs * fix(rest): Refactor code structure * fix(rest): Make the encoder static * fix(rest): Fix license --- .../parts/ApacheRequestBodyBuilder.java | 4 +- .../parts/ApacheRequestUriBuilder.java | 20 +--- .../apache/builder/parts/UrlEncoder.java | 52 +++++++++ .../apache/ApacheRequestFactoryTest.java | 107 +++++++++++++++--- .../apache/CustomApacheHttpClientTest.java | 18 +++ 5 files changed, 165 insertions(+), 36 deletions(-) create mode 100644 connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/UrlEncoder.java diff --git a/connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/ApacheRequestBodyBuilder.java b/connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/ApacheRequestBodyBuilder.java index f594804675..5ed0739e79 100644 --- a/connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/ApacheRequestBodyBuilder.java +++ b/connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/ApacheRequestBodyBuilder.java @@ -71,11 +71,11 @@ public void build(ClassicRequestBuilder builder, HttpCommonRequest request) { private HttpEntity createEntityForContentType( ContentType contentType, Map body, HttpCommonRequest request) { HttpEntity entity; - if (contentType.getMimeType().equals(MULTIPART_FORM_DATA.getMimeType())) { + if (contentType.getMimeType().equalsIgnoreCase(MULTIPART_FORM_DATA.getMimeType())) { entity = createMultiPartEntity(body, contentType); } else if (contentType .getMimeType() - .equals(ContentType.APPLICATION_FORM_URLENCODED.getMimeType())) { + .equalsIgnoreCase(ContentType.APPLICATION_FORM_URLENCODED.getMimeType())) { entity = createUrlEncodedFormEntity(body); } else { entity = createStringEntity(request); diff --git a/connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/ApacheRequestUriBuilder.java b/connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/ApacheRequestUriBuilder.java index 0661d57378..a424c2f96f 100644 --- a/connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/ApacheRequestUriBuilder.java +++ b/connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/ApacheRequestUriBuilder.java @@ -17,30 +17,12 @@ package io.camunda.connector.http.base.client.apache.builder.parts; import io.camunda.connector.http.base.model.HttpCommonRequest; -import java.net.MalformedURLException; -import java.net.URI; -import java.net.URISyntaxException; -import java.net.URL; import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; public class ApacheRequestUriBuilder implements ApacheRequestPartBuilder { @Override public void build(ClassicRequestBuilder builder, HttpCommonRequest request) { - try { - var url = new URL(request.getUrl()); - builder.setUri( - // Only this URI constructor escapes the URL properly - new URI( - url.getProtocol(), - url.getUserInfo(), - url.getHost(), - url.getPort(), - url.getPath(), - url.getQuery(), - null)); - } catch (MalformedURLException | URISyntaxException e) { - builder.setUri(request.getUrl()); - } + builder.setUri(UrlEncoder.toEncodedUri(request.getUrl())); } } diff --git a/connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/UrlEncoder.java b/connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/UrlEncoder.java new file mode 100644 index 0000000000..639a252992 --- /dev/null +++ b/connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/builder/parts/UrlEncoder.java @@ -0,0 +1,52 @@ +/* + * Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH + * under one or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information regarding copyright + * ownership. Camunda licenses this file to you under the Apache License, + * Version 2.0; 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.camunda.connector.http.base.client.apache.builder.parts; + +import java.net.MalformedURLException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class UrlEncoder { + private static final Logger LOG = LoggerFactory.getLogger(ApacheRequestUriBuilder.class); + + public static URI toEncodedUri(String requestUrl) { + try { + // We try to decode the URL first, because it might be encoded already + // which would lead to double encoding. Decoding is safe here, because it does nothing if + // the URL is not encoded. + var decodedUrl = URLDecoder.decode(requestUrl, StandardCharsets.UTF_8); + var url = new URL(decodedUrl); + // Only this URI constructor escapes the URL properly + return new URI( + url.getProtocol(), + url.getUserInfo(), + url.getHost(), + url.getPort(), + url.getPath(), + url.getQuery(), + null); + } catch (MalformedURLException | URISyntaxException e) { + LOG.error("Failed to parse URL {}, defaulting to requestUrl", requestUrl, e); + return URI.create(requestUrl); + } + } +} diff --git a/connectors/http/http-base/src/test/java/io/camunda/connector/http/base/client/apache/ApacheRequestFactoryTest.java b/connectors/http/http-base/src/test/java/io/camunda/connector/http/base/client/apache/ApacheRequestFactoryTest.java index 2b9b5da1a9..91b1106400 100644 --- a/connectors/http/http-base/src/test/java/io/camunda/connector/http/base/client/apache/ApacheRequestFactoryTest.java +++ b/connectors/http/http-base/src/test/java/io/camunda/connector/http/base/client/apache/ApacheRequestFactoryTest.java @@ -36,8 +36,11 @@ import io.camunda.connector.http.base.model.auth.BearerAuthentication; import io.camunda.connector.http.base.model.auth.OAuthAuthentication; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.stream.Stream; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ContentType; import org.apache.hc.core5.http.Header; @@ -46,7 +49,9 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import org.mockito.MockedStatic; @@ -60,6 +65,7 @@ public void shouldNotSetAuthentication_whenNotProvided() throws Exception { // given request without authentication HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.GET); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -74,6 +80,7 @@ public void shouldSetBasicAuthentication_whenProvided() throws Exception { HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.GET); request.setAuthentication(new BasicAuthentication("user", "password")); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -89,6 +96,7 @@ public void shouldSetBearerAuthentication_whenProvided() throws Exception { HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.GET); request.setAuthentication(new BearerAuthentication("token")); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -104,6 +112,7 @@ public void shouldSetOAuthAuthentication_whenProvided() throws Exception { HttpCommonResult result = new HttpCommonResult(200, null, "{\"access_token\":\"token\"}"); HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.GET); + request.setUrl("theurl"); request.setAuthentication( new OAuthAuthentication( "url", "clientId", "secret", "audience", OAuthConstants.CREDENTIALS_BODY, "scopes")); @@ -128,6 +137,7 @@ public void shouldSetApiKeyAuthenticationInHeaders_whenProvided() throws Excepti HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.GET); request.setAuthentication(new ApiKeyAuthentication(ApiKeyLocation.HEADERS, "name", "value")); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -142,6 +152,7 @@ public void shouldSetApiKeyAuthenticationInQuery_whenProvided() throws Exception HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.GET); request.setAuthentication(new ApiKeyAuthentication(ApiKeyLocation.QUERY, "name", "value")); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -160,6 +171,7 @@ public void shouldNotSetQueryParameters_whenNotProvided() throws Exception { // given request without query parameters HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.GET); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -174,6 +186,7 @@ public void shouldSetQueryParameters_whenProvided() throws Exception { HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.GET); request.setQueryParameters(Map.of("key", "value")); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -188,6 +201,7 @@ public void shouldSetQueryParameters_whenProvidedMultiple() throws Exception { HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.GET); request.setQueryParameters(Map.of("key", "value", "key2", "value2")); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -249,11 +263,66 @@ public void shouldSetUri_whenQueryHasParametersInUrl() throws Exception { @Nested class BodyTests { + private static Stream provideMultipartContentTypeHeaderWithWeirdCase() { + List weirdContentTypes = + List.of("content-type", "ContEnt-TyPe", "CONTENT-TYPE", "Content-type"); + List weirdMultipart = + List.of( + "multipart/form-data", + "MULTIPART/FORM-DATA", + "MuLtIpArT/fOrM-dAtA", + ContentType.MULTIPART_FORM_DATA.toString(), + ContentType.MULTIPART_FORM_DATA.withCharset(StandardCharsets.UTF_8).toString()); + List combinedCases = new ArrayList<>(); + for (String contentType : weirdContentTypes) { + for (String multipart : weirdMultipart) { + combinedCases.add(contentType); + combinedCases.add(multipart); + } + } + // combined values 2 by 2 + List arguments = new ArrayList<>(); + for (int i = 0; i < combinedCases.size(); i += 2) { + arguments.add(Arguments.of(combinedCases.get(i), combinedCases.get(i + 1))); + } + + return arguments.stream(); + } + + private static Stream provideFormUrlEncodedContentTypeHeaderWithWeirdCase() { + List weirdContentTypes = + List.of("content-type", "ContEnt-TyPe", "CONTENT-TYPE", "Content-type"); + List weirdFromUrlEncoded = + List.of( + "application/x-www-form-urlencodEd", + "APPLICATION/X-WWW-FORM-URLENCODED", + "AppLiCaTiOn/x-www-form-urlencoded", + ContentType.APPLICATION_FORM_URLENCODED.toString(), + ContentType.APPLICATION_FORM_URLENCODED + .withCharset(StandardCharsets.UTF_8) + .toString()); + List combinedCases = new ArrayList<>(); + for (String contentType : weirdContentTypes) { + for (String formUrlEncoded : weirdFromUrlEncoded) { + combinedCases.add(contentType); + combinedCases.add(formUrlEncoded); + } + } + // combined values 2 by 2 + List arguments = new ArrayList<>(); + for (int i = 0; i < combinedCases.size(); i += 2) { + arguments.add(Arguments.of(combinedCases.get(i), combinedCases.get(i + 1))); + } + + return arguments.stream(); + } + @Test public void shouldNotSetBody_whenBodyNotSupported() throws Exception { // given request with body HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.GET); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -268,6 +337,7 @@ public void shouldSetJsonBody_whenBodySupportedAndContentTypeNotProvided() throw HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.POST); request.setBody(Map.of("key", "value")); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -293,6 +363,7 @@ public void shouldNotSetJsonBody_whenBodySupportedAndContentTypeProvided() throw Map.of( HttpHeaders.CONTENT_TYPE, ContentType.TEXT_PLAIN.withCharset(StandardCharsets.UTF_8).toString())); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -318,6 +389,7 @@ public void shouldSetJsonBody_whenBodySupportedAndContentTypeProvided() throws E Map.of( HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.withCharset(StandardCharsets.UTF_8).toString())); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -344,6 +416,7 @@ public void shouldSetJsonBody_whenBodySupportedAndContentTypeProvidedAndBodyIsMa Map.of( HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.withCharset(StandardCharsets.UTF_8).toString())); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -374,6 +447,7 @@ public void shouldSetJsonBody_whenBodySupportedAndContentTypeProvidedAndBodyIsMa ContentType.APPLICATION_FORM_URLENCODED .withCharset(StandardCharsets.UTF_8) .toString())); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -408,6 +482,7 @@ public void shouldSetFormUrlEncodedBody_whenBodySupportedAndBodyIsMapAndHasNullV ContentType.APPLICATION_FORM_URLENCODED .withCharset(StandardCharsets.UTF_8) .toString())); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -427,19 +502,16 @@ public void shouldSetFormUrlEncodedBody_whenBodySupportedAndBodyIsMapAndHasNullV assertThat(content).contains("&"); } - @Test - public void shouldSetFormUrlEncodedBody_whenBodySupportedAndContentTypeProvidedAndBodyIsMap() - throws Exception { + @ParameterizedTest + @MethodSource("provideFormUrlEncodedContentTypeHeaderWithWeirdCase") + public void shouldSetFormUrlEncodedBody_whenBodySupportedAndContentTypeProvidedAndBodyIsMap( + String contentType, String formUrlEncodedValue) throws Exception { // given request with body HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.POST); request.setBody(Map.of("key", "value", "key2", "value2")); - request.setHeaders( - Map.of( - HttpHeaders.CONTENT_TYPE, - ContentType.APPLICATION_FORM_URLENCODED - .withCharset(StandardCharsets.UTF_8) - .toString())); + request.setHeaders(Map.of(contentType, formUrlEncodedValue)); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -458,16 +530,16 @@ public void shouldSetFormUrlEncodedBody_whenBodySupportedAndContentTypeProvidedA assertThat(content).contains("&"); } - @Test - public void shouldSetMultipartBody_whenBodySupportedAndContentTypeProvidedAndBodyIsMap() { + @ParameterizedTest + @MethodSource("provideMultipartContentTypeHeaderWithWeirdCase") + public void shouldSetMultipartBody_whenBodySupportedAndContentTypeProvidedAndBodyIsMap( + String contentType, String multipartValue) { // given request with body HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.POST); request.setBody(Map.of("key", "value", "key2", "value2")); - request.setHeaders( - Map.of( - HttpHeaders.CONTENT_TYPE, - ContentType.MULTIPART_FORM_DATA.withCharset(StandardCharsets.UTF_8).toString())); + request.setHeaders(Map.of(contentType, multipartValue)); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -495,6 +567,7 @@ public void shouldSetContentType_whenNullProvidedAndPost(HttpMethod method) headers.put(HttpHeaders.CONTENT_TYPE, null); headers.put(HttpHeaders.ACCEPT, null); headers.put("Other", null); + request.setUrl("theurl"); request.setHeaders(headers); // when @@ -514,6 +587,7 @@ public void shouldSetJsonContentType_WhenNotProvidedAndSupportsBody() throws Exc // given request without headers HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.POST); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -531,6 +605,7 @@ public void shouldSetJsonContentType_WhenNotProvidedAndSupportsBodyAndSomeHeader HttpCommonRequest request = new HttpCommonRequest(); request.setHeaders(Map.of("Authorization", "Bearer token")); request.setMethod(HttpMethod.POST); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -548,6 +623,7 @@ public void shouldNotSetJsonContentType_WhenNotProvidedAndDoesNotSupportBody() // given request without headers HttpCommonRequest request = new HttpCommonRequest(); request.setMethod(HttpMethod.GET); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); @@ -563,6 +639,7 @@ public void shouldNotSetJsonContentType_WhenProvided() throws Exception { HttpCommonRequest request = new HttpCommonRequest(); request.setHeaders(Map.of(HttpHeaders.CONTENT_TYPE, "text/plain")); request.setMethod(HttpMethod.POST); + request.setUrl("theurl"); // when ClassicHttpRequest httpRequest = ApacheRequestFactory.get().createHttpRequest(request); diff --git a/connectors/http/http-base/src/test/java/io/camunda/connector/http/base/client/apache/CustomApacheHttpClientTest.java b/connectors/http/http-base/src/test/java/io/camunda/connector/http/base/client/apache/CustomApacheHttpClientTest.java index b15d80a97f..b72df22ed9 100644 --- a/connectors/http/http-base/src/test/java/io/camunda/connector/http/base/client/apache/CustomApacheHttpClientTest.java +++ b/connectors/http/http-base/src/test/java/io/camunda/connector/http/base/client/apache/CustomApacheHttpClientTest.java @@ -232,6 +232,24 @@ public void shouldReturn200_whenSpaceInPathAndQueryParametersInPath( assertThat(result).isNotNull(); assertThat(result.status()).isEqualTo(200); } + + @ParameterizedTest + @EnumSource(HttpMethod.class) + public void shouldReturn200_whenEscapedSpaceInPathAndQueryParametersInPath( + HttpMethod method, WireMockRuntimeInfo wmRuntimeInfo) { + stubFor( + any(urlEqualTo("/path%20with%20spaces?andQuery=Param%20with%20space")) + .withQueryParams(Map.of("andQuery", equalTo("Param with space"))) + .willReturn(ok())); + + HttpCommonRequest request = new HttpCommonRequest(); + request.setMethod(method); + request.setUrl( + getHostAndPort(wmRuntimeInfo) + "/path%20with%20spaces?andQuery=Param with space"); + HttpCommonResult result = customApacheHttpClient.execute(request); + assertThat(result).isNotNull(); + assertThat(result.status()).isEqualTo(200); + } } @Nested