Fix path resolution for symlinks (emscripten-core#23072)

If `/a/link` is a symlink to directory `/b/c` then `/a/link/..` should
be resolved to `/b/c/..` which is `/b`. Currently, we cancel out
`link/..` to get `/a`. The problem is that we apply `PATH_FS.resolve`
and `PATH_FS.normalize` to paths. These functions cancel `..`
incorrectly.

This at least partially handles the situation. `lookupPath` is modified
to avoid calls that call `PATH_FS.normalize()`. We check that `mkdir`,
`open`, `stat`, `truncate`, and `chmod` now work correctly.

Author: hoodmane

src/library_fs.js

@@ -172,15 +172,17 @@ FS.staticInit();
     // paths
     //
     lookupPath(path, opts = {}) {
-      path = PATH_FS.resolve(path);
-
       if (!path) return { path: '', node: null };
       opts.follow_mount ??= true
 
+      if (!PATH.isAbs(path)) {
+        path = FS.cwd() + '/' + path;
+      }
+
       // limit max consecutive symlinks to 40 (SYMLOOP_MAX).
       linkloop: for (var nlinks = 0; nlinks < 40; nlinks++) {
         // split the absolute path
-        var parts = path.split('/').filter((p) => !!p);
+        var parts = path.split('/').filter((p) => !!p && (p !== '.'));
 
         // start at the root
         var current = FS.root;
@@ -193,6 +195,12 @@ FS.staticInit();
             break;
           }
 
+          if (parts[i] === '..') {
+            current_path = PATH.dirname(current_path);
+            current = current.parent;
+            continue;
+          }
+
           current_path = PATH.join2(current_path, parts[i]);
           try {
             current = FS.lookupNode(current, parts[i]);
@@ -218,7 +226,10 @@ FS.staticInit();
               throw new FS.ErrnoError({{{ cDefs.ENOSYS }}});
             }
             var link = current.node_ops.readlink(current);
-            path = PATH_FS.resolve(PATH.dirname(current_path), link, ...parts.slice(i + 1));
+            if (!PATH.isAbs(link)) {
+              link = PATH.dirname(current_path) + '/' + link;
+            }
+            path = link + '/' + parts.slice(i + 1).join('/');
             continue linkloop;
           }
         }
@@ -1045,7 +1056,6 @@ FS.staticInit();
       if (typeof path == 'object') {
         node = path;
       } else {
-        path = PATH.normalize(path);
         // noent_okay makes it so that if the final component of the path
         // doesn't exist, lookupPath returns `node: undefined`. `path` will be
         // updated to point to the target of all symlinks.

src/library_syscall.js

@@ -38,7 +38,7 @@ var SyscallsLibrary = {
         }
         return dir;
       }
-      return PATH.join2(dir, path);
+      return dir + '/' + path;
     },
 
     doStat(func, path, buf) {
@@ -833,10 +833,6 @@ var SyscallsLibrary = {
   __syscall_mkdirat: (dirfd, path, mode) => {
     path = SYSCALLS.getStr(path);
     path = SYSCALLS.calculateAt(dirfd, path);
-    // remove a trailing slash, if one - /a/b/ has basename of '', but
-    // we want to create b in the context of this function
-    path = PATH.normalize(path);
-    if (path[path.length-1] === '/') path = path.substr(0, path.length-1);
     FS.mkdir(path, mode, 0);
     return 0;
   },

test/fs/test_fs_symlink_resolution.c

@@ -0,0 +1,57 @@
+#include <unistd.h>
+#include <fcntl.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <assert.h>
+#include <stdio.h>
+#include <string.h>
+#if defined(__EMSCRIPTEN__)
+#include "emscripten.h"
+#endif
+
+void makedir(const char *dir) {
+  int rtn = mkdir(dir, 0777);
+  assert(rtn == 0);
+}
+
+void changedir(const char *dir) {
+  int rtn = chdir(dir);
+  assert(rtn == 0);
+}
+
+static void create_file(const char *path) {
+  printf("creating: %s\n", path);
+  int fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0777);
+  assert(fd >= 0);
+
+  close(fd);
+}
+
+void setup() {
+#if defined(__EMSCRIPTEN__) && defined(NODEFS)
+  makedir("working");
+  EM_ASM(FS.mount(NODEFS, { root: '.' }, 'working'));
+  changedir("working");
+#endif
+  makedir("a");
+  makedir("b");
+  makedir("b/c");
+  symlink("../b/c", "a/link");
+}
+
+
+int main() {
+  setup();
+  create_file("a/link/../x.txt");
+  struct stat statBuf;
+  assert(stat("a/link/../x.txt", &statBuf) == 0);
+  assert(stat("b/x.txt", &statBuf) == 0);
+  makedir("a/link/../d");
+  assert(stat("a/link/../d", &statBuf) == 0);
+  assert(stat("b/d", &statBuf) == 0);
+
+  assert(truncate("a/link/../x.txt", 0) == 0);
+  assert(chmod("a/link/../x.txt", 0777) == 0);
+  printf("success\n");
+}

test/test_core.py

@@ -5894,6 +5894,20 @@ def test_fs_64bit(self):
       self.set_setting('FORCE_FILESYSTEM')
     self.do_runf('fs/test_64bit.c', 'success')
 
+  @requires_node
+  @parameterized({
+    '': ([],),
+    'nodefs': (['-DNODEFS', '-lnodefs.js'],),
+    'noderawfs': (['-sNODERAWFS'],),
+  })
+  def test_fs_symlink_resolution(self, args):
+    nodefs = '-DNODEFS' in args or '-sNODERAWFS' in args
+    if self.get_setting('WASMFS'):
+      if nodefs:
+        self.skipTest('NODEFS in WasmFS')
+      self.set_setting('FORCE_FILESYSTEM')
+    self.do_runf('fs/test_fs_symlink_resolution.c', 'success', emcc_args=args)
+
   @parameterized({
     '': ([],),
     'nodefs': (['-DNODEFS', '-lnodefs.js'],),

test/wasmfs/wasmfs_mkdir.c

@@ -66,11 +66,7 @@ int main() {
   // Try to make the root directory.
   errno = 0;
   mkdir("/", 0777);
-#ifdef WASMFS
   assert(errno == EEXIST);
-#else
-  assert(errno == EINVAL);
-#endif
 
   // Try to make a directory that exists already.
   errno = 0;