Skip to content

Commit a1a3151

Browse files
committed
Add tests and update javadocs
1 parent 72a5150 commit a1a3151

File tree

2 files changed

+61
-9
lines changed

2 files changed

+61
-9
lines changed

core/sdk-core/src/main/java/software/amazon/awssdk/core/ResponseBytes.java

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,24 @@ public static <ResponseT> ResponseBytes<ResponseT> fromByteArrayUnsafe(ResponseT
7070
}
7171

7272
/**
73-
* Creates ResponseBytes from a ByteBuffer without copying the underlying data.
73+
* Create {@link ResponseBytes} from a {@link ByteBuffer} with minimal copying. This method attempts to avoid
74+
* copying data when possible, but introduces concurrency risks in specific scenarios.
7475
*
75-
* @param response the response object containing metadata
76-
* @param buffer the ByteBuffer containing the response body data
77-
* @return ResponseBytes wrapping the buffer data
76+
* <p><b>Behavior by buffer type:</b>
77+
* <ul>
78+
* <li><b>Array-backed ByteBuffer (perfect match):</b> When the buffer represents the entire backing array
79+
* (offset=0, remaining=array.length), the array is shared <b>without</b> copying. This introduces the same
80+
* concurrency risks as {@link #fromByteArrayUnsafe(Object, byte[])}: modifications to the original
81+
* backing array will affect the returned {@link ResponseBytes}.</li>
82+
* <li><b>Array-backed ByteBuffer (partial):</b> When the buffer represents only a portion of the backing array,
83+
* data is copied to a new array. No concurrency risks.</li>
84+
* <li><b>Direct ByteBuffer:</b> Data is always copied to a heap array. No concurrency risks.</li>
85+
* </ul>
86+
*
87+
* <p>The buffer's position is preserved and not modified by this operation.
88+
*
89+
* <p>As the method name implies, this is unsafe in the first scenario. Use a safe alternative unless you're
90+
* sure you know the risks.
7891
*/
7992
public static <ResponseT> ResponseBytes<ResponseT> fromByteBufferUnsafe(ResponseT response, ByteBuffer buffer) {
8093
byte[] array;
@@ -91,7 +104,9 @@ public static <ResponseT> ResponseBytes<ResponseT> fromByteBufferUnsafe(Response
91104
} else {
92105
// Direct buffer - must copy to array
93106
array = new byte[buffer.remaining()];
107+
int originalPosition = buffer.position();
94108
buffer.get(array);
109+
buffer.position(originalPosition);
95110
}
96111
return new ResponseBytes<>(response, array);
97112
}

core/sdk-core/src/test/java/software/amazon/awssdk/core/ResponseBytesTest.java

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,20 +49,57 @@ public void fromByteArrayUnsafeAndAsByteArrayUnsafeDoNotCopy() {
4949
}
5050

5151
@Test
52-
public void fromByteBufferUnsafe_doNotCopy() {
52+
public void fromByteBufferUnsafe_fullBuffer_doesNotCopy() {
5353
byte[] inputBytes = {'a'};
5454
ByteBuffer inputBuffer = ByteBuffer.wrap(inputBytes);
5555

5656
ResponseBytes<Object> responseBytes = ResponseBytes.fromByteBufferUnsafe(OBJECT, inputBuffer);
57-
58-
ByteBuffer outputBuffer = responseBytes.asByteBuffer();
5957
byte[] outputBytes = responseBytes.asByteArrayUnsafe();
6058

61-
assertThat(outputBuffer).isEqualTo(inputBuffer);
59+
assertThat(inputBuffer.hasArray()).isTrue();
60+
assertThat(inputBuffer.isDirect()).isFalse();
6261
assertThat(outputBytes).isSameAs(inputBytes);
6362

6463
inputBytes[0] = 'b';
65-
assertThat(outputBuffer).isEqualTo(inputBuffer);
64+
assertThat(outputBytes[0]).isEqualTo((byte) 'b');
65+
}
66+
67+
@Test
68+
public void fromByteBufferUnsafe_directBuffer_createsCopy() {
69+
byte[] inputBytes = {'a'};
70+
ByteBuffer directBuffer = ByteBuffer.allocateDirect(1);
71+
directBuffer.put(inputBytes);
72+
directBuffer.flip();
73+
74+
ResponseBytes<Object> responseBytes = ResponseBytes.fromByteBufferUnsafe(OBJECT, directBuffer);
75+
ByteBuffer outputBuffer = responseBytes.asByteBuffer();
76+
byte[] outputBytes = responseBytes.asByteArrayUnsafe();
77+
78+
assertThat(directBuffer.hasArray()).isFalse();
79+
assertThat(directBuffer.isDirect()).isTrue();
80+
assertThat(outputBuffer.isDirect()).isFalse();
6681
assertThat(outputBytes).isEqualTo(inputBytes);
82+
assertThat(outputBytes).isNotSameAs(inputBytes);
83+
84+
inputBytes[0] = 'b';
85+
assertThat(outputBytes[0]).isNotEqualTo((byte) 'b');
86+
}
87+
88+
@Test
89+
public void fromByteBufferUnsafe_bufferWithOffset_createsCopy() {
90+
byte[] inputBytes = "abcdefgh".getBytes();
91+
92+
ByteBuffer slicedBuffer = ByteBuffer.wrap(inputBytes, 2, 3); // "cde"
93+
94+
ResponseBytes<Object> responseBytes = ResponseBytes.fromByteBufferUnsafe(OBJECT, slicedBuffer);
95+
byte[] outputBytes = responseBytes.asByteArrayUnsafe();
96+
97+
assertThat(slicedBuffer.hasArray()).isTrue();
98+
assertThat(outputBytes).isEqualTo("cde".getBytes());
99+
assertThat(outputBytes.length).isEqualTo(3);
100+
assertThat(outputBytes).isNotSameAs(inputBytes);
101+
102+
inputBytes[0] = 'X';
103+
assertThat(outputBytes[0]).isEqualTo((byte) 'c');
67104
}
68105
}

0 commit comments

Comments
 (0)