Bug 684806. Make sure that zero-size files wrapped in buffered streams are correctly reopened by a seek after EOF. r=honza, sr=bsmedberg

netwerk/base/src/nsBufferedStreams.cpp

@@ -80,6 +80,7 @@ nsBufferedStream::nsBufferedStream()
       mFillPoint(0),
       mStream(nsnull),
       mBufferDisabled(false),
+      mEOF(false),
       mGetBufferCount(0)
 {
 }
@@ -186,8 +187,11 @@ nsBufferedStream::Seek(PRInt32 whence, PRInt64 offset)
     // between the current cursor and the mFillPoint "fencepost" -- the
     // client may never get around to a Read or Write after this Seek.
     // Read and Write worry about flushing and filling in that event.
+    // But if we're at EOF, make sure to pass the seek through to the
+    // underlying stream, because it may have auto-closed itself and
+    // needs to reopen.
     PRUint32 offsetInBuffer = PRUint32(absPos - mBufferStartOffset);
-    if (offsetInBuffer <= mFillPoint) {
+    if (offsetInBuffer <= mFillPoint && !mEOF) {
         METER(bufstats.mSeeksWithinBuffer++);
         mCursor = offsetInBuffer;
         return NS_OK;
@@ -202,6 +206,21 @@ nsBufferedStream::Seek(PRInt32 whence, PRInt64 offset)
     rv = ras->Seek(whence, offset);
     if (NS_FAILED(rv)) return rv;
 
+    mEOF = false;
+
+    // Recompute whether the offset we're seeking to is in our buffer.
+    // Note that we need to recompute because Flush() might have
+    // changed mBufferStartOffset.
+    offsetInBuffer = PRUint32(absPos - mBufferStartOffset);
+    if (offsetInBuffer <= mFillPoint) {
+        // It's safe to just set mCursor to offsetInBuffer.  In particular, we
+        // want to avoid calling Fill() here since we already have the data that
+        // was seeked to and calling Fill() might auto-close our underlying
+        // stream in some cases.
+        mCursor = offsetInBuffer;
+        return NS_OK;
+    }
+
     METER(if (bufstats.mBigSeekIndex < MAX_BIG_SEEKS)
               bufstats.mBigSeek[bufstats.mBigSeekIndex].mOldOffset =
                   mBufferStartOffset + PRInt64(mCursor));
@@ -246,7 +265,10 @@ nsBufferedStream::SetEOF()
     nsCOMPtr<nsISeekableStream> ras = do_QueryInterface(mStream, &rv);
     if (NS_FAILED(rv)) return rv;
 
-    return ras->SetEOF();
+    rv = ras->SetEOF();
+    if (NS_SUCCEEDED(rv))
+        mEOF = true;
+    return rv;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -327,8 +349,12 @@ nsBufferedInputStream::Read(char * buf, PRUint32 count, PRUint32 *result)
             return NS_OK;
         }
         nsresult rv = Source()->Read(buf, count, result);
-        if (NS_SUCCEEDED(rv))
+        if (NS_SUCCEEDED(rv)) {
             mBufferStartOffset += *result;  // so nsBufferedStream::Tell works
+            if (*result == 0) {
+                mEOF = true;
+            }
+        }
         return rv;
     }
 
@@ -399,6 +425,9 @@ nsBufferedInputStream::Fill()
     rv = Source()->Read(mBuffer + mFillPoint, mBufferSize - mFillPoint, &amt);
     if (NS_FAILED(rv)) return rv;
 
+    if (amt == 0)
+        mEOF = true;
+    
     mFillPoint += amt;
     return NS_OK;
 }

netwerk/base/src/nsBufferedStreams.h

@@ -83,6 +83,7 @@ protected:
     nsISupports*                mStream;        // cast to appropriate subclass
 
     bool                        mBufferDisabled;
+    bool                        mEOF;  // True if mStream is at EOF
     PRUint8                     mGetBufferCount;
 };
 

netwerk/test/unit/test_filestreams.js

@@ -206,6 +206,72 @@ function test_sync_operations_deferred()
   sync_operations(true);
 }
 
+function do_test_zero_size_buffered(disableBuffering)
+{
+  const LEAF_NAME = "filestreams-test-file.tmp";
+  const BUFFERSIZE = 4096;
+
+  let file = Cc["@mozilla.org/file/directory_service;1"].
+             getService(Ci.nsIProperties).
+             get("ProfD", Ci.nsIFile);
+  file.append(LEAF_NAME);
+  file.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, 0666);
+
+  let fstream = Cc["@mozilla.org/network/file-input-stream;1"].
+                createInstance(Ci.nsIFileInputStream);
+  fstream.init(file, -1, 0,
+               Ci.nsIFileInputStream.CLOSE_ON_EOF |
+               Ci.nsIFileInputStream.REOPEN_ON_REWIND);
+
+  var buffered = Cc["@mozilla.org/network/buffered-input-stream;1"].
+                   createInstance(Ci.nsIBufferedInputStream);
+  buffered.init(fstream, BUFFERSIZE);
+
+  if (disableBuffering) {
+      buffered.QueryInterface(Ci.nsIStreamBufferAccess).disableBuffering();
+  }
+
+  // Scriptable input streams clamp read sizes to the return value of
+  // available(), so don't quite do what we want here.
+  let cstream = Cc["@mozilla.org/intl/converter-input-stream;1"].
+                createInstance(Ci.nsIConverterInputStream);
+  cstream.init(buffered, "UTF-8", 0, 0);
+
+  do_check_eq(buffered.available(), 0);
+
+  // Now try reading from this stream
+  let string = {};
+  do_check_eq(cstream.readString(BUFFERSIZE, string), 0);
+  do_check_eq(string.value, "");
+
+  // Now check that available() throws
+  var exceptionThrown = false;
+  try {
+    do_check_eq(buffered.available(), 0);
+  } catch (e) {
+    exceptionThrown = true;
+  }
+  do_check_true(exceptionThrown);
+
+  // OK, now seek back to start
+  buffered.seek(Ci.nsISeekableStream.NS_SEEK_SET, 0);
+
+  // Now check that available() does not throw
+  exceptionThrown = false;
+  try {
+    do_check_eq(buffered.available(), 0);
+  } catch (e) {
+    exceptionThrown = true;
+  }
+  do_check_false(exceptionThrown);
+}
+
+function test_zero_size_buffered()
+{
+    do_test_zero_size_buffered(false);
+    do_test_zero_size_buffered(true);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //// Test Runner
 
@@ -220,6 +286,7 @@ let tests = [
   test_access_safe_defer_trick,
   test_sync_operations,
   test_sync_operations_deferred,
+  test_zero_size_buffered,
 ];
 
 function run_test()

xpcom/io/nsIStreamBufferAccess.idl

@@ -48,7 +48,7 @@
  * the current architecture (e.g., sizeof(double) for RISCy CPUs).  malloc(3)
  * satisfies this requirement.
  */
-[uuid(ac923b72-ac87-4892-ac7a-ca385d429435)]
+[scriptable, uuid(ac923b72-ac87-4892-ac7a-ca385d429435)]
 interface nsIStreamBufferAccess : nsISupports
 {
     /**
@@ -78,7 +78,7 @@ interface nsIStreamBufferAccess : nsISupports
      *    buffer has no room for aLength bytes starting at the next address A
      *    after the current position that satisfies (A & aAlignMask) == 0.
      */
-    [notxpcom] charPtr getBuffer(in PRUint32 aLength, in PRUint32 aAlignMask);
+    [notxpcom,noscript] charPtr getBuffer(in PRUint32 aLength, in PRUint32 aAlignMask);
 
     /**
      * Relinquish access to the stream's buffer, filling if at end of an input
@@ -93,7 +93,7 @@ interface nsIStreamBufferAccess : nsISupports
      *    The same count of contiguous bytes passed to the getBuffer call that
      *    returned aBuffer.
      */
-    [notxpcom] void putBuffer(in charPtr aBuffer, in PRUint32 aLength);
+    [notxpcom,noscript] void putBuffer(in charPtr aBuffer, in PRUint32 aLength);
 
     /**
      * Disable and enable buffering on the stream implementing this interface.