From 398cdaabb6aad9eb987a97f4b6d434d56d093a75 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Thu, 27 Jul 2017 13:51:50 -0400 Subject: [PATCH] google-cloud-nio: CloudStorageReadChannel: fix critical bug that could cause the channel position to overflow This fixes a critical bug in CloudStorageReadChannel in which the channel position (a long) was getting down-cast to int in CloudStorageReadChannel.innerOpen(). This could cause the channel position to become negative or a wildly incorrect positive value if the channel position was greater than MAX_INT. In practice, this bug would only manifest itself if the channel was constructed with a non-zero initial position, or if the channel was reopened in response to an error. Included a test case that fails without the fix, and passes with the fix. --- .../contrib/nio/CloudStorageReadChannel.java | 2 +- .../nio/CloudStorageReadChannelTest.java | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannel.java b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannel.java index 515871ea61d6..8df570f91093 100644 --- a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannel.java +++ b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannel.java @@ -82,7 +82,7 @@ private CloudStorageReadChannel(Storage gcsStorage, BlobId file, long position, private void innerOpen() throws IOException { this.channel = gcsStorage.reader(file); if (position > 0) { - channel.seek((int) position); + channel.seek(position); } } diff --git a/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannelTest.java b/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannelTest.java index 9a9f87c4aa8e..917bff79050f 100644 --- a/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannelTest.java +++ b/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannelTest.java @@ -37,6 +37,7 @@ import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; import javax.net.ssl.SSLHandshakeException; import java.io.IOException; @@ -191,4 +192,25 @@ public void testSetPosition() throws IOException { verify(gcsChannel).seek(1); verify(gcsChannel, times(5)).isOpen(); } + + /* + * This test case was crafted in response to a bug in CloudStorageReadChannel in which the + * channel position (a long) was getting truncated to an int when seeking on the encapsulated + * ReadChannel in innerOpen(). This test case fails when the bad long -> int cast is present, + * and passes when it's removed. + */ + @Test + public void testChannelPositionDoesNotGetTruncatedToInt() throws IOException { + // This position value will overflow to a negative value if a long -> int cast is attempted + long startPosition = 11918483280L; + ArgumentCaptor captor = ArgumentCaptor.forClass(Long.class); + + // Invoke CloudStorageReadChannel.create() to trigger a call to the private + // CloudStorageReadChannel.innerOpen() method, which does a seek on our gcsChannel. + CloudStorageReadChannel.create(gcsStorage, file, startPosition, 1); + + // Confirm that our position did not overflow during the seek in CloudStorageReadChannel.innerOpen() + verify(gcsChannel).seek(captor.capture()); + assertThat(captor.getValue()).isEqualTo(startPosition); + } }