Skip to content

fs: readdir fails when libuv returns UV_DIRENT_UNKNOWN and the first arg is Buffer #33395

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.

Sign up for GitHub

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

Closed
shackijj wants to merge 5 commits into from
Closed

fs: readdir fails when libuv returns UV_DIRENT_UNKNOWN and the first arg is Buffer #33395

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0cfa197
fs: test readdir with a buffer as a first param
shackijj May 14, 2020
a5328e2
fs: test readdir on macOS only
shackijj May 19, 2020
544ef53
fs: add tests for get-dirents
shackijj May 19, 2020
e6d39b3
fs: add join function which takes Buffers and strings as args
shackijj Jun 13, 2020
0435053
fs: add a space
shackijj Jun 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fs: add join function which takes Buffers and strings as args 
Refs: #33348
  • Loading branch information
@shackijj
shackijj committed Jun 23, 2020
commit e6d39b357bb6e96dea1d9fc4a6753ccdf06e5ac7
45 changes: 42 additions & 3 deletions lib/internal/fs/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,31 @@ function copyObject(source) {
return target;
}

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

function join(path, name) {
if ((typeof path === 'string' || isUint8Array(path)) &&
name === undefined) {
Comment thread
shackijj marked this conversation as resolved.
Outdated
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 @@ -185,7 +210,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 @@ -214,7 +246,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 @@ -225,7 +264,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
14 changes: 7 additions & 7 deletions test/parallel/test-fs-readdir-buffer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
'use strict';
const common = require('../common');
const fs = require('fs');

const common = require('../common');
if (!common.isOSX) {
common.skip('this tests works only on MacOS');
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);
})
Buffer.from('/dev'),
{ withFileTypes: true, encoding: 'buffer' },
common.mustCall((e, d) => {
assert.strictEqual(e, null);
})
);
126 changes: 104 additions & 22 deletions test/parallel/test-fs-utils-get-dirents.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,123 @@
// Flags: --expose-internals
'use strict';

const { getDirent, getDirents } = require('internal/fs/utils');
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 common = require('../common');
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), '')
// 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);
},
));
}
{
// string + string
getDirents(
tmpdir.path,
[[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
{
// Buffer + string
getDirents(
Buffer.from(tmpdir.path),
[[filename], [UV_DIRENT_UNKNOWN]],
common.mustCall((err, names) => {
assert.strictEqual(err, null);
assert.strictEqual(names.length, 1);
},
// 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(''));
},
));
}