Skip to content

Commit e0490c7

Browse files
authored
Merge pull request #1572 from dagnir/gh1564
Add scheme to destination region endpoint
2 parents 8cbd76a + 6ac80f1 commit e0490c7

File tree

3 files changed

+145
-2
lines changed

3 files changed

+145
-2
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"type": "bugfix",
3+
"category": "Amazon EC2",
4+
"description": "Fix NPE when calling `CopySnapshot`. Fixes [#1564](https://github.com/aws/aws-sdk-java-v2/issues/1564)"
5+
}

services/ec2/src/main/java/software/amazon/awssdk/services/ec2/transform/internal/GeneratePreSignUrlInterceptor.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
import static software.amazon.awssdk.auth.signer.AwsSignerExecutionAttribute.AWS_CREDENTIALS;
1919

2020
import java.net.URI;
21+
import java.time.Clock;
2122
import software.amazon.awssdk.annotations.SdkInternalApi;
23+
import software.amazon.awssdk.annotations.SdkTestInternalApi;
2224
import software.amazon.awssdk.auth.signer.Aws4Signer;
2325
import software.amazon.awssdk.auth.signer.params.Aws4PresignerParams;
2426
import software.amazon.awssdk.awscore.util.AwsHostNameUtils;
@@ -40,7 +42,6 @@
4042

4143
/**
4244
* ExecutionInterceptor that generates a pre-signed URL for copying encrypted snapshots
43-
* TODO: Is this actually right? What if a different interceptor modifies the message? Should this be treated as a signer?
4445
*/
4546
@SdkInternalApi
4647
public final class GeneratePreSignUrlInterceptor implements ExecutionInterceptor {
@@ -57,6 +58,17 @@ public final class GeneratePreSignUrlInterceptor implements ExecutionInterceptor
5758

5859
private static final CopySnapshotRequestMarshaller MARSHALLER = new CopySnapshotRequestMarshaller(PROTOCOL_FACTORY);
5960

61+
private final Clock testClock; // for testing only
62+
63+
public GeneratePreSignUrlInterceptor() {
64+
testClock = null;
65+
}
66+
67+
@SdkTestInternalApi
68+
GeneratePreSignUrlInterceptor(Clock testClock) {
69+
this.testClock = testClock;
70+
}
71+
6072
@Override
6173
public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, ExecutionAttributes executionAttributes) {
6274
SdkHttpRequest request = context.httpRequest();
@@ -122,6 +134,7 @@ private Aws4PresignerParams getPresignerParams(ExecutionAttributes attributes, S
122134
.signingRegion(Region.of(signingRegion))
123135
.signingName(signingName)
124136
.awsCredentials(attributes.getAttribute(AWS_CREDENTIALS))
137+
.signingClockOverride(testClock)
125138
.build();
126139
}
127140

@@ -152,6 +165,10 @@ private URI createEndpoint(String regionName, String serviceName) {
152165
.build();
153166
}
154167

155-
return Ec2Client.serviceMetadata().endpointFor(region);
168+
URI endpoint = Ec2Client.serviceMetadata().endpointFor(region);
169+
if (endpoint.getScheme() == null) {
170+
return URI.create("https://" + endpoint);
171+
}
172+
return endpoint;
156173
}
157174
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/*
2+
* Copyright 2010-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.services.ec2.transform.internal;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.mockito.Mockito.when;
20+
import static software.amazon.awssdk.auth.signer.AwsSignerExecutionAttribute.AWS_CREDENTIALS;
21+
import java.net.URI;
22+
import java.time.Clock;
23+
import java.time.Instant;
24+
import java.time.ZoneId;
25+
import java.time.ZonedDateTime;
26+
import org.junit.Test;
27+
import org.junit.runner.RunWith;
28+
import org.mockito.Mock;
29+
import org.mockito.runners.MockitoJUnitRunner;
30+
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
31+
import software.amazon.awssdk.core.interceptor.Context;
32+
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
33+
import software.amazon.awssdk.http.SdkHttpFullRequest;
34+
import software.amazon.awssdk.http.SdkHttpMethod;
35+
import software.amazon.awssdk.http.SdkHttpRequest;
36+
import software.amazon.awssdk.services.ec2.model.CopySnapshotRequest;
37+
38+
@RunWith(MockitoJUnitRunner.class)
39+
public class GeneratePreSignUrlInterceptorTest {
40+
private static final GeneratePreSignUrlInterceptor INTERCEPTOR = new GeneratePreSignUrlInterceptor();
41+
42+
@Mock
43+
private Context.ModifyHttpRequest mockContext;
44+
45+
@Test
46+
public void copySnapshotRequest_httpsProtocolAddedToEndpoint() {
47+
SdkHttpFullRequest request = SdkHttpFullRequest.builder()
48+
.uri(URI.create("https://ec2.us-west-2.amazonaws.com"))
49+
.method(SdkHttpMethod.POST)
50+
.build();
51+
52+
CopySnapshotRequest ec2Request = CopySnapshotRequest.builder()
53+
.sourceRegion("us-west-2")
54+
.destinationRegion("us-east-2")
55+
.build();
56+
57+
when(mockContext.httpRequest()).thenReturn(request);
58+
when(mockContext.request()).thenReturn(ec2Request);
59+
60+
ExecutionAttributes attrs = new ExecutionAttributes();
61+
attrs.putAttribute(AWS_CREDENTIALS, AwsBasicCredentials.create("foo", "bar"));
62+
63+
SdkHttpRequest modifiedRequest = INTERCEPTOR.modifyHttpRequest(mockContext, attrs);
64+
65+
String presignedUrl = modifiedRequest.rawQueryParameters().get("PresignedUrl").get(0);
66+
67+
assertThat(presignedUrl).startsWith("https://");
68+
}
69+
70+
@Test
71+
public void copySnapshotRequest_generatesCorrectPresignedUrl() {
72+
// Expected URL was derived by first making a request to EC2 using
73+
// valid credentials and a KMS encrypted snapshot and verifying that
74+
// the snapshot was copied to the destination region, also encrypted.
75+
// Then the same code was used to make a second request, changing only
76+
// the credentials and snapshot ID.
77+
String expectedPresignedUrl = "https://ec2.us-west-2.amazonaws.com?Action=CopySnapshot" +
78+
"&Version=2016-11-15" +
79+
"&DestinationRegion=us-east-1" +
80+
"&SourceRegion=us-west-2" +
81+
"&SourceSnapshotId=SNAPSHOT_ID" +
82+
"&X-Amz-Algorithm=AWS4-HMAC-SHA256" +
83+
"&X-Amz-Date=20200107T205609Z" +
84+
"&X-Amz-SignedHeaders=host" +
85+
"&X-Amz-Expires=604800" +
86+
"&X-Amz-Credential=akid%2F20200107%2Fus-west-2%2Fec2%2Faws4_request" +
87+
"&X-Amz-Signature=c1f5e34834292a86ff2b46b5e97cebaf2967b09641b4e2e60a382a37d137a03b";
88+
89+
ZoneId utcZone = ZoneId.of("UTC").normalized();
90+
91+
// Same signing date as the one used for the request above
92+
Instant signingInstant = ZonedDateTime.of(2020, 1, 7, 20, 56, 9, 0, utcZone).toInstant();
93+
Clock signingClock = Clock.fixed(signingInstant, utcZone);
94+
95+
GeneratePreSignUrlInterceptor interceptor = new GeneratePreSignUrlInterceptor(signingClock);
96+
97+
// These details don't really affect the test as they're not used in generating the signed URL
98+
SdkHttpFullRequest request = SdkHttpFullRequest.builder()
99+
.uri(URI.create("https://ec2.us-west-2.amazonaws.com"))
100+
.method(SdkHttpMethod.POST)
101+
.build();
102+
103+
CopySnapshotRequest ec2Request = CopySnapshotRequest.builder()
104+
.sourceRegion("us-west-2")
105+
.destinationRegion("us-east-1")
106+
.sourceSnapshotId("SNAPSHOT_ID")
107+
.build();
108+
109+
when(mockContext.httpRequest()).thenReturn(request);
110+
when(mockContext.request()).thenReturn(ec2Request);
111+
112+
ExecutionAttributes attrs = new ExecutionAttributes();
113+
attrs.putAttribute(AWS_CREDENTIALS, AwsBasicCredentials.create("akid", "skid"));
114+
115+
SdkHttpRequest modifiedRequest = interceptor.modifyHttpRequest(mockContext, attrs);
116+
117+
String generatedPresignedUrl = modifiedRequest.rawQueryParameters().get("PresignedUrl").get(0);
118+
119+
assertThat(generatedPresignedUrl).isEqualTo(expectedPresignedUrl);
120+
}
121+
}

0 commit comments

Comments
 (0)