Skip to content

Commit

Permalink
Merge branch 'master' into hdavidh/alpn-http-request
Browse files Browse the repository at this point in the history
  • Loading branch information
davidh44 authored Feb 4, 2025
2 parents 1264d9a + 7ca84b8 commit b4cc3a2
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 9 deletions.
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-f6370d6.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "AWS SDK for Java v2",
"contributor": "",
"description": "Fixed an issue in the SDK where it unnecessarily buffers the entire content for streaming operations, causing OOM error. See [#5850](https://github.com/aws/aws-sdk-java-v2/issues/5850)."
}
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,15 @@
<property name="ignoreComments" value="true"/>
</module>

<!-- Checks that we don't use RequestBody.fromContentProvider in the SDK core -->
<module name="Regexp">
<property name="format" value="RequestBody\.fromContentProvider"/>
<property name="illegalPattern" value="true"/>
<property name="message" value="DO NOT use fromContentProvider for streaming operations because it will buffer the entire content.
Add suppression if it's for a non-streaming operation"/>
<property name="ignoreComments" value="true"/>
</module>

<!-- Checks that we don't use AttributeKey.newInstance directly -->
<module name="Regexp">
<property name="format" value="AttributeKey\.newInstance"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ private static RequestBody getBody(SdkHttpFullRequest request) {
streamProvider = () -> new SdkLengthAwareInputStream(toWrap.newStream(), contentLength);
}

return RequestBody.fromContentProvider(streamProvider,
contentLength,
contentType);
return new SdkInternalOnlyRequestBody(streamProvider,
contentLength,
contentType);
}

return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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.core.internal.handler;

import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.sync.RequestBody;
import software.amazon.awssdk.http.ContentStreamProvider;

/**
* This class is needed as an alternative to {@link RequestBody#fromContentProvider(ContentStreamProvider, long, String)},
* which buffers the entire content.
*
* <p>
* THIS IS AN INTERNAL API. DO NOT USE IT OUTSIDE THE AWS SDK FOR JAVA V2.
*/
@SdkInternalApi
class SdkInternalOnlyRequestBody extends RequestBody {

protected SdkInternalOnlyRequestBody(ContentStreamProvider contentStreamProvider, Long contentLength, String contentType) {
super(contentStreamProvider, contentLength, contentType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Optional;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.core.internal.sync.BufferingContentStreamProvider;
import software.amazon.awssdk.core.internal.sync.FileContentStreamProvider;
Expand All @@ -43,16 +44,20 @@
/**
* Represents the body of an HTTP request. Must be provided for operations that have a streaming input.
* Offers various convenience factory methods from common sources of data (File, String, byte[], etc).
*
* <p>
* This class is NOT intended to be overridden.
*/
@SdkPublicApi
public final class RequestBody {
public class RequestBody {

// TODO Handle stream management (progress listener, orig input stream tracking, etc
private final ContentStreamProvider contentStreamProvider;
private final Long contentLength;
private final String contentType;

private RequestBody(ContentStreamProvider contentStreamProvider, Long contentLength, String contentType) {
@SdkInternalApi
protected RequestBody(ContentStreamProvider contentStreamProvider, Long contentLength, String contentType) {
this.contentStreamProvider = paramNotNull(contentStreamProvider, "contentStreamProvider");
this.contentLength = contentLength != null ? isNotNegative(contentLength, "Content-length") : null;
this.contentType = paramNotNull(contentType, "contentType");
Expand All @@ -61,7 +66,7 @@ private RequestBody(ContentStreamProvider contentStreamProvider, Long contentLen
/**
* @return RequestBody as an {@link InputStream}.
*/
public ContentStreamProvider contentStreamProvider() {
public final ContentStreamProvider contentStreamProvider() {
return contentStreamProvider;
}

Expand All @@ -70,7 +75,7 @@ public ContentStreamProvider contentStreamProvider() {
* @return Content length of {@link RequestBody}.
*/
@Deprecated
public long contentLength() {
public final long contentLength() {
validState(this.contentLength != null,
"Content length is invalid, please use optionalContentLength() for your case.");
return contentLength;
Expand All @@ -79,14 +84,14 @@ public long contentLength() {
/**
* @return Optional object of content length of {@link RequestBody}.
*/
public Optional<Long> optionalContentLength() {
public final Optional<Long> optionalContentLength() {
return Optional.ofNullable(contentLength);
}

/**
* @return Content type of {@link RequestBody}.
*/
public String contentType() {
public final String contentType() {
return contentType;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,13 @@ public Optional<RequestBody> modifyHttpContent(Context.ModifyHttpRequest context
if (context.request() instanceof SearchRequest) {
byte[] params = context.httpRequest().encodedQueryParametersAsFormData().orElse("")
.getBytes(StandardCharsets.UTF_8);
// CHECKSTYLE:OFF - Avoid flagging the use of fromContentProvider. This is fine here because it's non-streaming
// operation
return Optional.of(RequestBody.fromContentProvider(() -> new ByteArrayInputStream(params),
params.length,
"application/x-www-form-urlencoded; charset=" +
lowerCase(StandardCharsets.UTF_8.toString())));
// CHECKSTYLE:ON
}
return context.requestBody();
}
Expand Down

0 comments on commit b4cc3a2

Please sign in to comment.