Skip to content

Added an "unsafe" way to retrieve a byte array from SdkBytes and ResponseBytes without copying the data. #1977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/next-release/feature-AWSSDKforJavav2-67e3ac4.json
Original file line number Diff line number Diff line change
@@ -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."
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@
<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2" />
</Match>

<Match>
<!-- Explicitly an unsafe option that the customer must opt into. -->
<Class name="software.amazon.awssdk.core.BytesWrapper" />
<Method name="asByteArrayUnsafe" />
<Bug pattern="EI_EXPOSE_REP" />
</Match>

<!-- Delegate closes input stream. -->
<Match>
<Class name="software.amazon.awssdk.protocols.ion.internal.IonFactory" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,19 @@
*/
@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
BytesWrapper(byte[] bytes) {
this.bytes = Validate.paramNotNull(bytes, "bytes");
}

final byte[] wrappedBytes() {
return bytes;
}

/**
* @return The output as a read-only byte buffer.
*/
Expand All @@ -70,6 +64,22 @@ public final byte[] asByteArray() {
return Arrays.copyOf(bytes, bytes.length);
}

/**
* @return The output as a byte array. This <b>does not</b> 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.
*
* <p>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.</p>
*
* @see #asByteBuffer() to prevent creating an additional array copy safely.
*/
public final byte[] asByteArrayUnsafe() {
return bytes;
}

/**
* Retrieve the output as a string.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -31,15 +36,38 @@
public final class ResponseBytes<ResponseT> 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 <ResponseT> ResponseBytes<ResponseT> 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 <ResponseT> ResponseBytes<ResponseT> fromByteArray(ResponseT response, byte[] bytes) {
return new ResponseBytes<>(response, Arrays.copyOf(bytes, bytes.length));
}

/**
* Create {@link ResponseBytes} from a Byte array <b>without</b> 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.
*
* <p>As the method name implies, this is unsafe. Use {@link #fromByteArray(Object, byte[])} unless you're sure you know the
* risks.
*/
public static <ResponseT> ResponseBytes<ResponseT> fromByteArrayUnsafe(ResponseT response, byte[] bytes) {
return new ResponseBytes<>(response, bytes);
}

/**
* @return the unmarshalled response object from the service.
*/
Expand All @@ -51,7 +79,7 @@ public ResponseT response() {
public String toString() {
return ToString.builder("ResponseBytes")
.add("response", response)
.add("bytes", wrappedBytes())
.add("bytes", asByteArrayUnsafe())
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ private SdkBytes() {
* @see #fromUtf8String(String)
* @see #fromString(String, Charset)
*/
SdkBytes(byte[] bytes) {
private SdkBytes(byte[] bytes) {
super(bytes);
}

Expand All @@ -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 <b>without</b> 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.
*
* <p>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.
*/
Expand Down Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}