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

11 files changed

lines changed

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

});

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);

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

});

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

}

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 });

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

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

}

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');

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');

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+

}));