Skip to content

Commit

Permalink
fs: fix readdir failure when libuv returns UV_DIRENT_UNKNOWN
Browse files Browse the repository at this point in the history
Fixes: #33348

PR-URL: #33395
Refs: #33348
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
shackijj authored and codebytere committed Jul 14, 2020
1 parent d16e51b commit 47132ec
Show file tree
Hide file tree
Showing 3 changed files with 182 additions and 3 deletions.
45 changes: 42 additions & 3 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,31 @@ function copyObject(source) {
return target;
}

const bufferSep = Buffer.from(pathModule.sep);

function join(path, name) {
if ((typeof path === 'string' || isUint8Array(path)) &&
name === undefined) {
return path;
}

if (typeof path === 'string' && isUint8Array(name)) {
const pathBuffer = Buffer.from(pathModule.join(path, pathModule.sep));
return Buffer.concat([pathBuffer, name]);
}

if (typeof path === 'string' && typeof name === 'string') {
return pathModule.join(path, name);
}

if (isUint8Array(path) && isUint8Array(name)) {
return Buffer.concat([path, bufferSep, name]);
}

throw new ERR_INVALID_ARG_TYPE(
'path', ['string', 'Buffer'], path);
}

function getDirents(path, [names, types], callback) {
let i;
if (typeof callback === 'function') {
Expand All @@ -156,7 +181,14 @@ function getDirents(path, [names, types], callback) {
const name = names[i];
const idx = i;
toFinish++;
lazyLoadFs().lstat(pathModule.join(path, name), (err, stats) => {
let filepath;
try {
filepath = join(path, name);
} catch (err) {
callback(err);
return;
}
lazyLoadFs().lstat(filepath, (err, stats) => {
if (err) {
callback(err);
return;
Expand Down Expand Up @@ -185,7 +217,14 @@ function getDirents(path, [names, types], callback) {
function getDirent(path, name, type, callback) {
if (typeof callback === 'function') {
if (type === UV_DIRENT_UNKNOWN) {
lazyLoadFs().lstat(pathModule.join(path, name), (err, stats) => {
let filepath;
try {
filepath = join(path, name);
} catch (err) {
callback(err);
return;
}
lazyLoadFs().lstat(filepath, (err, stats) => {
if (err) {
callback(err);
return;
Expand All @@ -196,7 +235,7 @@ function getDirent(path, name, type, callback) {
callback(null, new Dirent(name, type));
}
} else if (type === UV_DIRENT_UNKNOWN) {
const stats = lazyLoadFs().lstatSync(pathModule.join(path, name));
const stats = lazyLoadFs().lstatSync(join(path, name));
return new DirentFromStats(name, stats);
} else {
return new Dirent(name, type);
Expand Down
17 changes: 17 additions & 0 deletions test/parallel/test-fs-readdir-buffer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
'use strict';
const common = require('../common');
const fs = require('fs');

if (!common.isOSX) {
common.skip('this tests works only on MacOS');
}

const assert = require('assert');

fs.readdir(
Buffer.from('/dev'),
{ withFileTypes: true, encoding: 'buffer' },
common.mustCall((e, d) => {
assert.strictEqual(e, null);
})
);
123 changes: 123 additions & 0 deletions test/parallel/test-fs-utils-get-dirents.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
// Flags: --expose-internals
'use strict';

const common = require('../common');
const { getDirents, getDirent } = require('internal/fs/utils');
const assert = require('assert');
const { internalBinding } = require('internal/test/binding');
const { UV_DIRENT_UNKNOWN } = internalBinding('constants').fs;
const fs = require('fs');
const path = require('path');

const tmpdir = require('../common/tmpdir');
const filename = 'foo';

{
// setup
tmpdir.refresh();
fs.writeFileSync(path.join(tmpdir.path, filename), '');
}
// getDirents
{
// string + string
getDirents(
tmpdir.path,
[[filename], [UV_DIRENT_UNKNOWN]],
common.mustCall((err, names) => {
assert.strictEqual(err, null);
assert.strictEqual(names.length, 1);
},
));
}
{
// string + Buffer
getDirents(
tmpdir.path,
[[Buffer.from(filename)], [UV_DIRENT_UNKNOWN]],
common.mustCall((err, names) => {
assert.strictEqual(err, null);
assert.strictEqual(names.length, 1);
},
));
}
{
// Buffer + Buffer
getDirents(
Buffer.from(tmpdir.path),
[[Buffer.from(filename)], [UV_DIRENT_UNKNOWN]],
common.mustCall((err, names) => {
assert.strictEqual(err, null);
assert.strictEqual(names.length, 1);
},
));
}
{
// wrong combination
getDirents(
42,
[[Buffer.from(filename)], [UV_DIRENT_UNKNOWN]],
common.mustCall((err) => {
assert.strictEqual(
err.message,
[
'The "path" argument must be of type string or an ' +
'instance of Buffer. Received type number (42)'
].join(''));
},
));
}
// getDirent
{
// string + string
getDirent(
tmpdir.path,
filename,
UV_DIRENT_UNKNOWN,
common.mustCall((err, dirent) => {
assert.strictEqual(err, null);
assert.strictEqual(dirent.name, filename);
},
));
}
{
// string + Buffer
const filenameBuffer = Buffer.from(filename);
getDirent(
tmpdir.path,
filenameBuffer,
UV_DIRENT_UNKNOWN,
common.mustCall((err, dirent) => {
assert.strictEqual(err, null);
assert.strictEqual(dirent.name, filenameBuffer);
},
));
}
{
// Buffer + Buffer
const filenameBuffer = Buffer.from(filename);
getDirent(
Buffer.from(tmpdir.path),
filenameBuffer,
UV_DIRENT_UNKNOWN,
common.mustCall((err, dirent) => {
assert.strictEqual(err, null);
assert.strictEqual(dirent.name, filenameBuffer);
},
));
}
{
// wrong combination
getDirent(
42,
Buffer.from(filename),
UV_DIRENT_UNKNOWN,
common.mustCall((err) => {
assert.strictEqual(
err.message,
[
'The "path" argument must be of type string or an ' +
'instance of Buffer. Received type number (42)'
].join(''));
},
));
}

0 comments on commit 47132ec

Please sign in to comment.