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

File tree

13 files changed

+97
-83
lines changed

13 files changed

+97
-83
lines changed

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

Lines changed: 1 addition & 5 deletions
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

Lines changed: 1 addition & 3 deletions
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

Lines changed: 10 additions & 4 deletions
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

Lines changed: 5 additions & 5 deletions
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

Lines changed: 11 additions & 9 deletions
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

Lines changed: 5 additions & 6 deletions
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

Lines changed: 3 additions & 4 deletions
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

Lines changed: 0 additions & 1 deletion
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

Lines changed: 2 additions & 2 deletions
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

Lines changed: 0 additions & 33 deletions
This file was deleted.

0 commit comments

Comments
 (0)