Rewrote realpath implementation solving all known failing tests (also added a bunch of new test cases)

lib/fs.js

@@ -292,86 +292,132 @@ exports.unwatchFile = function (filename) {
 // Realpath
 
 var path = require('path');
-var dirname = path.dirname,
-    basename = path.basename,
-    normalize = path.normalize;
-
-function readlinkDeepSync(path, stats) {
-  var seen_links = {}, resolved_link, stats, file_id;
-  while (true) {
-    stats = stats || exports.lstatSync(path);
-    file_id = stats.dev.toString(32)+":"+stats.ino.toString(32);
-    if (file_id in seen_links) {
-      throw new Error("cyclic symbolic link at "+path);
+var normalize = path.normalize
+    normalizeArray = path.normalizeArray;
+
+exports.realpathSync = function (path) {
+  var seen_links = {}, knownHards = {}, buf, i = 0, part, x, stats;
+  if (path.charAt(0) !== '/') {
+    var cwd = process.cwd().split('/');
+    path = cwd.concat(path.split('/'));
+    path = normalizeArray(path);
+    i = cwd.length;
+    buf = [].concat(cwd);
+  } else {
+    path = normalizeArray(path.split('/'));
+    buf = [''];
+  }
+  for (; i<path.length; i++) {
+    part = path.slice(0, i+1).join('/');
+    if (part.length !== 0) {
+      if (part in knownHards) {
+        buf.push(path[i]);
       } else {
-      seen_links[file_id] = 1;
+        stats = exports.lstatSync(part);
         if (stats.isSymbolicLink()) {
-        var newpath = exports.readlinkSync(path);
-        if (newpath.charAt(0) === '/') {
-          path = newpath;
+          x = stats.dev.toString(32)+":"+stats.ino.toString(32);
+          if (x in seen_links)
+            throw new Error("cyclic link at "+part);
+          seen_links[x] = true;
+          part = exports.readlinkSync(part);
+          if (part.charAt(0) === '/') {
+            // absolute
+            path = normalizeArray(part.split('/'));
+            buf = [''];
+            i = 0;
+          } else {
+            // relative
+            Array.prototype.splice.apply(path, [i, 1].concat(part.split('/')));
+            part = normalizeArray(path);
+            var y = 0, L = Math.max(path.length, part.length), delta;
+            for (; y<L && path[y] === part[y]; y++);
+            if (y !== L) {
+              path = part;
+              delta = i-y;
+              i = y-1;
+              if (delta > 0) buf.splice(y, delta);
             } else {
-          var dir = dirname(path);
-          path = (dir !== '') ? dir + '/' + newpath : newpath;
+              i--;
+            }
           }
         } else {
-        return normalize(path);
+          buf.push(path[i]);
+          knownHards[buf.join('/')] = true;
         }
       }
-    stats = null;
     }
   }
+  return buf.join('/');
+}
+
 
-function readlinkDeep(path, stats, callback) {
-  var seen_links = {}, resolved_link, file_id;
-  function next(stats) {
-    file_id = stats.dev.toString(32)+":"+stats.ino.toString(32);
-    if (file_id in seen_links) {
-      callback(new Error("cyclic symbolic link at "+path));
+exports.realpath = function (path, callback) {
+  var seen_links = {}, knownHards = {}, buf = [''], i = 0, part, x;
+  if (path.charAt(0) !== '/') {
+    // assumes cwd is canonical
+    var cwd = process.cwd().split('/');
+    path = cwd.concat(path.split('/'));
+    path = normalizeArray(path);
+    i = cwd.length-1;
+    buf = [].concat(cwd);
   } else {
-      seen_links[file_id] = 1;
+    path = normalizeArray(path.split('/'));
+  }
+  function done(err) {
+    if (callback) {
+      if (!err) callback(err, buf.join('/'));
+      else callback(err);
+    }
+  }
+  function next() {
+    if (++i === path.length) return done();
+    part = path.slice(0, i+1).join('/');
+    if (part.length === 0) return next();
+    if (part in knownHards) {
+      buf.push(path[i]);
+      next();
+    } else {
+      exports.lstat(part, function(err, stats){
+        if (err) return done(err);
         if (stats.isSymbolicLink()) {
-        exports.readlink(path, function(err, newpath) {
-          if (err) callback(err);
-          if (newpath.charAt(0) === '/') {
-            path = newpath;
+          x = stats.dev.toString(32)+":"+stats.ino.toString(32);
+          if (x in seen_links)
+            return done(new Error("cyclic link at "+part));
+          seen_links[x] = true;
+          exports.readlink(part, function(err, npart){
+            if (err) return done(err);
+            part = npart;
+            if (part.charAt(0) === '/') {
+              // absolute
+              path = normalizeArray(part.split('/'));
+              buf = [''];
+              i = 0;
             } else {
-            var dir = dirname(path);
-            path = (dir !== '') ? dir + '/' + newpath : newpath;
+              // relative
+              Array.prototype.splice.apply(path, [i, 1].concat(part.split('/')));
+              part = normalizeArray(path);
+              var y = 0, L = Math.max(path.length, part.length), delta;
+              for (; y<L && path[y] === part[y]; y++);
+              if (y !== L) {
+                path = part;
+                delta = i-y;
+                i = y-1; // resolve new node if needed
+                if (delta > 0) buf.splice(y, delta);
               }
-          _next();
-        });
-      } else {
-        callback(null, normalize(path));
+              else {
+                i--; // resolve new node if needed
               }
             }
+            next();
+          }); // fs.readlink
         }
-  function _next() {
-    exports.lstat(path, function(err, stats){
-      if (err) callback(err);
-      else next(stats);
-    });
+        else {
+          buf.push(path[i]);
+          knownHards[buf.join('/')] = true;
+          next();
         }
-  if (stats) next(stats);
-  else _next();
+      }); // fs.lstat
     }
-
-exports.realpathSync = function(path) {
-  var stats = exports.lstatSync(path);
-  if (stats.isSymbolicLink())
-    return readlinkDeepSync(path, stats);
-  else
-    return normalize(path);
   }
-
-exports.realpath = function(path, callback) {
-  var resolved_path = path;
-  if (!callback) return;
-  exports.lstat(path, function(err, stats){
-    if (err)
-      callback(err);
-    else if (stats.isSymbolicLink())
-      readlinkDeep(path, stats, callback);
-    else
-      callback(null, normalize(path));
-  });
+  next();
 }

test/simple/test-fs-realpath.js

@@ -1,56 +1,251 @@
 process.mixin(require("../common"));
 
-var async_completed = 0, async_expected = 0;
+var async_completed = 0, async_expected = 0, unlink = [];
 
-// a. deep relative file symlink
-var dstPath = path.join(fixturesDir, 'cycles', 'root.js');
+function asynctest(testBlock, args, callback, assertBlock) {
+  async_expected++;
+  testBlock.apply(testBlock, args.concat([function(err){
+    var ignoreError = false;
+    if (assertBlock) {
+      try {
+        ignoreError = assertBlock.apply(assertBlock, 
+          Array.prototype.slice.call(arguments));
+      }
+      catch (e) {
+        err = e;
+      }
+    }
+    async_completed++;
+    callback(ignoreError ? null : err);
+  }]));
+}
+
+function bashRealpath(path, callback) {
+  exec("cd '"+path.replace("'","\\'")+"' && pwd -P",function (err, o) {
+    callback(err, o.trim());
+  });
+}
+
+// sub-tests:
+
+function test_simple_relative_symlink(callback) {
+  var entry = fixturesDir+'/cycles/symlink',
+      expected = fixturesDir+'/cycles/root.js';
+  [
+    [entry, 'root.js'],
+  ].forEach(function(t) {
+    try {fs.unlinkSync(t[0]);}catch(e){}
+    fs.symlinkSync(t[1], t[0]);
+    unlink.push(t[0]);
+  });
+  var result = fs.realpathSync(entry);
+  assert.equal(result, expected,
+      'got '+inspect(result)+' expected '+inspect(expected));
+  asynctest(fs.realpath, [entry], callback, function(err, result){
+    assert.equal(result, expected,
+      'got '+inspect(result)+' expected '+inspect(expected));
+  });
+}
+
+function test_simple_absolute_symlink(callback) {
+  bashRealpath(fixturesDir, function(err, fixturesAbsDir) {
+    if (err) return callback(err);
+    var entry = fixturesAbsDir+'/cycles/symlink',
+        expected = fixturesAbsDir+'/nested-index/one/index.js';
+    [
+      [entry, expected],
+    ].forEach(function(t) {
+      try {fs.unlinkSync(t[0]);}catch(e){}
+      fs.symlinkSync(t[1], t[0]);
+      unlink.push(t[0]);
+    });
+    var result = fs.realpathSync(entry);
+    assert.equal(result, expected,
+        'got '+inspect(result)+' expected '+inspect(expected));
+    asynctest(fs.realpath, [entry], callback, function(err, result){
+      assert.equal(result, expected,
+        'got '+inspect(result)+' expected '+inspect(expected));
+    });
+  });
+}
+
+function test_deep_relative_file_symlink(callback) {
+  var expected = path.join(fixturesDir, 'cycles', 'root.js');
   var linkData1 = "../../cycles/root.js";
   var linkPath1 = path.join(fixturesDir, "nested-index", 'one', 'symlink1.js');
   try {fs.unlinkSync(linkPath1);}catch(e){}
   fs.symlinkSync(linkData1, linkPath1);
 
   var linkData2 = "../one/symlink1.js";
-var linkPath2 = path.join(fixturesDir, "nested-index", 'two', 'symlink1-b.js');
-try {fs.unlinkSync(linkPath2);}catch(e){}
-fs.symlinkSync(linkData2, linkPath2);
+  var entry = path.join(fixturesDir, "nested-index", 'two', 'symlink1-b.js');
+  try {fs.unlinkSync(entry);}catch(e){}
+  fs.symlinkSync(linkData2, entry);
+  unlink.push(linkPath1);
+  unlink.push(entry);
 
-// b. deep relative directory symlink
-var dstPath_b = path.join(fixturesDir, 'cycles', 'folder');
+  assert.equal(fs.realpathSync(entry), expected);
+  asynctest(fs.realpath, [entry], callback, function(err, result){
+    assert.equal(result, expected,
+      'got '+inspect(result)+' expected '+inspect(expected));
+  });
+}
+
+function test_deep_relative_dir_symlink(callback) {
+  var expected = path.join(fixturesDir, 'cycles', 'folder');
   var linkData1b = "../../cycles/folder";
   var linkPath1b = path.join(fixturesDir, "nested-index", 'one', 'symlink1-dir');
   try {fs.unlinkSync(linkPath1b);}catch(e){}
   fs.symlinkSync(linkData1b, linkPath1b);
 
   var linkData2b = "../one/symlink1-dir";
-var linkPath2b = path.join(fixturesDir, "nested-index", 'two', 'symlink12-dir');
-try {fs.unlinkSync(linkPath2b);}catch(e){}
-fs.symlinkSync(linkData2b, linkPath2b);
+  var entry = path.join(fixturesDir, "nested-index", 'two', 'symlink12-dir');
+  try {fs.unlinkSync(entry);}catch(e){}
+  fs.symlinkSync(linkData2b, entry);
+  unlink.push(linkPath1b);
+  unlink.push(entry);
 
-assert.equal(fs.realpathSync(linkPath2), dstPath);
-assert.equal(fs.realpathSync(linkPath2b), dstPath_b);
+  assert.equal(fs.realpathSync(entry), expected);
   
-async_expected++;
-fs.realpath(linkPath2, function(err, rpath) {
-  if (err) throw err;
-  assert.equal(rpath, dstPath);
-  async_completed++;
+  asynctest(fs.realpath, [entry], callback, function(err, result){
+    assert.equal(result, expected,
+      'got '+inspect(result)+' expected '+inspect(expected));
   });
+}
 
-async_expected++;
-fs.realpath(linkPath2b, function(err, rpath) {
-  if (err) throw err;
-  assert.equal(rpath, dstPath_b);
-  async_completed++;
+function test_cyclic_link_protection(callback) {
+  var entry = fixturesDir+'/cycles/realpath-3a';
+  [
+    [entry, '../cycles/realpath-3b'],
+    [fixturesDir+'/cycles/realpath-3b', '../cycles/realpath-3c'],
+    [fixturesDir+'/cycles/realpath-3c', '../cycles/realpath-3a'],
+  ].forEach(function(t) {
+    try {fs.unlinkSync(t[0]);}catch(e){}
+    fs.symlinkSync(t[1], t[0]);
+    unlink.push(t[0]);
+  });
+  assert.throws(function(){ fs.realpathSync(entry); });
+  asynctest(fs.realpath, [entry], callback, function(err, result){
+    assert.ok(err && true);
+    return true;
+  });
+}
+
+function test_relative_input_cwd(callback) {
+  var p = fixturesDir.lastIndexOf('/');
+  var entrydir = fixturesDir.substr(0, p);
+  var entry = fixturesDir.substr(p+1)+'/cycles/realpath-3a';
+  var expected = fixturesDir+'/cycles/root.js';
+  [
+    [entry, '../cycles/realpath-3b'],
+    [fixturesDir+'/cycles/realpath-3b', '../cycles/realpath-3c'],
+    [fixturesDir+'/cycles/realpath-3c', 'root.js'],
+  ].forEach(function(t) {
+    var fn = t[0];
+    if (fn.charAt(0) !== '/') fn = entrydir + '/' + fn;
+    try {fs.unlinkSync(fn);}catch(e){}
+    fs.symlinkSync(t[1], fn);
+    unlink.push(fn);
+  });
+  var origcwd = process.cwd();
+  process.chdir(entrydir);
+  assert.equal(fs.realpathSync(entry), expected);
+  asynctest(fs.realpath, [entry], callback, function(err, result){
+    process.chdir(origcwd);
+    assert.equal(result, expected,
+      'got '+inspect(result)+' expected '+inspect(expected));
+    return true;
+  });
+}
+
+function test_deep_symlink_mix(callback) {
+  // todo: check to see that fixturesDir is not rooted in the
+  //       same directory as our test symlink.
+  // obtain our current realpath using bash (so we can test ourselves)
+  bashRealpath(fixturesDir, function(err, fixturesAbsDir) {
+    if (err) return callback(err);
+    /*
+    /tmp/node-test-realpath-f1 -> ../tmp/node-test-realpath-d1/foo
+    /tmp/node-test-realpath-d1 -> ../node-test-realpath-d2
+    /tmp/node-test-realpath-d2/foo -> ../node-test-realpath-f2
+    /tmp/node-test-realpath-f2 
+      -> /node/test/fixtures/nested-index/one/realpath-c
+    /node/test/fixtures/nested-index/one/realpath-c 
+      -> /node/test/fixtures/nested-index/two/realpath-c
+    /node/test/fixtures/nested-index/two/realpath-c -> ../../cycles/root.js
+    /node/test/fixtures/cycles/root.js (hard)
+    */
+    var entry = '/tmp/node-test-realpath-f1';
+    try {fs.unlinkSync('/tmp/node-test-realpath-d2/foo');}catch(e){}
+    try {fs.rmdirSync('/tmp/node-test-realpath-d2');}catch(e){}
+    fs.mkdirSync('/tmp/node-test-realpath-d2', 0700);
+    try {
+      [
+        [entry, '../tmp/node-test-realpath-d1/foo'],
+        ['/tmp/node-test-realpath-d1', '../tmp/node-test-realpath-d2'],
+        ['/tmp/node-test-realpath-d2/foo', '../node-test-realpath-f2'],
+        ['/tmp/node-test-realpath-f2', fixturesAbsDir+'/nested-index/one/realpath-c'],
+        [fixturesAbsDir+'/nested-index/one/realpath-c', fixturesAbsDir+'/nested-index/two/realpath-c'],
+        [fixturesAbsDir+'/nested-index/two/realpath-c', '../../cycles/root.js'],
+      ].forEach(function(t) {
+        //debug('setting up '+t[0]+' -> '+t[1]);
+        try {fs.unlinkSync(t[0]);}catch(e){}
+        fs.symlinkSync(t[1], t[0]);
+        unlink.push(t[0]);
       });
+    } finally {
+      unlink.push('/tmp/node-test-realpath-d2');
+    }
+    var expected = fixturesAbsDir+'/cycles/root.js';
+    assert.equal(fs.realpathSync(entry), expected);
+    asynctest(fs.realpath, [entry], callback, function(err, result){
+      assert.equal(result, expected,
+        'got '+inspect(result)+' expected '+inspect(expected));
+      return true;
+    });
+  });
+}
+
+function test_non_symlinks(callback) {
+  bashRealpath(fixturesDir, function(err, fixturesAbsDir) {
+    if (err) return callback(err);
+    var p = fixturesAbsDir.lastIndexOf('/');
+    var entrydir = fixturesAbsDir.substr(0, p);
+    var entry = fixturesAbsDir.substr(p+1)+'/cycles/root.js';
+    var expected = fixturesAbsDir+'/cycles/root.js';
+    var origcwd = process.cwd();
+    process.chdir(entrydir);
+    assert.equal(fs.realpathSync(entry), expected);
+    asynctest(fs.realpath, [entry], callback, function(err, result){
+      process.chdir(origcwd);
+      assert.equal(result, expected,
+        'got '+inspect(result)+' expected '+inspect(expected));
+      return true;
+    });
+  });
+}
 
-// todo: test shallow symlinks (file & dir)
-// todo: test non-symlinks (file & dir)
-// todo: test error on cyclic symlinks
+// ----------------------------------------------------------------------------
+
+var tests = [
+  test_simple_relative_symlink,
+  test_simple_absolute_symlink,
+  test_deep_relative_file_symlink,
+  test_deep_relative_dir_symlink,
+  test_cyclic_link_protection,
+  test_relative_input_cwd,
+  test_deep_symlink_mix,
+  test_non_symlinks,
+];
+var numtests = tests.length;
+function runNextTest(err) {
+  if (err) throw err;
+  var test = tests.shift()
+  if (!test) puts(numtests+' subtests completed OK for fs.realpath');
+  else test(runNextTest);
+}
+runNextTest();
 
 process.addListener("exit", function () {
-  try {fs.unlinkSync(linkPath1);}catch(e){}
-  try {fs.unlinkSync(linkPath2);}catch(e){}
-  try {fs.unlinkSync(linkPath1b);}catch(e){}
-  try {fs.unlinkSync(linkPath2b);}catch(e){}
+  unlink.forEach(function(path){ try {fs.unlinkSync(path);}catch(e){} });
   assert.equal(async_completed, async_expected);
 });