Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v8.x] Backport tty fixes #25351

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions doc/api/errors.md
Expand Up @@ -1147,12 +1147,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: 2 additions & 4 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 @@ -1937,6 +1934,7 @@ cases:
[`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
[`domain`]: domain.html
[`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');
});
}
3 changes: 3 additions & 0 deletions test/pseudo-tty/test-stdin-write.js
@@ -0,0 +1,3 @@
'use strict';
require('../common');
process.stdin.end('foobar\n');
1 change: 1 addition & 0 deletions test/pseudo-tty/test-stdin-write.out
@@ -0,0 +1 @@
foobar
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>
8 changes: 8 additions & 0 deletions test/pseudo-tty/test-tty-stdin-call-end.js
@@ -0,0 +1,8 @@
'use strict';

require('../common');

// This tests verifies that process.stdin.end() does not
// crash the process with ENOTCONN

process.stdin.end();
Empty file.
12 changes: 9 additions & 3 deletions test/pseudo-tty/testcfg.py
Expand Up @@ -35,10 +35,11 @@

class TTYTestCase(test.TestCase):

def __init__(self, path, file, expected, arch, mode, context, config):
def __init__(self, path, file, expected, input, arch, mode, context, config):
super(TTYTestCase, self).__init__(context, path, arch, mode)
self.file = file
self.expected = expected
self.input = input
self.config = config
self.arch = arch
self.mode = mode
Expand Down Expand Up @@ -104,12 +105,16 @@ def GetSource(self):
+ open(self.expected).read())

def RunCommand(self, command, env):
input = None
if self.input is not None and exists(self.input):
input = open(self.input).read()
full_command = self.context.processor(command)
output = test.Execute(full_command,
self.context,
self.context.GetTimeout(self.mode),
env,
True)
faketty=True,
input=input)
self.Cleanup()
return test.TestOutput(self,
full_command,
Expand Down Expand Up @@ -140,11 +145,12 @@ def ListTests(self, current_path, path, arch, mode):
if self.Contains(path, test):
file_prefix = join(self.root, reduce(join, test[1:], ""))
file_path = file_prefix + ".js"
input_path = file_prefix + ".in"
output_path = file_prefix + ".out"
if not exists(output_path):
raise Exception("Could not find %s" % output_path)
result.append(TTYTestCase(test, file_path, output_path,
arch, mode, self.context, self))
input_path, arch, mode, self.context, self))
return result

def GetBuildRequirements(self):
Expand Down
13 changes: 12 additions & 1 deletion tools/test.py
Expand Up @@ -717,12 +717,23 @@ def CheckedUnlink(name):
PrintError("os.unlink() " + str(e))
break

def Execute(args, context, timeout=None, env={}, faketty=False, disable_core_files=False):
def Execute(args, context, timeout=None, env={}, faketty=False, disable_core_files=False, input=None):
if faketty:
import pty
(out_master, fd_out) = pty.openpty()
fd_in = fd_err = fd_out
pty_out = out_master

if input is not None:
# Before writing input data, disable echo so the input doesn't show
# up as part of the output.
import termios
attr = termios.tcgetattr(fd_in)
attr[3] = attr[3] & ~termios.ECHO
termios.tcsetattr(fd_in, termios.TCSADRAIN, attr)

os.write(pty_out, input)
os.write(pty_out, '\x04') # End-of-file marker (Ctrl+D)
else:
(fd_out, outname) = tempfile.mkstemp()
(fd_err, errname) = tempfile.mkstemp()
Expand Down