Skip to content
This repository

Fix fs.realpath to return on error #2045

Closed
wants to merge 1 commit into from

2 participants

Benjamin Pasero Ben Noordhuis
Benjamin Pasero

There is a missing return statement on error for fs.realpath.

Ben Noordhuis

Patch LGTM. Can you sign the CLA?

Sanity check: what does test/simple/test-fs-realpath.js with the below patch applied do with and without your patch?

diff --git a/test/simple/test-fs-realpath.js b/test/simple/test-fs-realpath.js
index 27ac0b8..b2c8e9b 100644
--- a/test/simple/test-fs-realpath.js
+++ b/test/simple/test-fs-realpath.js
@@ -79,6 +79,19 @@ function bashRealpath(path, callback) {
 }

 // sub-tests:
+function test_simple_error_callback() {
+  var ncalls = 0;
+
+  fs.realpath('/this/path/does/not/exist', function(err, s) {
+    assert(err);
+    assert(!s);
+    ncalls++;
+  });
+
+  process.on('exit', function() {
+    assert.equal(ncalls, 1);
+  });
+}

 function test_simple_relative_symlink(callback) {
   console.log('test_simple_relative_symlink');
@@ -415,6 +428,7 @@ function test_lying_cache_liar(cb) {
 // ----------------------------------------------------------------------------

 var tests = [
+  test_simple_error_callback,
   test_simple_relative_symlink,
   test_simple_absolute_symlink,
   test_deep_relative_file_symlink,

@piscisaureus: Can you confirm the issue?

Benjamin Pasero

I signed the CLA electronically. I hope its fine, otherwise let me know. Thanks.

Benjamin Pasero

Your test shows that the callback is being called twice, it demonstrates the issue if my patch is not applied (ncalls will be 2 in that case).

Ben Noordhuis

Thanks, Benjamin. Merged in b1bb17f.

Benjamin Pasero

That was quick, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Nov 08, 2011
Benjamin Pasero bpasero Fix fs.realpath to return on error a81eea0
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 1 addition and 1 deletion. Show diff stats Hide diff stats

  1. +1 1  lib/fs.js
2  lib/fs.js
@@ -774,7 +774,7 @@ if (isWindows) {
774 774 return cb(null, cache[p]);
775 775 }
776 776 fs.stat(p, function(err) {
777   - if (err) cb(err);
  777 + if (err) return cb(err);
778 778 if (cache) cache[p] = p;
779 779 cb(null, p);
780 780 });

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.