From a6c18472cd31c1367dd85ca0d864b1136027811b Mon Sep 17 00:00:00 2001 From: isaacs Date: Thu, 31 Jan 2013 13:33:37 -0800 Subject: [PATCH] stream: Don't stop reading on zero-length decoded output Fixes regression introduced in 7e1cf84c9efd491d72b25968a70656458ecb6b7c --- lib/_stream_readable.js | 6 +- ...st-stream2-readable-empty-buffer-no-eof.js | 137 +++++++++++------- 2 files changed, 88 insertions(+), 55 deletions(-) diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index ee2db01747..c46399c7da 100644 --- a/lib/_stream_readable.js +++ b/lib/_stream_readable.js @@ -291,9 +291,6 @@ function onread(stream, er, chunk) { return; } - if (state.decoder) - chunk = state.decoder.write(chunk); - // at this point, if we got a zero-length buffer or string, // and we're not in object-mode, then there's really no point // continuing. it means that there is nothing to read right @@ -305,6 +302,9 @@ function onread(stream, er, chunk) { 0 === chunk.length) return; + if (state.decoder) + chunk = state.decoder.write(chunk); + // update the buffer info. state.length += state.objectMode ? 1 : chunk.length; state.buffer.push(chunk); diff --git a/test/simple/test-stream2-readable-empty-buffer-no-eof.js b/test/simple/test-stream2-readable-empty-buffer-no-eof.js index 18d45966c0..6cc7c73f08 100644 --- a/test/simple/test-stream2-readable-empty-buffer-no-eof.js +++ b/test/simple/test-stream2-readable-empty-buffer-no-eof.js @@ -24,62 +24,95 @@ var assert = require('assert'); var Readable = require('stream').Readable; -var r = new Readable(); +test1(); +test2(); -// should not end when we get a Buffer(0) or '' as the _read result -// that just means that there is *temporarily* no data, but to go -// ahead and try again later. -// -// note that this is very unusual. it only works for crypto streams -// because the other side of the stream will call read(0) to cycle -// data through openssl. that's why we set the timeouts to call -// r.read(0) again later, otherwise there is no more work being done -// and the process just exits. +function test1() { + var r = new Readable(); -var buf = new Buffer(5); -buf.fill('x'); -var reads = 5; -r._read = function(n, cb) { - switch (reads--) { - case 0: - return cb(null, null); // EOF - case 1: - return cb(null, buf); - case 2: - setTimeout(r.read.bind(r, 0), 10); - return cb(null, new Buffer(0)); // Not-EOF! - case 3: - setTimeout(r.read.bind(r, 0), 10); - return process.nextTick(function() { - return cb(null, new Buffer(0)); - }); - case 4: - setTimeout(r.read.bind(r, 0), 10); - return setTimeout(function() { - return cb(null, new Buffer(0)); - }); - case 5: - return setTimeout(function() { + // should not end when we get a Buffer(0) or '' as the _read result + // that just means that there is *temporarily* no data, but to go + // ahead and try again later. + // + // note that this is very unusual. it only works for crypto streams + // because the other side of the stream will call read(0) to cycle + // data through openssl. that's why we set the timeouts to call + // r.read(0) again later, otherwise there is no more work being done + // and the process just exits. + + var buf = new Buffer(5); + buf.fill('x'); + var reads = 5; + r._read = function(n, cb) { + switch (reads--) { + case 0: + return cb(null, null); // EOF + case 1: return cb(null, buf); - }); - default: - throw new Error('unreachable'); + case 2: + setTimeout(r.read.bind(r, 0), 10); + return cb(null, new Buffer(0)); // Not-EOF! + case 3: + setTimeout(r.read.bind(r, 0), 10); + return process.nextTick(function() { + return cb(null, new Buffer(0)); + }); + case 4: + setTimeout(r.read.bind(r, 0), 10); + return setTimeout(function() { + return cb(null, new Buffer(0)); + }); + case 5: + return setTimeout(function() { + return cb(null, buf); + }); + default: + throw new Error('unreachable'); + } + }; + + var results = []; + function flow() { + var chunk; + while (null !== (chunk = r.read())) + results.push(chunk + ''); } -}; + r.on('readable', flow); + r.on('end', function() { + results.push('EOF'); + }); + flow(); -var results = []; -function flow() { - var chunk; - while (null !== (chunk = r.read())) - results.push(chunk + ''); + process.on('exit', function() { + assert.deepEqual(results, [ 'xxxxx', 'xxxxx', 'EOF' ]); + console.log('ok'); + }); } -r.on('readable', flow); -r.on('end', function() { - results.push('EOF'); -}); -flow(); -process.on('exit', function() { - assert.deepEqual(results, [ 'xxxxx', 'xxxxx', 'EOF' ]); - console.log('ok'); -}); +function test2() { + var r = new Readable({ encoding: 'base64' }); + var reads = 5; + r._read = function(n, cb) { + if (!reads--) + return cb(null, null); // EOF + else + return cb(null, new Buffer('x')); + }; + + var results = []; + function flow() { + var chunk; + while (null !== (chunk = r.read())) + results.push(chunk + ''); + } + r.on('readable', flow); + r.on('end', function() { + results.push('EOF'); + }); + flow(); + + process.on('exit', function() { + assert.deepEqual(results, [ 'eHh4', 'eHg=', 'EOF' ]); + console.log('ok'); + }); +}