[nodefs] Don't try to resolve absolute symlinks outside of the VFS (#21805)

This PR reverts the behaviour that was added in #3277 and suggested in #3222.

Author: mho22

ChangeLog.md

@@ -18,6 +18,11 @@ to browse the changes between the tags.
 
 See docs/process.md for more on how version tagging works.
 
+3.1.61 - 05/08/24
+-----------------
+- Under nodefs, symbolic links to files outside of mount locations no longer work.
+  This reverts the previous behaviour added in #3277. (#21805)
+
 3.1.60 (in development)
 -----------------------
 - The `EXPORTED_FUNCTIONS` list can now include JS library symbols even if they

src/library_nodefs.js

@@ -234,9 +234,7 @@ addToLibrary({
       readlink(node) {
         var path = NODEFS.realPath(node);
         try {
-          path = fs.readlinkSync(path);
-          path = nodePath.relative(nodePath.resolve(node.mount.opts.root), path);
-          return path;
+          return fs.readlinkSync(path);
         } catch (e) {
           if (!e.code) throw e;
           // node under windows can return code 'UNKNOWN' here:

test/unistd/links.c

@@ -17,39 +17,70 @@
 #include <emscripten.h>
 #endif
 
-void setup() {
-  int rtn = mkdir("working", 0777);
-  assert(rtn == 0);
-#if defined(__EMSCRIPTEN__) && defined(NODEFS)
-  EM_ASM(FS.mount(NODEFS, { root: '.' }, 'working'));
-#endif
-  rtn = chdir("working");
+void makedir(const char *dir) {
+  int rtn = mkdir(dir, 0777);
   assert(rtn == 0);
-  symlink("../test/../there!", "link");
-  int fd = open("file", O_RDWR | O_CREAT, 0777);
+}
+
+void makefile(const char *file, const char *content) {
+  int fd = open(file, O_RDWR | O_CREAT, 0777);
   assert(fd >= 0);
-  rtn = write(fd, "test", 5);
-  assert(rtn == 5);
+  int rtn = write(fd, content, strlen(content));
+  assert(rtn == strlen(content));
   close(fd);
-  rtn = mkdir("folder", 0777);
+}
+
+void makelink(const char *link, const char *path) {
+  int rtn = symlink(link, path);
   assert(rtn == 0);
 }
 
-int main() {
-  setup();
+void changedir(const char *dir) {
+  int rtn = chdir(dir);
+  assert(rtn == 0);
+}
 
-  char* files[] = {"link", "file", "folder"};
-  char buffer[256] = {0};
-  int ret;
+void setup() {
+  makedir("working");
+#if defined(__EMSCRIPTEN__) && defined(NODEFS)
+  EM_ASM(FS.mount(NODEFS, { root: '.' }, 'working'));
+#endif
+  changedir("working");
+  makelink("../test/../there!", "link");
+  makefile("file", "test");
+  makedir("directory");
+  makedir("directory/subdirectory");
+  makefile("directory/subdirectory/file", "Subdirectory");
+
+  makedir("relative");
+  makedir("relative/subrelative");
+  makefile("relative/file", "Relative");
+  makefile("relative/subrelative/file", "Subrelative");
+  makelink("../relative/file", "directory/relative");
+  makelink("../../relative/subrelative/file", "directory/subdirectory/subrelative");
+  makelink("directory/subdirectory/file", "subdirectoryrelative");
+
+  makedir("absolute");
+  makedir("absolute/subabsolute");
+  makefile("absolute/file", "Absolute");
+  makefile("absolute/subabsolute/file", "Subabsolute");
+  makelink("/working/absolute/file", "/working/directory/absolute");
+  makelink("/working/absolute/subabsolute/file", "/working/directory/subdirectory/subabsolute");
+  makelink("/working/directory/subdirectory/file", "/working/subdirectoryabsolute");
+}
+
+void test_reading_existing_symlinks() {
+  char* files[] = {"link", "file", "directory"};
 
   for (int i = 0; i < sizeof files / sizeof files[0]; i++) {
-    printf("readlink(%s)\n", files[i]);
-    ret = readlink(files[i], buffer, 256);
-    printf("errno: %s\n", strerror(errno));
-    if (ret < 0) {
-      printf("not a link\n\n");
+    char buffer[256] = {0};
+    int rtn = readlink(files[i], buffer, 256);
+    printf("readlink: '%s'\n", files[i]);
+    printf("errno: %s\n\n", strerror(errno));
+    if (rtn < 0) {
       continue;
     }
+
     // WASMFS behaves the same as Linux (and as best as I can tell, the spec),
     // seeing the symlink as a string. The old JS FS instead normalizes it and
     // returns something modified.
@@ -59,56 +90,125 @@ int main() {
 #else
     assert(strcmp(buffer, "/there!") == 0);
 #endif
-    assert(strlen(buffer) == ret);
+    assert(strlen(buffer) == rtn);
     errno = 0;
-    printf("\n");
   }
+}
 
-  printf("symlink/overwrite\n");
-  ret = symlink("new-nonexistent-path", "link");
-  assert(ret == -1);
+void test_overwriting_symlink() {
+  int rtn = symlink("new-nonexistent-path", "link");
+  assert(rtn == -1);
   assert(errno == EEXIST);
   errno = 0;
+}
 
-  printf("\nsymlink/normal\n");
-  ret = symlink("new-nonexistent-path", "folder/link");
-  assert(ret == 0);
+void test_creating_symlink() {
+  int rtn = symlink("new-nonexistent-path", "directory/link");
+  assert(rtn == 0);
   assert(errno == 0);
   errno = 0;
 
-  printf("\nreadlink(created link)\n");
-  ret = readlink("folder/link", buffer, 256);
+  char buffer[256] = {0};
+  rtn = readlink("directory/link", buffer, 256);
   assert(errno == 0);
 #if !defined(__EMSCRIPTEN__) || defined(WASMFS)
   assert(strcmp(buffer, "new-nonexistent-path") == 0);
 #else
-  assert(strcmp(buffer, "/working/folder/new-nonexistent-path") == 0);
+  assert(strcmp(buffer, "/working/directory/new-nonexistent-path") == 0);
 #endif
-  assert(strlen(buffer) == ret);
+  assert(strlen(buffer) == rtn);
   errno = 0;
+}
 
+void test_reading_shortened_symlink() {
+  char buffer[256] = {0};
+  readlink("directory/link", buffer, 256);
   buffer[0] = buffer[1] = buffer[2] = buffer[3] = buffer[4] = buffer[5] = '*';
-  printf("\nreadlink(short buffer)\n");
-  ret = readlink("link", buffer, 4);
+  int rtn = readlink("link", buffer, 4);
   assert(errno == 0);
-  assert(ret == 4);
+  assert(rtn == 4);
 #if !defined(__EMSCRIPTEN__) || defined(WASMFS)
   assert(strcmp(buffer, "../t**nexistent-path") == 0);
 #else
-  assert(strcmp(buffer, "/the**ng/folder/new-nonexistent-path") == 0);
+  assert(strcmp(buffer, "/the**ng/directory/new-nonexistent-path") == 0);
 #endif
   errno = 0;
+}
 
+void test_noticing_loop_in_symlink() {
   // FS.lookupPath should notice the symlink loop and return ELOOP, not go into
   // an infinite recurse.
   //
   // This test doesn't work in wasmfs -- probably because access sees the
   // symlink and returns true without bothering to chase the symlink
-  symlink("./linkX/inside", "./linkX");
-  ret = access("linkX", F_OK);
-  assert(ret == -1);
+  symlink("./loop-link/inside", "./loop-link");
+  int rtn = access("loop-link", F_OK);
+  assert(rtn == -1);
   assert(errno == ELOOP);
   errno = 0;
+}
+
+
+void test_relative_path_symlinks() {
+  char* parents[] = {
+    "/working/directory/",
+    "/working/directory/subdirectory/",
+    "/working/"
+  };
 
+  char* links[] = {
+    "relative",
+    "subrelative",
+    "subdirectoryrelative",
+  };
+
+  for (int i = 0; i < sizeof links / sizeof links[0]; i++) {
+    int rtn = chdir(parents[i]);
+    assert(rtn == 0);
+    char symlink[256] = {0};
+    strcat(strcpy(symlink, parents[i]), links[i]);
+    printf("symlink: '%s'\n", symlink);
+    char buf[256] = {0};
+    rtn = readlink(links[i], buf, 256);
+    FILE *fd = fopen(buf, "r");
+    assert(fd);
+    char buffer[13] = {0};
+    rtn = fread(buffer, 1, 13, fd);
+    assert(rtn <= 13);
+    printf("buffer: '%s'\n\n", buffer);
+    fclose(fd);
+  }
+}
+
+void test_absolute_path_symlinks() {
+  char* links[] = {
+    "/working/directory/absolute",
+    "/working/directory/subdirectory/subabsolute",
+    "/working/subdirectoryabsolute"
+  };
+
+  for (int i = 0; i < sizeof links / sizeof links[0]; i++) {
+    printf("symlink: '%s'\n", links[i]);
+    char buf[256] = {0};
+    readlink(links[i], buf, 256);
+    FILE *fd = fopen(buf, "r");
+    assert(fd);
+    char buffer[13] = {0};
+    int rtn = fread(buffer, 1, 13, fd);
+    assert(rtn <= 13);
+    printf("buffer: '%s'\n\n", buffer);
+    fclose(fd);
+  }
+}
+
+int main() {
+  setup();
+  test_reading_existing_symlinks();
+  test_overwriting_symlink();
+  test_creating_symlink();
+  test_reading_shortened_symlink();
+  test_noticing_loop_in_symlink();
+  test_relative_path_symlinks();
+  test_absolute_path_symlinks();
   return 0;
 }

test/unistd/links.out

@@ -1,18 +1,26 @@
-readlink(link)
+readlink: 'link'
 errno: No error information
 
-readlink(file)
+readlink: 'file'
 errno: Invalid argument
-not a link
 
-readlink(folder)
+readlink: 'directory'
 errno: Invalid argument
-not a link
 
-symlink/overwrite
+symlink: '/working/directory/relative'
+buffer: 'Relative'
 
-symlink/normal
+symlink: '/working/directory/subdirectory/subrelative'
+buffer: 'Subrelative'
 
-readlink(created link)
+symlink: '/working/subdirectoryrelative'
+buffer: 'Subdirectory'
 
-readlink(short buffer)
+symlink: '/working/directory/absolute'
+buffer: 'Absolute'
+
+symlink: '/working/directory/subdirectory/subabsolute'
+buffer: 'Subabsolute'
+
+symlink: '/working/subdirectoryabsolute'
+buffer: 'Subdirectory'

test/unistd/symlink_on_nodefs.c

@@ -17,37 +17,60 @@
 
 void setup() {
   EM_ASM(
-    fs.mkdirSync('new-directory', '0777');
-    fs.writeFileSync('new-directory/test', 'Link it');
-    fs.symlinkSync(fs.realpathSync('new-directory'), 'symlink');
+    fs.mkdirSync('directory', '0777');
+    fs.writeFileSync('directory/test', 'Link it');
+    fs.symlinkSync('/working/directory', 'inside-symlink');
+    fs.symlinkSync(fs.realpathSync('directory'), 'outside-symlink');
 
     FS.mkdir('working');
     FS.mount(NODEFS, { root: '.' }, 'working');
 
-    FS.mkdir('direct-link');
-    FS.mount(NODEFS, { root: 'symlink' }, 'direct-link');
+    FS.mkdir('mount-link');
+    FS.mount(NODEFS, { root: 'inside-symlink' }, 'mount-link');
   );
 }
 
-int main() {
-  setup();
-
-  char buf[1024];
-  readlink("/working/symlink", buf, 1024);
-  printf("readlink: %s\n", buf);
-
-  FILE* fd = fopen("/working/symlink/test", "r");
+void test_inside_symlink()
+{
+  char buf[256] = {0};
+  readlink("/working/inside-symlink", buf, 256);
+  printf("readlink: '%s'\n", buf);
+  FILE* fd = fopen("/working/inside-symlink/test", "r");
   assert(fd);
   char buffer[8] = {0};
   int rtn = fread(buffer, 1, 7, fd);
   assert(rtn == 7);
-  printf("buffer is '%s'\n", buffer);
+  printf("buffer: '%s'\n", buffer);
   fclose(fd);
+}
 
-  // This should fail, since it resolves to ../new-directory which is not
-  // mounted
-  fd = fopen("/direct-link/test", "r");
+void test_outside_symlink()
+{
+  // outside-symlink is link to an absolute path which is not part of the emscripten VFS
+  // and so we should be able to open it.
+  FILE* fd = fopen("/working/outside-symlink/test", "r");
   assert(fd == NULL);
+  assert(errno == ENOENT);
+}
 
+void test_mount_link()
+{
+  char buf[256] = {0};
+  readlink("/mount-link", buf, 256);
+  printf("\nreadlink: '%s'\n", buf);
+  FILE* fd = fopen("/mount-link/test", "r");
+  assert(fd);
+  char buffer[8] = {0};
+  int rtn = fread(buffer, 1, 7, fd);
+  assert(rtn == 7);
+  printf("buffer: '%s'\n", buffer);
+  fclose(fd);
+}
+
+int main() {
+  setup();
+  test_inside_symlink();
+  test_outside_symlink();
+  test_mount_link();
   return 0;
 }

test/unistd/symlink_on_nodefs.out

@@ -1,2 +1,5 @@
-readlink: /working/new-directory
-buffer is 'Link it'
+readlink: '/working/directory'
+buffer: 'Link it'
+
+readlink: '/working/directory'
+buffer: 'Link it'