Skip to content

Commit

Permalink
stream2: flush extant data on read of ended stream
Browse files Browse the repository at this point in the history
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 nodejs#7914.

Signed-off-by: Fedor Indutny <[email protected]>
  • Loading branch information
chrisdickinson authored and indutny committed Jul 15, 2014
1 parent 7f86baf commit a96d660
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 2 deletions.
23 changes: 21 additions & 2 deletions lib/_stream_readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
58 changes: 58 additions & 0 deletions test/simple/test-stream2-base64-single-char-read-end.js
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit a96d660

Please sign in to comment.