stream2: flush extant data on read of ended stream
authorChris Dickinson <christopher.s.dickinson@gmail.com>
Wed, 9 Jul 2014 09:16:45 +0000 (02:16 -0700)
committerFedor Indutny <fedor@indutny.com>
Tue, 15 Jul 2014 08:38:23 +0000 (12:38 +0400)
A ReadableStream with a base64 StringDecoder backed by only
one or two bytes would fail to output its partial data before
ending. This fix adds a check to see if the `read` was triggered
by an internal `flow`, and if so, empties any remaining data.

fixes #7914.

Signed-off-by: Fedor Indutny <fedor@indutny.com>
lib/_stream_readable.js
test/simple/test-stream2-base64-single-char-read-end.js [new file with mode: 0644]

index bf47e5e..ae04f22 100755 (executable)
@@ -253,6 +253,7 @@ Readable.prototype.read = function(n) {
   var state = this._readableState;
   state.calledRead = true;
   var nOrig = n;
+  var ret;
 
   if (typeof n !== 'number' || n > 0)
     state.emittedReadable = false;
@@ -271,9 +272,28 @@ Readable.prototype.read = function(n) {
 
   // if we've ended, and we're now clear, then finish it up.
   if (n === 0 && state.ended) {
+    ret = null;
+
+    // In cases where the decoder did not receive enough data
+    // to produce a full chunk, then immediately received an
+    // EOF, state.buffer will contain [<Buffer >, <Buffer 00 ...>].
+    // howMuchToRead will see this and coerce the amount to
+    // read to zero (because it's looking at the length of the
+    // first <Buffer > in state.buffer), and we'll end up here.
+    //
+    // This can only happen via state.decoder -- no other venue
+    // exists for pushing a zero-length chunk into state.buffer
+    // and triggering this behavior. In this case, we return our
+    // remaining data and end the stream, if appropriate.
+    if (state.length > 0 && state.decoder) {
+      ret = fromList(n, state);
+      state.length -= ret.length;
+    }
+
     if (state.length === 0)
       endReadable(this);
-    return null;
+
+    return ret;
   }
 
   // All the actual chunk generation logic needs to be
@@ -327,7 +347,6 @@ Readable.prototype.read = function(n) {
   if (doRead && !state.reading)
     n = howMuchToRead(nOrig, state);
 
-  var ret;
   if (n > 0)
     ret = fromList(n, state);
   else
diff --git a/test/simple/test-stream2-base64-single-char-read-end.js b/test/simple/test-stream2-base64-single-char-read-end.js
new file mode 100644 (file)
index 0000000..5a38341
--- /dev/null
@@ -0,0 +1,58 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+
+var common = require('../common.js');
+var R = require('_stream_readable');
+var W = require('_stream_writable');
+var assert = require('assert');
+
+var src = new R({encoding: 'base64'});
+var dst = new W();
+var hasRead = false;
+var accum = [];
+var timeout;
+
+src._read = function(n) {
+  if(!hasRead) {
+    hasRead = true;
+    process.nextTick(function() {
+      src.push(new Buffer('1'));
+      src.push(null);
+    });
+  };
+};
+
+dst._write = function(chunk, enc, cb) {
+  accum.push(chunk);
+  cb();
+};
+
+src.on('end', function() {
+  assert.equal(Buffer.concat(accum) + '', 'MQ==');
+  clearTimeout(timeout);
+})
+
+src.pipe(dst);
+
+timeout = setTimeout(function() {
+  assert.fail('timed out waiting for _write');
+}, 100);