Skip to content

Commit 954217a

Browse files
ronagronag
committed
stream: error Duplex write/read if not writable/readable
If writable/readable has been explicitly disabled then using a Duplex as writable/readable should fail. Fixes: #34374 PR-URL: #34385 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 1544e69 commit 954217a

13 files changed

+97
-83
lines changed

‎lib/internal/bootstrap/switches/is_main_thread.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,7 @@ function getStdin() {
148148
switch (guessHandleType(fd)) {
149149
case 'TTY':
150150
const tty = require('tty');
151-
stdin = new tty.ReadStream(fd, {
152-
highWaterMark: 0,
153-
readable: true,
154-
writable: false
155-
});
151+
stdin = new tty.ReadStream(fd);
156152
break;
157153

158154
case 'FILE':

‎lib/internal/child_process.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ function flushStdio(subprocess) {
324324

325325

326326
function createSocket(pipe, readable) {
327-
return net.Socket({ handle: pipe, readable, writable: !readable });
327+
return net.Socket({ handle: pipe, readable });
328328
}
329329

330330

@@ -442,8 +442,6 @@ ChildProcess.prototype.spawn = function(options) {
442442
}
443443

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

‎lib/internal/http2/core.js

+10-4
Original file line numberDiff line numberDiff line change
@@ -358,18 +358,23 @@ function onSessionHeaders(handle, id, cat, flags, headers, sensitiveHeaders) {
358358
handle.destroy();
359359
return;
360360
}
361-
const opts = { readable: !endOfStream };
362361
// session[kType] can be only one of two possible values
363362
if (type === NGHTTP2_SESSION_SERVER) {
364-
stream = new ServerHttp2Stream(session, handle, id, opts, obj);
363+
stream = new ServerHttp2Stream(session, handle, id, {}, obj);
364+
if (endOfStream) {
365+
stream.push(null);
366+
}
365367
if (obj[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD) {
366368
// For head requests, there must not be a body...
367369
// end the writable side immediately.
368370
stream.end();
369371
stream[kState].flags |= STREAM_FLAGS_HEAD_REQUEST;
370372
}
371373
} else {
372-
stream = new ClientHttp2Stream(session, handle, id, opts);
374+
stream = new ClientHttp2Stream(session, handle, id, {});
375+
if (endOfStream) {
376+
stream.push(null);
377+
}
373378
stream.end();
374379
}
375380
if (endOfStream)
@@ -2675,7 +2680,6 @@ class ServerHttp2Stream extends Http2Stream {
26752680
let headRequest = false;
26762681
if (headers[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD)
26772682
headRequest = options.endStream = true;
2678-
options.readable = false;
26792683

26802684
const headersList = mapToHeaders(headers);
26812685

@@ -2703,6 +2707,8 @@ class ServerHttp2Stream extends Http2Stream {
27032707
const stream = new ServerHttp2Stream(session, ret, id, options, headers);
27042708
stream[kSentHeaders] = headers;
27052709

2710+
stream.push(null);
2711+
27062712
if (options.endStream)
27072713
stream.end();
27082714

‎lib/internal/streams/destroy.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,8 @@ function undestroy() {
206206
r.errored = null;
207207
r.errorEmitted = false;
208208
r.reading = false;
209-
r.ended = false;
210-
r.endEmitted = false;
209+
r.ended = r.readable === false;
210+
r.endEmitted = r.readable === false;
211211
}
212212

213213
if (w) {
@@ -217,11 +217,11 @@ function undestroy() {
217217
w.closeEmitted = false;
218218
w.errored = null;
219219
w.errorEmitted = false;
220-
w.ended = false;
221-
w.ending = false;
222220
w.finalCalled = false;
223221
w.prefinished = false;
224-
w.finished = false;
222+
w.ended = w.writable === false;
223+
w.ending = w.writable === false;
224+
w.finished = w.writable === false;
225225
}
226226
}
227227

‎lib/internal/streams/duplex.js

+11-9
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,20 @@ function Duplex(options) {
5555

5656
Readable.call(this, options);
5757
Writable.call(this, options);
58-
this.allowHalfOpen = true;
5958

60-
if (options) {
61-
if (options.readable === false)
62-
this.readable = false;
59+
this.allowHalfOpen = options?.allowHalfOpen !== false;
6360

64-
if (options.writable === false)
65-
this.writable = false;
61+
if (options?.readable === false) {
62+
this._readableState.readable = false;
63+
this._readableState.ended = true;
64+
this._readableState.endEmitted = true;
65+
}
6666

67-
if (options.allowHalfOpen === false) {
68-
this.allowHalfOpen = false;
69-
}
67+
if (options?.writable === false) {
68+
this._writableState.writable = false;
69+
this._writableState.ending = true;
70+
this._writableState.ended = true;
71+
this._writableState.finished = true;
7072
}
7173
}
7274

‎lib/tty.js

+5-6
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,15 @@ function ReadStream(fd, options) {
5151
throw new ERR_INVALID_FD(fd);
5252

5353
const ctx = {};
54-
const tty = new TTY(fd, true, ctx);
54+
const tty = new TTY(fd, ctx);
5555
if (ctx.code !== undefined) {
5656
throw new ERR_TTY_INIT_FAILED(ctx);
5757
}
5858

5959
net.Socket.call(this, {
6060
highWaterMark: 0,
61-
readable: true,
62-
writable: false,
6361
handle: tty,
62+
manualStart: true,
6463
...options
6564
});
6665

@@ -89,15 +88,15 @@ function WriteStream(fd) {
8988
throw new ERR_INVALID_FD(fd);
9089

9190
const ctx = {};
92-
const tty = new TTY(fd, false, ctx);
91+
const tty = new TTY(fd, ctx);
9392
if (ctx.code !== undefined) {
9493
throw new ERR_TTY_INIT_FAILED(ctx);
9594
}
9695

9796
net.Socket.call(this, {
97+
highWaterMark: 0,
9898
handle: tty,
99-
readable: false,
100-
writable: true
99+
manualStart: true
101100
});
102101

103102
// Prevents interleaved or dropped stdout/stderr output for terminals.

‎src/tty_wrap.cc

+3-4
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,9 @@ void TTYWrap::New(const FunctionCallbackInfo<Value>& args) {
121121
CHECK_GE(fd, 0);
122122

123123
int err = 0;
124-
new TTYWrap(env, args.This(), fd, args[1]->IsTrue(), &err);
124+
new TTYWrap(env, args.This(), fd, &err);
125125
if (err != 0) {
126-
env->CollectUVExceptionInfo(args[2], err, "uv_tty_init");
126+
env->CollectUVExceptionInfo(args[1], err, "uv_tty_init");
127127
args.GetReturnValue().SetUndefined();
128128
}
129129
}
@@ -132,13 +132,12 @@ void TTYWrap::New(const FunctionCallbackInfo<Value>& args) {
132132
TTYWrap::TTYWrap(Environment* env,
133133
Local<Object> object,
134134
int fd,
135-
bool readable,
136135
int* init_err)
137136
: LibuvStreamWrap(env,
138137
object,
139138
reinterpret_cast<uv_stream_t*>(&handle_),
140139
AsyncWrap::PROVIDER_TTYWRAP) {
141-
*init_err = uv_tty_init(env->event_loop(), &handle_, fd, readable);
140+
*init_err = uv_tty_init(env->event_loop(), &handle_, fd, 0);
142141
set_fd(fd);
143142
if (*init_err != 0)
144143
MarkAsUninitialized();

‎src/tty_wrap.h

-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ class TTYWrap : public LibuvStreamWrap {
4646
TTYWrap(Environment* env,
4747
v8::Local<v8::Object> object,
4848
int fd,
49-
bool readable,
5049
int* init_err);
5150

5251
static void IsTTY(const v8::FunctionCallbackInfo<v8::Value>& args);

‎test/parallel/test-http2-compat-socket-set.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ server.on('request', common.mustCall(function(request, response) {
2626
assert.strictEqual(request.stream.destroyed, true);
2727
request.socket.destroyed = false;
2828

29-
assert.strictEqual(request.stream.readable, false);
30-
request.socket.readable = true;
3129
assert.strictEqual(request.stream.readable, true);
30+
request.socket.readable = false;
31+
assert.strictEqual(request.stream.readable, false);
3232

3333
assert.strictEqual(request.stream.writable, true);
3434
request.socket.writable = false;

‎test/parallel/test-stdio-readable-writable.js

-33
This file was deleted.

‎test/parallel/test-stream-duplex-readable-writable.js

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const { Duplex } = require('stream');
5+
const assert = require('assert');
6+
7+
{
8+
const duplex = new Duplex({
9+
readable: false
10+
});
11+
assert.strictEqual(duplex.readable, false);
12+
duplex.push('asd');
13+
duplex.on('error', common.mustCall((err) => {
14+
assert.strictEqual(err.code, 'ERR_STREAM_PUSH_AFTER_EOF');
15+
}));
16+
duplex.on('data', common.mustNotCall());
17+
duplex.on('end', common.mustNotCall());
18+
}
19+
20+
{
21+
const duplex = new Duplex({
22+
writable: false,
23+
write: common.mustNotCall()
24+
});
25+
assert.strictEqual(duplex.writable, false);
26+
duplex.write('asd');
27+
duplex.on('error', common.mustCall((err) => {
28+
assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END');
29+
}));
30+
duplex.on('finish', common.mustNotCall());
31+
}
32+
33+
{
34+
const duplex = new Duplex({
35+
readable: false
36+
});
37+
assert.strictEqual(duplex.readable, false);
38+
duplex.on('data', common.mustNotCall());
39+
duplex.on('end', common.mustNotCall());
40+
async function run() {
41+
for await (const chunk of duplex) {
42+
assert(false, chunk);
43+
}
44+
}
45+
run().then(common.mustCall());
46+
}

‎test/pseudo-tty/repl-dumb-tty.js

+11-9
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,13 @@
11
'use strict';
22
const common = require('../common');
3+
const process = require('process');
34

45
process.env.TERM = 'dumb';
56

67
const repl = require('repl');
78
const ArrayStream = require('../common/arraystream');
89

910
repl.start('> ');
10-
process.stdin.push('conso'); // No completion preview.
11-
process.stdin.push('le.log("foo")\n');
12-
process.stdin.push('1 + 2'); // No input preview.
13-
process.stdin.push('\n');
14-
process.stdin.push('"str"\n');
15-
process.stdin.push('console.dir({ a: 1 })\n');
16-
process.stdin.push('{ a: 1 }\n');
17-
process.stdin.push('\n');
18-
process.stdin.push('.exit\n');
1911

2012
// Verify <ctrl> + D support.
2113
{
@@ -34,3 +26,13 @@ process.stdin.push('.exit\n');
3426
replServer.write(null, { ctrl: true, name: 's' });
3527
replServer.write(null, { ctrl: true, name: 'd' });
3628
}
29+
30+
process.stdin.push('conso'); // No completion preview.
31+
process.stdin.push('le.log("foo")\n');
32+
process.stdin.push('1 + 2'); // No input preview.
33+
process.stdin.push('\n');
34+
process.stdin.push('"str"\n');
35+
process.stdin.push('console.dir({ a: 1 })\n');
36+
process.stdin.push('{ a: 1 }\n');
37+
process.stdin.push('\n');
38+
process.stdin.push('.exit\n');

‎test/pseudo-tty/repl-dumb-tty.out

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
> console.log("foo")
1+
> >
2+
console.log("foo")
23
foo
34
undefined
45
> 1 + 2
@@ -12,4 +13,3 @@ undefined
1213
{ a: 1 }
1314
>
1415
> .exit
15-
>

0 commit comments

Comments
 (0)