Repository: streamich/memfs
Pull request #1081 (2024-12-21 01:26:51)

fix: don't fail on closing fd after reset has been called (#550)
Spent a day tracking this down so I figured I could put in a fix. To review, the issue is that if you have a stream that happens to be open when you call `reset`, you get an error like this (assuming jest, but I'm sure it comes as a surprise to anyone on any testing platform):
```
> npx jest
Error: Unhandled error. (Error: EBADF: bad file descriptor, close
    at createError (/path/to/project/node_modules/memfs/src/node/util.ts:146:17)
    at Volume.getFileByFdOrThrow (/path/to/project/node_modules/memfs/src/volume.ts:540:33)
    at Volume.closeSync (/path/to/project/node_modules/memfs/src/volume.ts:818:23)
    at Immediate._onImmediate (/path/to/project/node_modules/memfs/src/volume.ts:578:25)
    at processImmediate (node:internal/timers:478:21)
    at process.topLevelDomainCallback (node:domain:160:15)
    at process.callbackTrampoline (node:internal/async_hooks:128:24) {
  code: 'EBADF'
})
    at Volume.ReadStream.emit (node:events:507:17)
    at Volume.ReadStream.emit (node:domain:488:12)
    at /path/to/project/node_modules/memfs/src/volume.ts:2397:18
    at Immediate._onImmediate (/path/to/project/node_modules/memfs/src/volume.ts:580:9)
    at processImmediate (node:internal/timers:478:21)
    at process.topLevelDomainCallback (node:domain:160:15)
    at process.callbackTrampoline (node:internal/async_hooks:128:24) {
  code: 'ERR_UNHANDLED_ERROR',
  context: Error: EBADF: bad file descriptor, close
      at createError (/path/to/project/node_modules/memfs/src/node/util.ts:146:17)
      at Volume.getFileByFdOrThrow (/path/to/project/node_modules/memfs/src/volume.ts:540:33)
      at Volume.closeSync (/path/to/project/node_modules/memfs/src/volume.ts:818:23)
      at Immediate._onImmediate (/path/to/project/node_modules/memfs/src/volume.ts:578:25)
      at processImmediate (node:internal/timers:478:21)
      at process.topLevelDomainCallback (node:domain:160:15)
      at process.callbackTrampoline (node:internal/async_hooks:128:24) {
    code: 'EBADF'
  }
}
```
I find it difficult to imagine someone calling `reset` and expecting an error when the `setImmediate` callback fires, so that's where I made my change. Other ways of fixing I thought about were:
- add a `waitForFilesToClose`, but potentially you're testing something where they wouldn't close? also it requires an extra command that you'd only use for this purpose.
- if you could somehow "reset" the actual file system, I'd imagine it would abort all the ongoing operations as well, so this seems ideologically correct (to me)

G-Rath: Can you confirm that this doesn't happen with the native fs? And if so, could you add a test too?
adiktofsugar: @G-Rath you can't reset the native fs, so I'm not sure what I'd be comparing to. The closest I can think of would be if the node process closed while a stream is open, but node keeps itself open in that case unless it detects a cycle. I can definitely add a test, though.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1784dd78..b4cf3a41 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,10 @@
+## [4.15.1](https://github.com/streamich/memfs/compare/v4.15.0...v4.15.1) (2024-12-22)
+
+
+### Bug Fixes
+
+* resolve relative symlinks to the current directory ([#1079](https://github.com/streamich/memfs/issues/1079)) ([63e3873](https://github.com/streamich/memfs/commit/63e38735fe08b728da02b9328d16be4d132b9327)), closes [#725](https://github.com/streamich/memfs/issues/725)
+
 # [4.15.0](https://github.com/streamich/memfs/compare/v4.14.1...v4.15.0) (2024-12-09)
 
 
diff --git a/package.json b/package.json
index 148e1c70..c80fc43c 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "memfs",
-  "version": "4.15.0",
+  "version": "4.15.1",
   "description": "In-memory file-system with Node's fs API.",
   "keywords": [
     "fs",
diff --git a/src/node.ts b/src/node.ts
index 8128e364..1921fb00 100644
--- a/src/node.ts
+++ b/src/node.ts
@@ -36,8 +36,8 @@ export class Node extends EventEmitter {
   // Number of hard links pointing at this Node.
   private _nlink = 1;
 
-  // Steps to another node, if this node is a symlink.
-  symlink: string[];
+  // Path to another node, if this is a symlink.
+  symlink: string;
 
   constructor(ino: number, perm: number = 0o666) {
     super();
@@ -163,9 +163,9 @@ export class Node extends EventEmitter {
     return (this.mode & S_IFMT) === S_IFLNK;
   }
 
-  makeSymlink(steps: string[]) {
-    this.symlink = steps;
-    this.setIsSymlink();
+  makeSymlink(symlink: string) {
+    this.mode = S_IFLNK;
+    this.symlink = symlink;
   }
 
   write(buf: Buffer, off: number = 0, len: number = buf.length, pos: number = 0): number {
diff --git a/src/volume.ts b/src/volume.ts
index f685f610..77f3067d 100644
--- a/src/volume.ts
+++ b/src/volume.ts
@@ -472,7 +472,11 @@ export class Volume implements FsCallbackApi, FsSynchronousApi {
       node = curr?.getNode();
       // Resolve symlink
       if (resolveSymlinks && node.isSymlink()) {
-        steps = node.symlink.concat(steps.slice(i + 1));
+        const resolvedPath = pathModule.isAbsolute(node.symlink)
+          ? node.symlink
+          : join(pathModule.dirname(curr.getPath()), node.symlink); // Relative to symlink's parent
+
+        steps = filenameToSteps(resolvedPath).concat(steps.slice(i + 1));
         curr = this.root;
         i = 0;
         continue;
@@ -821,7 +825,9 @@ export class Volume implements FsCallbackApi, FsSynchronousApi {
 
   close(fd: number, callback: TCallback<void>) {
     validateFd(fd);
-    this.wrapAsync(this.closeSync, [fd], callback);
+    const file = this.getFileByFdOrThrow(fd, 'close');
+    // NOTE: not calling closeSync because we can reset in between close and closeSync
+    this.wrapAsync(this.closeFile, [file], callback);
   }
 
   private openFileOrGetById(id: TFileId, flagsNum: number, modeNum?: number): File {
@@ -1294,7 +1300,8 @@ export class Volume implements FsCallbackApi, FsSynchronousApi {
 
     // Create symlink.
     const symlink: Link = dirLink.createChild(name);
-    symlink.getNode().makeSymlink(filenameToSteps(targetFilename));
+    symlink.getNode().makeSymlink(targetFilename);
+
     return symlink;
   }
 
@@ -1637,8 +1644,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi {
 
     if (!node.isSymlink()) throw createError(EINVAL, 'readlink', filename);
 
-    const str = sep + node.symlink.join(sep);
-    return strToEncoding(str, encoding);
+    return strToEncoding(node.symlink, encoding);
   }
 
   readlinkSync(path: PathLike, options?: opts.IOptions): TDataOut {


Test patch:
diff --git a/src/__tests__/volume.test.ts b/src/__tests__/volume.test.ts
index 7c7798d1..e70f624e 100644
--- a/src/__tests__/volume.test.ts
+++ b/src/__tests__/volume.test.ts
@@ -361,6 +361,23 @@ describe('volume', () => {
           '/good': 'bye',
         });
       });
+      it('Open streams should not be affected', async () => {
+        const vol = new Volume();
+        const json = {
+          '/hello': 'world',
+        };
+        vol.fromJSON(json);
+        await new Promise((resolve, reject) => {
+          vol
+            .createReadStream('/hello')
+            .on('data', () => null)
+            .on('close', resolve)
+            .on('end', () => {
+              vol.reset();
+            })
+            .on('error', reject);
+        });
+      });
     });
     describe('.openSync(path, flags[, mode])', () => {
       const vol = new Volume();
@@ -795,6 +812,62 @@ describe('volume', () => {
           expect(vol.readFileSync('/c1/c2/c3/c4/c5/final/a3/a4/a5/hello.txt', 'utf8')).toBe('world a');
         });
       });
+      describe('Relative paths', () => {
+        it('Creates symlinks with relative paths correctly', () => {
+          const vol = Volume.fromJSON({
+            '/test/target': 'foo',
+            '/test/folder': null,
+          });
+
+          // Create symlink using relative path
+          vol.symlinkSync('../target', '/test/folder/link');
+
+          // Verify we can read through the symlink
+          expect(vol.readFileSync('/test/folder/link', 'utf8')).toBe('foo');
+
+          // Verify the symlink points to the correct location
+          const linkPath = vol.readlinkSync('/test/folder/link');
+          expect(linkPath).toBe('../target');
+        });
+
+        it('Handles nested relative symlinks', () => {
+          const vol = Volume.fromJSON({
+            '/a/b/target.txt': 'content',
+            '/a/c/d': null,
+          });
+
+          // Create symlink in nested directory using relative path
+          vol.symlinkSync('../../b/target.txt', '/a/c/d/link');
+
+          // Should be able to read through the symlink
+          expect(vol.readFileSync('/a/c/d/link', 'utf8')).toBe('content');
+
+          // Create another symlink pointing to the first symlink
+          vol.symlinkSync('./d/link', '/a/c/link2');
+
+          // Should be able to read through both symlinks
+          expect(vol.readFileSync('/a/c/link2', 'utf8')).toBe('content');
+        });
+
+        it('Maintains relative paths when reading symlinks', () => {
+          const vol = Volume.fromJSON({
+            '/x/y/file.txt': 'test content',
+            '/x/z': null,
+          });
+
+          // Create symlinks with different relative path patterns
+          vol.symlinkSync('../y/file.txt', '/x/z/link1');
+          vol.symlinkSync('../../x/y/file.txt', '/x/z/link2');
+
+          // Verify that readlink returns the original relative paths
+          expect(vol.readlinkSync('/x/z/link1')).toBe('../y/file.txt');
+          expect(vol.readlinkSync('/x/z/link2')).toBe('../../x/y/file.txt');
+
+          // Verify that all symlinks resolve correctly
+          expect(vol.readFileSync('/x/z/link1', 'utf8')).toBe('test content');
+          expect(vol.readFileSync('/x/z/link2', 'utf8')).toBe('test content');
+        });
+      });
     });
     describe('.symlink(target, path[, type], callback)', () => {
       xit('...', () => {});
@@ -806,7 +879,7 @@ describe('volume', () => {
       mootools.getNode().setString(data);
 
       const symlink = vol.root.createChild('mootools.link.js');
-      symlink.getNode().makeSymlink(['mootools.js']);
+      symlink.getNode().makeSymlink('mootools.js');
 
       it('Symlink works', () => {
         const resolved = vol.resolveSymlinks(symlink);
@@ -828,7 +901,7 @@ describe('volume', () => {
       mootools.getNode().setString(data);
 
       const symlink = vol.root.createChild('mootools.link.js');
-      symlink.getNode().makeSymlink(['mootools.js']);
+      symlink.getNode().makeSymlink('mootools.js');
 
       it('Basic one-jump symlink resolves', done => {
         vol.realpath('/mootools.link.js', (err, path) => {