Skip to content
Browse files

stream: Writable.end(chunk) after end is an error

Calling end(data) calls write(data).  Doing this after end should
raise a 'write after end' error.

However, because end() calls were previously ignored on already
ended streams, this error was confusingly suppressed, even though the
data never is written, and cannot get to the other side.

This is a re-hash of 5222d19, but
without assuming that the data passed to end() is valid, and thus
breaking a bunch of tests.
  • Loading branch information...
1 parent 80ea63b commit a3ed0e19348635961db04d07e4a3e290bef4a8c4 @isaacs committed Mar 2, 2013
Showing with 34 additions and 13 deletions.
  1. +18 −13 lib/_stream_writable.js
  2. +16 −0 test/simple/test-stream2-writable.js
View
31 lib/_stream_writable.js
@@ -304,10 +304,6 @@ Writable.prototype._write = function(chunk, cb) {
Writable.prototype.end = function(chunk, encoding, cb) {
var state = this._writableState;
- // ignore unnecessary end() calls.
- if (state.ending || state.ended || state.finished)
- return;
-
if (typeof chunk === 'function') {
cb = chunk;
chunk = null;
@@ -317,17 +313,26 @@ Writable.prototype.end = function(chunk, encoding, cb) {
encoding = null;
}
+ if (typeof chunk !== 'undefined' && chunk !== null)
+ this.write(chunk, encoding);
+
+ // ignore unnecessary end() calls.
+ if (!state.ending && !state.ended && !state.finished)
+ endWritable(this, state, cb);
+};
+
+function endWritable(stream, state, cb) {
state.ending = true;
- if (chunk)
- this.write(chunk, encoding, cb);
- else if (state.length === 0 && !state.finishing && !state.finished) {
+ if (state.length === 0 && !state.finishing) {
state.finishing = true;
- this.emit('finish');
+ stream.emit('finish');
state.finished = true;
- if (cb) process.nextTick(cb);
- } else if (cb) {
- this.once('finish', cb);
}
-
+ if (cb) {
+ if (state.finished || state.finishing)
+ process.nextTick(cb);
+ else
+ stream.once('finish', cb);
+ }
state.ended = true;
-};
+}
View
16 test/simple/test-stream2-writable.js
@@ -311,3 +311,19 @@ test('duplexes are pipable', function(t) {
assert(!gotError);
t.end();
});
+
+test('end(chunk) two times is an error', function(t) {
+ var w = new W();
+ w._write = function() {};
+ var gotError = false;
+ w.on('error', function(er) {
+ gotError = true;
+ t.equal(er.message, 'write after end');
+ });
+ w.end('this is the end');
+ w.end('and so is this');
+ process.nextTick(function() {
+ assert(gotError);
+ t.end();
+ });
+});

0 comments on commit a3ed0e1

Please sign in to comment.
Something went wrong with that request. Please try again.