From d7ad627e4e9aaa446f836fdd425f41c7c14caaeb Mon Sep 17 00:00:00 2001 From: Sergey Chernyshev Date: Thu, 25 Apr 2024 01:55:05 +0200 Subject: [PATCH] fs: make `mkdtemp` accept buffers and URL PR-URL: https://github.com/nodejs/node/pull/48828 Backport-PR-URL: https://github.com/nodejs/node/pull/50669 Reviewed-By: Mohammed Keyvanzadeh Reviewed-By: Antoine du Hamel --- graal-nodejs/doc/api/fs.md | 15 ++- graal-nodejs/lib/fs.js | 30 +++-- graal-nodejs/lib/internal/fs/promises.js | 14 ++- graal-nodejs/lib/internal/fs/utils.js | 5 +- graal-nodejs/test/parallel/test-fs-mkdtemp.js | 110 +++++++++++++++--- 5 files changed, 139 insertions(+), 35 deletions(-) diff --git a/graal-nodejs/doc/api/fs.md b/graal-nodejs/doc/api/fs.md index 893eb7befb7..711ed4a86da 100644 --- a/graal-nodejs/doc/api/fs.md +++ b/graal-nodejs/doc/api/fs.md @@ -1153,6 +1153,9 @@ makeDirectory().catch(console.error); -* `prefix` {string} +* `prefix` {string|Buffer|URL} * `options` {string|Object} * `encoding` {string} **Default:** `'utf8'` * Returns: {Promise} Fulfills with a string containing the file system path @@ -3225,6 +3228,9 @@ See the POSIX mkdir(2) documentation for more details. -* `prefix` {string} +* `prefix` {string|Buffer|URL} * `options` {string|Object} * `encoding` {string} **Default:** `'utf8'` * `callback` {Function} @@ -5478,6 +5484,9 @@ See the POSIX mkdir(2) documentation for more details. -* `prefix` {string} +* `prefix` {string|Buffer|URL} * `options` {string|Object} * `encoding` {string} **Default:** `'utf8'` * Returns: {string} diff --git a/graal-nodejs/lib/fs.js b/graal-nodejs/lib/fs.js index 8b6af16e5e5..09cace93b54 100644 --- a/graal-nodejs/lib/fs.js +++ b/graal-nodejs/lib/fs.js @@ -139,7 +139,6 @@ const { validateFunction, validateInteger, validateObject, - validateString, } = require('internal/validators'); let truncateWarn = true; @@ -2884,7 +2883,7 @@ realpath.native = (path, options, callback) => { /** * Creates a unique temporary directory. - * @param {string} prefix + * @param {string | Buffer | URL} prefix * @param {string | { encoding?: string; }} [options] * @param {( * err?: Error, @@ -2896,27 +2895,40 @@ function mkdtemp(prefix, options, callback) { callback = makeCallback(typeof options === 'function' ? options : callback); options = getOptions(options); - validateString(prefix, 'prefix'); - nullCheck(prefix, 'prefix'); + prefix = getValidatedPath(prefix, 'prefix'); warnOnNonPortableTemplate(prefix); + + let path; + if (typeof prefix === 'string') { + path = `${prefix}XXXXXX`; + } else { + path = Buffer.concat([prefix, Buffer.from('XXXXXX')]); + } + const req = new FSReqCallback(); req.oncomplete = callback; - binding.mkdtemp(`${prefix}XXXXXX`, options.encoding, req); + binding.mkdtemp(path, options.encoding, req); } /** * Synchronously creates a unique temporary directory. - * @param {string} prefix + * @param {string | Buffer | URL} prefix * @param {string | { encoding?: string; }} [options] * @returns {string} */ function mkdtempSync(prefix, options) { options = getOptions(options); - validateString(prefix, 'prefix'); - nullCheck(prefix, 'prefix'); + prefix = getValidatedPath(prefix, 'prefix'); warnOnNonPortableTemplate(prefix); - const path = `${prefix}XXXXXX`; + + let path; + if (typeof prefix === 'string') { + path = `${prefix}XXXXXX`; + } else { + path = Buffer.concat([prefix, Buffer.from('XXXXXX')]); + } + const ctx = { path }; const result = binding.mkdtemp(path, options.encoding, undefined, ctx); diff --git a/graal-nodejs/lib/internal/fs/promises.js b/graal-nodejs/lib/internal/fs/promises.js index 4cf97f2253a..c4409d51b9d 100644 --- a/graal-nodejs/lib/internal/fs/promises.js +++ b/graal-nodejs/lib/internal/fs/promises.js @@ -59,7 +59,6 @@ const { getStatsFromBinding, getValidatedPath, getValidMode, - nullCheck, preprocessSymlinkDestination, stringToFlags, stringToSymlinkType, @@ -976,10 +975,17 @@ async function realpath(path, options) { async function mkdtemp(prefix, options) { options = getOptions(options); - validateString(prefix, 'prefix'); - nullCheck(prefix); + prefix = getValidatedPath(prefix, 'prefix'); warnOnNonPortableTemplate(prefix); - return binding.mkdtemp(`${prefix}XXXXXX`, options.encoding, kUsePromises); + + let path; + if (typeof prefix === 'string') { + path = `${prefix}XXXXXX`; + } else { + path = Buffer.concat([prefix, Buffer.from('XXXXXX')]); + } + + return binding.mkdtemp(path, options.encoding, kUsePromises); } async function writeFile(path, data, options) { diff --git a/graal-nodejs/lib/internal/fs/utils.js b/graal-nodejs/lib/internal/fs/utils.js index 23865845bac..00889ffccc4 100644 --- a/graal-nodejs/lib/internal/fs/utils.js +++ b/graal-nodejs/lib/internal/fs/utils.js @@ -21,6 +21,7 @@ const { StringPrototypeEndsWith, StringPrototypeIncludes, Symbol, + TypedArrayPrototypeAt, TypedArrayPrototypeIncludes, } = primordials; @@ -736,7 +737,9 @@ let nonPortableTemplateWarn = true; function warnOnNonPortableTemplate(template) { // Template strings passed to the mkdtemp() family of functions should not // end with 'X' because they are handled inconsistently across platforms. - if (nonPortableTemplateWarn && StringPrototypeEndsWith(template, 'X')) { + if (nonPortableTemplateWarn && + ((typeof template === 'string' && StringPrototypeEndsWith(template, 'X')) || + (typeof template !== 'string' && TypedArrayPrototypeAt(template, -1) === 0x58))) { process.emitWarning('mkdtemp() templates ending with X are not portable. ' + 'For details see: https://nodejs.org/api/fs.html'); nonPortableTemplateWarn = false; diff --git a/graal-nodejs/test/parallel/test-fs-mkdtemp.js b/graal-nodejs/test/parallel/test-fs-mkdtemp.js index 950a524368c..b7bb0bbafe0 100644 --- a/graal-nodejs/test/parallel/test-fs-mkdtemp.js +++ b/graal-nodejs/test/parallel/test-fs-mkdtemp.js @@ -8,29 +8,103 @@ const path = require('path'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); -const tmpFolder = fs.mkdtempSync(path.join(tmpdir.path, 'foo.')); - -assert.strictEqual(path.basename(tmpFolder).length, 'foo.XXXXXX'.length); -assert(fs.existsSync(tmpFolder)); - -const utf8 = fs.mkdtempSync(path.join(tmpdir.path, '\u0222abc.')); -assert.strictEqual(Buffer.byteLength(path.basename(utf8)), - Buffer.byteLength('\u0222abc.XXXXXX')); -assert(fs.existsSync(utf8)); - function handler(err, folder) { assert.ifError(err); assert(fs.existsSync(folder)); assert.strictEqual(this, undefined); } -fs.mkdtemp(path.join(tmpdir.path, 'bar.'), common.mustCall(handler)); +// Test with plain string +{ + const tmpFolder = fs.mkdtempSync(path.join(tmpdir.path, 'foo.')); + + assert.strictEqual(path.basename(tmpFolder).length, 'foo.XXXXXX'.length); + assert(fs.existsSync(tmpFolder)); + + const utf8 = fs.mkdtempSync(path.join(tmpdir.path, '\u0222abc.')); + assert.strictEqual(Buffer.byteLength(path.basename(utf8)), + Buffer.byteLength('\u0222abc.XXXXXX')); + assert(fs.existsSync(utf8)); + + fs.mkdtemp(path.join(tmpdir.path, 'bar.'), common.mustCall(handler)); + + // Same test as above, but making sure that passing an options object doesn't + // affect the way the callback function is handled. + fs.mkdtemp(path.join(tmpdir.path, 'bar.'), {}, common.mustCall(handler)); + + const warningMsg = 'mkdtemp() templates ending with X are not portable. ' + + 'For details see: https://nodejs.org/api/fs.html'; + common.expectWarning('Warning', warningMsg); + fs.mkdtemp(path.join(tmpdir.path, 'bar.X'), common.mustCall(handler)); +} + +// Test with URL object +{ + tmpdir.url = new URL(`file://${tmpdir.path}`); + const urljoin = (base, path) => new URL(path, base); + + const tmpFolder = fs.mkdtempSync(urljoin(tmpdir.url, 'foo.')); + + assert.strictEqual(path.basename(tmpFolder).length, 'foo.XXXXXX'.length); + assert(fs.existsSync(tmpFolder)); + + const utf8 = fs.mkdtempSync(urljoin(tmpdir.url, '\u0222abc.')); + assert.strictEqual(Buffer.byteLength(path.basename(utf8)), + Buffer.byteLength('\u0222abc.XXXXXX')); + assert(fs.existsSync(utf8)); + + fs.mkdtemp(urljoin(tmpdir.url, 'bar.'), common.mustCall(handler)); + + // Same test as above, but making sure that passing an options object doesn't + // affect the way the callback function is handled. + fs.mkdtemp(urljoin(tmpdir.url, 'bar.'), {}, common.mustCall(handler)); + + // Warning fires only once + fs.mkdtemp(urljoin(tmpdir.url, 'bar.X'), common.mustCall(handler)); +} + +// Test with Buffer +{ + const tmpFolder = fs.mkdtempSync(Buffer.from(path.join(tmpdir.path, 'foo.'))); + + assert.strictEqual(path.basename(tmpFolder).length, 'foo.XXXXXX'.length); + assert(fs.existsSync(tmpFolder)); -// Same test as above, but making sure that passing an options object doesn't -// affect the way the callback function is handled. -fs.mkdtemp(path.join(tmpdir.path, 'bar.'), {}, common.mustCall(handler)); + const utf8 = fs.mkdtempSync(Buffer.from(path.join(tmpdir.path, '\u0222abc.'))); + assert.strictEqual(Buffer.byteLength(path.basename(utf8)), + Buffer.byteLength('\u0222abc.XXXXXX')); + assert(fs.existsSync(utf8)); + + fs.mkdtemp(Buffer.from(path.join(tmpdir.path, 'bar.')), common.mustCall(handler)); + + // Same test as above, but making sure that passing an options object doesn't + // affect the way the callback function is handled. + fs.mkdtemp(Buffer.from(path.join(tmpdir.path, 'bar.')), {}, common.mustCall(handler)); + + // Warning fires only once + fs.mkdtemp(Buffer.from(path.join(tmpdir.path, 'bar.X')), common.mustCall(handler)); +} -const warningMsg = 'mkdtemp() templates ending with X are not portable. ' + - 'For details see: https://nodejs.org/api/fs.html'; -common.expectWarning('Warning', warningMsg); -fs.mkdtemp(path.join(tmpdir.path, 'bar.X'), common.mustCall(handler)); +// Test with Uint8Array +{ + const encoder = new TextEncoder(); + + const tmpFolder = fs.mkdtempSync(encoder.encode(path.join(tmpdir.path, 'foo.'))); + + assert.strictEqual(path.basename(tmpFolder).length, 'foo.XXXXXX'.length); + assert(fs.existsSync(tmpFolder)); + + const utf8 = fs.mkdtempSync(encoder.encode(path.join(tmpdir.path, '\u0222abc.'))); + assert.strictEqual(Buffer.byteLength(path.basename(utf8)), + Buffer.byteLength('\u0222abc.XXXXXX')); + assert(fs.existsSync(utf8)); + + fs.mkdtemp(encoder.encode(path.join(tmpdir.path, 'bar.')), common.mustCall(handler)); + + // Same test as above, but making sure that passing an options object doesn't + // affect the way the callback function is handled. + fs.mkdtemp(encoder.encode(path.join(tmpdir.path, 'bar.')), {}, common.mustCall(handler)); + + // Warning fires only once + fs.mkdtemp(encoder.encode(path.join(tmpdir.path, 'bar.X')), common.mustCall(handler)); +}