From ad79524382fc7ff8b3b8d4b8464337ed94874b81 Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Wed, 27 Jul 2016 00:18:35 +0200 Subject: [PATCH 1/7] fs: restore JS implementation of realpath This reverts parts of https://github.com/nodejs/node/commit/b488b19eaf2b2e7a3ca5eccd2445e245847a5f76 restoring javascript implementation of realpath and realpathSync. Fixes: https://github.com/nodejs/node/issues/7175 Fixes: https://github.com/nodejs/node/issues/6861 Fixes: https://github.com/nodejs/node/issues/7294 Fixes: https://github.com/nodejs/node/issues/7192 Fixes: https://github.com/nodejs/node/issues/7044 Fixes: https://github.com/nodejs/node/issues/6624 Fixes: https://github.com/nodejs/node/issues/6978 --- lib/fs.js | 251 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 224 insertions(+), 27 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index e3bfdabe885734..f52b20f94a6983 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1563,39 +1563,236 @@ fs.unwatchFile = function(filename, listener) { }; -fs.realpathSync = function realpathSync(path, options) { - if (!options) - options = {}; - else if (typeof options === 'string') - options = {encoding: options}; - else if (typeof options !== 'object') - throw new TypeError('"options" must be a string or an object'); - nullCheck(path); - return binding.realpath(pathModule._makeLong(path), options.encoding); -}; +// Regexp that finds the next partion of a (partial) path +// result is [base_with_slash, base], e.g. ['somedir/', 'somedir'] +const nextPartRe = isWindows ? + /(.*?)(?:[\/\\]+|$)/g : + /(.*?)(?:[\/]+|$)/g; + +// Regex to find the device root, including trailing slash. E.g. 'c:\\'. +const splitRootRe = isWindows ? + /^(?:[a-zA-Z]:|[\\\/]{2}[^\\\/]+[\\\/][^\\\/]+)?[\\\/]*/ : + /^[\/]*/; + +fs.realpathSync = function realpathSync(p, cache) { + // make p is absolute + p = pathModule.resolve(p); + + if (cache && Object.prototype.hasOwnProperty.call(cache, p)) { + return cache[p]; + } + + const original = p; + const seenLinks = {}; + const knownHard = {}; + + // current character position in p + var pos; + // the partial path so far, including a trailing slash if any + var current; + // the partial path without a trailing slash (except when pointing at a root) + var base; + // the partial path scanned in the previous round, with slash + var previous; + + start(); + + function start() { + // Skip over roots + var m = splitRootRe.exec(p); + pos = m[0].length; + current = m[0]; + base = m[0]; + previous = ''; + + // On windows, check that the root exists. On unix there is no need. + if (isWindows && !knownHard[base]) { + fs.lstatSync(base); + knownHard[base] = true; + } + } + // walk down the path, swapping out linked pathparts for their real + // values + // NB: p.length changes. + while (pos < p.length) { + // find the next part + nextPartRe.lastIndex = pos; + var result = nextPartRe.exec(p); + previous = current; + current += result[0]; + base = previous + result[1]; + pos = nextPartRe.lastIndex; + + // continue if not a symlink + if (knownHard[base] || (cache && cache[base] === base)) { + continue; + } -fs.realpath = function realpath(path, options, callback) { - if (!options) { - options = {}; - } else if (typeof options === 'function') { - callback = options; - options = {}; - } else if (typeof options === 'string') { - options = {encoding: options}; - } else if (typeof options !== 'object') { - throw new TypeError('"options" must be a string or an object'); + var resolvedLink; + if (cache && Object.prototype.hasOwnProperty.call(cache, base)) { + // some known symbolic link. no need to stat again. + resolvedLink = cache[base]; + } else { + var stat = fs.lstatSync(base); + if (!stat.isSymbolicLink()) { + knownHard[base] = true; + if (cache) cache[base] = base; + continue; + } + + // read the link if it wasn't read before + // dev/ino always return 0 on windows, so skip the check. + var linkTarget = null; + if (!isWindows) { + var id = stat.dev.toString(32) + ':' + stat.ino.toString(32); + if (seenLinks.hasOwnProperty(id)) { + linkTarget = seenLinks[id]; + } + } + if (linkTarget === null) { + fs.statSync(base); + linkTarget = fs.readlinkSync(base); + } + resolvedLink = pathModule.resolve(previous, linkTarget); + // track this, if given a cache. + if (cache) cache[base] = resolvedLink; + if (!isWindows) seenLinks[id] = linkTarget; + } + + // resolve the link, then start over + p = pathModule.resolve(resolvedLink, p.slice(pos)); + start(); } - callback = makeCallback(callback); - if (!nullCheck(path, callback)) - return; - var req = new FSReqWrap(); - req.oncomplete = callback; - binding.realpath(pathModule._makeLong(path), options.encoding, req); - return; + + if (cache) cache[original] = p; + + return p; }; +fs.realpath = function realpath(p, cache, cb) { + if (typeof cb !== 'function') { + cb = maybeCallback(cache); + cache = null; + } + + // make p is absolute + p = pathModule.resolve(p); + + if (cache && Object.prototype.hasOwnProperty.call(cache, p)) { + return process.nextTick(cb.bind(null, null, cache[p])); + } + + const original = p; + const seenLinks = {}; + const knownHard = {}; + + // current character position in p + var pos; + // the partial path so far, including a trailing slash if any + var current; + // the partial path without a trailing slash (except when pointing at a root) + var base; + // the partial path scanned in the previous round, with slash + var previous; + + start(); + + function start() { + // Skip over roots + var m = splitRootRe.exec(p); + pos = m[0].length; + current = m[0]; + base = m[0]; + previous = ''; + + // On windows, check that the root exists. On unix there is no need. + if (isWindows && !knownHard[base]) { + fs.lstat(base, function(err) { + if (err) return cb(err); + knownHard[base] = true; + LOOP(); + }); + } else { + process.nextTick(LOOP); + } + } + + // walk down the path, swapping out linked pathparts for their real + // values + function LOOP() { + // stop if scanned past end of path + if (pos >= p.length) { + if (cache) cache[original] = p; + return cb(null, p); + } + + // find the next part + nextPartRe.lastIndex = pos; + var result = nextPartRe.exec(p); + previous = current; + current += result[0]; + base = previous + result[1]; + pos = nextPartRe.lastIndex; + + // continue if not a symlink + if (knownHard[base] || (cache && cache[base] === base)) { + return process.nextTick(LOOP); + } + + if (cache && Object.prototype.hasOwnProperty.call(cache, base)) { + // known symbolic link. no need to stat again. + return gotResolvedLink(cache[base]); + } + + return fs.lstat(base, gotStat); + } + + function gotStat(err, stat) { + if (err) return cb(err); + + // if not a symlink, skip to the next path part + if (!stat.isSymbolicLink()) { + knownHard[base] = true; + if (cache) cache[base] = base; + return process.nextTick(LOOP); + } + + // stat & read the link if not read before + // call gotTarget as soon as the link target is known + // dev/ino always return 0 on windows, so skip the check. + if (!isWindows) { + var id = stat.dev.toString(32) + ':' + stat.ino.toString(32); + if (seenLinks.hasOwnProperty(id)) { + return gotTarget(null, seenLinks[id], base); + } + } + fs.stat(base, function(err) { + if (err) return cb(err); + + fs.readlink(base, function(err, target) { + if (!isWindows) seenLinks[id] = target; + gotTarget(err, target); + }); + }); + } + + function gotTarget(err, target, base) { + if (err) return cb(err); + + var resolvedLink = pathModule.resolve(previous, target); + if (cache) cache[base] = resolvedLink; + gotResolvedLink(resolvedLink); + } + + function gotResolvedLink(resolvedLink) { + // resolve the link, then start over + p = pathModule.resolve(resolvedLink, p.slice(pos)); + start(); + } +}; + fs.mkdtemp = function(prefix, options, callback) { if (!prefix || typeof prefix !== 'string') throw new TypeError('filename prefix is required'); From 32cc5d25c1c990f118d9d3667e0ca69600657835 Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Wed, 27 Jul 2016 01:04:24 +0200 Subject: [PATCH 2/7] fixup: reimplement new realpath API Removes cache argument and adds support for new 'options' parameter. --- lib/fs.js | 124 +++++++++--------- .../test-fs-realpath-on-substed-drive.js | 53 ++++++++ 2 files changed, 116 insertions(+), 61 deletions(-) create mode 100644 test/parallel/test-fs-realpath-on-substed-drive.js diff --git a/lib/fs.js b/lib/fs.js index f52b20f94a6983..f4b801b9b596ad 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1574,15 +1574,28 @@ const splitRootRe = isWindows ? /^(?:[a-zA-Z]:|[\\\/]{2}[^\\\/]+[\\\/][^\\\/]+)?[\\\/]*/ : /^[\/]*/; -fs.realpathSync = function realpathSync(p, cache) { - // make p is absolute - p = pathModule.resolve(p); - - if (cache && Object.prototype.hasOwnProperty.call(cache, p)) { - return cache[p]; +function encodeRealpathResult(result, options, err) { + if (!options || !options.encoding || options.encoding === 'utf8' || err) + return result; + const asBuffer = Buffer.from(result); + if (options.encoding === 'buffer') { + return asBuffer; + } else { + return asBuffer.toString(options.encoding); } +} + +fs.realpathSync = function realpathSync(p, options) { + if (!options) + options = {}; + else if (typeof options === 'string') + options = {encoding: options}; + else if (typeof options !== 'object') + throw new TypeError('"options" must be a string or an object'); + nullCheck(p); + + p = pathModule.resolve(p); - const original = p; const seenLinks = {}; const knownHard = {}; @@ -1625,66 +1638,63 @@ fs.realpathSync = function realpathSync(p, cache) { pos = nextPartRe.lastIndex; // continue if not a symlink - if (knownHard[base] || (cache && cache[base] === base)) { + if (knownHard[base]) { continue; } var resolvedLink; - if (cache && Object.prototype.hasOwnProperty.call(cache, base)) { - // some known symbolic link. no need to stat again. - resolvedLink = cache[base]; - } else { - var stat = fs.lstatSync(base); - if (!stat.isSymbolicLink()) { - knownHard[base] = true; - if (cache) cache[base] = base; - continue; - } + var stat = fs.lstatSync(base); + if (!stat.isSymbolicLink()) { + knownHard[base] = true; + continue; + } - // read the link if it wasn't read before - // dev/ino always return 0 on windows, so skip the check. - var linkTarget = null; - if (!isWindows) { - var id = stat.dev.toString(32) + ':' + stat.ino.toString(32); - if (seenLinks.hasOwnProperty(id)) { - linkTarget = seenLinks[id]; - } - } - if (linkTarget === null) { - fs.statSync(base); - linkTarget = fs.readlinkSync(base); + // read the link if it wasn't read before + // dev/ino always return 0 on windows, so skip the check. + var linkTarget = null; + if (!isWindows) { + var id = stat.dev.toString(32) + ':' + stat.ino.toString(32); + if (seenLinks.hasOwnProperty(id)) { + linkTarget = seenLinks[id]; } - resolvedLink = pathModule.resolve(previous, linkTarget); - // track this, if given a cache. - if (cache) cache[base] = resolvedLink; - if (!isWindows) seenLinks[id] = linkTarget; } + if (linkTarget === null) { + fs.statSync(base); + linkTarget = fs.readlinkSync(base); + } + resolvedLink = pathModule.resolve(previous, linkTarget); + + if (!isWindows) seenLinks[id] = linkTarget; // resolve the link, then start over p = pathModule.resolve(resolvedLink, p.slice(pos)); start(); } - if (cache) cache[original] = p; - - return p; + return encodeRealpathResult(p, options); }; -fs.realpath = function realpath(p, cache, cb) { - if (typeof cb !== 'function') { - cb = maybeCallback(cache); - cache = null; +fs.realpath = function realpath(p, options, callback) { + if (typeof callback !== 'function') { + callback = maybeCallback(options); + options = {}; } - // make p is absolute - p = pathModule.resolve(p); - - if (cache && Object.prototype.hasOwnProperty.call(cache, p)) { - return process.nextTick(cb.bind(null, null, cache[p])); + if (!options) { + options = {}; + } else if (typeof options === 'function') { + options = {}; + } else if (typeof options === 'string') { + options = {encoding: options}; + } else if (typeof options !== 'object') { + throw new TypeError('"options" must be a string or an object'); } + if (!nullCheck(p, callback)) + return; + + p = pathModule.resolve(p); - const original = p; const seenLinks = {}; const knownHard = {}; @@ -1710,7 +1720,7 @@ fs.realpath = function realpath(p, cache, cb) { // On windows, check that the root exists. On unix there is no need. if (isWindows && !knownHard[base]) { fs.lstat(base, function(err) { - if (err) return cb(err); + if (err) return callback(err); knownHard[base] = true; LOOP(); }); @@ -1724,8 +1734,7 @@ fs.realpath = function realpath(p, cache, cb) { function LOOP() { // stop if scanned past end of path if (pos >= p.length) { - if (cache) cache[original] = p; - return cb(null, p); + return callback(null, encodeRealpathResult(p, options)); } // find the next part @@ -1737,25 +1746,19 @@ fs.realpath = function realpath(p, cache, cb) { pos = nextPartRe.lastIndex; // continue if not a symlink - if (knownHard[base] || (cache && cache[base] === base)) { + if (knownHard[base]) { return process.nextTick(LOOP); } - if (cache && Object.prototype.hasOwnProperty.call(cache, base)) { - // known symbolic link. no need to stat again. - return gotResolvedLink(cache[base]); - } - return fs.lstat(base, gotStat); } function gotStat(err, stat) { - if (err) return cb(err); + if (err) return callback(err); // if not a symlink, skip to the next path part if (!stat.isSymbolicLink()) { knownHard[base] = true; - if (cache) cache[base] = base; return process.nextTick(LOOP); } @@ -1769,7 +1772,7 @@ fs.realpath = function realpath(p, cache, cb) { } } fs.stat(base, function(err) { - if (err) return cb(err); + if (err) return callback(err); fs.readlink(base, function(err, target) { if (!isWindows) seenLinks[id] = target; @@ -1779,10 +1782,9 @@ fs.realpath = function realpath(p, cache, cb) { } function gotTarget(err, target, base) { - if (err) return cb(err); + if (err) return callback(err); var resolvedLink = pathModule.resolve(previous, target); - if (cache) cache[base] = resolvedLink; gotResolvedLink(resolvedLink); } diff --git a/test/parallel/test-fs-realpath-on-substed-drive.js b/test/parallel/test-fs-realpath-on-substed-drive.js new file mode 100644 index 00000000000000..c9109362691845 --- /dev/null +++ b/test/parallel/test-fs-realpath-on-substed-drive.js @@ -0,0 +1,53 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const spawnSync = require('child_process').spawnSync; + +if (!common.isWindows) { + common.skip('Test for Windows only'); + return; +} +let result; + +// create a subst drive +const driveLetters = 'ABCDEFGHIJKLMNOPQRSTUWXYZ'; +let drive; +for (var i = 0; i < driveLetters.length; ++i) { + drive = `${driveLetters[i]}:`; + result = spawnSync('subst', [drive, common.fixturesDir]); + if (result.status === 0) + break; +} +if (i === driveLetters.length) { + common.skip('Cannot create subst drive'); + return; +} + +// schedule cleanup (and check if all callbacks where called) +process.on('exit', function() { + spawnSync('subst', ['/d', drive]); +}); + +// test: +const filename = `${drive}\\empty.js`; +const filenameBuffer = Buffer.from(filename); + +result = fs.realpathSync(filename); +assert.strictEqual(result, filename); + +result = fs.realpathSync(filename, 'buffer'); +assert(Buffer.isBuffer(result)); +assert(result.equals(filenameBuffer)); + +fs.realpath(filename, common.mustCall(function(err, result) { + assert(!err); + assert.strictEqual(result, filename); +})); + +fs.realpath(filename, 'buffer', common.mustCall(function(err, result) { + assert(!err); + assert(Buffer.isBuffer(result)); + assert(result.equals(filenameBuffer)); +})); From f806237e7fb430fd2ec925e44f4492d972a75920 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Thu, 14 Jul 2016 11:57:26 -0600 Subject: [PATCH 3/7] test,bench: add tests/bench for fs.realpath() fix The test/benchmarks included also work for the previous JS implementation of fs.realpath(). In case the new implementation of realpath() needs to be reverted, we want these changes to stick around. --- benchmark/fs/bench-realpath.js | 46 ++++++++++++++++++ benchmark/fs/bench-realpathSync.js | 39 ++++++++++++++++ test/parallel/test-fs-realpath.js | 75 +++++++++++++++++++++++++++++- 3 files changed, 158 insertions(+), 2 deletions(-) create mode 100644 benchmark/fs/bench-realpath.js create mode 100644 benchmark/fs/bench-realpathSync.js diff --git a/benchmark/fs/bench-realpath.js b/benchmark/fs/bench-realpath.js new file mode 100644 index 00000000000000..1a181935f14ec2 --- /dev/null +++ b/benchmark/fs/bench-realpath.js @@ -0,0 +1,46 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const path = require('path'); +const resolved_path = path.resolve(__dirname, '../../lib/'); +const relative_path = path.relative(__dirname, '../../lib/'); + +const bench = common.createBenchmark(main, { + n: [1e4], + type: ['relative', 'resolved'], +}); + + +function main(conf) { + const n = conf.n >>> 0; + const type = conf.type; + + bench.start(); + if (type === 'relative') + relativePath(n); + else if (type === 'resolved') + resolvedPath(n); + else + throw new Error('unknown "type": ' + type); +} + +function relativePath(n) { + (function r(cntr) { + if (--cntr <= 0) + return bench.end(n); + fs.realpath(relative_path, function() { + r(cntr); + }); + }(n)); +} + +function resolvedPath(n) { + (function r(cntr) { + if (--cntr <= 0) + return bench.end(n); + fs.realpath(resolved_path, function() { + r(cntr); + }); + }(n)); +} diff --git a/benchmark/fs/bench-realpathSync.js b/benchmark/fs/bench-realpathSync.js new file mode 100644 index 00000000000000..ae1c78d30d1b35 --- /dev/null +++ b/benchmark/fs/bench-realpathSync.js @@ -0,0 +1,39 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const path = require('path'); +const resolved_path = path.resolve(__dirname, '../../lib/'); +const relative_path = path.relative(__dirname, '../../lib/'); + +const bench = common.createBenchmark(main, { + n: [1e4], + type: ['relative', 'resolved'], +}); + + +function main(conf) { + const n = conf.n >>> 0; + const type = conf.type; + + bench.start(); + if (type === 'relative') + relativePath(n); + else if (type === 'resolved') + resolvedPath(n); + else + throw new Error('unknown "type": ' + type); + bench.end(n); +} + +function relativePath(n) { + for (var i = 0; i < n; i++) { + fs.realpathSync(relative_path); + } +} + +function resolvedPath(n) { + for (var i = 0; i < n; i++) { + fs.realpathSync(resolved_path); + } +} diff --git a/test/parallel/test-fs-realpath.js b/test/parallel/test-fs-realpath.js index 976ad92a6ea560..6770fd904688b3 100644 --- a/test/parallel/test-fs-realpath.js +++ b/test/parallel/test-fs-realpath.js @@ -197,7 +197,7 @@ function test_cyclic_link_protection(callback) { fs.symlinkSync(t[1], t[0], 'dir'); unlink.push(t[0]); }); - assert.throws(function() { fs.realpathSync(entry); }); + assert.throws(function() { fs.realpathSync(entry); }, /ELOOP/); asynctest(fs.realpath, [entry], callback, function(err, result) { assert.ok(err && true); return true; @@ -454,6 +454,76 @@ function test_abs_with_kids(cb) { }); } +function test_deep_symlink_eloop(callback) { + console.log('test_deep_symlink_eloop'); + if (skipSymlinks) { + common.skip('symlink test (no privs)'); + return runNextTest(); + } + + const deepsymPath = path.join(targetsAbsDir, 'deep-symlink'); + const aPath = path.join(deepsymPath, 'a'); + const bSympath = path.join(aPath, 'b'); + const cleanupPaths = [bSympath]; + const pRepeat = 33; + + function cleanup() { + while (cleanupPaths.length > 0) { + try {fs.unlinkSync(cleanupPaths.pop());} catch (e) {} + } + } + + fs.mkdirSync(deepsymPath); + fs.mkdirSync(aPath); + fs.mkdirSync(path.join(targetsAbsDir, 'deep-symlink', 'c')); + try {fs.unlinkSync(bSympath);} catch (e) {} + fs.symlinkSync(deepsymPath, bSympath); + + // First test sync calls. + + const testPath = aPath + '/b/a'.repeat(pRepeat) + '/b/c'; + const resolvedPath = fs.realpathSync(testPath); + assert.equal(path.relative(deepsymPath, resolvedPath), 'c'); + + var reallyBigSymPath = deepsymPath; + var prev = null; + + // Make massively deep set of symlinks + for (var i = 97; i < 105; i++) { + for (var j = 97; j < 101; j++) { + const link = String.fromCharCode(i) + String.fromCharCode(j); + const link_path = path.join(deepsymPath, link); + cleanupPaths.push(link_path); + try {fs.unlinkSync(link_path);} catch (e) {} + if (prev) + fs.symlinkSync(link_path, prev); + reallyBigSymPath += '/' + link; + prev = link_path; + } + } + fs.symlinkSync(deepsymPath, prev); + reallyBigSymPath += '/' + path.basename(prev); + + assert.throws(() => fs.realpathSync(reallyBigSymPath), /ELOOP/); + + // Now test async calls. + + fs.realpath(testPath, (err, resolvedPath) => { + if (err) throw err; + assert.equal(path.relative(deepsymPath, resolvedPath), 'c'); + checkAsyncReallyBigSymPath(); + }); + + function checkAsyncReallyBigSymPath() { + fs.realpath(reallyBigSymPath, (err, path) => { + assert.ok(err); + assert.ok(/ELOOP/.test(err.message)); + cleanup(); + runNextTest(); + }); + } +} + // ---------------------------------------------------------------------------- var tests = [ @@ -469,7 +539,8 @@ var tests = [ test_non_symlinks, test_escape_cwd, test_abs_with_kids, - test_up_multiple + test_up_multiple, + test_deep_symlink_eloop, ]; var numtests = tests.length; var testsRun = 0; From 5a1a7907375b383c5bb9d37bda748beff913c934 Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Fri, 29 Jul 2016 12:43:01 +0200 Subject: [PATCH 4/7] fixup: remove test_deep_symlink_eloop test --- test/parallel/test-fs-realpath.js | 75 +------------------------------ 1 file changed, 2 insertions(+), 73 deletions(-) diff --git a/test/parallel/test-fs-realpath.js b/test/parallel/test-fs-realpath.js index 6770fd904688b3..976ad92a6ea560 100644 --- a/test/parallel/test-fs-realpath.js +++ b/test/parallel/test-fs-realpath.js @@ -197,7 +197,7 @@ function test_cyclic_link_protection(callback) { fs.symlinkSync(t[1], t[0], 'dir'); unlink.push(t[0]); }); - assert.throws(function() { fs.realpathSync(entry); }, /ELOOP/); + assert.throws(function() { fs.realpathSync(entry); }); asynctest(fs.realpath, [entry], callback, function(err, result) { assert.ok(err && true); return true; @@ -454,76 +454,6 @@ function test_abs_with_kids(cb) { }); } -function test_deep_symlink_eloop(callback) { - console.log('test_deep_symlink_eloop'); - if (skipSymlinks) { - common.skip('symlink test (no privs)'); - return runNextTest(); - } - - const deepsymPath = path.join(targetsAbsDir, 'deep-symlink'); - const aPath = path.join(deepsymPath, 'a'); - const bSympath = path.join(aPath, 'b'); - const cleanupPaths = [bSympath]; - const pRepeat = 33; - - function cleanup() { - while (cleanupPaths.length > 0) { - try {fs.unlinkSync(cleanupPaths.pop());} catch (e) {} - } - } - - fs.mkdirSync(deepsymPath); - fs.mkdirSync(aPath); - fs.mkdirSync(path.join(targetsAbsDir, 'deep-symlink', 'c')); - try {fs.unlinkSync(bSympath);} catch (e) {} - fs.symlinkSync(deepsymPath, bSympath); - - // First test sync calls. - - const testPath = aPath + '/b/a'.repeat(pRepeat) + '/b/c'; - const resolvedPath = fs.realpathSync(testPath); - assert.equal(path.relative(deepsymPath, resolvedPath), 'c'); - - var reallyBigSymPath = deepsymPath; - var prev = null; - - // Make massively deep set of symlinks - for (var i = 97; i < 105; i++) { - for (var j = 97; j < 101; j++) { - const link = String.fromCharCode(i) + String.fromCharCode(j); - const link_path = path.join(deepsymPath, link); - cleanupPaths.push(link_path); - try {fs.unlinkSync(link_path);} catch (e) {} - if (prev) - fs.symlinkSync(link_path, prev); - reallyBigSymPath += '/' + link; - prev = link_path; - } - } - fs.symlinkSync(deepsymPath, prev); - reallyBigSymPath += '/' + path.basename(prev); - - assert.throws(() => fs.realpathSync(reallyBigSymPath), /ELOOP/); - - // Now test async calls. - - fs.realpath(testPath, (err, resolvedPath) => { - if (err) throw err; - assert.equal(path.relative(deepsymPath, resolvedPath), 'c'); - checkAsyncReallyBigSymPath(); - }); - - function checkAsyncReallyBigSymPath() { - fs.realpath(reallyBigSymPath, (err, path) => { - assert.ok(err); - assert.ok(/ELOOP/.test(err.message)); - cleanup(); - runNextTest(); - }); - } -} - // ---------------------------------------------------------------------------- var tests = [ @@ -539,8 +469,7 @@ var tests = [ test_non_symlinks, test_escape_cwd, test_abs_with_kids, - test_up_multiple, - test_deep_symlink_eloop, + test_up_multiple ]; var numtests = tests.length; var testsRun = 0; From cad4d82f5c30ea60061b72b97589877b5e0f1c96 Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Tue, 9 Aug 2016 11:04:10 +0200 Subject: [PATCH 5/7] fixup: handle buffer parameters --- lib/fs.js | 2 + .../test-fs-realpath-buffer-encoding.js | 88 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 test/parallel/test-fs-realpath-buffer-encoding.js diff --git a/lib/fs.js b/lib/fs.js index f4b801b9b596ad..5ddc83a194f29d 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1594,6 +1594,7 @@ fs.realpathSync = function realpathSync(p, options) { throw new TypeError('"options" must be a string or an object'); nullCheck(p); + p = p.toString('utf8'); p = pathModule.resolve(p); const seenLinks = {}; @@ -1693,6 +1694,7 @@ fs.realpath = function realpath(p, options, callback) { if (!nullCheck(p, callback)) return; + p = p.toString('utf8'); p = pathModule.resolve(p); const seenLinks = {}; diff --git a/test/parallel/test-fs-realpath-buffer-encoding.js b/test/parallel/test-fs-realpath-buffer-encoding.js new file mode 100644 index 00000000000000..78ec02549bbd27 --- /dev/null +++ b/test/parallel/test-fs-realpath-buffer-encoding.js @@ -0,0 +1,88 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); + +const string_dir = fs.realpathSync(common.fixturesDir); +const buffer_dir = Buffer.from(string_dir); + +const encodings = ['ascii', 'utf8', 'utf16le', 'ucs2', + 'base64', 'binary', 'hex']; +var expected = {}; +encodings.forEach((encoding) => { + expected[encoding] = buffer_dir.toString(encoding); +}); + + +// test sync version +for (var encoding in expected) { + const expected_value = expected[encoding]; + let result; + + result = fs.realpathSync(string_dir, {encoding: encoding}); + assert.strictEqual(result, expected_value); + + result = fs.realpathSync(string_dir, encoding); + assert.strictEqual(result, expected_value); + + result = fs.realpathSync(buffer_dir, {encoding: encoding}); + assert.strictEqual(result, expected_value); + + result = fs.realpathSync(buffer_dir, encoding); + assert.strictEqual(result, expected_value); +} + +let buffer_result; +buffer_result = fs.realpathSync(string_dir, {encoding: 'buffer'}); +assert.deepStrictEqual(buffer_result, buffer_dir); + +buffer_result = fs.realpathSync(string_dir, 'buffer'); +assert.deepStrictEqual(buffer_result, buffer_dir); + +buffer_result = fs.realpathSync(buffer_dir, {encoding: 'buffer'}); +assert.deepStrictEqual(buffer_result, buffer_dir); + +buffer_result = fs.realpathSync(buffer_dir, 'buffer'); +assert.deepStrictEqual(buffer_result, buffer_dir); + +// test async version +for (encoding in expected) { + const expected_value = expected[encoding]; + + fs.realpath(string_dir, {encoding: encoding}, common.mustCall((err, res) => { + assert(!err); + assert.strictEqual(res, expected_value); + })); + fs.realpath(string_dir, encoding, common.mustCall((err, res) => { + assert(!err); + assert.strictEqual(res, expected_value); + })); + fs.realpath(buffer_dir, {encoding: encoding}, common.mustCall((err, res) => { + assert(!err); + assert.strictEqual(res, expected_value); + })); + fs.realpath(buffer_dir, encoding, common.mustCall((err, res) => { + assert(!err); + assert.strictEqual(res, expected_value); + })); +} + +fs.realpath(string_dir, {encoding: 'buffer'}, common.mustCall((err, res) => { + assert(!err); + assert.deepStrictEqual(res, buffer_dir); +})); + +fs.realpath(string_dir, 'buffer', common.mustCall((err, res) => { + assert(!err); + assert.deepStrictEqual(res, buffer_dir); +})); + +fs.realpath(buffer_dir, {encoding: 'buffer'}, common.mustCall((err, res) => { + assert(!err); + assert.deepStrictEqual(res, buffer_dir); +})); + +fs.realpath(buffer_dir, 'buffer', common.mustCall((err, res) => { + assert(!err); + assert.deepStrictEqual(res, buffer_dir); +})); From 94b4214eb8fed2b6538e0ee3da567921bcd65f41 Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Wed, 10 Aug 2016 15:50:19 +0200 Subject: [PATCH 6/7] Add note about buffer parameters to documentation --- doc/api/fs.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 1984b5642e305b..ac1db3b1177af8 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1213,6 +1213,8 @@ added: v0.1.31 Asynchronous realpath(3). The `callback` gets two arguments `(err, resolvedPath)`. May use `process.cwd` to resolve relative paths. +Only paths that can be converted to UTF8 strings are supported. + The optional `options` argument can be a string specifying an encoding, or an object with an `encoding` property specifying the character encoding to use for the path passed to the callback. If the `encoding` is set to `'buffer'`, @@ -1229,10 +1231,12 @@ added: v0.1.31 Synchronous realpath(3). Returns the resolved path. +Only paths that can be converted to UTF8 strings are supported. + The optional `options` argument can be a string specifying an encoding, or an object with an `encoding` property specifying the character encoding to use for -the path passed to the callback. If the `encoding` is set to `'buffer'`, -the path returned will be passed as a `Buffer` object. +the returned value. If the `encoding` is set to `'buffer'`, the path returned +will be passed as a `Buffer` object. ## fs.rename(oldPath, newPath, callback)