From 0cfa1975f798c2ccfa1308c87f437f182f1776cb Mon Sep 17 00:00:00 2001 From: Kirill Shatskiy Date: Thu, 14 May 2020 10:32:38 +0300 Subject: [PATCH 1/5] fs: test readdir with a buffer as a first param Refs: https://github.com/nodejs/node/issues/33348 --- test/parallel/test-fs-readdir-buffer.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 test/parallel/test-fs-readdir-buffer.js diff --git a/test/parallel/test-fs-readdir-buffer.js b/test/parallel/test-fs-readdir-buffer.js new file mode 100644 index 00000000000000..217bd41f2a253f --- /dev/null +++ b/test/parallel/test-fs-readdir-buffer.js @@ -0,0 +1,17 @@ +'use strict'; +const fs = require('fs'); + +const common = require('../common'); +if (common.isWindows) { + common.skip('windows doesnt support /dev'); +} + +const assert = require('assert'); + +fs.readdir( + Buffer.from("/dev"), + {withFileTypes: true, encoding: "buffer"}, + common.mustCall((e,d) => { + assert.strictEqual(e, null); + }) +); From a5328e27de188d6c96e904cd9713ab85ab832f2c Mon Sep 17 00:00:00 2001 From: Kirill Shatskiy Date: Tue, 19 May 2020 08:05:20 +0300 Subject: [PATCH 2/5] fs: test readdir on macOS only --- test/parallel/test-fs-readdir-buffer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-fs-readdir-buffer.js b/test/parallel/test-fs-readdir-buffer.js index 217bd41f2a253f..63b4d9114291ba 100644 --- a/test/parallel/test-fs-readdir-buffer.js +++ b/test/parallel/test-fs-readdir-buffer.js @@ -2,8 +2,8 @@ const fs = require('fs'); const common = require('../common'); -if (common.isWindows) { - common.skip('windows doesnt support /dev'); +if (!common.isOSX) { + common.skip('this tests works only on MacOS'); } const assert = require('assert'); From 544ef5307997856f1739a29f7d29ac552c6d443b Mon Sep 17 00:00:00 2001 From: Kirill Shatskiy Date: Tue, 19 May 2020 09:11:58 +0300 Subject: [PATCH 3/5] fs: add tests for get-dirents --- test/parallel/test-fs-utils-get-dirents.js | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 test/parallel/test-fs-utils-get-dirents.js diff --git a/test/parallel/test-fs-utils-get-dirents.js b/test/parallel/test-fs-utils-get-dirents.js new file mode 100644 index 00000000000000..ebf305cd813e5e --- /dev/null +++ b/test/parallel/test-fs-utils-get-dirents.js @@ -0,0 +1,41 @@ +// Flags: --expose-internals +'use strict'; + +const { getDirent, getDirents } = 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), '') +} +{ + // string + string + getDirents( + tmpdir.path, + [[filename], [UV_DIRENT_UNKNOWN]], + common.mustCall((err, names) => { + assert.strictEqual(err, null); + assert.strictEqual(names.length, 1); + }, + )); +} +{ + // Buffer + string + getDirents( + Buffer.from(tmpdir.path), + [[filename], [UV_DIRENT_UNKNOWN]], + common.mustCall((err, names) => { + assert.strictEqual(err, null); + assert.strictEqual(names.length, 1); + }, + )); +} \ No newline at end of file From e6d39b357bb6e96dea1d9fc4a6753ccdf06e5ac7 Mon Sep 17 00:00:00 2001 From: Kirill Shatskiy Date: Sat, 13 Jun 2020 12:08:32 +0300 Subject: [PATCH 4/5] fs: add join function which takes Buffers and strings as args Refs: https://github.com/nodejs/node/issues/33348 --- lib/internal/fs/utils.js | 45 +++++++- test/parallel/test-fs-readdir-buffer.js | 14 +-- test/parallel/test-fs-utils-get-dirents.js | 126 +++++++++++++++++---- 3 files changed, 153 insertions(+), 32 deletions(-) diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 04ae34e41fd606..8c97e16e25eaec 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -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) { + 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') { @@ -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; @@ -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; @@ -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); diff --git a/test/parallel/test-fs-readdir-buffer.js b/test/parallel/test-fs-readdir-buffer.js index 63b4d9114291ba..a679aad01bf447 100644 --- a/test/parallel/test-fs-readdir-buffer.js +++ b/test/parallel/test-fs-readdir-buffer.js @@ -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); + }) ); diff --git a/test/parallel/test-fs-utils-get-dirents.js b/test/parallel/test-fs-utils-get-dirents.js index ebf305cd813e5e..6afe6dbca3e529 100644 --- a/test/parallel/test-fs-utils-get-dirents.js +++ b/test/parallel/test-fs-utils-get-dirents.js @@ -1,11 +1,11 @@ // 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'); @@ -13,29 +13,111 @@ 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); + }, )); -} \ No newline at end of file +} +{ + // 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('')); + }, + )); +} From 04350530c5a3ff4702b3e789a009cf262526a89a Mon Sep 17 00:00:00 2001 From: Kirill Shatskiy Date: Thu, 25 Jun 2020 14:52:27 +0300 Subject: [PATCH 5/5] fs: add a space Refs #33348 Co-authored-by: James M Snell --- lib/internal/fs/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 8c97e16e25eaec..712bd87d85f6e6 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -177,7 +177,7 @@ const bufferSep = Buffer.from(pathModule.sep); function join(path, name) { if ((typeof path === 'string' || isUint8Array(path)) && - name === undefined) { + name === undefined) { return path; }