NODEFS readlink should take current mountpoint into account

Author: janjongboom

AUTHORS

@@ -171,4 +171,5 @@ a license to everyone to use it as detailed in LICENSE.)
 * Petr Babicka <[email protected]>
 * Akira Takahashi <[email protected]>
 * Victor Costan <[email protected]>
+* Jan Jongboom <[email protected]> (copyright owned by Telenor Digital AS)
 

src/library_fs.js

@@ -97,7 +97,7 @@ mergeInto(LibraryManager.library, {
           while (FS.isLink(current.mode)) {
             var link = FS.readlink(current_path);
             current_path = PATH.resolve(PATH.dirname(current_path), link);
-            
+
             var lookup = FS.lookupPath(current_path, { recurse_count: opts.recurse_count });
             current = lookup.node;
 
@@ -427,7 +427,7 @@ mergeInto(LibraryManager.library, {
     // we employ a simple trick: the pointer to a stream is its fd plus 1.  This
     // means that all valid streams have a valid non-zero pointer while allowing
     // the fs for stdin to be the standard value of zero.
-    // 
+    //
     //
     getStreamFromPtr: function(ptr) {
       return FS.streams[ptr - 1];
@@ -838,7 +838,7 @@ mergeInto(LibraryManager.library, {
       if (!link.node_ops.readlink) {
         throw new FS.ErrnoError(ERRNO_CODES.EINVAL);
       }
-      return link.node_ops.readlink(link);
+      return PATH.resolve('.', link.node_ops.readlink(link));
     },
     stat: function(path, dontFollow) {
       var lookup = FS.lookupPath(path, { follow: !dontFollow });

src/library_nodefs.js

@@ -1,6 +1,6 @@
 mergeInto(LibraryManager.library, {
   $NODEFS__deps: ['$FS', '$PATH'],
-  $NODEFS__postset: 'if (ENVIRONMENT_IS_NODE) { var fs = require("fs"); NODEFS.staticInit(); }',
+  $NODEFS__postset: 'if (ENVIRONMENT_IS_NODE) { var fs = require("fs"); var NODEJS_PATH = require("path"); NODEFS.staticInit(); }',
   $NODEFS: {
     isWindows: false,
     staticInit: function() {
@@ -24,7 +24,7 @@ mergeInto(LibraryManager.library, {
       try {
         stat = fs.lstatSync(path);
         if (NODEFS.isWindows) {
-          // On Windows, directories return permission bits 'rw-rw-rw-', even though they have 'rwxrwxrwx', so 
+          // On Windows, directories return permission bits 'rw-rw-rw-', even though they have 'rwxrwxrwx', so
           // propagate write bits to execute bits.
           stat.mode = stat.mode | ((stat.mode & 146) >> 1);
         }
@@ -203,7 +203,9 @@ mergeInto(LibraryManager.library, {
       readlink: function(node) {
         var path = NODEFS.realPath(node);
         try {
-          return fs.readlinkSync(path);
+          path = fs.readlinkSync(path);
+          path = NODEJS_PATH.relative(NODEJS_PATH.resolve(node.mount.opts.root), path);
+          return PATH.join(node.mount.mountpoint, path);
         } catch (e) {
           if (!e.code) throw e;
           throw new FS.ErrnoError(ERRNO_CODES[e.code]);

tests/test_core.py

@@ -4709,6 +4709,22 @@ def test_unistd_links(self):
       Building.COMPILER_TEST_OPTS = orig_compiler_opts + ['-D' + fs]
       self.do_run(src, expected, js_engines=[NODE_JS])
 
+  def test_unistd_symlink_on_nodefs(self):
+    self.clear()
+    if not self.is_emscripten_abi(): return self.skip('asmjs-unknown-emscripten needed for inline js')
+    orig_compiler_opts = Building.COMPILER_TEST_OPTS[:]
+    for fs in ['NODEFS']:
+      if WINDOWS and fs == 'NODEFS':
+        print >> sys.stderr, 'Skipping NODEFS part of this test for test_unistd_symlink_on_nodefs on Windows, since it would require administrative privileges.'
+        # Also, other detected discrepancies if you do end up running this test on NODEFS:
+        # test expects /, but Windows gives \ as path slashes.
+        # Calling readlink() on a non-link gives error 22 EINVAL on Unix, but simply error 0 OK on Windows.
+        continue
+      src = open(path_from_root('tests', 'unistd', 'symlink_on_nodefs.c'), 'r').read()
+      expected = open(path_from_root('tests', 'unistd', 'symlink_on_nodefs.out'), 'r').read()
+      Building.COMPILER_TEST_OPTS = orig_compiler_opts + ['-D' + fs]
+      self.do_run(src, expected, js_engines=[NODE_JS])
+
   def test_unistd_sleep(self):
     src = open(path_from_root('tests', 'unistd', 'sleep.c'), 'r').read()
     expected = open(path_from_root('tests', 'unistd', 'sleep.out'), 'r').read()

tests/unistd/links.out

@@ -1,17 +1,17 @@
 readlink(link)
-ret: 17
+ret: 7
 errno: 0
-result: ../test/../there!
+result: /there!
 
 readlink(file)
 ret: -1
 errno: 22
-result: ../test/../there!
+result: /there!
 
 readlink(folder)
 ret: -1
 errno: 22
-result: ../test/../there!
+result: /there!
 
 symlink/overwrite
 ret: -1
@@ -21,11 +21,11 @@ symlink/normal
 ret: 0
 errno: 0
 readlink(created link)
-ret: 20
+ret: 29
 errno: 0
-result: new-nonexistent-path
+result: /working/new-nonexistent-path
 
 readlink(short buffer)
 ret: 3
 errno: 0
-result: ../-nonexistent-path
+result: /thrking/new-nonexistent-path

tests/unistd/symlink_on_nodefs.c

@@ -0,0 +1,57 @@
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <emscripten.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <limits.h>
+#include <stdlib.h>
+
+int main() {
+  EM_ASM(
+    fs.mkdirSync('./new-directory', '0777');
+    fs.writeFileSync('./new-directory/test', 'Link it');
+    fs.symlinkSync(fs.realpathSync('./new-directory'), './symlink');
+
+    FS.mkdir('working');
+    FS.mount(NODEFS, { root: '.' }, 'working');
+
+    FS.mkdir('direct-link');
+    FS.mount(NODEFS, { root: './symlink' }, 'direct-link');
+  );
+
+  {
+    char* path = "/working/symlink/test";
+    printf("reading %s\n", path);
+
+    FILE* fd = fopen(path, "r");
+    if (fd == NULL) {
+      printf("failed to open file %s\n", path);
+    }
+    else {
+      char buffer[8];
+      fread(buffer, 1, 7, fd);
+      printf("buffer is %s\n", buffer);
+      fclose(fd);
+    }
+  }
+
+  printf("\n");
+
+  {
+    char* path = "/direct-link/test";
+    printf("reading %s\n", path);
+
+    FILE* fd = fopen(path, "r");
+    if (fd != NULL) {
+      // This should not happen, it resolves to ../new-directory which is not mounted
+      printf("opened file %s\n", path);
+      fclose(fd);
+    }
+    else {
+      printf("failed to open file %s\n", path);
+    }
+  }
+  
+  return 0;
+}

tests/unistd/symlink_on_nodefs.out

@@ -0,0 +1,5 @@
+reading /working/symlink/test
+buffer is Link it
+
+reading /direct-link/test
+failed to open file /direct-link/test