Allow slicing a ByteSource starting at an offset that is >= the sourc…

…e's total size.

In this case, the sliced source will be empty. Previously, the call to slice() would succeed but an exception would occur when the user attempted to read the returned source. There is precedent for this behavior: FileChannel/SeekableByteChannel allow setting the channel's position beyond the end of the file, in which case reads return EOF.

This is a change to existing behavior, but I think it's probably a good change to make:

- Previously, it was unspecified what happens if offset is >= source.size. Now it's specified.
- Previously, a call to slice could succeed but return an (effectively) invalid ByteSource that would always throw.
- The override of ByteArrayByteSource.slice is currently using this behavior, because I thought that was the behavior that the normal slice implementation was already using. I could change that, but I think this is preferable.
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=102276980

guava-tests/test/com/google/common/io/ByteSourceTest.java

@@ -30,15 +30,16 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.hash.Hashing;
+import com.google.common.primitives.UnsignedBytes;
 import com.google.common.testing.TestLogHandler;
 
 import junit.framework.TestSuite;
 
 import java.io.ByteArrayOutputStream;
-import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.util.Arrays;
 import java.util.EnumSet;
 
 /**
@@ -210,11 +211,78 @@ public void testSlice() throws IOException {
     assertCorrectSlice(100, 5, 100, 95);
     assertCorrectSlice(100, 100, 0, 0);
     assertCorrectSlice(100, 100, 10, 0);
+    assertCorrectSlice(100, 101, 10, 0);
+  }
 
-    try {
-      assertCorrectSlice(100, 101, 10, 0);
-      fail();
-    } catch (EOFException expected) {
+  /**
+   * Tests that the default slice() behavior is correct when the source is sliced starting at an
+   * offset that is greater than the current length of the source, a stream is then opened to that
+   * source, and finally additional bytes are appended to the source before the stream is read.
+   *
+   * <p>Without special handling, it's possible to have reads of the open stream start <i>before</i>
+   * the offset at which the slice is supposed to start.
+   */
+  // TODO(cgdecker): Maybe add a test for this to ByteSourceTester
+  public void testSlice_appendingAfterSlicing() throws IOException {
+    // Source of length 5
+    AppendableByteSource source = new AppendableByteSource(newPreFilledByteArray(5));
+
+    // Slice it starting at offset 10.
+    ByteSource slice = source.slice(10, 5);
+
+    // Open a stream to the slice.
+    InputStream in = slice.openStream();
+
+    // Append 10 more bytes to the source.
+    source.append(newPreFilledByteArray(5, 10));
+
+    // The stream reports no bytes... importantly, it doesn't read the byte at index 5 when it
+    // should be reading the byte at index 10.
+    // We could use a custom InputStream instead to make the read start at index 10, but since this
+    // is a racy situation anyway, this behavior seems reasonable.
+    assertEquals(-1, in.read());
+  }
+
+  private static class AppendableByteSource extends ByteSource {
+    private byte[] bytes;
+
+    public AppendableByteSource(byte[] initialBytes) {
+      this.bytes = initialBytes.clone();
+    }
+
+    @Override
+    public InputStream openStream() {
+      return new In();
+    }
+
+    public void append(byte[] b) {
+      byte[] newBytes = Arrays.copyOf(bytes, bytes.length + b.length);
+      System.arraycopy(b, 0, newBytes, bytes.length, b.length);
+      bytes = newBytes;
+    }
+
+    private class In extends InputStream {
+      private int pos;
+
+      @Override
+      public int read() throws IOException {
+        byte[] b = new byte[1];
+        return read(b) == -1
+            ? -1
+            : UnsignedBytes.toInt(b[0]);
+      }
+
+      @Override
+      public int read(byte[] b, int off, int len) {
+        if (pos >= bytes.length) {
+          return -1;
+        }
+
+        int lenToRead = Math.min(len, bytes.length - pos);
+        System.arraycopy(bytes, pos, b, off, lenToRead);
+        pos += lenToRead;
+        return lenToRead;
+      }
     }
   }
 

guava-tests/test/com/google/common/io/ByteSourceTester.java

@@ -98,6 +98,12 @@ private static TestSuite suiteForBytes(ByteSourceFactory factory, byte[] bytes,
           factory, off, Long.MAX_VALUE);
       suite.addTest(suiteForBytes(slicedLongMaxValue, bytes, name + ".slice[long, Long.MAX_VALUE]",
           desc, false));
+
+      // test a slice() of the ByteSource starting at an offset greater than its size
+      ByteSourceFactory slicedOffsetPastEnd = SourceSinkFactories.asSlicedByteSourceFactory(
+          factory, expected.length + 2, expected.length + 10);
+      suite.addTest(suiteForBytes(slicedOffsetPastEnd, bytes, name + ".slice[size + 2, long]",
+          desc, false));
     }
 
     return suite;

guava/src/com/google/common/io/ByteSource.java

@@ -105,7 +105,10 @@ public InputStream openBufferedStream() throws IOException {
 
   /**
    * Returns a view of a slice of this byte source that is at most {@code length} bytes long
-   * starting at the given {@code offset}.
+   * starting at the given {@code offset}. If {@code offset} is greater than the size of this
+   * source, the returned source will be empty. If {@code offset + length} is greater than the size
+   * of this source, the returned source will contain the slice starting at {@code offset} and
+   * ending at the end of this source.
    *
    * @throws IllegalArgumentException if {@code offset} or {@code length} is negative
    */
@@ -492,8 +495,9 @@ public InputStream openBufferedStream() throws IOException {
 
     private InputStream sliceStream(InputStream in) throws IOException {
       if (offset > 0) {
+        long skipped;
         try {
-          ByteStreams.skipFully(in, offset);
+          skipped = ByteStreams.skipUpTo(in, offset);
         } catch (Throwable e) {
           Closer closer = Closer.create();
           closer.register(in);
@@ -503,6 +507,12 @@ private InputStream sliceStream(InputStream in) throws IOException {
             closer.close();
           }
         }
+
+        if (skipped < offset) {
+          // offset was beyond EOF
+          in.close();
+          return new ByteArrayInputStream(new byte[0]);
+        }
       }
       return ByteStreams.limit(in, length);
     }

guava/src/com/google/common/io/ByteStreams.java

@@ -704,21 +704,39 @@ public static void readFully(
    *     support skipping
    */
   public static void skipFully(InputStream in, long n) throws IOException {
-    long toSkip = n;
-    while (n > 0) {
-      long amt = in.skip(n);
-      if (amt == 0) {
+    long skipped = skipUpTo(in, n);
+    if (skipped < n) {
+      throw new EOFException("reached end of stream after skipping "
+          + skipped + " bytes; " + n + " bytes expected");
+    }
+  }
+
+  /**
+   * Discards up to {@code n} bytes of data from the input stream. This method
+   * will block until either the full amount has been skipped or until the end
+   * of the stream is reached, whichever happens first. Returns the total number
+   * of bytes skipped.
+   */
+  static long skipUpTo(InputStream in, final long n) throws IOException {
+    long totalSkipped = 0;
+
+    while (totalSkipped < n) {
+      long skipped = in.skip(n - totalSkipped);
+
+      if (skipped == 0) {
         // Force a blocking read to avoid infinite loop
         if (in.read() == -1) {
-          long skipped = toSkip - n;
-          throw new EOFException("reached end of stream after skipping "
-              + skipped + " bytes; " + toSkip + " bytes expected");
+          // Reached EOF
+          break;
+        } else {
+          skipped = 1;
         }
-        n--;
-      } else {
-        n -= amt;
       }
+
+      totalSkipped += skipped;
     }
+
+    return totalSkipped;
   }
 
   /**