From 7ca84b8916ca3dd2491f4f62994d933eac675bad Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Mon, 3 Feb 2025 18:36:32 -0800 Subject: [PATCH] Fixed an issue that the SDK unnecessarily bufferred the entire content for streaming operations (#5855) --- .../bugfix-AWSSDKforJavav2-f6370d6.json | 6 ++++ .../software/amazon/awssdk/checkstyle.xml | 9 +++++ .../internal/handler/BaseClientHandler.java | 6 ++-- .../handler/SdkInternalOnlyRequestBody.java | 35 +++++++++++++++++++ .../amazon/awssdk/core/sync/RequestBody.java | 17 +++++---- .../internal/SwitchToPostInterceptor.java | 3 ++ 6 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 .changes/next-release/bugfix-AWSSDKforJavav2-f6370d6.json create mode 100644 core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/SdkInternalOnlyRequestBody.java diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-f6370d6.json b/.changes/next-release/bugfix-AWSSDKforJavav2-f6370d6.json new file mode 100644 index 000000000000..8180ede127fe --- /dev/null +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-f6370d6.json @@ -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)." +} diff --git a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle.xml b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle.xml index 532429b1cfd2..783d61377a20 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle.xml @@ -392,6 +392,15 @@ + + + + + + + + diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseClientHandler.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseClientHandler.java index 393770d898f7..521155a0cd0c 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseClientHandler.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/BaseClientHandler.java @@ -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; diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/SdkInternalOnlyRequestBody.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/SdkInternalOnlyRequestBody.java new file mode 100644 index 000000000000..00df8e43e01b --- /dev/null +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/handler/SdkInternalOnlyRequestBody.java @@ -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. + * + *

+ * 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); + } +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/RequestBody.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/RequestBody.java index bb90ab29e2f6..e66a8b61cbc7 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/RequestBody.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/RequestBody.java @@ -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; @@ -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). + * + *

+ * 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"); @@ -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; } @@ -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; @@ -79,14 +84,14 @@ public long contentLength() { /** * @return Optional object of content length of {@link RequestBody}. */ - public Optional optionalContentLength() { + public final Optional optionalContentLength() { return Optional.ofNullable(contentLength); } /** * @return Content type of {@link RequestBody}. */ - public String contentType() { + public final String contentType() { return contentType; } diff --git a/services/cloudsearchdomain/src/main/java/software/amazon/awssdk/services/cloudsearchdomain/internal/SwitchToPostInterceptor.java b/services/cloudsearchdomain/src/main/java/software/amazon/awssdk/services/cloudsearchdomain/internal/SwitchToPostInterceptor.java index 7c3721a8c9b7..876cd052c4ea 100644 --- a/services/cloudsearchdomain/src/main/java/software/amazon/awssdk/services/cloudsearchdomain/internal/SwitchToPostInterceptor.java +++ b/services/cloudsearchdomain/src/main/java/software/amazon/awssdk/services/cloudsearchdomain/internal/SwitchToPostInterceptor.java @@ -54,10 +54,13 @@ public Optional 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(); }