Skip to content

Commit d72f134

Browse files
zoewanggzoewangg
committed
Update s3 createMultipartRequest content-type and update apache http client to not set the wrong default content type
1 parent 1835a9d commit d72f134

File tree

7 files changed

+96
-11
lines changed

7 files changed

+96
-11
lines changed

‎.changes/next-release/bugfix-AmazonS3-6113bf7.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "Amazon S3",
3+
"type": "bugfix",
4+
"description": "Set `Content-Type` to `binary/octet-stream` for `S3#createMultipartRequest`. See [#1092](https://github.com/aws/aws-sdk-java-v2/issues/1092)"
5+
}

‎.changes/next-release/bugfix-ApacheHttpClient-f1d9a7d.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "Apache Http Client",
3+
"type": "bugfix",
4+
"description": "Updated to not set a default `Content-Type` if the header does not exist. Per [RFC7231](https://tools.ietf.org/html/rfc7231#page-11), we should let the recipient to decide if not known."
5+
}

‎http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package software.amazon.awssdk.http.apache.internal.impl;
1717

1818
import static software.amazon.awssdk.utils.NumericUtils.saturatedCast;
19-
import static software.amazon.awssdk.utils.StringUtils.lowerCase;
2019

2120
import java.net.URI;
2221
import java.util.Arrays;
@@ -151,14 +150,6 @@ private void addHeadersToRequest(HttpRequestBase httpRequest, SdkHttpRequest req
151150
*/
152151
.filter(e -> !IGNORE_HEADERS.contains(e.getKey()))
153152
.forEach(e -> e.getValue().forEach(h -> httpRequest.addHeader(e.getKey(), h)));
154-
155-
/* Set content type and encoding */
156-
if (httpRequest.getHeaders(HttpHeaders.CONTENT_TYPE) == null ||
157-
httpRequest.getHeaders(HttpHeaders.CONTENT_TYPE).length == 0) {
158-
httpRequest.addHeader(HttpHeaders.CONTENT_TYPE,
159-
"application/x-www-form-urlencoded; " +
160-
"charset=" + lowerCase(DEFAULT_ENCODING));
161-
}
162153
}
163154

164155
private String getHostHeaderValue(SdkHttpRequest request) {

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/CreateMultipartUploadRequestInterceptor.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
package software.amazon.awssdk.services.s3.internal.handlers;
1717

18+
import static software.amazon.awssdk.http.Header.CONTENT_LENGTH;
19+
import static software.amazon.awssdk.http.Header.CONTENT_TYPE;
20+
1821
import java.io.ByteArrayInputStream;
1922
import java.util.Optional;
2023
import software.amazon.awssdk.annotations.SdkInternalApi;
@@ -44,7 +47,8 @@ public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context,
4447
if (context.request() instanceof CreateMultipartUploadRequest) {
4548
return context.httpRequest()
4649
.toBuilder()
47-
.putHeader("Content-Length", String.valueOf(0))
50+
.putHeader(CONTENT_LENGTH, String.valueOf(0))
51+
.putHeader(CONTENT_TYPE, "binary/octet-stream")
4852
.build();
4953
}
5054

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/MultipartUploadTest.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020
import static com.github.tomakehurst.wiremock.client.WireMock.anyRequestedFor;
2121
import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
2222
import static com.github.tomakehurst.wiremock.client.WireMock.containing;
23+
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
2324
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
2425
import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
2526
import static com.github.tomakehurst.wiremock.client.WireMock.verify;
27+
import static software.amazon.awssdk.http.Header.CONTENT_TYPE;
2628

2729
import com.github.tomakehurst.wiremock.junit.WireMockRule;
2830
import java.net.URI;
@@ -65,6 +67,7 @@ public void syncCreateMultipartUpload_shouldHaveUploadsQueryParam() {
6567
s3Client.createMultipartUpload(b -> b.key("key").bucket("bucket"));
6668

6769
verify(anyRequestedFor(anyUrl()).withQueryParam("uploads", containing("")));
70+
verify(anyRequestedFor(anyUrl()).withHeader(CONTENT_TYPE, equalTo("binary/octet-stream")));
6871
}
6972

7073
@Test
@@ -74,5 +77,6 @@ public void asyncCreateMultipartUpload_shouldHaveUploadsQueryParam() {
7477
s3AsyncClient.createMultipartUpload(b -> b.key("key").bucket("bucket")).join();
7578

7679
verify(anyRequestedFor(anyUrl()).withQueryParam("uploads", containing("")));
80+
verify(anyRequestedFor(anyUrl()).withHeader(CONTENT_TYPE, equalTo("binary/octet-stream")));
7781
}
7882
}

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/CreateMultipartUploadRequestInterceptorTest.java

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
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.s3.internal.handlers;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static software.amazon.awssdk.http.Header.CONTENT_LENGTH;
20+
import static software.amazon.awssdk.http.Header.CONTENT_TYPE;
21+
22+
import java.util.Collections;
23+
import java.util.Optional;
24+
import org.junit.Test;
25+
import software.amazon.awssdk.core.interceptor.Context;
26+
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
27+
import software.amazon.awssdk.core.sync.RequestBody;
28+
import software.amazon.awssdk.http.SdkHttpRequest;
29+
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadRequest;
30+
import software.amazon.awssdk.services.s3.model.GetObjectAclRequest;
31+
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
32+
import software.amazon.awssdk.services.s3.utils.InterceptorTestUtils;
33+
34+
public class CreateMultipartUploadRequestInterceptorTest {
35+
36+
private final CreateMultipartUploadRequestInterceptor interceptor = new CreateMultipartUploadRequestInterceptor();
37+
38+
@Test
39+
public void createMultipartRequest_shouldModifyHttpContent() {
40+
Context.ModifyHttpRequest modifyHttpRequest =
41+
InterceptorTestUtils.modifyHttpRequestContext(CreateMultipartUploadRequest.builder().build());
42+
Optional<RequestBody> requestBody =
43+
interceptor.modifyHttpContent(modifyHttpRequest,
44+
new ExecutionAttributes());
45+
assertThat(modifyHttpRequest.requestBody().get()).isNotEqualTo(requestBody.get());
46+
}
47+
48+
@Test
49+
public void createMultipartRequest_shouldModifyHttpRequest() {
50+
Context.ModifyHttpRequest modifyHttpRequest =
51+
InterceptorTestUtils.modifyHttpRequestContext(CreateMultipartUploadRequest.builder().build());
52+
SdkHttpRequest httpRequest = interceptor.modifyHttpRequest(modifyHttpRequest, new ExecutionAttributes());
53+
assertThat(httpRequest).isNotEqualTo(modifyHttpRequest.httpRequest());
54+
assertThat(httpRequest.headers()).containsEntry(CONTENT_LENGTH, Collections.singletonList("0"));
55+
assertThat(httpRequest.headers()).containsEntry(CONTENT_TYPE, Collections.singletonList("binary/octet-stream"));
56+
}
57+
58+
@Test
59+
public void nonCreateMultipartRequest_shouldNotModifyHttpContent() {
60+
Context.ModifyHttpRequest modifyHttpRequest =
61+
InterceptorTestUtils.modifyHttpRequestContext(PutObjectRequest.builder().build());
62+
Optional<RequestBody> requestBody =
63+
interceptor.modifyHttpContent(modifyHttpRequest,
64+
new ExecutionAttributes());
65+
assertThat(modifyHttpRequest.requestBody().get()).isEqualTo(requestBody.get());
66+
}
67+
68+
@Test
69+
public void nonCreateMultipartRequest_shouldNotModifyHttpRequest() {
70+
Context.ModifyHttpRequest modifyHttpRequest =
71+
InterceptorTestUtils.modifyHttpRequestContext(GetObjectAclRequest.builder().build());
72+
SdkHttpRequest httpRequest = interceptor.modifyHttpRequest(modifyHttpRequest, new ExecutionAttributes());
73+
assertThat(httpRequest).isEqualTo(modifyHttpRequest.httpRequest());
74+
}
75+
}

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/utils/InterceptorTestUtils.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,12 @@ public SdkRequest request() {
8989
public static Context.ModifyHttpRequest modifyHttpRequestContext(SdkRequest request) {
9090
Optional<RequestBody> requestBody = Optional.of(RequestBody.fromString("helloworld"));
9191
Optional<AsyncRequestBody> asyncRequestBody = Optional.of(AsyncRequestBody.fromString("helloworld"));
92+
SdkHttpFullRequest sdkHttpFullRequest = sdkHttpFullRequest();
9293

9394
return new Context.ModifyHttpRequest() {
9495
@Override
9596
public SdkHttpRequest httpRequest() {
96-
return sdkHttpFullRequest();
97+
return sdkHttpFullRequest;
9798
}
9899

99100
@Override

0 commit comments

Comments
 (0)