From c6bebf968075a615435497ae4fd81b20d9b2aa84 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Wed, 12 Aug 2020 10:32:55 -0700 Subject: [PATCH] Added an "unsafe" way to retrieve a byte array from `SdkBytes` and `ResponseBytes` without copying the data. --- .../feature-AWSSDKforJavav2-67e3ac4.json | 5 ++ .../amazon/awssdk/spotbugs-suppressions.xml | 7 +++ .../amazon/awssdk/core/BytesWrapper.java | 24 ++++++--- .../amazon/awssdk/core/ResponseBytes.java | 32 +++++++++++- .../software/amazon/awssdk/core/SdkBytes.java | 16 +++++- .../amazon/awssdk/core/ResponseBytesTest.java | 49 +++++++++++++++++++ .../amazon/awssdk/core/SdkBytesTest.java | 48 ++++++++++++++++++ 7 files changed, 170 insertions(+), 11 deletions(-) create mode 100644 .changes/next-release/feature-AWSSDKforJavav2-67e3ac4.json create mode 100644 core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseBytesTest.java create mode 100644 core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkBytesTest.java diff --git a/.changes/next-release/feature-AWSSDKforJavav2-67e3ac4.json b/.changes/next-release/feature-AWSSDKforJavav2-67e3ac4.json new file mode 100644 index 000000000000..f3b0da0569e2 --- /dev/null +++ b/.changes/next-release/feature-AWSSDKforJavav2-67e3ac4.json @@ -0,0 +1,5 @@ +{ + "type": "feature", + "category": "AWS SDK for Java v2", + "description": "Added an \"unsafe\" way to retrieve a byte array from `SdkBytes` and `ResponseBytes` without copying the data." +} diff --git a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml index d1809a003d7d..35311e288a77 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml @@ -32,6 +32,13 @@ + + + + + + + diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/BytesWrapper.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/BytesWrapper.java index e2e735b31c09..52889e1c9890 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/BytesWrapper.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/BytesWrapper.java @@ -36,14 +36,12 @@ */ @SdkPublicApi public abstract class BytesWrapper { - private static final byte[] EMPTY_BYTES = new byte[0]; - private final byte[] bytes; // Needed for serialization @SdkInternalApi BytesWrapper() { - this(EMPTY_BYTES); + this(new byte[0]); } @SdkInternalApi @@ -51,10 +49,6 @@ public abstract class BytesWrapper { this.bytes = Validate.paramNotNull(bytes, "bytes"); } - final byte[] wrappedBytes() { - return bytes; - } - /** * @return The output as a read-only byte buffer. */ @@ -70,6 +64,22 @@ public final byte[] asByteArray() { return Arrays.copyOf(bytes, bytes.length); } + /** + * @return The output as a byte array. This does not create a copy of the underlying byte array. This introduces + * concurrency risks, allowing: (1) the caller to modify the byte array stored in this object implementation AND + * (2) the original creator of this object, if they created it using the unsafe method. + * + *

Consider using {@link #asByteBuffer()}, which is a safer method to avoid an additional array copy because it does not + * provide a way to modify the underlying buffer. As the method name implies, this is unsafe. If you're not sure, don't use + * this. The only guarantees given to the user of this method is that the SDK itself won't modify the underlying byte + * array.

+ * + * @see #asByteBuffer() to prevent creating an additional array copy safely. + */ + public final byte[] asByteArrayUnsafe() { + return bytes; + } + /** * Retrieve the output as a string. * diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseBytes.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseBytes.java index 61da73085c1b..2b3ed0523cca 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseBytes.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseBytes.java @@ -15,10 +15,15 @@ package software.amazon.awssdk.core; +import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely; + +import java.io.InputStream; +import java.io.UncheckedIOException; import java.util.Arrays; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.core.sync.ResponseTransformer; +import software.amazon.awssdk.utils.IoUtils; import software.amazon.awssdk.utils.ToString; import software.amazon.awssdk.utils.Validate; @@ -31,15 +36,38 @@ public final class ResponseBytes extends BytesWrapper { private final ResponseT response; - ResponseBytes(ResponseT response, byte[] bytes) { + private ResponseBytes(ResponseT response, byte[] bytes) { super(bytes); this.response = Validate.paramNotNull(response, "response"); } + /** + * Create {@link ResponseBytes} from a Byte array. This will copy the contents of the byte array. + */ + public static ResponseBytes fromInputStream(ResponseT response, InputStream stream) + throws UncheckedIOException { + return new ResponseBytes<>(response, invokeSafely(() -> IoUtils.toByteArray(stream))); + } + + /** + * Create {@link ResponseBytes} from a Byte array. This will copy the contents of the byte array. + */ public static ResponseBytes fromByteArray(ResponseT response, byte[] bytes) { return new ResponseBytes<>(response, Arrays.copyOf(bytes, bytes.length)); } + /** + * Create {@link ResponseBytes} from a Byte array without copying the contents of the byte array. This introduces + * concurrency risks, allowing: (1) the caller to modify the byte array stored in this {@code SdkBytes} implementation AND + * (2) any users of {@link #asByteArrayUnsafe()} to modify the byte array passed into this {@code SdkBytes} implementation. + * + *

As the method name implies, this is unsafe. Use {@link #fromByteArray(Object, byte[])} unless you're sure you know the + * risks. + */ + public static ResponseBytes fromByteArrayUnsafe(ResponseT response, byte[] bytes) { + return new ResponseBytes<>(response, bytes); + } + /** * @return the unmarshalled response object from the service. */ @@ -51,7 +79,7 @@ public ResponseT response() { public String toString() { return ToString.builder("ResponseBytes") .add("response", response) - .add("bytes", wrappedBytes()) + .add("bytes", asByteArrayUnsafe()) .build(); } diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkBytes.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkBytes.java index 065794fc0b4a..1677f5806803 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkBytes.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkBytes.java @@ -52,7 +52,7 @@ private SdkBytes() { * @see #fromUtf8String(String) * @see #fromString(String, Charset) */ - SdkBytes(byte[] bytes) { + private SdkBytes(byte[] bytes) { super(bytes); } @@ -72,6 +72,18 @@ public static SdkBytes fromByteArray(byte[] bytes) { return new SdkBytes(Arrays.copyOf(bytes, bytes.length)); } + /** + * Create {@link SdkBytes} from a Byte array without copying the contents of the byte array. This introduces + * concurrency risks, allowing: (1) the caller to modify the byte array stored in this {@code SdkBytes} implementation AND + * (2) any users of {@link #asByteArrayUnsafe()} to modify the byte array passed into this {@code SdkBytes} implementation. + * + *

As the method name implies, this is unsafe. Use {@link #fromByteArray(byte[])} unless you're sure you know the risks. + */ + public static SdkBytes fromByteArrayUnsafe(byte[] bytes) { + Validate.paramNotNull(bytes, "bytes"); + return new SdkBytes(bytes); + } + /** * Create {@link SdkBytes} from a string, using the provided charset. */ @@ -100,7 +112,7 @@ public static SdkBytes fromInputStream(InputStream inputStream) { @Override public String toString() { return ToString.builder("SdkBytes") - .add("bytes", wrappedBytes()) + .add("bytes", asByteArrayUnsafe()) .build(); } } diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseBytesTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseBytesTest.java new file mode 100644 index 000000000000..f41a0179b6ad --- /dev/null +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseBytesTest.java @@ -0,0 +1,49 @@ +/* + * 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; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.Test; + +public class ResponseBytesTest { + private static final Object OBJECT = new Object(); + @Test + public void fromByteArrayCreatesCopy() { + byte[] input = new byte[] { 'a' }; + byte[] output = ResponseBytes.fromByteArray(OBJECT, input).asByteArrayUnsafe(); + + input[0] = 'b'; + assertThat(output).isNotEqualTo(input); + } + + @Test + public void asByteArrayCreatesCopy() { + byte[] input = new byte[] { 'a' }; + byte[] output = ResponseBytes.fromByteArrayUnsafe(OBJECT, input).asByteArray(); + + input[0] = 'b'; + assertThat(output).isNotEqualTo(input); + } + + @Test + public void fromByteArrayUnsafeAndAsByteArrayUnsafeDoNotCopy() { + byte[] input = new byte[] { 'a' }; + byte[] output = ResponseBytes.fromByteArrayUnsafe(OBJECT, input).asByteArrayUnsafe(); + + assertThat(output).isSameAs(input); + } +} diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkBytesTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkBytesTest.java new file mode 100644 index 000000000000..1503f08ae54b --- /dev/null +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/SdkBytesTest.java @@ -0,0 +1,48 @@ +/* + * 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; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.Test; + +public class SdkBytesTest { + @Test + public void fromByteArrayCreatesCopy() { + byte[] input = new byte[] { 'a' }; + byte[] output = SdkBytes.fromByteArray(input).asByteArrayUnsafe(); + + input[0] = 'b'; + assertThat(output).isNotEqualTo(input); + } + + @Test + public void asByteArrayCreatesCopy() { + byte[] input = new byte[] { 'a' }; + byte[] output = SdkBytes.fromByteArrayUnsafe(input).asByteArray(); + + input[0] = 'b'; + assertThat(output).isNotEqualTo(input); + } + + @Test + public void fromByteArrayUnsafeAndAsByteArrayUnsafeDoNotCopy() { + byte[] input = new byte[] { 'a' }; + byte[] output = SdkBytes.fromByteArrayUnsafe(input).asByteArrayUnsafe(); + + assertThat(output).isSameAs(input); + } +}