Skip to content

Commit

Permalink
stream: error Duplex write/read if not writable/readable
Browse files Browse the repository at this point in the history
If writable/readable has been explicitly disabled then using
a Duplex as writable/readable should fail.

Fixes: nodejs#34374
  • Loading branch information
ronag committed Jan 5, 2021
1 parent db79783 commit 97bdb5a
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 26 deletions.
4 changes: 1 addition & 3 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ function flushStdio(subprocess) {


function createSocket(pipe, readable) {
return net.Socket({ handle: pipe, readable, writable: !readable });
return net.Socket({ handle: pipe, readable });
}


Expand Down Expand Up @@ -439,8 +439,6 @@ ChildProcess.prototype.spawn = function(options) {
}

if (stream.handle) {
// When i === 0 - we're dealing with stdin
// (which is the only one writable pipe).
stream.socket = createSocket(this.pid !== 0 ?
stream.handle : null, i > 0);

Expand Down
14 changes: 10 additions & 4 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -350,18 +350,23 @@ function onSessionHeaders(handle, id, cat, flags, headers, sensitiveHeaders) {
handle.destroy();
return;
}
const opts = { readable: !endOfStream };
// session[kType] can be only one of two possible values
if (type === NGHTTP2_SESSION_SERVER) {
stream = new ServerHttp2Stream(session, handle, id, opts, obj);
stream = new ServerHttp2Stream(session, handle, id, {}, obj);
if (endOfStream) {
stream.push(null);
}
if (obj[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD) {
// For head requests, there must not be a body...
// end the writable side immediately.
stream.end();
stream[kState].flags |= STREAM_FLAGS_HEAD_REQUEST;
}
} else {
stream = new ClientHttp2Stream(session, handle, id, opts);
stream = new ClientHttp2Stream(session, handle, id, {});
if (endOfStream) {
stream.push(null);
}
stream.end();
}
if (endOfStream)
Expand Down Expand Up @@ -2629,7 +2634,6 @@ class ServerHttp2Stream extends Http2Stream {
let headRequest = false;
if (headers[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD)
headRequest = options.endStream = true;
options.readable = false;

const headersList = mapToHeaders(headers);

Expand Down Expand Up @@ -2657,6 +2661,8 @@ class ServerHttp2Stream extends Http2Stream {
const stream = new ServerHttp2Stream(session, ret, id, options, headers);
stream[kSentHeaders] = headers;

stream.push(null);

if (options.endStream)
stream.end();

Expand Down
10 changes: 5 additions & 5 deletions lib/internal/streams/destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ function undestroy() {
r.errored = null;
r.errorEmitted = false;
r.reading = false;
r.ended = false;
r.endEmitted = false;
r.ended = r.readable === false;
r.endEmitted = r.readable === false;
}

if (w) {
Expand All @@ -211,11 +211,11 @@ function undestroy() {
w.closeEmitted = false;
w.errored = null;
w.errorEmitted = false;
w.ended = false;
w.ending = false;
w.finalCalled = false;
w.prefinished = false;
w.finished = false;
w.ended = w.writable === false;
w.ending = w.writable === false;
w.finished = w.writable === false;
}
}

Expand Down
20 changes: 11 additions & 9 deletions lib/internal/streams/duplex.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,20 @@ function Duplex(options) {

Readable.call(this, options);
Writable.call(this, options);
this.allowHalfOpen = true;

if (options) {
if (options.readable === false)
this.readable = false;
this.allowHalfOpen = options?.allowHalfOpen !== false;

if (options.writable === false)
this.writable = false;
if (options?.readable === false) {
this._readableState.readable = false;
this._readableState.ended = true;
this._readableState.endEmitted = true;
}

if (options.allowHalfOpen === false) {
this.allowHalfOpen = false;
}
if (options?.writable === false) {
this._writableState.writable = false;
this._writableState.ending = true;
this._writableState.ended = true;
this._writableState.finished = true;
}
}

Expand Down
6 changes: 3 additions & 3 deletions lib/tty.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function ReadStream(fd, options) {
net.Socket.call(this, {
highWaterMark: 0,
readable: true,
writable: false,
writable: true,
handle: tty,
...options
});
Expand Down Expand Up @@ -88,14 +88,14 @@ function WriteStream(fd) {
throw new ERR_INVALID_FD(fd);

const ctx = {};
const tty = new TTY(fd, false, ctx);
const tty = new TTY(fd, true, ctx);
if (ctx.code !== undefined) {
throw new ERR_TTY_INIT_FAILED(ctx);
}

net.Socket.call(this, {
handle: tty,
readable: false,
readable: true,
writable: true
});

Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-compat-socket-set.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ server.on('request', common.mustCall(function(request, response) {
assert.strictEqual(request.stream.destroyed, true);
request.socket.destroyed = false;

assert.strictEqual(request.stream.readable, false);
request.socket.readable = true;
assert.strictEqual(request.stream.readable, true);
request.socket.readable = false;
assert.strictEqual(request.stream.readable, false);

assert.strictEqual(request.stream.writable, true);
request.socket.writable = false;
Expand Down
46 changes: 46 additions & 0 deletions test/parallel/test-stream-duplex-readable-writable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';

const common = require('../common');
const { Duplex } = require('stream');
const assert = require('assert');

{
const duplex = new Duplex({
readable: false
});
assert.strictEqual(duplex.readable, false);
duplex.push('asd');
duplex.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'ERR_STREAM_PUSH_AFTER_EOF');
}));
duplex.on('data', common.mustNotCall());
duplex.on('end', common.mustNotCall());
}

{
const duplex = new Duplex({
writable: false,
write: common.mustNotCall()
});
assert.strictEqual(duplex.writable, false);
duplex.write('asd');
duplex.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END');
}));
duplex.on('finish', common.mustNotCall());
}

{
const duplex = new Duplex({
readable: false
});
assert.strictEqual(duplex.readable, false);
duplex.on('data', common.mustNotCall());
duplex.on('end', common.mustNotCall());
async function run() {
for await (const chunk of duplex) {
assert(false, chunk);
}
}
run().then(common.mustCall());
}

0 comments on commit 97bdb5a

Please sign in to comment.