Skip to content

Commit

Permalink
Fixed an issue that the SDK unnecessarily bufferred the entire conten…
Browse files Browse the repository at this point in the history
…t for streaming operations (#5855)
  • Loading branch information
zoewangg authored Feb 4, 2025
1 parent bf0f2c7 commit 7ca84b8
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 7ca84b8

Please sign in to comment.