Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
process: allow reading from stdout/stderr sockets
Allow reading from stdio streams that are conventionally
associated with process output, since this is only convention.

This involves disabling the oddness around closing stdio
streams. Its purpose is to prevent the file descriptors
0 through 2 from being closed, since doing so can lead
to information leaks when new file descriptors are being
opened; instead, not doing anything seems like a more
reasonable choice.

Fixes: #21203

Backport-PR-URL: #25351
PR-URL: #23053
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and BethGriggs committed Mar 19, 2019
1 parent 1a9582b commit ea5628e
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 20 deletions.
20 changes: 20 additions & 0 deletions doc/api/errors.md
Expand Up @@ -1172,12 +1172,32 @@ A call was made and the UDP subsystem was not running.
<a id="ERR_STDERR_CLOSE"></a>
### ERR_STDERR_CLOSE

<!-- YAML
removed: REPLACEME
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/23053
description: Rather than emitting an error, `process.stderr.end()` now
only closes the stream side but not the underlying resource,
making this error obsolete.
-->

An attempt was made to close the `process.stderr` stream. By design, Node.js
does not allow `stdout` or `stderr` streams to be closed by user code.

<a id="ERR_STDOUT_CLOSE"></a>
### ERR_STDOUT_CLOSE

<!-- YAML
removed: REPLACEME
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/23053
description: Rather than emitting an error, `process.stderr.end()` now
only closes the stream side but not the underlying resource,
making this error obsolete.
-->

An attempt was made to close the `process.stdout` stream. By design, Node.js
does not allow `stdout` or `stderr` streams to be closed by user code.

Expand Down
6 changes: 1 addition & 5 deletions doc/api/process.md
Expand Up @@ -1703,9 +1703,7 @@ important ways:

1. They are used internally by [`console.log()`][] and [`console.error()`][],
respectively.
2. They cannot be closed ([`end()`][] will throw).
3. They will never emit the [`'finish'`][] event.
4. Writes may be synchronous depending on what the stream is connected to
2. Writes may be synchronous depending on what the stream is connected to
and whether the system is Windows or POSIX:
- Files: *synchronous* on Windows and POSIX
- TTYs (Terminals): *asynchronous* on Windows, *synchronous* on POSIX
Expand Down Expand Up @@ -1925,7 +1923,6 @@ cases:


[`'exit'`]: #process_event_exit
[`'finish'`]: stream.html#stream_event_finish
[`'message'`]: child_process.html#child_process_event_message
[`'rejectionHandled'`]: #process_event_rejectionhandled
[`'uncaughtException'`]: #process_event_uncaughtexception
Expand All @@ -1936,7 +1933,6 @@ cases:
[`EventEmitter`]: events.html#events_class_eventemitter
[`console.error()`]: console.html#console_console_error_data_args
[`console.log()`]: console.html#console_console_log_data_args
[`end()`]: stream.html#stream_writable_end_chunk_encoding_callback
[`net.Server`]: net.html#net_class_net_server
[`net.Socket`]: net.html#net_class_net_socket
[`process.argv`]: #process_process_argv
Expand Down
2 changes: 0 additions & 2 deletions lib/internal/errors.js
Expand Up @@ -427,8 +427,6 @@ E('ERR_SOCKET_BUFFER_SIZE',
(reason) => `Could not get or set buffer size: ${reason}`);
E('ERR_SOCKET_CANNOT_SEND', 'Unable to send data');
E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running');
E('ERR_STDERR_CLOSE', 'process.stderr cannot be closed');
E('ERR_STDOUT_CLOSE', 'process.stdout cannot be closed');
E('ERR_UNKNOWN_BUILTIN_MODULE', (id) => `No such built-in module: ${id}`);
E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension: %s');
E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s');
Expand Down
18 changes: 7 additions & 11 deletions lib/internal/process/stdio.js
@@ -1,9 +1,11 @@
'use strict';

const errors = require('internal/errors');
const errors = require('internal/errors').codes;

exports.setup = setupStdio;

function dummyDestroy(err, cb) { cb(err); }

function setupStdio() {
var stdin;
var stdout;
Expand All @@ -13,11 +15,8 @@ function setupStdio() {
if (stdout) return stdout;
stdout = createWritableStdioStream(1);
stdout.destroySoon = stdout.destroy;
stdout._destroy = function(er, cb) {
// Avoid errors if we already emitted
er = er || new errors.Error('ERR_STDOUT_CLOSE');
cb(er);
};
// Override _destroy so that the fd is never actually closed.
stdout._destroy = dummyDestroy;
if (stdout.isTTY) {
process.on('SIGWINCH', () => stdout._refreshSize());
}
Expand All @@ -28,11 +27,8 @@ function setupStdio() {
if (stderr) return stderr;
stderr = createWritableStdioStream(2);
stderr.destroySoon = stderr.destroy;
stderr._destroy = function(er, cb) {
// Avoid errors if we already emitted
er = er || new errors.Error('ERR_STDERR_CLOSE');
cb(er);
};
// Override _destroy so that the fd is never actually closed.
stdout._destroy = dummyDestroy;
if (stderr.isTTY) {
process.on('SIGWINCH', () => stderr._refreshSize());
}
Expand Down
Expand Up @@ -24,9 +24,9 @@ function parent() {
});

child.on('close', function(code, signal) {
assert(code);
assert.strictEqual(code, 0);
assert.strictEqual(err, '');
assert.strictEqual(out, 'foo');
assert(/process\.stdout cannot be closed/.test(err));
console.log('ok');
});
}
1 change: 1 addition & 0 deletions test/pseudo-tty/test-stdout-read.in
@@ -0,0 +1 @@
Hello!
3 changes: 3 additions & 0 deletions test/pseudo-tty/test-stdout-read.js
@@ -0,0 +1,3 @@
'use strict';
const common = require('../common');
process.stderr.on('data', common.mustCall(console.log));
1 change: 1 addition & 0 deletions test/pseudo-tty/test-stdout-read.out
@@ -0,0 +1 @@
<Buffer 48 65 6c 6c 6f 21 0a>

0 comments on commit ea5628e

Please sign in to comment.