GeezerVison is showing github.com — test: cover `realpathSync` resolving symlinks after a FIFO stat · nodejs/node@5ded71f
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/fs.js +18-7Lines changed: 18 additions & 7 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -84,7 +84,6 @@ const { | 84 | 84 |
| 85 | 85 | const { | 86 | 86 | FSReqCallback, | 87 | | - statValues, | 88 | 87 | } = binding; | 89 | 88 | const { toPathIfFileURL } = require('internal/url'); | 90 | 89 | const { | @@ -2787,6 +2786,11 @@ function realpathSync(p, options) { | 2787 | 2786 | const seenLinks = new SafeMap(); | 2788 | 2787 | const knownHard = new SafeSet(); | 2789 | 2788 | const original = p; | | 2789 | + // Whether the symlink this walk resolved last pointed at a pipe or a | | 2790 | + // socket, which is where the walk stops. It cannot be read back from the | | 2791 | + // shared stat buffer, which holds the last stat made anywhere in the | | 2792 | + // process rather than the last one made here. | | 2793 | + let reachedPipeOrSocket = false; | 2790 | 2794 |
| 2791 | 2795 | // Current character position in p | 2792 | 2796 | let pos; | @@ -2830,8 +2834,7 @@ function realpathSync(p, options) { | 2830 | 2834 |
| 2831 | 2835 | // Continue if not a symlink, break if a pipe/socket | 2832 | 2836 | if (knownHard.has(base) || cache?.get(base) === base) { | 2833 | | - if (isFileType(statValues, S_IFIFO) || | 2834 | | - isFileType(statValues, S_IFSOCK)) { | | 2837 | + if (reachedPipeOrSocket) { | 2835 | 2838 | break; | 2836 | 2839 | } | 2837 | 2840 | continue; | @@ -2869,7 +2872,9 @@ function realpathSync(p, options) { | 2869 | 2872 | } | 2870 | 2873 | } | 2871 | 2874 | if (linkTarget === null) { | 2872 | | - binding.stat(base, false, undefined, true); | | 2875 | + const targetStats = binding.stat(base, false, undefined, true); | | 2876 | + reachedPipeOrSocket = isFileType(targetStats, S_IFIFO) || | | 2877 | + isFileType(targetStats, S_IFSOCK); | 2873 | 2878 | linkTarget = binding.readlink(base, undefined); | 2874 | 2879 | } | 2875 | 2880 | resolvedLink = pathModule.resolve(previous, linkTarget); | @@ -2941,6 +2946,11 @@ function realpath(p, options, callback) { | 2941 | 2946 |
| 2942 | 2947 | const seenLinks = new SafeMap(); | 2943 | 2948 | const knownHard = new SafeSet(); | | 2949 | + // Whether the symlink this walk resolved last pointed at a pipe or a | | 2950 | + // socket, which is where the walk stops. It cannot be read back from the | | 2951 | + // shared stat buffer, which holds the last stat made anywhere in the | | 2952 | + // process rather than the last one made here. | | 2953 | + let reachedPipeOrSocket = false; | 2944 | 2954 |
| 2945 | 2955 | // Current character position in p | 2946 | 2956 | let pos; | @@ -2989,8 +2999,7 @@ function realpath(p, options, callback) { | 2989 | 2999 |
| 2990 | 3000 | // Continue if not a symlink, break if a pipe/socket | 2991 | 3001 | if (knownHard.has(base)) { | 2992 | | - if (isFileType(statValues, S_IFIFO) || | 2993 | | - isFileType(statValues, S_IFSOCK)) { | | 3002 | + if (reachedPipeOrSocket) { | 2994 | 3003 | return callback(null, encodeRealpathResult(p, options)); | 2995 | 3004 | } | 2996 | 3005 | return process.nextTick(LOOP); | @@ -3020,9 +3029,11 @@ function realpath(p, options, callback) { | 3020 | 3029 | return gotTarget(null, seenLinks.get(id)); | 3021 | 3030 | } | 3022 | 3031 | } | 3023 | | - fs.stat(base, (err) => { | | 3032 | + fs.stat(base, (err, targetStats) => { | 3024 | 3033 | if (err) return callback(err); | 3025 | 3034 |
| | 3035 | + reachedPipeOrSocket = targetStats.isFIFO() || targetStats.isSocket(); | | 3036 | + | 3026 | 3037 | fs.readlink(base, (err, target) => { | 3027 | 3038 | if (!isWindows) seenLinks.set(id, target); | 3028 | 3039 | gotTarget(err, target); | |
Collapse file Copy file name to clipboard +62Lines changed: 62 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +'use strict'; | | 2 | + | | 3 | +// The async realpath() reads the shared stat buffer the same way realpathSync() | | 4 | +// did, to decide whether the walk has reached a pipe or a socket. The walk's | | 5 | +// own fs.stat() does leave the right value there, but it is not read until | | 6 | +// after fs.readlink() and a process.nextTick(), and any stat completing in that | | 7 | +// window replaces it. | | 8 | +// | | 9 | +// Truncating the walk only costs something when a second symlink follows the | | 10 | +// one being resolved, so the path used here has two. | | 11 | + | | 12 | +const common = require('../common'); | | 13 | + | | 14 | +if (common.isWindows) | | 15 | + common.skip('no mkfifo on Windows'); | | 16 | + | | 17 | +const assert = require('assert'); | | 18 | +const fs = require('fs'); | | 19 | +const path = require('path'); | | 20 | +const { execFileSync } = require('child_process'); | | 21 | +const tmpdir = require('../common/tmpdir'); | | 22 | + | | 23 | +tmpdir.refresh(); | | 24 | + | | 25 | +const real = tmpdir.resolve('real'); | | 26 | +const pkg = tmpdir.resolve('pkg'); | | 27 | +const fifo = tmpdir.resolve('fifo'); | | 28 | + | | 29 | +fs.mkdirSync(real); | | 30 | +fs.mkdirSync(pkg); | | 31 | +fs.writeFileSync(path.join(real, 'index.js'), ''); | | 32 | +fs.symlinkSync(path.join('..', 'real'), path.join(pkg, 'sub')); | | 33 | +fs.symlinkSync('pkg', tmpdir.resolve('link')); | | 34 | +execFileSync('mkfifo', [fifo]); | | 35 | + | | 36 | +const throughLinks = tmpdir.resolve('link', 'sub', 'index.js'); | | 37 | +const expected = path.join(real, 'index.js'); | | 38 | + | | 39 | +// Keep stats of the FIFO completing for as long as the walk runs, so that one | | 40 | +// of them lands in the buffer during the window. | | 41 | +let settled = false; | | 42 | +(function statFifo() { | | 43 | + if (settled) return; | | 44 | + fs.stat(fifo, statFifo); | | 45 | +})(); | | 46 | + | | 47 | +let error; | | 48 | +let resolvedPath; | | 49 | + | | 50 | +fs.realpath(throughLinks, common.mustCall((err, resolved) => { | | 51 | + settled = true; | | 52 | + error = err; | | 53 | + resolvedPath = resolved; | | 54 | +})); | | 55 | + | | 56 | +// Asserted on exit rather than in the callback. An assertion that fails inside | | 57 | +// this callback is lost: it does not reach an `uncaughtException` handler and | | 58 | +// the process still exits 0, so the test would pass over the bug it covers. | | 59 | +process.on('exit', () => { | | 60 | + assert.ifError(error); | | 61 | + assert.strictEqual(resolvedPath, expected); | | 62 | +}); |
Collapse file Copy file name to clipboard +73Lines changed: 73 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +// Flags: --expose-internals | | 2 | +'use strict'; | | 3 | + | | 4 | +// Resolving a path must not depend on what was stat'ed before it. | | 5 | +// | | 6 | +// While walking a path, realpath skips the components it already knows are | | 7 | +// real, and in that branch it consulted the shared stat buffer to decide | | 8 | +// whether the walk had reached a pipe or a socket. That buffer holds the result | | 9 | +// of the last stat made anywhere in the process, so an unrelated stat of a FIFO | | 10 | +// made the walk stop early and hand back the path with its symlinks unresolved. | | 11 | +// The unresolved path is then cached, so every later resolution repeats it. | | 12 | + | | 13 | +const common = require('../common'); | | 14 | + | | 15 | +if (common.isWindows) | | 16 | + common.skip('no mkfifo on Windows'); | | 17 | + | | 18 | +const assert = require('assert'); | | 19 | +const fs = require('fs'); | | 20 | +const path = require('path'); | | 21 | +const { execFileSync } = require('child_process'); | | 22 | +const { realpathCacheKey } = require('internal/fs/utils'); | | 23 | +const tmpdir = require('../common/tmpdir'); | | 24 | + | | 25 | +tmpdir.refresh(); | | 26 | + | | 27 | +const pkg = tmpdir.resolve('pkg'); | | 28 | +const link = tmpdir.resolve('pkg-link'); | | 29 | +const fifo = tmpdir.resolve('fifo'); | | 30 | + | | 31 | +fs.mkdirSync(pkg); | | 32 | +fs.writeFileSync(path.join(pkg, 'index.js'), 'module.exports = {};\n'); | | 33 | +fs.writeFileSync(tmpdir.resolve('warm.js'), 'module.exports = {};\n'); | | 34 | +fs.symlinkSync('pkg', link); | | 35 | +execFileSync('mkfifo', [fifo]); | | 36 | + | | 37 | +const throughLink = path.join(link, 'index.js'); | | 38 | +const throughReal = path.join(pkg, 'index.js'); | | 39 | + | | 40 | +// The walk only skips a component once something has established it as real. A | | 41 | +// cache carrying the ancestors is that state, and it is the state the module | | 42 | +// loader's own cache is in after it has resolved anything else under the | | 43 | +// directory. | | 44 | +function ancestorCache() { | | 45 | + const cache = new Map(); | | 46 | + let dir = ''; | | 47 | + for (const part of tmpdir.path.split(path.sep).slice(1)) { | | 48 | + dir += path.sep + part; | | 49 | + cache.set(dir, dir); | | 50 | + } | | 51 | + return cache; | | 52 | +} | | 53 | + | | 54 | +fs.statSync(path.join(pkg, 'index.js')); | | 55 | +assert.strictEqual( | | 56 | + fs.realpathSync(throughLink, { [realpathCacheKey]: ancestorCache() }), | | 57 | + throughReal, | | 58 | +); | | 59 | + | | 60 | +fs.statSync(fifo); | | 61 | +assert.strictEqual( | | 62 | + fs.realpathSync(throughLink, { [realpathCacheKey]: ancestorCache() }), | | 63 | + throughReal, | | 64 | +); | | 65 | + | | 66 | +// What the stale read costs through the module loader, whose cache puts the | | 67 | +// walk in that same state: the symlink stays unresolved, so the file is loaded | | 68 | +// a second time under a second name. | | 69 | +require(tmpdir.resolve('warm.js')); | | 70 | +fs.statSync(fifo); | | 71 | + | | 72 | +assert.strictEqual(require.resolve(throughLink), throughReal); | | 73 | +assert.strictEqual(require(throughLink), require(throughReal)); |
|