Handle cyclic links smarter in fs.realpath

isaacs · ry · commit 987cbbc191c0 · 2010-10-27T17:02:42.000-07:00
Rather than aborting in the face of *any* repeated link in a given path,
instead only abort if such a cycle actually makes a given path unresolvable.

Test for this by doing a normal stat.  Still use the seenLinks object to
cache link contents so as to cut own a little bit on readlink calls.

Also add a pathological test that fails without the change to fs.js.
diff --git a/lib/fs.js b/lib/fs.js
@@ -508,9 +508,11 @@ function realpathSync (p) {
       continue;
     }
     var id = stat.dev.toString(32)+':'+stat.ino.toString(32);
-    if (seenLinks[id]) throw new Error("cyclic link at "+part);
-    seenLinks[id] = true;
-    var target = fs.readlinkSync(part);
+    if (!seenLinks[id]) {
+      fs.statSync(part);
+      seenLinks[id] = fs.readlinkSync(part);
+    }
+    var target = seenLinks[id];
     if (target.charAt(0) === '/') {
       // absolute. Start over.
       buf = [''];
@@ -565,9 +567,13 @@ function realpath (p, cb) {
       return process.nextTick(LOOP);
     }
     var id = stat.dev.toString(32)+':'+stat.ino.toString(32);
-    if (seenLinks[id]) return cb(new Error("cyclic link at "+part));
-    seenLinks[id] = true;
-    fs.readlink(part, gotTarget);
+    if (seenLinks[id]) return gotTarget(null, seenLinks[id]);
+    fs.stat(part, function (er) {
+      if (er) return cb(er)
+      fs.readlink(part, function (er, target) {
+        gotTarget(er, seenLinks[id] = target);
+      });
+    })
   }
   function gotTarget (er, target) {
     if (er) return cb(er);
diff --git a/test/simple/test-fs-realpath.js b/test/simple/test-fs-realpath.js
@@ -32,6 +32,7 @@ function bashRealpath(path, callback) {
 // sub-tests:
 
 function test_simple_relative_symlink(callback) {
+  console.log("test_simple_relative_symlink");
   var entry = common.fixturesDir+'/cycles/symlink',
       expected = common.fixturesDir+'/cycles/root.js';
   [
@@ -51,6 +52,7 @@ function test_simple_relative_symlink(callback) {
 }
 
 function test_simple_absolute_symlink(callback) {
+  console.log("test_simple_absolute_symlink");
   bashRealpath(common.fixturesDir, function(err, fixturesAbsDir) {
     if (err) return callback(err);
     var entry = fixturesAbsDir+'/cycles/symlink',
@@ -73,6 +75,7 @@ function test_simple_absolute_symlink(callback) {
 }
 
 function test_deep_relative_file_symlink(callback) {
+  console.log("test_deep_relative_file_symlink");
   var expected = path.join(common.fixturesDir, 'cycles', 'root.js');
   var linkData1 = "../../cycles/root.js";
   var linkPath1 = path.join(common.fixturesDir, "nested-index", 'one', 'symlink1.js');
@@ -94,6 +97,7 @@ function test_deep_relative_file_symlink(callback) {
 }
 
 function test_deep_relative_dir_symlink(callback) {
+  console.log("test_deep_relative_dir_symlink");
   var expected = path.join(common.fixturesDir, 'cycles', 'folder');
   var linkData1b = "../../cycles/folder";
   var linkPath1b = path.join(common.fixturesDir, "nested-index", 'one', 'symlink1-dir');
@@ -116,6 +120,7 @@ function test_deep_relative_dir_symlink(callback) {
 }
 
 function test_cyclic_link_protection(callback) {
+  console.log("test_cyclic_link_protection");
   var entry = common.fixturesDir+'/cycles/realpath-3a';
   [
     [entry, '../cycles/realpath-3b'],
@@ -133,7 +138,24 @@ function test_cyclic_link_protection(callback) {
   });
 }
 
+function test_cyclic_link_overprotection (callback) {
+  console.log("test_cyclic_link_overprotection");
+  var cycles = common.fixturesDir+'/cycles';
+  var expected = fs.realpathSync(cycles);
+  var folder = cycles+'/folder';
+  var link = folder+'/cycles';
+  var testPath = cycles;
+  for (var i = 0; i < 10; i ++) testPath += '/folder/cycles';
+  try {fs.unlinkSync(link)} catch (ex) {}
+  fs.symlinkSync(cycles, link);
+  assert.equal(fs.realpathSync(testPath), expected);
+  asynctest(fs.realpath, [testPath], callback, function (er, res) {
+    assert.equal(res, expected);
+  });
+}
+
 function test_relative_input_cwd(callback) {
+  console.log("test_relative_input_cwd");
   var p = common.fixturesDir.lastIndexOf('/');
   var entrydir = common.fixturesDir.substr(0, p);
   var entry = common.fixturesDir.substr(p+1)+'/cycles/realpath-3a';
@@ -161,6 +183,7 @@ function test_relative_input_cwd(callback) {
 }
 
 function test_deep_symlink_mix(callback) {
+  console.log("test_deep_symlink_mix");
   // todo: check to see that common.fixturesDir is not rooted in the
   //       same directory as our test symlink.
   // obtain our current realpath using bash (so we can test ourselves)
@@ -209,6 +232,7 @@ function test_deep_symlink_mix(callback) {
 }
 
 function test_non_symlinks(callback) {
+  console.log("test_non_symlinks");
   bashRealpath(common.fixturesDir, function(err, fixturesAbsDir) {
     if (err) return callback(err);
     var p = fixturesAbsDir.lastIndexOf('/');
@@ -229,6 +253,7 @@ function test_non_symlinks(callback) {
 
 var upone = path.join(process.cwd(), "..");
 function test_escape_cwd (cb) {
+  console.log("test_escape_cwd");
   asynctest(fs.realpath, [".."], cb, function(er, uponeActual){
     assert.equal(upone, uponeActual,
       "realpath('..') expected: "+upone+" actual:"+uponeActual);
@@ -247,6 +272,7 @@ assert.equal(upone, uponeActual,
 //     `-- link -> /tmp/node-test-realpath-abs-kids/a/b/
 // realpath(root+'/a/link/c/x.txt') ==> root+'/a/b/c/x.txt'
 function test_abs_with_kids (cb) {
+  console.log("test_abs_with_kids");
   bashRealpath(common.fixturesDir, function(err, fixturesAbsDir) {
     var root = fixturesAbsDir+'/node-test-realpath-abs-kids';
     function cleanup () {
@@ -298,11 +324,12 @@ var tests = [
   test_deep_relative_file_symlink,
   test_deep_relative_dir_symlink,
   test_cyclic_link_protection,
+  test_cyclic_link_overprotection,
   test_relative_input_cwd,
   test_deep_symlink_mix,
   test_non_symlinks,
   test_escape_cwd,
-  test_abs_with_kids  
+  test_abs_with_kids
 ];
 var numtests = tests.length;
 function runNextTest(err) {
