nodejs/node · #66352

vfs: fix readlink, realpath and mkdtemp encoding

christianaurichzm · merged Oct 25, 20265 files · 164 + / 54 −
lib/fs.js1 + / 10 −
@@ -102,6 +102,7 @@ const {   collectRecursiveReaddirResult,   copyObject,   Dirent,+  encodeRealpathResult,   getDirents,   getRecursiveDirents,   getOptions,@@ -3319,16 +3320,6 @@ if (isWindows) {  } -function encodeRealpathResult(result, options) {-  if (!options || !options.encoding || options.encoding === 'utf8')-    return result;-  const asBuffer = Buffer.from(result);-  if (options.encoding === 'buffer') {-    return asBuffer;-  }-  return asBuffer.toString(options.encoding);-}- // Finds the next portion of a (partial) path, up to the next path delimiter let nextPart; if (isWindows) {
lib/internal/fs/utils.js11 + / 0 −
@@ -427,6 +427,16 @@ function getOptions(options, defaultOptions = kEmptyObject) {   return options; } +function encodeRealpathResult(result, options) {+  if (!options || !options.encoding || options.encoding === 'utf8')+    return result;+  const asBuffer = Buffer.from(result);+  if (options.encoding === 'buffer') {+    return asBuffer;+  }+  return asBuffer.toString(options.encoding);+}+ /**  * @param {InternalFSBinding.FSSyncContext} ctx  */@@ -1209,6 +1219,7 @@ module.exports = {   copyObject,   Dirent,   DirentFromStats,+  encodeRealpathResult,   getDirent,   getDirents,   getOptions,
lib/internal/vfs/file_system.js22 + / 15 −
@@ -35,6 +35,7 @@ const {   createEBUSY,   createEISDIR, } = require('internal/vfs/errors');+const { encodeRealpathResult, getOptions } = require('internal/fs/utils'); const { VirtualReadStream, VirtualWriteStream } = require('internal/vfs/streams'); const { VirtualDir } = require('internal/vfs/dir'); const { emitExperimentalWarning, kEmptyObject } = require('internal/util');@@ -457,24 +458,26 @@ class VirtualFileSystem {   /**    * Gets the real path by resolving all symlinks.    * @param {string} filePath The path-   * @param {object} [options] Options-   * @returns {string}+   * @param {object|string} [options] Options or encoding+   * @returns {string|Buffer}    */   realpathSync(filePath, options) {+    options = getOptions(options);     const providerPath = this.#toProviderPath(filePath);-    const realProviderPath = this[kProvider].realpathSync(providerPath, options);-    return this.#toMountedPath(realProviderPath);+    const realProviderPath = this[kProvider].realpathSync(providerPath);+    return encodeRealpathResult(this.#toMountedPath(realProviderPath), options);   }    /**    * Reads the target of a symbolic link.    * @param {string} linkPath The symlink path-   * @param {object} [options] Options-   * @returns {string}+   * @param {object|string} [options] Options or encoding+   * @returns {string|Buffer}    */   readlinkSync(linkPath, options) {+    options = getOptions(options);     const providerPath = this.#toProviderPath(linkPath);-    return this[kProvider].readlinkSync(providerPath, options);+    return encodeRealpathResult(this[kProvider].readlinkSync(providerPath), options);   }    /**@@ -825,34 +828,36 @@ class VirtualFileSystem {   /**    * Gets the real path asynchronously.    * @param {string} filePath The path-   * @param {object|Function} [options] Options or callback+   * @param {object|string|Function} [options] Options, encoding, or callback    * @param {Function} [callback] Callback (err, resolvedPath)    */   realpath(filePath, options, callback) {     if (typeof options === 'function') {       callback = options;       options = undefined;     }+    options = getOptions(options); -    this[kProvider].realpath(this.#toProviderPath(filePath), options)-      .then((realPath) => callback(null, this.#toMountedPath(realPath)),+    this[kProvider].realpath(this.#toProviderPath(filePath))+      .then((realPath) => callback(null, encodeRealpathResult(this.#toMountedPath(realPath), options)),             (err) => callback(err));   }    /**    * Reads symlink target asynchronously.    * @param {string} linkPath The symlink path-   * @param {object|Function} [options] Options or callback+   * @param {object|string|Function} [options] Options, encoding, or callback    * @param {Function} [callback] Callback (err, target)    */   readlink(linkPath, options, callback) {     if (typeof options === 'function') {       callback = options;       options = undefined;     }+    options = getOptions(options); -    this[kProvider].readlink(this.#toProviderPath(linkPath), options)-      .then((target) => callback(null, target), (err) => callback(err));+    this[kProvider].readlink(this.#toProviderPath(linkPath))+      .then((target) => callback(null, encodeRealpathResult(target, options)), (err) => callback(err));   }    /**@@ -1265,13 +1270,15 @@ class VirtualFileSystem {       },        async realpath(filePath, options) {+        options = getOptions(options);         const providerPath = toProviderPath(filePath);-        return toMountedPath(await provider.realpath(providerPath, options));+        return encodeRealpathResult(toMountedPath(await provider.realpath(providerPath)), options);       },        async readlink(linkPath, options) {+        options = getOptions(options);         const providerPath = toProviderPath(linkPath);-        return provider.readlink(providerPath, options);+        return encodeRealpathResult(await provider.readlink(providerPath), options);       },        async symlink(target, path, type) {
lib/internal/vfs/setup.js26 + / 29 −
@@ -37,7 +37,12 @@ const { } = require('internal/vfs/router'); const { ReservedRootProvider } = require('internal/vfs/root'); const { getVirtualFd, closeVirtualFd, createVfsFileHandle } = require('internal/vfs/fd');-const { assertEncoding, setVfsHandlers } = require('internal/fs/utils');+const {+  assertEncoding,+  encodeRealpathResult,+  getOptions,+  setVfsHandlers,+} = require('internal/fs/utils'); const permission = require('internal/process/permission'); const { getOptionValue } = require('internal/options'); const nativeModulesBinding = internalBinding('modules');@@ -73,6 +78,14 @@ function toWriteBuffer(data, options) {   return Buffer.from(data, encoding || 'utf8'); } +function getMkdtempOptions(prefix, options) {+  options = getOptions(options);+  if (Buffer.isBuffer(prefix)) {+    options = { __proto__: null, ...options, encoding: 'buffer' };+  }+  return options;+}+ function writeFileSyncFd(fd, data, options) {   const vfd = getVirtualFd(fd);   if (vfd === undefined) return undefined;@@ -433,11 +446,7 @@ function createVfsHandlers() {       return vfsRead(path, 'stat', (vfs, n) => vfs.statSync(n, options));     },     realpathSync(path, options) {-      const result = vfsRead(path, 'realpath', (vfs, n) => vfs.realpathSync(n));-      if (result !== undefined && options?.encoding === 'buffer') {-        return Buffer.from(result);-      }-      return result;+      return vfsRead(path, 'realpath', (vfs, n) => vfs.realpathSync(n, options));     },     accessSync(path, mode) {       const pathStr = toPathStr(path);@@ -458,9 +467,7 @@ function createVfsHandlers() {       if (pathStr === null) return undefined;       const r = findVFS(pathStr);       if (r === null) return undefined;-      const result = r.vfs.readlinkSync(pathStr, options);-      if (options?.encoding === 'buffer') return Buffer.from(result);-      return result;+      return r.vfs.readlinkSync(pathStr, options);     },     statfsSync(path, options) {       const pathStr = toPathStr(path);@@ -526,11 +533,10 @@ function createVfsHandlers() {       });     },     mkdtempSync(prefix, options) {-      const result = vfsOp(prefix, (vfs, n) => vfs.mkdtempSync(n));-      if (result !== undefined && options?.encoding === 'buffer') {-        return Buffer.from(result);-      }-      return result;+      return vfsOp(prefix, (vfs, n) => {+        options = getMkdtempOptions(prefix, options);+        return encodeRealpathResult(vfs.mkdtempSync(n), options);+      });     },     mkstempSync(prefix, options) {       const result = vfsOp(prefix, (vfs, n) => vfs.mkstempSync(n));@@ -768,11 +774,7 @@ function createVfsHandlers() {       return vfsOp(path, (vfs, n) => vfs.promises.readFile(n, options));     },     realpath(path, options) {-      const promise = vfsOp(path, (vfs, n) => vfs.promises.realpath(n, options));-      if (promise !== undefined && options?.encoding === 'buffer') {-        return promise.then((result) => Buffer.from(result));-      }-      return promise;+      return vfsOp(path, (vfs, n) => vfs.promises.realpath(n, options));     },     access(path, mode) {       return vfsOp(path, (vfs, n) => {@@ -787,11 +789,7 @@ function createVfsHandlers() {       if (pathStr === null) return undefined;       const r = findVFS(pathStr);       if (r === null) return undefined;-      const promise = r.vfs.promises.readlink(pathStr, options);-      if (options?.encoding === 'buffer') {-        return promise.then((result) => Buffer.from(result));-      }-      return promise;+      return r.vfs.promises.readlink(pathStr, options);     },     chown: (path, uid, gid) =>       vfsOp(path, (vfs, n) => vfs.promises.chown(n, uid, gid).then(() => true)),@@ -863,11 +861,10 @@ function createVfsHandlers() {       });     },     mkdtemp(prefix, options) {-      const promise = vfsOp(prefix, (vfs, n) => vfs.promises.mkdtemp(n));-      if (promise !== undefined && options?.encoding === 'buffer') {-        return promise.then((result) => Buffer.from(result));-      }-      return promise;+      return vfsOp(prefix, (vfs, n) => {+        options = getMkdtempOptions(prefix, options);+        return vfs.promises.mkdtemp(n).then((result) => encodeRealpathResult(result, options));+      });     },     // Wrapped in async functions so provider throws become rejections.     mkstemp(prefix, options) {
test/parallel/test-vfs-path-encoding.jsadded104 + / 0 −
@@ -0,0 +1,104 @@+// Flags: --disable-warning=ExperimentalWarning+'use strict';++// readlink(), realpath() and mkdtemp() on a mounted VFS apply the encoding+// option the way fs does: given as a string or in an object, 'buffer' or any+// other encoding, and a Buffer mkdtemp() prefix always gives a Buffer. The+// VirtualFileSystem readlink and realpath methods do the same.++const common = require('../common');+const assert = require('assert');+const fs = require('fs');+const path = require('path');+const { promisify } = require('util');+const vfs = require('node:vfs');+const tmpdir = require('../common/tmpdir');++function decode(value, encoding) {+  if (encoding === 'buffer') {+    assert.ok(Buffer.isBuffer(value));+    return value.toString();+  }+  assert.strictEqual(typeof value, 'string');+  return Buffer.from(value, encoding).toString();+}++const memoryVfs = vfs.create();+memoryVfs.writeFileSync('/file.txt', 'x');+memoryVfs.symlinkSync('file.txt', '/link');+memoryVfs.symlinkSync('/file.txt', '/abs-link');+const vfsInstances = [memoryVfs];++if (common.canCreateSymLink()) {+  tmpdir.refresh();+  fs.writeFileSync(tmpdir.resolve('file.txt'), 'x');+  fs.symlinkSync('file.txt', tmpdir.resolve('link'));+  // RealFSProvider returns an absolute target inside its root as a VFS path.+  fs.symlinkSync(tmpdir.resolve('file.txt'), tmpdir.resolve('abs-link'));+  vfsInstances.push(vfs.create(new vfs.RealFSProvider(tmpdir.path)));+}++const readlinks = [+  fs.readlinkSync,+  promisify(fs.readlink),+  fs.promises.readlink,+];+const realpaths = [+  fs.realpathSync,+  fs.realpathSync.native,+  promisify(fs.realpath),+  promisify(fs.realpath.native),+  fs.promises.realpath,+];+const mkdtemps = [+  fs.mkdtempSync,+  promisify(fs.mkdtemp),+  fs.promises.mkdtemp,+];++(async () => {+  for (const myVfs of vfsInstances) {+    const mountPoint = myVfs.mount();+    const prefix = path.join(mountPoint, 'tmp-');+    const vfsReadlinks = [+      myVfs.readlinkSync.bind(myVfs),+      promisify(myVfs.readlink.bind(myVfs)),+      myVfs.promises.readlink,+    ];+    const vfsRealpaths = [+      myVfs.realpathSync.bind(myVfs),+      promisify(myVfs.realpath.bind(myVfs)),+      myVfs.promises.realpath,+    ];++    for (const options of ['buffer', { encoding: 'buffer' }, 'hex', { encoding: 'hex' }]) {+      const encoding = options.encoding ?? options;++      for (const name of ['link', 'abs-link']) {+        const link = path.join(mountPoint, name);+        for (const readlink of [...readlinks, ...vfsReadlinks]) {+          assert.strictEqual(decode(await readlink(link, options), encoding),+                             fs.readlinkSync(link));+        }+        for (const realpath of [...realpaths, ...vfsRealpaths]) {+          assert.strictEqual(decode(await realpath(link, options), encoding),+                             fs.realpathSync(link));+        }+      }++      for (const mkdtemp of mkdtemps) {+        const dir = decode(await mkdtemp(prefix, options), encoding);+        assert.ok(dir.startsWith(prefix));+        assert.ok(fs.statSync(dir).isDirectory());+      }+    }++    for (const mkdtemp of mkdtemps) {+      const dir = await mkdtemp(Buffer.from(prefix), 'hex');+      assert.ok(Buffer.isBuffer(dir));+      assert.ok(fs.statSync(dir).isDirectory());+    }++    myVfs.unmount();+  }+})().then(common.mustCall());