Skip to content

Commit 3307cf8

Browse files
committed
fs: prevent multiple invocations of callback in Dir class
1 parent 264cad7 commit 3307cf8

File tree

2 files changed

+50
-10
lines changed

2 files changed

+50
-10
lines changed

lib/internal/fs/dir.js

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -115,22 +115,22 @@ class Dir {
115115
return;
116116
}
117117

118+
let dirent;
118119
if (this.#processHandlerQueue()) {
119120
try {
120-
const dirent = ArrayPrototypeShift(this.#bufferedEntries);
121+
dirent = ArrayPrototypeShift(this.#bufferedEntries);
121122

122123
if (this.#options.recursive && dirent.isDirectory()) {
123124
this.readSyncRecursive(dirent);
124125
}
125-
126-
if (maybeSync)
127-
process.nextTick(callback, null, dirent);
128-
else
129-
callback(null, dirent);
130-
return;
131126
} catch (error) {
132127
return callback(error);
133128
}
129+
if (maybeSync)
130+
process.nextTick(callback, null, dirent);
131+
else
132+
callback(null, dirent);
133+
return;
134134
}
135135

136136
const req = new FSReqCallback();
@@ -145,16 +145,17 @@ class Dir {
145145
return callback(err, result);
146146
}
147147

148+
let dirent;
148149
try {
149150
this.processReadResult(this.#path, result);
150-
const dirent = ArrayPrototypeShift(this.#bufferedEntries);
151+
dirent = ArrayPrototypeShift(this.#bufferedEntries);
151152
if (this.#options.recursive && dirent.isDirectory()) {
152153
this.readSyncRecursive(dirent);
153154
}
154-
callback(null, dirent);
155155
} catch (error) {
156-
callback(error);
156+
return callback(error);
157157
}
158+
return callback(null, dirent);
158159
};
159160

160161
this.#operationQueue = [];

test/parallel/test-fs-opendir.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const common = require('../common');
44
const assert = require('assert');
55
const fs = require('fs');
66
const path = require('path');
7+
const test = require('node:test');
78

89
const tmpdir = require('../common/tmpdir');
910

@@ -287,3 +288,41 @@ doConcurrentAsyncMixedOps().then(common.mustCall());
287288
dir.closeSync();
288289
assert.rejects(dir.close(), dirclosedError).then(common.mustCall());
289290
}
291+
292+
test('fs.opendir double callback check', async (t) => {
293+
let callbackCount = 0;
294+
295+
await new Promise((resolve, reject) => {
296+
fs.opendir('.', (err, dir) => {
297+
if (err) return reject(err);
298+
299+
const cleanup = () => {
300+
dir.close((err) => {
301+
if (err) {
302+
reject(err);
303+
}
304+
});
305+
};
306+
307+
dir.read((err, dirent) => {
308+
callbackCount++;
309+
if (callbackCount === 1) throw new Error('User error');
310+
if (callbackCount > 1) {
311+
reject(new Error('Callback was called multiple times'));
312+
}
313+
});
314+
315+
const timeout = setTimeout(() => {
316+
cleanup();
317+
resolve();
318+
}, 1000);
319+
320+
process.nextTick(() => {
321+
if (callbackCount > 1) {
322+
clearTimeout(timeout);
323+
cleanup();
324+
}
325+
});
326+
});
327+
});
328+
});

0 commit comments

Comments
 (0)