GeezerVison is showing github.com — test: cover `realpathSync` resolving symlinks after a FIFO stat · nodejs/node@87ee7bf
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 |
|---|
@@ -81,7 +81,6 @@ const { | 81 | 81 |
| 82 | 82 | const { | 83 | 83 | FSReqCallback, | 84 | | - statValues, | 85 | 84 | } = binding; | 86 | 85 | const { toPathIfFileURL } = require('internal/url'); | 87 | 86 | const { | @@ -3254,6 +3253,11 @@ function realpathSync(p, options) { | 3254 | 3253 | const seenLinks = new SafeMap(); | 3255 | 3254 | const knownHard = new SafeSet(); | 3256 | 3255 | const original = p; | | 3256 | + // Whether the symlink this walk resolved last pointed at a pipe or a | | 3257 | + // socket, which is where the walk stops. It cannot be read back from the | | 3258 | + // shared stat buffer, which holds the last stat made anywhere in the | | 3259 | + // process rather than the last one made here. | | 3260 | + let reachedPipeOrSocket = false; | 3257 | 3261 |
| 3258 | 3262 | // Current character position in p | 3259 | 3263 | let pos; | @@ -3297,8 +3301,7 @@ function realpathSync(p, options) { | 3297 | 3301 |
| 3298 | 3302 | // Continue if not a symlink, break if a pipe/socket | 3299 | 3303 | if (knownHard.has(base) || cache?.get(base) === base) { | 3300 | | - if (isFileType(statValues, S_IFIFO) || | 3301 | | - isFileType(statValues, S_IFSOCK)) { | | 3304 | + if (reachedPipeOrSocket) { | 3302 | 3305 | break; | 3303 | 3306 | } | 3304 | 3307 | continue; | @@ -3336,7 +3339,9 @@ function realpathSync(p, options) { | 3336 | 3339 | } | 3337 | 3340 | } | 3338 | 3341 | if (linkTarget === null) { | 3339 | | - binding.stat(base, false, undefined, true); | | 3342 | + const targetStats = binding.stat(base, false, undefined, true); | | 3343 | + reachedPipeOrSocket = isFileType(targetStats, S_IFIFO) || | | 3344 | + isFileType(targetStats, S_IFSOCK); | 3340 | 3345 | linkTarget = binding.readlink(base, undefined); | 3341 | 3346 | } | 3342 | 3347 | resolvedLink = pathModule.resolve(previous, linkTarget); | @@ -3418,6 +3423,11 @@ function realpath(p, options, callback) { | 3418 | 3423 |
| 3419 | 3424 | const seenLinks = new SafeMap(); | 3420 | 3425 | const knownHard = new SafeSet(); | | 3426 | + // Whether the symlink this walk resolved last pointed at a pipe or a | | 3427 | + // socket, which is where the walk stops. It cannot be read back from the | | 3428 | + // shared stat buffer, which holds the last stat made anywhere in the | | 3429 | + // process rather than the last one made here. | | 3430 | + let reachedPipeOrSocket = false; | 3421 | 3431 |
| 3422 | 3432 | // Current character position in p | 3423 | 3433 | let pos; | @@ -3466,8 +3476,7 @@ function realpath(p, options, callback) { | 3466 | 3476 |
| 3467 | 3477 | // Continue if not a symlink, break if a pipe/socket | 3468 | 3478 | if (knownHard.has(base)) { | 3469 | | - if (isFileType(statValues, S_IFIFO) || | 3470 | | - isFileType(statValues, S_IFSOCK)) { | | 3479 | + if (reachedPipeOrSocket) { | 3471 | 3480 | return callback(null, encodeRealpathResult(p, options)); | 3472 | 3481 | } | 3473 | 3482 | return process.nextTick(LOOP); | @@ -3497,9 +3506,11 @@ function realpath(p, options, callback) { | 3497 | 3506 | return gotTarget(null, seenLinks.get(id)); | 3498 | 3507 | } | 3499 | 3508 | } | 3500 | | - fs.stat(base, (err) => { | | 3509 | + fs.stat(base, (err, targetStats) => { | 3501 | 3510 | if (err) return callback(err); | 3502 | 3511 |
| | 3512 | + reachedPipeOrSocket = targetStats.isFIFO() || targetStats.isSocket(); | | 3513 | + | 3503 | 3514 | fs.readlink(base, (err, target) => { | 3504 | 3515 | if (!isWindows) seenLinks.set(id, target); | 3505 | 3516 | 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)); |
|