diff --git a/.changes/next-release/bugfix-S3-7ca353f.json b/.changes/next-release/bugfix-S3-7ca353f.json new file mode 100644 index 000000000000..c5544ea8e7c9 --- /dev/null +++ b/.changes/next-release/bugfix-S3-7ca353f.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "S3", + "contributor": "", + "description": "Fixed single-byte `read()` on empty S3 objects returning checksum metadata instead of EOF" +} diff --git a/services/s3/src/it/java/software/amazon/awssdk/services/s3/urlconnection/EmptyFileS3IntegrationTest.java b/services/s3/src/it/java/software/amazon/awssdk/services/s3/urlconnection/EmptyFileS3IntegrationTest.java index 7de782e87bf6..c06536d10bd0 100644 --- a/services/s3/src/it/java/software/amazon/awssdk/services/s3/urlconnection/EmptyFileS3IntegrationTest.java +++ b/services/s3/src/it/java/software/amazon/awssdk/services/s3/urlconnection/EmptyFileS3IntegrationTest.java @@ -18,11 +18,17 @@ import static org.assertj.core.api.Assertions.assertThat; import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName; +import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import software.amazon.awssdk.core.ResponseInputStream; +import software.amazon.awssdk.core.async.AsyncRequestBody; +import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.core.sync.RequestBody; +import software.amazon.awssdk.services.s3.S3AsyncClient; import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.GetObjectResponse; public class EmptyFileS3IntegrationTest extends UrlHttpConnectionS3IntegrationTestBase { private static final String BUCKET = temporaryBucketName(EmptyFileS3IntegrationTest.class); @@ -54,4 +60,50 @@ public void s3EmptyFileContentLengthIsCorrectWithoutChecksumValidationEnabled() assertThat(s3.getObject(r -> r.bucket(BUCKET).key("x")).response().contentLength()).isEqualTo(0); } } + + @Test + public void s3EmptyFileGetAsInputStreamReturnsEOF() throws Exception{ + try (S3Client s3 = s3ClientBuilder().build()) { + s3.putObject(r -> r.bucket(BUCKET).key("x"), RequestBody.empty()); + + ResponseInputStream stream = + s3.getObject(r -> r.bucket(BUCKET).key("x")); + + assertThat(stream.read()).isEqualTo(-1); + + stream.close(); + } + } + + @Test + public void syncEmptyObjectWithChecksumValidation_arrayRead_returnsEOF() throws Exception { + try (S3Client s3 = s3ClientBuilder().build()) { + s3.putObject(r -> r.bucket(BUCKET).key("x"), RequestBody.empty()); + + ResponseInputStream stream = + s3.getObject(r -> r.bucket(BUCKET).key("x")); + + byte[] buffer = new byte[100]; + int bytesRead = stream.read(buffer); + + assertThat(bytesRead).isEqualTo(-1); + + stream.close(); + } + } + + @Test + public void asyncEmptyObjectWithChecksumValidation_returnsEmpty() throws Exception { + try (S3AsyncClient s3Async = S3AsyncClient.builder().region(DEFAULT_REGION).build()) { + s3Async.putObject(r -> r.bucket(BUCKET).key("x"),AsyncRequestBody.empty()).join(); + + + CompletableFuture future = s3Async.getObject(r -> r.bucket(BUCKET).key("x"), + AsyncResponseTransformer.toBytes()).thenApply( + res -> res.asByteArray()); + + byte[] content = future.join(); + assertThat(content).isEmpty(); + } + } } diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/checksums/S3ChecksumValidatingInputStream.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/checksums/S3ChecksumValidatingInputStream.java index 986308ca1825..e04dde11f2d7 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/checksums/S3ChecksumValidatingInputStream.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/checksums/S3ChecksumValidatingInputStream.java @@ -58,6 +58,18 @@ public S3ChecksumValidatingInputStream(InputStream in, SdkChecksum cksum, long s */ @Override public int read() throws IOException { + if (strippedLength == 0 && lengthRead == 0) { + for (int i = 0; i < CHECKSUM_SIZE; i++) { + int b = inputStream.read(); + if (b != -1) { + streamChecksum[i] = (byte) b; + } + } + lengthRead = CHECKSUM_SIZE; + validateAndThrow(); + return -1; + } + int read = inputStream.read(); if (read != -1 && lengthRead < strippedLength) { diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/checksums/S3ChecksumValidatingInputStreamTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/checksums/S3ChecksumValidatingInputStreamTest.java index cf9163c54e6e..9419a91c9708 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/checksums/S3ChecksumValidatingInputStreamTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/checksums/S3ChecksumValidatingInputStreamTest.java @@ -16,6 +16,7 @@ package software.amazon.awssdk.services.s3.checksums; import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; import java.io.ByteArrayInputStream; @@ -56,12 +57,43 @@ public static void populateData() { @Test public void validChecksumSucceeds() throws IOException { - InputStream validatingInputStream = newValidatingStream(testData); + InputStream validatingInputStream = newValidatingStream(testData, TEST_DATA_SIZE, CHECKSUM_SIZE); byte[] dataFromValidatingStream = IoUtils.toByteArray(validatingInputStream); assertArrayEquals(testDataWithoutChecksum, dataFromValidatingStream); } + @Test + public void emptyObjectSingleByteReadReturnsEOF() throws IOException { + Md5Checksum checksum = new Md5Checksum(); + byte[] checksumBytes = checksum.getChecksumBytes(); + byte[] emptyWithChecksum = new byte[CHECKSUM_SIZE]; + + for (int i = 0; i < CHECKSUM_SIZE; i++) { + emptyWithChecksum[i] = checksumBytes[i]; + } + + InputStream validatingInputStream = newValidatingStream(emptyWithChecksum, 0, CHECKSUM_SIZE); + + assertEquals(-1, validatingInputStream.read()); + } + + @Test + public void emptyObjectByteArrayReadReturnsEOF() throws IOException { + Md5Checksum checksum = new Md5Checksum(); + byte[] checksumBytes = checksum.getChecksumBytes(); + byte[] emptyWithChecksum = new byte[CHECKSUM_SIZE]; + + for (int i = 0; i < CHECKSUM_SIZE; i++) { + emptyWithChecksum[i] = checksumBytes[i]; + } + + InputStream validatingInputStream = newValidatingStream(emptyWithChecksum, 0, CHECKSUM_SIZE); + byte[] buffer = new byte[1]; + + assertEquals(-1, validatingInputStream.read(buffer)); + } + @Test public void invalidChecksumFails() throws IOException { for (int i = 0; i < testData.length; i++) { @@ -69,7 +101,7 @@ public void invalidChecksumFails() throws IOException { byte[] corruptedChecksumData = Arrays.copyOf(testData, testData.length); corruptedChecksumData[i] = (byte) ~corruptedChecksumData[i]; - InputStream validatingInputStream = newValidatingStream(corruptedChecksumData); + InputStream validatingInputStream = newValidatingStream(corruptedChecksumData, TEST_DATA_SIZE, CHECKSUM_SIZE); try { IoUtils.toByteArray(validatingInputStream); @@ -80,9 +112,9 @@ public void invalidChecksumFails() throws IOException { } } - private InputStream newValidatingStream(byte[] dataFromS3) { + private InputStream newValidatingStream(byte[] dataFromS3, int testDataSize, int checkSumSize) { return new S3ChecksumValidatingInputStream(new ByteArrayInputStream(dataFromS3), new Md5Checksum(), - TEST_DATA_SIZE + CHECKSUM_SIZE); + testDataSize + checkSumSize); } } \ No newline at end of file diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/checksums/S3ChecksumValidatingPublisherTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/checksums/S3ChecksumValidatingPublisherTest.java index 02d59acf6abb..81abc862e297 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/checksums/S3ChecksumValidatingPublisherTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/checksums/S3ChecksumValidatingPublisherTest.java @@ -169,6 +169,28 @@ public void checksumValidationFailure_throwsSdkClientException_NotNPE() { assertFalse(s.hasCompleted()); } + @Test + public void emptyObjectReturnsNoData() { + Md5Checksum checksum = new Md5Checksum(); + byte[] checksumBytes = checksum.getChecksumBytes(); + byte[] emptyWithChecksum = new byte[CHECKSUM_SIZE]; + for (int i = 0; i < CHECKSUM_SIZE; i++) { + emptyWithChecksum[i] = checksumBytes[i]; + } + + final TestPublisher driver = new TestPublisher(); + final TestSubscriber s = new TestSubscriber(); + final S3ChecksumValidatingPublisher p = new S3ChecksumValidatingPublisher(driver, new Md5Checksum(), CHECKSUM_SIZE); + p.subscribe(s); + + driver.doOnNext(ByteBuffer.wrap(emptyWithChecksum)); + driver.doOnComplete(); + + assertArrayEquals(new byte[0], s.receivedData()); + assertTrue(s.hasCompleted()); + assertFalse(s.isOnErrorCalled()); + } + private class TestSubscriber implements Subscriber { final List received; boolean completed;