From 30660f43ec485b20b39d7ea6743bdf43b2b7faa1 Mon Sep 17 00:00:00 2001 From: Bert Roex Date: Tue, 13 Oct 2020 19:01:26 +0200 Subject: [PATCH] feature 2034: SdkHttpFullRequest builder.uri keep query parameters of provided URI --- .../feature-HTTPclientspi-25d1ed9.json | 5 ++ .../awssdk/http/SdkHttpFullRequest.java | 12 +++- .../amazon/awssdk/http/SdkHttpRequest.java | 12 +++- .../http/SdkHttpRequestResponseTest.java | 64 ++++++++++++++++++- .../awssdk/utils/http/SdkHttpUtils.java | 19 ++++++ .../amazon/awssdk/utils/SdkHttpUtilsTest.java | 47 ++++++++++++++ 6 files changed, 152 insertions(+), 7 deletions(-) create mode 100644 .changes/next-release/feature-HTTPclientspi-25d1ed9.json diff --git a/.changes/next-release/feature-HTTPclientspi-25d1ed9.json b/.changes/next-release/feature-HTTPclientspi-25d1ed9.json new file mode 100644 index 000000000000..375e9a107605 --- /dev/null +++ b/.changes/next-release/feature-HTTPclientspi-25d1ed9.json @@ -0,0 +1,5 @@ +{ + "category": "HTTP Client SPI", + "type": "feature", + "description": "Calling the SdkHttpFullRequest uri() builder method, query parameters of the provided URI will be kept.\nThis can be useful in case you want to provide an already fully formed URI like a callback URI." +} diff --git a/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpFullRequest.java b/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpFullRequest.java index 474a6b49a8cf..b4e945e17327 100644 --- a/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpFullRequest.java +++ b/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpFullRequest.java @@ -57,18 +57,24 @@ static SdkHttpFullRequest.Builder builder() { */ interface Builder extends SdkHttpRequest.Builder { /** - * Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()}, and - * {@link #encodedPath()} from a {@link URI} object. + * Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()}, + * {@link #encodedPath()} and extracts query parameters from a {@link URI} object. * * @param uri URI containing protocol, host, port and path. * @return This builder for method chaining. */ @Override default Builder uri(URI uri) { - return this.protocol(uri.getScheme()) + Builder builder = this.protocol(uri.getScheme()) .host(uri.getHost()) .port(uri.getPort()) .encodedPath(SdkHttpUtils.appendUri(uri.getRawPath(), encodedPath())); + if (uri.getRawQuery() != null) { + builder.clearQueryParameters(); + SdkHttpUtils.uriParams(uri) + .forEach(this::putRawQueryParameter); + } + return builder; } /** diff --git a/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java b/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java index d49f824d3a9b..088d9cd2c69e 100644 --- a/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java +++ b/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java @@ -135,17 +135,23 @@ default URI getUri() { */ interface Builder extends CopyableBuilder { /** - * Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()}, and - * {@link #encodedPath()} from a {@link URI} object. + * Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()}, + * {@link #encodedPath()} and extracts query parameters from a {@link URI} object. * * @param uri URI containing protocol, host, port and path. * @return This builder for method chaining. */ default Builder uri(URI uri) { - return this.protocol(uri.getScheme()) + Builder builder = this.protocol(uri.getScheme()) .host(uri.getHost()) .port(uri.getPort()) .encodedPath(SdkHttpUtils.appendUri(uri.getRawPath(), encodedPath())); + if (uri.getRawQuery() != null) { + builder.clearQueryParameters(); + SdkHttpUtils.uriParams(uri) + .forEach(this::putRawQueryParameter); + } + return builder; } /** diff --git a/http-client-spi/src/test/java/software/amazon/awssdk/http/SdkHttpRequestResponseTest.java b/http-client-spi/src/test/java/software/amazon/awssdk/http/SdkHttpRequestResponseTest.java index 3848b6407b78..875e664d4303 100644 --- a/http-client-spi/src/test/java/software/amazon/awssdk/http/SdkHttpRequestResponseTest.java +++ b/http-client-spi/src/test/java/software/amazon/awssdk/http/SdkHttpRequestResponseTest.java @@ -22,9 +22,10 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.net.URI; +import java.net.URISyntaxException; import java.util.AbstractMap; import java.util.Arrays; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -401,13 +402,74 @@ public Map> getMap() { }); } + @Test + public void testSdkHttpFullRequestBuilderNoQueryParams() { + URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034"); + final SdkHttpFullRequest sdkHttpFullRequest = SdkHttpFullRequest.builder().method(SdkHttpMethod.POST).uri(uri).build(); + assertThat(sdkHttpFullRequest.getUri().getQuery()).isNullOrEmpty(); + } + + @Test + public void testSdkHttpFullRequestBuilderUriWithQueryParams() { + URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456%26reqParam%3D5678"); + final SdkHttpFullRequest sdkHttpFullRequest = + SdkHttpFullRequest.builder().method(SdkHttpMethod.POST).uri(uri).build(); + assertThat(sdkHttpFullRequest.getUri().getQuery()).contains("reqParam=1234", "oParam=3456&reqParam=5678"); + } + + @Test + public void testSdkHttpFullRequestBuilderUriWithQueryParamWithoutValue() { + final String expected = "https://github.com/aws/aws-sdk-for-java-v2?foo"; + URI myUri = URI.create(expected); + SdkHttpFullRequest actual = SdkHttpFullRequest.builder().method(SdkHttpMethod.POST).uri(myUri).build(); + assertThat(actual.getUri()).hasToString(expected); + } + + @Test + public void testSdkHttpRequestBuilderNoQueryParams() { + URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034"); + final SdkHttpRequest sdkHttpRequest = SdkHttpRequest.builder().method(SdkHttpMethod.POST).uri(uri).build(); + assertThat(sdkHttpRequest.getUri().getQuery()).isNullOrEmpty(); + } + + @Test + public void testSdkHttpRequestBuilderUriWithQueryParams() { + URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456%26reqParam%3D5678"); + final SdkHttpRequest sdkHttpRequest = + SdkHttpRequest.builder().method(SdkHttpMethod.POST).uri(uri).build(); + assertThat(sdkHttpRequest.getUri().getQuery()).contains("reqParam=1234", "oParam=3456&reqParam=5678"); + } + + @Test + public void testSdkHttpRequestBuilderUriWithQueryParamsIgnoreOtherQueryParams() { + URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456%26reqParam%3D5678"); + final SdkHttpRequest sdkHttpRequest = + SdkHttpRequest.builder().method(SdkHttpMethod.POST).appendRawQueryParameter("clean", "up").uri(uri).build(); + assertThat(sdkHttpRequest.getUri().getQuery()).contains("reqParam=1234", "oParam=3456&reqParam=5678") + .doesNotContain("clean"); + } + + @Test + public void testSdkHttpRequestBuilderUriWithQueryParamWithoutValue() { + final String expected = "https://github.com/aws/aws-sdk-for-java-v2?foo"; + URI myUri = URI.create(expected); + SdkHttpRequest actual = SdkHttpRequest.builder().method(SdkHttpMethod.POST).uri(myUri).build(); + assertThat(actual.getUri()).hasToString(expected); + } + private interface BuilderProxy { BuilderProxy setValue(String key, String value); + BuilderProxy appendValue(String key, String value); + BuilderProxy setValues(String key, List values); + BuilderProxy removeValue(String key); + BuilderProxy clearValues(); + BuilderProxy setMap(Map> map); + Map> getMap(); } diff --git a/utils/src/main/java/software/amazon/awssdk/utils/http/SdkHttpUtils.java b/utils/src/main/java/software/amazon/awssdk/utils/http/SdkHttpUtils.java index b1d2fa7773c9..263850cfe3f8 100644 --- a/utils/src/main/java/software/amazon/awssdk/utils/http/SdkHttpUtils.java +++ b/utils/src/main/java/software/amazon/awssdk/utils/http/SdkHttpUtils.java @@ -15,6 +15,9 @@ package software.amazon.awssdk.utils.http; +import static java.util.stream.Collectors.groupingBy; +import static java.util.stream.Collectors.mapping; +import static java.util.stream.Collectors.toList; import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely; import java.io.UnsupportedEncodingException; @@ -30,6 +33,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.UnaryOperator; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; import software.amazon.awssdk.annotations.SdkProtectedApi; @@ -52,6 +56,9 @@ public final class SdkHttpUtils { private static final String[] ENCODED_CHARACTERS_WITHOUT_SLASHES = new String[] {"+", "*", "%7E"}; private static final String[] ENCODED_CHARACTERS_WITHOUT_SLASHES_REPLACEMENTS = new String[] {"%20", "%2A", "~"}; + private static final String QUERY_PARAM_DELIMITER_REGEX = "\\s*&\\s*"; + private static final Pattern QUERY_PARAM_DELIMITER_PATTERN = Pattern.compile(QUERY_PARAM_DELIMITER_REGEX); + // List of headers that may appear only once in a request; i.e. is not a list of values. // Taken from https://github.com/apache/httpcomponents-client/blob/81c1bc4dc3ca5a3134c5c60e8beff08be2fd8792/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java#L69-L85 with modifications: // removed: accept-ranges, if-match, if-none-match, vary since it looks like they're defined as lists @@ -62,6 +69,7 @@ public final class SdkHttpUtils { "proxy-authorization", "range", "referer", "retry-after", "server", "user-agent") .collect(Collectors.toSet()); + private SdkHttpUtils() { } @@ -322,4 +330,15 @@ public static Optional firstMatchingHeaderFromCollection(Map> uriParams(URI uri) { + return QUERY_PARAM_DELIMITER_PATTERN + .splitAsStream(uri.getRawQuery().trim()) + .map(s -> s.contains("=") ? s.split("=", 2) : new String[] {s, null}) + .collect(groupingBy(a -> urlDecode(a[0]), mapping(a -> urlDecode(a[1]), toList()))); + } + } diff --git a/utils/src/test/java/software/amazon/awssdk/utils/SdkHttpUtilsTest.java b/utils/src/test/java/software/amazon/awssdk/utils/SdkHttpUtilsTest.java index 0d885deb410e..e514974efc90 100644 --- a/utils/src/test/java/software/amazon/awssdk/utils/SdkHttpUtilsTest.java +++ b/utils/src/test/java/software/amazon/awssdk/utils/SdkHttpUtilsTest.java @@ -19,7 +19,12 @@ import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.entry; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; @@ -173,4 +178,46 @@ public void headersFromCollectionWorksCorrectly() { assertThat(SdkHttpUtils.firstMatchingHeaderFromCollection(headers, asList("foo", "nothing"))).hasValue("bar"); assertThat(SdkHttpUtils.firstMatchingHeaderFromCollection(headers, asList("foo", "other"))).hasValue("foo"); } + + @Test + public void isSingleHeader() { + assertThat(SdkHttpUtils.isSingleHeader("age")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("authorization")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("content-length")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("content-location")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("content-md5")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("content-range")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("content-type")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("date")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("etag")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("expires")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("from")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("host")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("if-modified-since")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("if-range")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("if-unmodified-since")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("last-modified")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("location")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("max-forwards")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("proxy-authorization")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("range")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("referer")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("retry-after")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("server")).isTrue(); + assertThat(SdkHttpUtils.isSingleHeader("user-agent")).isTrue(); + + assertThat(SdkHttpUtils.isSingleHeader("custom")).isFalse(); + } + + @Test + public void uriParams() throws URISyntaxException { + URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456&reqParam=5678&noval" + + "&decoded%26Part=equals%3Dval"); + Map> uriParams = SdkHttpUtils.uriParams(uri); + assertThat(uriParams).contains(entry("reqParam", Arrays.asList("1234", "5678")), + entry("oParam", Collections.singletonList("3456")), + entry("noval", Arrays.asList((String)null)), + entry("decoded&Part", Arrays.asList("equals=val"))); + } + }