fix: resolve relative symlinks to the current directory (#1079)

kylecarbs · web-flow · commit 63e38735fe08 · 2024-12-22T14:28:51.000+13:00
Fixes #725. Symlinks are intended to be stored as relative paths to their target file.
diff --git a/src/__tests__/volume.test.ts b/src/__tests__/volume.test.ts
@@ -795,6 +795,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 +862,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 +884,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) => {
diff --git 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
@@ -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;
@@ -1294,7 +1298,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 +1642,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 {
