FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

readline: add stricter validation for functions called after closed · nodejs/node@462c4b0 · GitHub

/ node Public

Commit 462c4b0

Browse files
dario-piotrowicz
authored and
targos
committed
readline: add stricter validation for functions called after closed
PR-URL: #58283 Fixes: #57678 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent 1091066 commit 462c4b0

11 files changed

Lines changed: 93 additions & 19 deletions

‎lib/internal/main/repl.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) {
4545
}
4646
repl.on('exit', () => {
4747
if (repl._flushing) {
48-
repl.pause();
4948
return repl.once('flushHistory', () => {
5049
process.exit();
5150
});

‎lib/internal/readline/interface.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,9 @@ class Interface extends InterfaceConstructor {
611611
* @returns {void | Interface}
612612
*/
613613
pause() {
614+
if (this.closed) {
615+
throw new ERR_USE_AFTER_CLOSE('readline');
616+
}
614617
if (this.paused) return;
615618
this.input.pause();
616619
this.paused = true;
@@ -623,6 +626,9 @@ class Interface extends InterfaceConstructor {
623626
* @returns {void | Interface}
624627
*/
625628
resume() {
629+
if (this.closed) {
630+
throw new ERR_USE_AFTER_CLOSE('readline');
631+
}
626632
if (!this.paused) return;
627633
this.input.resume();
628634
this.paused = false;
@@ -643,6 +649,9 @@ class Interface extends InterfaceConstructor {
643649
* @returns {void}
644650
*/
645651
write(d, key) {
652+
if (this.closed) {
653+
throw new ERR_USE_AFTER_CLOSE('readline');
654+
}
646655
if (this.paused) this.resume();
647656
if (this.terminal) {
648657
this[kTtyWrite](d, key);

‎lib/internal/repl/history.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,10 @@ function setupHistory(repl, historyPath, ready) {
116116

117117
// Reading the file data out erases it
118118
repl.once('flushHistory', function() {
119-
repl.resume();
120-
ready(null, repl);
119+
if (!repl.closed) {
120+
repl.resume();
121+
ready(null, repl);
122+
}
121123
});
122124
flushHistory();
123125
});

‎lib/repl.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -974,9 +974,9 @@ function REPLServer(prompt,
974974
self.output.write(self.writer(ret) + '\n');
975975
}
976976

977-
// Display prompt again (unless we already did by emitting the 'error'
978-
// event on the domain instance).
979-
if (!e) {
977+
// If the REPL sever hasn't closed display prompt again (unless we already
978+
// did by emitting the 'error' event on the domain instance).
979+
if (!self.closed && !e) {
980980
self[kLastCommandErrored] = false;
981981
self.displayPrompt();
982982
}

‎test/parallel/test-readline-interface.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,6 +1202,47 @@ for (let i = 0; i < 12; i++) {
12021202
fi.emit('data', 'Node.js\n');
12031203
}
12041204

1205+
// Call write after close
1206+
{
1207+
const [rli, fi] = getInterface({ terminal });
1208+
rli.question('What\'s your name?', common.mustCall((name) => {
1209+
assert.strictEqual(name, 'Node.js');
1210+
rli.close();
1211+
assert.throws(() => {
1212+
rli.write('I said Node.js');
1213+
}, {
1214+
name: 'Error',
1215+
code: 'ERR_USE_AFTER_CLOSE'
1216+
});
1217+
}));
1218+
fi.emit('data', 'Node.js\n');
1219+
}
1220+
1221+
// Call pause/resume after close
1222+
{
1223+
const [rli, fi] = getInterface({ terminal });
1224+
rli.question('What\'s your name?', common.mustCall((name) => {
1225+
assert.strictEqual(name, 'Node.js');
1226+
rli.close();
1227+
// No 'resume' nor 'pause' event should be emitted after close
1228+
rli.on('resume', common.mustNotCall());
1229+
rli.on('pause', common.mustNotCall());
1230+
assert.throws(() => {
1231+
rli.pause();
1232+
}, {
1233+
name: 'Error',
1234+
code: 'ERR_USE_AFTER_CLOSE'
1235+
});
1236+
assert.throws(() => {
1237+
rli.resume();
1238+
}, {
1239+
name: 'Error',
1240+
code: 'ERR_USE_AFTER_CLOSE'
1241+
});
1242+
}));
1243+
fi.emit('data', 'Node.js\n');
1244+
}
1245+
12051246
// Can create a new readline Interface with a null output argument
12061247
{
12071248
const [rli, fi] = getInterface({ output: null, terminal });

‎test/parallel/test-readline-promises-interface.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ function assertCursorRowsAndCols(rli, rows, cols) {
204204
fi.emit('data', character);
205205
}
206206
fi.emit('data', '\n');
207-
rli.close();
207+
fi.end();
208208
}
209209

210210
// \t when there is no completer function should behave like an ordinary

‎test/parallel/test-readline-promises-tab-complete.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ if (process.env.TERM === 'dumb') {
8080
output = '';
8181
});
8282
}
83-
rli.close();
83+
fi.end();
8484
});
8585
});
8686
});
@@ -114,5 +114,5 @@ if (process.env.TERM === 'dumb') {
114114
assert.match(output, /^Tab completion error: Error: message/);
115115
output = '';
116116
});
117-
rli.close();
117+
fi.end();
118118
}

‎test/parallel/test-repl-close.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const cp = require('child_process');
5+
6+
const child = cp.spawn(process.execPath, ['--interactive']);
7+
8+
let output = '';
9+
child.stdout.on('data', (data) => {
10+
output += data;
11+
});
12+
13+
child.on('exit', common.mustCall(() => {
14+
assert.doesNotMatch(output, /Uncaught Error/);
15+
}));
16+
17+
child.stdin.write('await null;\n');
18+
child.stdin.write('.exit\n');

‎test/parallel/test-repl-import-referrer.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ child.stdout.on('data', (data) => {
1515
});
1616

1717
child.on('exit', common.mustCall(() => {
18-
const results = output.replace(/^> /mg, '').split('\n').slice(2);
19-
assert.deepStrictEqual(
20-
results,
21-
['[Module: null prototype] { message: \'A message\' }', '']
22-
);
18+
const result = output.replace(/^> /mg, '').split('\n').slice(2);
19+
assert.deepStrictEqual(result, [
20+
'[Module: null prototype] { message: \'A message\' }',
21+
'',
22+
]);
2323
}));
2424

2525
child.stdin.write('await import(\'./message.mjs\');\n');

‎test/parallel/test-repl-no-terminal.js‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
'use strict';
22
const common = require('../common');
3-
3+
const ArrayStream = require('../common/arraystream');
44
const repl = require('repl');
5-
const r = repl.start({ terminal: false });
6-
r.setupHistory('/nonexistent/file', common.mustSucceed());
7-
process.stdin.unref?.();
5+
6+
const stream = new ArrayStream();
7+
8+
const replServer = repl.start({ terminal: false, input: stream, output: stream });
9+
10+
replServer.setupHistory('/nonexistent/file', common.mustSucceed(() => {
11+
replServer.close();
12+
}));

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL