diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/presigner/PresignedRequest.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/presigner/PresignedRequest.java index f3da23bf11f5..464007e5947a 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/presigner/PresignedRequest.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/presigner/PresignedRequest.java @@ -27,6 +27,7 @@ import software.amazon.awssdk.core.SdkBytes; import software.amazon.awssdk.http.SdkHttpRequest; import software.amazon.awssdk.utils.Validate; +import software.amazon.awssdk.utils.http.SdkHttpUtils; /** @@ -49,7 +50,13 @@ protected PresignedRequest(DefaultBuilder builder) { this.signedHeaders = Validate.notEmpty(builder.signedHeaders, "signedHeaders"); this.signedPayload = builder.signedPayload; this.httpRequest = Validate.notNull(builder.httpRequest, "httpRequest"); - this.url = invokeSafely(httpRequest.getUri()::toURL); + this.url = toUrl(httpRequest); + } + + private static URL toUrl(SdkHttpRequest request) { + String queryString = request.encodedQueryParameters().map(query -> "?" + query).orElse(""); + int port = SdkHttpUtils.isUsingStandardPort(request.protocol(), request.port()) ? -1 : request.port(); + return invokeSafely(() -> new URL(request.protocol(), request.host(), port, request.encodedPath() + queryString)); } /** @@ -230,4 +237,4 @@ private B thisBuilder() { return (B) this; } } -} \ No newline at end of file +} diff --git a/core/aws-core/src/test/java/software/amazon/awssdk/awscore/presigner/PresignedRequestTest.java b/core/aws-core/src/test/java/software/amazon/awssdk/awscore/presigner/PresignedRequestTest.java new file mode 100644 index 000000000000..14be00015c9b --- /dev/null +++ b/core/aws-core/src/test/java/software/amazon/awssdk/awscore/presigner/PresignedRequestTest.java @@ -0,0 +1,163 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.awscore.presigner; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URI; +import java.time.Instant; +import java.util.Collections; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.http.SdkHttpFullRequest; +import software.amazon.awssdk.http.SdkHttpMethod; +import software.amazon.awssdk.http.SdkHttpRequest; + +class PresignedRequestTest { + @Test + void url_withStandardHttpsPort_omitsPort() throws Exception { + SdkHttpRequest httpRequest = requestBuilder() + .protocol("https") + .host("example.com") + .port(443) + .encodedPath("/resource") + .build(); + + assertEquivalentToUriConversion(httpRequest); + assertThat(presignedRequest(httpRequest).url().getPort()).isEqualTo(-1); + assertThat(presignedRequest(httpRequest).url()).hasToString("https://example.com/resource"); + } + + @Test + void url_withStandardHttpPort_omitsPort() throws Exception { + SdkHttpRequest httpRequest = requestBuilder() + .protocol("http") + .host("example.com") + .port(80) + .encodedPath("/resource") + .build(); + + assertEquivalentToUriConversion(httpRequest); + assertThat(presignedRequest(httpRequest).url().getPort()).isEqualTo(-1); + assertThat(presignedRequest(httpRequest).url()).hasToString("http://example.com/resource"); + } + + @Test + void url_withImplicitDefaultPort_omitsPort() throws Exception { + SdkHttpRequest httpRequest = requestBuilder() + .protocol("https") + .host("example.com") + .encodedPath("/resource") + .build(); + + assertThat(httpRequest.port()).isEqualTo(443); + assertEquivalentToUriConversion(httpRequest); + assertThat(presignedRequest(httpRequest).url().getPort()).isEqualTo(-1); + } + + @Test + void url_withCustomPort_preservesPort() throws Exception { + SdkHttpRequest httpRequest = requestBuilder() + .protocol("https") + .host("example.com") + .port(8443) + .encodedPath("/resource") + .build(); + + assertEquivalentToUriConversion(httpRequest); + assertThat(presignedRequest(httpRequest).url().getPort()).isEqualTo(8443); + assertThat(presignedRequest(httpRequest).url()).hasToString("https://example.com:8443/resource"); + } + + @Test + void url_withEncodedPathAndQuery_preservesEncodedValues() throws Exception { + SdkHttpRequest httpRequest = requestBuilder() + .protocol("https") + .host("example.com") + .encodedPath("/a%20path/%2Fvalue%3Fquery%23fragment%25percent") + .putRawQueryParameter("key with space", "value/with?#%characters") + .build(); + + assertEquivalentToUriConversion(httpRequest); + assertThat(presignedRequest(httpRequest).url()) + .hasToString("https://example.com/a%20path/%2Fvalue%3Fquery%23fragment%25percent" + + "?key%20with%20space=value%2Fwith%3F%23%25characters"); + } + + @Test + void url_withEmptyPath_preservesEmptyPath() throws Exception { + SdkHttpRequest httpRequest = requestBuilder() + .protocol("https") + .host("example.com") + .build(); + + assertEquivalentToUriConversion(httpRequest); + assertThat(presignedRequest(httpRequest).url()).hasToString("https://example.com"); + } + + @Test + void url_withEmptyPathAndQuery_preservesQuery() throws Exception { + SdkHttpRequest httpRequest = requestBuilder() + .protocol("https") + .host("example.com") + .putRawQueryParameter("foo", "bar") + .build(); + + assertEquivalentToUriConversion(httpRequest); + assertThat(presignedRequest(httpRequest).url()).hasToString("https://example.com?foo=bar"); + } + + @Test + void url_withIpv6Host_preservesHost() throws Exception { + SdkHttpRequest httpRequest = requestBuilder() + .uri(URI.create("https://[2001:db8::1]:8443/resource")) + .build(); + + assertEquivalentToUriConversion(httpRequest); + assertThat(presignedRequest(httpRequest).url()) + .hasToString("https://[2001:db8::1]:8443/resource"); + } + + private static void assertEquivalentToUriConversion(SdkHttpRequest httpRequest) throws Exception { + assertThat(presignedRequest(httpRequest).url().toExternalForm()) + .isEqualTo(httpRequest.getUri().toURL().toExternalForm()); + } + + private static SdkHttpFullRequest.Builder requestBuilder() { + return SdkHttpFullRequest.builder().method(SdkHttpMethod.GET); + } + + private static TestPresignedRequest presignedRequest(SdkHttpRequest httpRequest) { + return new TestBuilder() + .expiration(Instant.EPOCH) + .isBrowserExecutable(true) + .signedHeaders(Collections.singletonMap("host", Collections.singletonList(httpRequest.host()))) + .httpRequest(httpRequest) + .build(); + } + + private static final class TestPresignedRequest extends PresignedRequest { + private TestPresignedRequest(TestBuilder builder) { + super(builder); + } + } + + private static final class TestBuilder extends PresignedRequest.DefaultBuilder { + @Override + public TestPresignedRequest build() { + return new TestPresignedRequest(this); + } + } +} diff --git a/test/sdk-benchmarks/pom.xml b/test/sdk-benchmarks/pom.xml index f784a7e11ff3..b88804822bf1 100644 --- a/test/sdk-benchmarks/pom.xml +++ b/test/sdk-benchmarks/pom.xml @@ -118,6 +118,11 @@ cloudfront ${awsjavasdk.version} + + software.amazon.awssdk + s3 + ${awsjavasdk.version} + software.amazon.awssdk sts diff --git a/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/presigner/S3PresignerBenchmark.java b/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/presigner/S3PresignerBenchmark.java new file mode 100644 index 000000000000..68f814eac2b3 --- /dev/null +++ b/test/sdk-benchmarks/src/main/java/software/amazon/awssdk/benchmark/presigner/S3PresignerBenchmark.java @@ -0,0 +1,80 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.benchmark.presigner; + +import java.time.Duration; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Warmup; +import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; +import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.s3.model.GetObjectRequest; +import software.amazon.awssdk.services.s3.presigner.S3Presigner; +import software.amazon.awssdk.services.s3.presigner.model.GetObjectPresignRequest; +import software.amazon.awssdk.services.s3.presigner.model.PresignedGetObjectRequest; + +@State(Scope.Thread) +@Warmup(iterations = 5) +@Measurement(iterations = 5) +@Fork(2) +@BenchmarkMode(Mode.Throughput) +public class S3PresignerBenchmark { + private static final String BUCKET = "bucket"; + private static final String KEY = "key"; + + private S3Presigner presigner; + private GetObjectPresignRequest request; + + @Setup(Level.Trial) + public void setup() { + presigner = S3Presigner.builder() + .region(Region.US_EAST_1) + .credentialsProvider(StaticCredentialsProvider.create( + AwsBasicCredentials.create("dummykey", "dummysecret"))) + .build(); + + GetObjectRequest getObjectRequest = + GetObjectRequest.builder() + .bucket(BUCKET) + .key(KEY) + .build(); + + request = GetObjectPresignRequest.builder() + .signatureDuration(Duration.ofMinutes(15)) + .getObjectRequest(getObjectRequest) + .build(); + } + + @TearDown(Level.Trial) + public void tearDown() { + presigner.close(); + } + + @Benchmark + public PresignedGetObjectRequest presignGetObject() { + return presigner.presignGetObject(request); + } +}