test: cover realpathSync resolving symlinks after a FIFO stat

While walking a path, `realpathSync` skips the components it already
knows are real, and in that branch it reads the shared stat buffer to
decide whether the walk has reached a pipe or a socket. That buffer
holds the result of the last stat made anywhere in the process rather
than the last one made by the walk, so an unrelated stat of a FIFO ends
the walk early and the path comes back with its symlinks unresolved. The
unresolved path is then written to the cache, so every later resolution
repeats it.
The walk only takes that branch once the ancestors are established as
real, which is the state the module loader's cache is in. The test goes
through `require()` to reach it, where the stale read costs a second
copy of a module reached through a symlink.
Signed-off-by: Hendrik Liebau <mail@hendrik-liebau.de>
PR-URL: #65113
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Author: unstubbable

lib/fs.js

@@ -81,7 +81,6 @@ const {
 
 const{
  FSReqCallback,
- statValues,
 }=binding;
 const{ toPathIfFileURL }=require('internal/url');
 const{
@@ -3254,6 +3253,11 @@ function realpathSync(p, options) {
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
 constoriginal=p;
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3297,8 +3301,7 @@ function realpathSync(p, options) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)||cache?.get(base)===base){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 break;
 }
 continue;
@@ -3336,7 +3339,9 @@ function realpathSync(p, options) {
 }
 }
 if(linkTarget===null){
-binding.stat(base,false,undefined,true);
+consttargetStats=binding.stat(base,false,undefined,true);
+reachedPipeOrSocket=isFileType(targetStats,S_IFIFO)||
+isFileType(targetStats,S_IFSOCK);
 linkTarget=binding.readlink(base,undefined);
 }
 resolvedLink=pathModule.resolve(previous,linkTarget);
@@ -3418,6 +3423,11 @@ function realpath(p, options, callback) {
 
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3466,8 +3476,7 @@ function realpath(p, options, callback) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 returncallback(null,encodeRealpathResult(p,options));
 }
 returnprocess.nextTick(LOOP);
@@ -3497,9 +3506,11 @@ function realpath(p, options, callback) {
 returngotTarget(null,seenLinks.get(id));
 }
 }
-fs.stat(base,(err)=>{
+fs.stat(base,(err,targetStats)=>{
 if(err)returncallback(err);
 
+reachedPipeOrSocket=targetStats.isFIFO()||targetStats.isSocket();
+
 fs.readlink(base,(err,target)=>{
 if(!isWindows)seenLinks.set(id,target);
 gotTarget(err,target);

test/parallel/test-fs-realpath-async-stale-stat-values.js

@@ -0,0 +1,62 @@
+'use strict';
+
+// The async realpath() reads the shared stat buffer the same way realpathSync()
+// did, to decide whether the walk has reached a pipe or a socket. The walk's
+// own fs.stat() does leave the right value there, but it is not read until
+// after fs.readlink() and a process.nextTick(), and any stat completing in that
+// window replaces it.
+//
+// Truncating the walk only costs something when a second symlink follows the
+// one being resolved, so the path used here has two.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constreal=tmpdir.resolve('real');
+constpkg=tmpdir.resolve('pkg');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(real);
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(real,'index.js'),'');
+fs.symlinkSync(path.join('..','real'),path.join(pkg,'sub'));
+fs.symlinkSync('pkg',tmpdir.resolve('link'));
+execFileSync('mkfifo',[fifo]);
+
+constthroughLinks=tmpdir.resolve('link','sub','index.js');
+constexpected=path.join(real,'index.js');
+
+// Keep stats of the FIFO completing for as long as the walk runs, so that one
+// of them lands in the buffer during the window.
+letsettled=false;
+(functionstatFifo(){
+if(settled)return;
+fs.stat(fifo,statFifo);
+})();
+
+leterror;
+letresolvedPath;
+
+fs.realpath(throughLinks,common.mustCall((err,resolved)=>{
+settled=true;
+error=err;
+resolvedPath=resolved;
+}));
+
+// Asserted on exit rather than in the callback. An assertion that fails inside
+// this callback is lost: it does not reach an `uncaughtException` handler and
+// the process still exits 0, so the test would pass over the bug it covers.
+process.on('exit',()=>{
+assert.ifError(error);
+assert.strictEqual(resolvedPath,expected);
+});

test/parallel/test-fs-realpath-stale-stat-values.js

@@ -0,0 +1,73 @@
+// Flags: --expose-internals
+'use strict';
+
+// Resolving a path must not depend on what was stat'ed before it.
+//
+// While walking a path, realpath skips the components it already knows are
+// real, and in that branch it consulted the shared stat buffer to decide
+// whether the walk had reached a pipe or a socket. That buffer holds the result
+// of the last stat made anywhere in the process, so an unrelated stat of a FIFO
+// made the walk stop early and hand back the path with its symlinks unresolved.
+// The unresolved path is then cached, so every later resolution repeats it.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+const{ realpathCacheKey }=require('internal/fs/utils');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constpkg=tmpdir.resolve('pkg');
+constlink=tmpdir.resolve('pkg-link');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(pkg,'index.js'),'module.exports = {};\n');
+fs.writeFileSync(tmpdir.resolve('warm.js'),'module.exports = {};\n');
+fs.symlinkSync('pkg',link);
+execFileSync('mkfifo',[fifo]);
+
+constthroughLink=path.join(link,'index.js');
+constthroughReal=path.join(pkg,'index.js');
+
+// The walk only skips a component once something has established it as real. A
+// cache carrying the ancestors is that state, and it is the state the module
+// loader's own cache is in after it has resolved anything else under the
+// directory.
+functionancestorCache(){
+constcache=newMap();
+letdir='';
+for(constpartoftmpdir.path.split(path.sep).slice(1)){
+dir+=path.sep+part;
+cache.set(dir,dir);
+}
+returncache;
+}
+
+fs.statSync(path.join(pkg,'index.js'));
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+fs.statSync(fifo);
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+// What the stale read costs through the module loader, whose cache puts the
+// walk in that same state: the symlink stays unresolved, so the file is loaded
+// a second time under a second name.
+require(tmpdir.resolve('warm.js'));
+fs.statSync(fifo);
+
+assert.strictEqual(require.resolve(throughLink),throughReal);
+assert.strictEqual(require(throughLink),require(throughReal));

lib/fs.js

@@ -81,7 +81,6 @@ const {
 
 const{
  FSReqCallback,
- statValues,
 }=binding;
 const{ toPathIfFileURL }=require('internal/url');
 const{
@@ -3254,6 +3253,11 @@ function realpathSync(p, options) {
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
 constoriginal=p;
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3297,8 +3301,7 @@ function realpathSync(p, options) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)||cache?.get(base)===base){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 break;
 }
 continue;
@@ -3336,7 +3339,9 @@ function realpathSync(p, options) {
 }
 }
 if(linkTarget===null){
-binding.stat(base,false,undefined,true);
+consttargetStats=binding.stat(base,false,undefined,true);
+reachedPipeOrSocket=isFileType(targetStats,S_IFIFO)||
+isFileType(targetStats,S_IFSOCK);
 linkTarget=binding.readlink(base,undefined);
 }
 resolvedLink=pathModule.resolve(previous,linkTarget);
@@ -3418,6 +3423,11 @@ function realpath(p, options, callback) {
 
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3466,8 +3476,7 @@ function realpath(p, options, callback) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 returncallback(null,encodeRealpathResult(p,options));
 }
 returnprocess.nextTick(LOOP);
@@ -3497,9 +3506,11 @@ function realpath(p, options, callback) {
 returngotTarget(null,seenLinks.get(id));
 }
 }
-fs.stat(base,(err)=>{
+fs.stat(base,(err,targetStats)=>{
 if(err)returncallback(err);
 
+reachedPipeOrSocket=targetStats.isFIFO()||targetStats.isSocket();
+
 fs.readlink(base,(err,target)=>{
 if(!isWindows)seenLinks.set(id,target);
 gotTarget(err,target);

test/parallel/test-fs-realpath-async-stale-stat-values.js

@@ -0,0 +1,62 @@
+'use strict';
+
+// The async realpath() reads the shared stat buffer the same way realpathSync()
+// did, to decide whether the walk has reached a pipe or a socket. The walk's
+// own fs.stat() does leave the right value there, but it is not read until
+// after fs.readlink() and a process.nextTick(), and any stat completing in that
+// window replaces it.
+//
+// Truncating the walk only costs something when a second symlink follows the
+// one being resolved, so the path used here has two.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constreal=tmpdir.resolve('real');
+constpkg=tmpdir.resolve('pkg');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(real);
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(real,'index.js'),'');
+fs.symlinkSync(path.join('..','real'),path.join(pkg,'sub'));
+fs.symlinkSync('pkg',tmpdir.resolve('link'));
+execFileSync('mkfifo',[fifo]);
+
+constthroughLinks=tmpdir.resolve('link','sub','index.js');
+constexpected=path.join(real,'index.js');
+
+// Keep stats of the FIFO completing for as long as the walk runs, so that one
+// of them lands in the buffer during the window.
+letsettled=false;
+(functionstatFifo(){
+if(settled)return;
+fs.stat(fifo,statFifo);
+})();
+
+leterror;
+letresolvedPath;
+
+fs.realpath(throughLinks,common.mustCall((err,resolved)=>{
+settled=true;
+error=err;
+resolvedPath=resolved;
+}));
+
+// Asserted on exit rather than in the callback. An assertion that fails inside
+// this callback is lost: it does not reach an `uncaughtException` handler and
+// the process still exits 0, so the test would pass over the bug it covers.
+process.on('exit',()=>{
+assert.ifError(error);
+assert.strictEqual(resolvedPath,expected);
+});

test/parallel/test-fs-realpath-stale-stat-values.js

@@ -0,0 +1,73 @@
+// Flags: --expose-internals
+'use strict';
+
+// Resolving a path must not depend on what was stat'ed before it.
+//
+// While walking a path, realpath skips the components it already knows are
+// real, and in that branch it consulted the shared stat buffer to decide
+// whether the walk had reached a pipe or a socket. That buffer holds the result
+// of the last stat made anywhere in the process, so an unrelated stat of a FIFO
+// made the walk stop early and hand back the path with its symlinks unresolved.
+// The unresolved path is then cached, so every later resolution repeats it.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+const{ realpathCacheKey }=require('internal/fs/utils');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constpkg=tmpdir.resolve('pkg');
+constlink=tmpdir.resolve('pkg-link');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(pkg,'index.js'),'module.exports = {};\n');
+fs.writeFileSync(tmpdir.resolve('warm.js'),'module.exports = {};\n');
+fs.symlinkSync('pkg',link);
+execFileSync('mkfifo',[fifo]);
+
+constthroughLink=path.join(link,'index.js');
+constthroughReal=path.join(pkg,'index.js');
+
+// The walk only skips a component once something has established it as real. A
+// cache carrying the ancestors is that state, and it is the state the module
+// loader's own cache is in after it has resolved anything else under the
+// directory.
+functionancestorCache(){
+constcache=newMap();
+letdir='';
+for(constpartoftmpdir.path.split(path.sep).slice(1)){
+dir+=path.sep+part;
+cache.set(dir,dir);
+}
+returncache;
+}
+
+fs.statSync(path.join(pkg,'index.js'));
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+fs.statSync(fifo);
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+// What the stale read costs through the module loader, whose cache puts the
+// walk in that same state: the symlink stays unresolved, so the file is loaded
+// a second time under a second name.
+require(tmpdir.resolve('warm.js'));
+fs.statSync(fifo);
+
+assert.strictEqual(require.resolve(throughLink),throughReal);
+assert.strictEqual(require(throughLink),require(throughReal));

lib/fs.js

@@ -81,7 +81,6 @@ const {
 
 const{
  FSReqCallback,
- statValues,
 }=binding;
 const{ toPathIfFileURL }=require('internal/url');
 const{
@@ -3254,6 +3253,11 @@ function realpathSync(p, options) {
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
 constoriginal=p;
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3297,8 +3301,7 @@ function realpathSync(p, options) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)||cache?.get(base)===base){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 break;
 }
 continue;
@@ -3336,7 +3339,9 @@ function realpathSync(p, options) {
 }
 }
 if(linkTarget===null){
-binding.stat(base,false,undefined,true);
+consttargetStats=binding.stat(base,false,undefined,true);
+reachedPipeOrSocket=isFileType(targetStats,S_IFIFO)||
+isFileType(targetStats,S_IFSOCK);
 linkTarget=binding.readlink(base,undefined);
 }
 resolvedLink=pathModule.resolve(previous,linkTarget);
@@ -3418,6 +3423,11 @@ function realpath(p, options, callback) {
 
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3466,8 +3476,7 @@ function realpath(p, options, callback) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 returncallback(null,encodeRealpathResult(p,options));
 }
 returnprocess.nextTick(LOOP);
@@ -3497,9 +3506,11 @@ function realpath(p, options, callback) {
 returngotTarget(null,seenLinks.get(id));
 }
 }
-fs.stat(base,(err)=>{
+fs.stat(base,(err,targetStats)=>{
 if(err)returncallback(err);
 
+reachedPipeOrSocket=targetStats.isFIFO()||targetStats.isSocket();
+
 fs.readlink(base,(err,target)=>{
 if(!isWindows)seenLinks.set(id,target);
 gotTarget(err,target);

test/parallel/test-fs-realpath-async-stale-stat-values.js

@@ -0,0 +1,62 @@
+'use strict';
+
+// The async realpath() reads the shared stat buffer the same way realpathSync()
+// did, to decide whether the walk has reached a pipe or a socket. The walk's
+// own fs.stat() does leave the right value there, but it is not read until
+// after fs.readlink() and a process.nextTick(), and any stat completing in that
+// window replaces it.
+//
+// Truncating the walk only costs something when a second symlink follows the
+// one being resolved, so the path used here has two.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constreal=tmpdir.resolve('real');
+constpkg=tmpdir.resolve('pkg');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(real);
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(real,'index.js'),'');
+fs.symlinkSync(path.join('..','real'),path.join(pkg,'sub'));
+fs.symlinkSync('pkg',tmpdir.resolve('link'));
+execFileSync('mkfifo',[fifo]);
+
+constthroughLinks=tmpdir.resolve('link','sub','index.js');
+constexpected=path.join(real,'index.js');
+
+// Keep stats of the FIFO completing for as long as the walk runs, so that one
+// of them lands in the buffer during the window.
+letsettled=false;
+(functionstatFifo(){
+if(settled)return;
+fs.stat(fifo,statFifo);
+})();
+
+leterror;
+letresolvedPath;
+
+fs.realpath(throughLinks,common.mustCall((err,resolved)=>{
+settled=true;
+error=err;
+resolvedPath=resolved;
+}));
+
+// Asserted on exit rather than in the callback. An assertion that fails inside
+// this callback is lost: it does not reach an `uncaughtException` handler and
+// the process still exits 0, so the test would pass over the bug it covers.
+process.on('exit',()=>{
+assert.ifError(error);
+assert.strictEqual(resolvedPath,expected);
+});

test/parallel/test-fs-realpath-stale-stat-values.js

@@ -0,0 +1,73 @@
+// Flags: --expose-internals
+'use strict';
+
+// Resolving a path must not depend on what was stat'ed before it.
+//
+// While walking a path, realpath skips the components it already knows are
+// real, and in that branch it consulted the shared stat buffer to decide
+// whether the walk had reached a pipe or a socket. That buffer holds the result
+// of the last stat made anywhere in the process, so an unrelated stat of a FIFO
+// made the walk stop early and hand back the path with its symlinks unresolved.
+// The unresolved path is then cached, so every later resolution repeats it.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+const{ realpathCacheKey }=require('internal/fs/utils');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constpkg=tmpdir.resolve('pkg');
+constlink=tmpdir.resolve('pkg-link');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(pkg,'index.js'),'module.exports = {};\n');
+fs.writeFileSync(tmpdir.resolve('warm.js'),'module.exports = {};\n');
+fs.symlinkSync('pkg',link);
+execFileSync('mkfifo',[fifo]);
+
+constthroughLink=path.join(link,'index.js');
+constthroughReal=path.join(pkg,'index.js');
+
+// The walk only skips a component once something has established it as real. A
+// cache carrying the ancestors is that state, and it is the state the module
+// loader's own cache is in after it has resolved anything else under the
+// directory.
+functionancestorCache(){
+constcache=newMap();
+letdir='';
+for(constpartoftmpdir.path.split(path.sep).slice(1)){
+dir+=path.sep+part;
+cache.set(dir,dir);
+}
+returncache;
+}
+
+fs.statSync(path.join(pkg,'index.js'));
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+fs.statSync(fifo);
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+// What the stale read costs through the module loader, whose cache puts the
+// walk in that same state: the symlink stays unresolved, so the file is loaded
+// a second time under a second name.
+require(tmpdir.resolve('warm.js'));
+fs.statSync(fifo);
+
+assert.strictEqual(require.resolve(throughLink),throughReal);
+assert.strictEqual(require(throughLink),require(throughReal));

lib/fs.js

@@ -81,7 +81,6 @@ const {
 
 const{
  FSReqCallback,
- statValues,
 }=binding;
 const{ toPathIfFileURL }=require('internal/url');
 const{
@@ -3254,6 +3253,11 @@ function realpathSync(p, options) {
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
 constoriginal=p;
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3297,8 +3301,7 @@ function realpathSync(p, options) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)||cache?.get(base)===base){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 break;
 }
 continue;
@@ -3336,7 +3339,9 @@ function realpathSync(p, options) {
 }
 }
 if(linkTarget===null){
-binding.stat(base,false,undefined,true);
+consttargetStats=binding.stat(base,false,undefined,true);
+reachedPipeOrSocket=isFileType(targetStats,S_IFIFO)||
+isFileType(targetStats,S_IFSOCK);
 linkTarget=binding.readlink(base,undefined);
 }
 resolvedLink=pathModule.resolve(previous,linkTarget);
@@ -3418,6 +3423,11 @@ function realpath(p, options, callback) {
 
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3466,8 +3476,7 @@ function realpath(p, options, callback) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 returncallback(null,encodeRealpathResult(p,options));
 }
 returnprocess.nextTick(LOOP);
@@ -3497,9 +3506,11 @@ function realpath(p, options, callback) {
 returngotTarget(null,seenLinks.get(id));
 }
 }
-fs.stat(base,(err)=>{
+fs.stat(base,(err,targetStats)=>{
 if(err)returncallback(err);
 
+reachedPipeOrSocket=targetStats.isFIFO()||targetStats.isSocket();
+
 fs.readlink(base,(err,target)=>{
 if(!isWindows)seenLinks.set(id,target);
 gotTarget(err,target);

test/parallel/test-fs-realpath-async-stale-stat-values.js

@@ -0,0 +1,62 @@
+'use strict';
+
+// The async realpath() reads the shared stat buffer the same way realpathSync()
+// did, to decide whether the walk has reached a pipe or a socket. The walk's
+// own fs.stat() does leave the right value there, but it is not read until
+// after fs.readlink() and a process.nextTick(), and any stat completing in that
+// window replaces it.
+//
+// Truncating the walk only costs something when a second symlink follows the
+// one being resolved, so the path used here has two.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constreal=tmpdir.resolve('real');
+constpkg=tmpdir.resolve('pkg');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(real);
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(real,'index.js'),'');
+fs.symlinkSync(path.join('..','real'),path.join(pkg,'sub'));
+fs.symlinkSync('pkg',tmpdir.resolve('link'));
+execFileSync('mkfifo',[fifo]);
+
+constthroughLinks=tmpdir.resolve('link','sub','index.js');
+constexpected=path.join(real,'index.js');
+
+// Keep stats of the FIFO completing for as long as the walk runs, so that one
+// of them lands in the buffer during the window.
+letsettled=false;
+(functionstatFifo(){
+if(settled)return;
+fs.stat(fifo,statFifo);
+})();
+
+leterror;
+letresolvedPath;
+
+fs.realpath(throughLinks,common.mustCall((err,resolved)=>{
+settled=true;
+error=err;
+resolvedPath=resolved;
+}));
+
+// Asserted on exit rather than in the callback. An assertion that fails inside
+// this callback is lost: it does not reach an `uncaughtException` handler and
+// the process still exits 0, so the test would pass over the bug it covers.
+process.on('exit',()=>{
+assert.ifError(error);
+assert.strictEqual(resolvedPath,expected);
+});

test/parallel/test-fs-realpath-stale-stat-values.js

@@ -0,0 +1,73 @@
+// Flags: --expose-internals
+'use strict';
+
+// Resolving a path must not depend on what was stat'ed before it.
+//
+// While walking a path, realpath skips the components it already knows are
+// real, and in that branch it consulted the shared stat buffer to decide
+// whether the walk had reached a pipe or a socket. That buffer holds the result
+// of the last stat made anywhere in the process, so an unrelated stat of a FIFO
+// made the walk stop early and hand back the path with its symlinks unresolved.
+// The unresolved path is then cached, so every later resolution repeats it.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+const{ realpathCacheKey }=require('internal/fs/utils');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constpkg=tmpdir.resolve('pkg');
+constlink=tmpdir.resolve('pkg-link');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(pkg,'index.js'),'module.exports = {};\n');
+fs.writeFileSync(tmpdir.resolve('warm.js'),'module.exports = {};\n');
+fs.symlinkSync('pkg',link);
+execFileSync('mkfifo',[fifo]);
+
+constthroughLink=path.join(link,'index.js');
+constthroughReal=path.join(pkg,'index.js');
+
+// The walk only skips a component once something has established it as real. A
+// cache carrying the ancestors is that state, and it is the state the module
+// loader's own cache is in after it has resolved anything else under the
+// directory.
+functionancestorCache(){
+constcache=newMap();
+letdir='';
+for(constpartoftmpdir.path.split(path.sep).slice(1)){
+dir+=path.sep+part;
+cache.set(dir,dir);
+}
+returncache;
+}
+
+fs.statSync(path.join(pkg,'index.js'));
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+fs.statSync(fifo);
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+// What the stale read costs through the module loader, whose cache puts the
+// walk in that same state: the symlink stays unresolved, so the file is loaded
+// a second time under a second name.
+require(tmpdir.resolve('warm.js'));
+fs.statSync(fifo);
+
+assert.strictEqual(require.resolve(throughLink),throughReal);
+assert.strictEqual(require(throughLink),require(throughReal));

lib/fs.js

@@ -81,7 +81,6 @@ const {
 
 const{
  FSReqCallback,
- statValues,
 }=binding;
 const{ toPathIfFileURL }=require('internal/url');
 const{
@@ -3254,6 +3253,11 @@ function realpathSync(p, options) {
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
 constoriginal=p;
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3297,8 +3301,7 @@ function realpathSync(p, options) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)||cache?.get(base)===base){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 break;
 }
 continue;
@@ -3336,7 +3339,9 @@ function realpathSync(p, options) {
 }
 }
 if(linkTarget===null){
-binding.stat(base,false,undefined,true);
+consttargetStats=binding.stat(base,false,undefined,true);
+reachedPipeOrSocket=isFileType(targetStats,S_IFIFO)||
+isFileType(targetStats,S_IFSOCK);
 linkTarget=binding.readlink(base,undefined);
 }
 resolvedLink=pathModule.resolve(previous,linkTarget);
@@ -3418,6 +3423,11 @@ function realpath(p, options, callback) {
 
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3466,8 +3476,7 @@ function realpath(p, options, callback) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 returncallback(null,encodeRealpathResult(p,options));
 }
 returnprocess.nextTick(LOOP);
@@ -3497,9 +3506,11 @@ function realpath(p, options, callback) {
 returngotTarget(null,seenLinks.get(id));
 }
 }
-fs.stat(base,(err)=>{
+fs.stat(base,(err,targetStats)=>{
 if(err)returncallback(err);
 
+reachedPipeOrSocket=targetStats.isFIFO()||targetStats.isSocket();
+
 fs.readlink(base,(err,target)=>{
 if(!isWindows)seenLinks.set(id,target);
 gotTarget(err,target);

test/parallel/test-fs-realpath-async-stale-stat-values.js

@@ -0,0 +1,62 @@
+'use strict';
+
+// The async realpath() reads the shared stat buffer the same way realpathSync()
+// did, to decide whether the walk has reached a pipe or a socket. The walk's
+// own fs.stat() does leave the right value there, but it is not read until
+// after fs.readlink() and a process.nextTick(), and any stat completing in that
+// window replaces it.
+//
+// Truncating the walk only costs something when a second symlink follows the
+// one being resolved, so the path used here has two.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constreal=tmpdir.resolve('real');
+constpkg=tmpdir.resolve('pkg');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(real);
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(real,'index.js'),'');
+fs.symlinkSync(path.join('..','real'),path.join(pkg,'sub'));
+fs.symlinkSync('pkg',tmpdir.resolve('link'));
+execFileSync('mkfifo',[fifo]);
+
+constthroughLinks=tmpdir.resolve('link','sub','index.js');
+constexpected=path.join(real,'index.js');
+
+// Keep stats of the FIFO completing for as long as the walk runs, so that one
+// of them lands in the buffer during the window.
+letsettled=false;
+(functionstatFifo(){
+if(settled)return;
+fs.stat(fifo,statFifo);
+})();
+
+leterror;
+letresolvedPath;
+
+fs.realpath(throughLinks,common.mustCall((err,resolved)=>{
+settled=true;
+error=err;
+resolvedPath=resolved;
+}));
+
+// Asserted on exit rather than in the callback. An assertion that fails inside
+// this callback is lost: it does not reach an `uncaughtException` handler and
+// the process still exits 0, so the test would pass over the bug it covers.
+process.on('exit',()=>{
+assert.ifError(error);
+assert.strictEqual(resolvedPath,expected);
+});

test/parallel/test-fs-realpath-stale-stat-values.js

@@ -0,0 +1,73 @@
+// Flags: --expose-internals
+'use strict';
+
+// Resolving a path must not depend on what was stat'ed before it.
+//
+// While walking a path, realpath skips the components it already knows are
+// real, and in that branch it consulted the shared stat buffer to decide
+// whether the walk had reached a pipe or a socket. That buffer holds the result
+// of the last stat made anywhere in the process, so an unrelated stat of a FIFO
+// made the walk stop early and hand back the path with its symlinks unresolved.
+// The unresolved path is then cached, so every later resolution repeats it.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+const{ realpathCacheKey }=require('internal/fs/utils');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constpkg=tmpdir.resolve('pkg');
+constlink=tmpdir.resolve('pkg-link');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(pkg,'index.js'),'module.exports = {};\n');
+fs.writeFileSync(tmpdir.resolve('warm.js'),'module.exports = {};\n');
+fs.symlinkSync('pkg',link);
+execFileSync('mkfifo',[fifo]);
+
+constthroughLink=path.join(link,'index.js');
+constthroughReal=path.join(pkg,'index.js');
+
+// The walk only skips a component once something has established it as real. A
+// cache carrying the ancestors is that state, and it is the state the module
+// loader's own cache is in after it has resolved anything else under the
+// directory.
+functionancestorCache(){
+constcache=newMap();
+letdir='';
+for(constpartoftmpdir.path.split(path.sep).slice(1)){
+dir+=path.sep+part;
+cache.set(dir,dir);
+}
+returncache;
+}
+
+fs.statSync(path.join(pkg,'index.js'));
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+fs.statSync(fifo);
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+// What the stale read costs through the module loader, whose cache puts the
+// walk in that same state: the symlink stays unresolved, so the file is loaded
+// a second time under a second name.
+require(tmpdir.resolve('warm.js'));
+fs.statSync(fifo);
+
+assert.strictEqual(require.resolve(throughLink),throughReal);
+assert.strictEqual(require(throughLink),require(throughReal));

lib/fs.js

@@ -81,7 +81,6 @@ const {
 
 const{
  FSReqCallback,
- statValues,
 }=binding;
 const{ toPathIfFileURL }=require('internal/url');
 const{
@@ -3254,6 +3253,11 @@ function realpathSync(p, options) {
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
 constoriginal=p;
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3297,8 +3301,7 @@ function realpathSync(p, options) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)||cache?.get(base)===base){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 break;
 }
 continue;
@@ -3336,7 +3339,9 @@ function realpathSync(p, options) {
 }
 }
 if(linkTarget===null){
-binding.stat(base,false,undefined,true);
+consttargetStats=binding.stat(base,false,undefined,true);
+reachedPipeOrSocket=isFileType(targetStats,S_IFIFO)||
+isFileType(targetStats,S_IFSOCK);
 linkTarget=binding.readlink(base,undefined);
 }
 resolvedLink=pathModule.resolve(previous,linkTarget);
@@ -3418,6 +3423,11 @@ function realpath(p, options, callback) {
 
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3466,8 +3476,7 @@ function realpath(p, options, callback) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 returncallback(null,encodeRealpathResult(p,options));
 }
 returnprocess.nextTick(LOOP);
@@ -3497,9 +3506,11 @@ function realpath(p, options, callback) {
 returngotTarget(null,seenLinks.get(id));
 }
 }
-fs.stat(base,(err)=>{
+fs.stat(base,(err,targetStats)=>{
 if(err)returncallback(err);
 
+reachedPipeOrSocket=targetStats.isFIFO()||targetStats.isSocket();
+
 fs.readlink(base,(err,target)=>{
 if(!isWindows)seenLinks.set(id,target);
 gotTarget(err,target);

test/parallel/test-fs-realpath-async-stale-stat-values.js

@@ -0,0 +1,62 @@
+'use strict';
+
+// The async realpath() reads the shared stat buffer the same way realpathSync()
+// did, to decide whether the walk has reached a pipe or a socket. The walk's
+// own fs.stat() does leave the right value there, but it is not read until
+// after fs.readlink() and a process.nextTick(), and any stat completing in that
+// window replaces it.
+//
+// Truncating the walk only costs something when a second symlink follows the
+// one being resolved, so the path used here has two.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constreal=tmpdir.resolve('real');
+constpkg=tmpdir.resolve('pkg');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(real);
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(real,'index.js'),'');
+fs.symlinkSync(path.join('..','real'),path.join(pkg,'sub'));
+fs.symlinkSync('pkg',tmpdir.resolve('link'));
+execFileSync('mkfifo',[fifo]);
+
+constthroughLinks=tmpdir.resolve('link','sub','index.js');
+constexpected=path.join(real,'index.js');
+
+// Keep stats of the FIFO completing for as long as the walk runs, so that one
+// of them lands in the buffer during the window.
+letsettled=false;
+(functionstatFifo(){
+if(settled)return;
+fs.stat(fifo,statFifo);
+})();
+
+leterror;
+letresolvedPath;
+
+fs.realpath(throughLinks,common.mustCall((err,resolved)=>{
+settled=true;
+error=err;
+resolvedPath=resolved;
+}));
+
+// Asserted on exit rather than in the callback. An assertion that fails inside
+// this callback is lost: it does not reach an `uncaughtException` handler and
+// the process still exits 0, so the test would pass over the bug it covers.
+process.on('exit',()=>{
+assert.ifError(error);
+assert.strictEqual(resolvedPath,expected);
+});

test/parallel/test-fs-realpath-stale-stat-values.js

@@ -0,0 +1,73 @@
+// Flags: --expose-internals
+'use strict';
+
+// Resolving a path must not depend on what was stat'ed before it.
+//
+// While walking a path, realpath skips the components it already knows are
+// real, and in that branch it consulted the shared stat buffer to decide
+// whether the walk had reached a pipe or a socket. That buffer holds the result
+// of the last stat made anywhere in the process, so an unrelated stat of a FIFO
+// made the walk stop early and hand back the path with its symlinks unresolved.
+// The unresolved path is then cached, so every later resolution repeats it.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+const{ realpathCacheKey }=require('internal/fs/utils');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constpkg=tmpdir.resolve('pkg');
+constlink=tmpdir.resolve('pkg-link');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(pkg,'index.js'),'module.exports = {};\n');
+fs.writeFileSync(tmpdir.resolve('warm.js'),'module.exports = {};\n');
+fs.symlinkSync('pkg',link);
+execFileSync('mkfifo',[fifo]);
+
+constthroughLink=path.join(link,'index.js');
+constthroughReal=path.join(pkg,'index.js');
+
+// The walk only skips a component once something has established it as real. A
+// cache carrying the ancestors is that state, and it is the state the module
+// loader's own cache is in after it has resolved anything else under the
+// directory.
+functionancestorCache(){
+constcache=newMap();
+letdir='';
+for(constpartoftmpdir.path.split(path.sep).slice(1)){
+dir+=path.sep+part;
+cache.set(dir,dir);
+}
+returncache;
+}
+
+fs.statSync(path.join(pkg,'index.js'));
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+fs.statSync(fifo);
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+// What the stale read costs through the module loader, whose cache puts the
+// walk in that same state: the symlink stays unresolved, so the file is loaded
+// a second time under a second name.
+require(tmpdir.resolve('warm.js'));
+fs.statSync(fifo);
+
+assert.strictEqual(require.resolve(throughLink),throughReal);
+assert.strictEqual(require(throughLink),require(throughReal));

lib/fs.js

@@ -81,7 +81,6 @@ const {
 
 const{
  FSReqCallback,
- statValues,
 }=binding;
 const{ toPathIfFileURL }=require('internal/url');
 const{
@@ -3254,6 +3253,11 @@ function realpathSync(p, options) {
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
 constoriginal=p;
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3297,8 +3301,7 @@ function realpathSync(p, options) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)||cache?.get(base)===base){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 break;
 }
 continue;
@@ -3336,7 +3339,9 @@ function realpathSync(p, options) {
 }
 }
 if(linkTarget===null){
-binding.stat(base,false,undefined,true);
+consttargetStats=binding.stat(base,false,undefined,true);
+reachedPipeOrSocket=isFileType(targetStats,S_IFIFO)||
+isFileType(targetStats,S_IFSOCK);
 linkTarget=binding.readlink(base,undefined);
 }
 resolvedLink=pathModule.resolve(previous,linkTarget);
@@ -3418,6 +3423,11 @@ function realpath(p, options, callback) {
 
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3466,8 +3476,7 @@ function realpath(p, options, callback) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 returncallback(null,encodeRealpathResult(p,options));
 }
 returnprocess.nextTick(LOOP);
@@ -3497,9 +3506,11 @@ function realpath(p, options, callback) {
 returngotTarget(null,seenLinks.get(id));
 }
 }
-fs.stat(base,(err)=>{
+fs.stat(base,(err,targetStats)=>{
 if(err)returncallback(err);
 
+reachedPipeOrSocket=targetStats.isFIFO()||targetStats.isSocket();
+
 fs.readlink(base,(err,target)=>{
 if(!isWindows)seenLinks.set(id,target);
 gotTarget(err,target);

test/parallel/test-fs-realpath-async-stale-stat-values.js

@@ -0,0 +1,62 @@
+'use strict';
+
+// The async realpath() reads the shared stat buffer the same way realpathSync()
+// did, to decide whether the walk has reached a pipe or a socket. The walk's
+// own fs.stat() does leave the right value there, but it is not read until
+// after fs.readlink() and a process.nextTick(), and any stat completing in that
+// window replaces it.
+//
+// Truncating the walk only costs something when a second symlink follows the
+// one being resolved, so the path used here has two.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constreal=tmpdir.resolve('real');
+constpkg=tmpdir.resolve('pkg');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(real);
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(real,'index.js'),'');
+fs.symlinkSync(path.join('..','real'),path.join(pkg,'sub'));
+fs.symlinkSync('pkg',tmpdir.resolve('link'));
+execFileSync('mkfifo',[fifo]);
+
+constthroughLinks=tmpdir.resolve('link','sub','index.js');
+constexpected=path.join(real,'index.js');
+
+// Keep stats of the FIFO completing for as long as the walk runs, so that one
+// of them lands in the buffer during the window.
+letsettled=false;
+(functionstatFifo(){
+if(settled)return;
+fs.stat(fifo,statFifo);
+})();
+
+leterror;
+letresolvedPath;
+
+fs.realpath(throughLinks,common.mustCall((err,resolved)=>{
+settled=true;
+error=err;
+resolvedPath=resolved;
+}));
+
+// Asserted on exit rather than in the callback. An assertion that fails inside
+// this callback is lost: it does not reach an `uncaughtException` handler and
+// the process still exits 0, so the test would pass over the bug it covers.
+process.on('exit',()=>{
+assert.ifError(error);
+assert.strictEqual(resolvedPath,expected);
+});

test/parallel/test-fs-realpath-stale-stat-values.js

@@ -0,0 +1,73 @@
+// Flags: --expose-internals
+'use strict';
+
+// Resolving a path must not depend on what was stat'ed before it.
+//
+// While walking a path, realpath skips the components it already knows are
+// real, and in that branch it consulted the shared stat buffer to decide
+// whether the walk had reached a pipe or a socket. That buffer holds the result
+// of the last stat made anywhere in the process, so an unrelated stat of a FIFO
+// made the walk stop early and hand back the path with its symlinks unresolved.
+// The unresolved path is then cached, so every later resolution repeats it.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+const{ realpathCacheKey }=require('internal/fs/utils');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constpkg=tmpdir.resolve('pkg');
+constlink=tmpdir.resolve('pkg-link');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(pkg,'index.js'),'module.exports = {};\n');
+fs.writeFileSync(tmpdir.resolve('warm.js'),'module.exports = {};\n');
+fs.symlinkSync('pkg',link);
+execFileSync('mkfifo',[fifo]);
+
+constthroughLink=path.join(link,'index.js');
+constthroughReal=path.join(pkg,'index.js');
+
+// The walk only skips a component once something has established it as real. A
+// cache carrying the ancestors is that state, and it is the state the module
+// loader's own cache is in after it has resolved anything else under the
+// directory.
+functionancestorCache(){
+constcache=newMap();
+letdir='';
+for(constpartoftmpdir.path.split(path.sep).slice(1)){
+dir+=path.sep+part;
+cache.set(dir,dir);
+}
+returncache;
+}
+
+fs.statSync(path.join(pkg,'index.js'));
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+fs.statSync(fifo);
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+// What the stale read costs through the module loader, whose cache puts the
+// walk in that same state: the symlink stays unresolved, so the file is loaded
+// a second time under a second name.
+require(tmpdir.resolve('warm.js'));
+fs.statSync(fifo);
+
+assert.strictEqual(require.resolve(throughLink),throughReal);
+assert.strictEqual(require(throughLink),require(throughReal));

lib/fs.js

@@ -81,7 +81,6 @@ const {
 
 const{
  FSReqCallback,
- statValues,
 }=binding;
 const{ toPathIfFileURL }=require('internal/url');
 const{
@@ -3254,6 +3253,11 @@ function realpathSync(p, options) {
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
 constoriginal=p;
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3297,8 +3301,7 @@ function realpathSync(p, options) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)||cache?.get(base)===base){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 break;
 }
 continue;
@@ -3336,7 +3339,9 @@ function realpathSync(p, options) {
 }
 }
 if(linkTarget===null){
-binding.stat(base,false,undefined,true);
+consttargetStats=binding.stat(base,false,undefined,true);
+reachedPipeOrSocket=isFileType(targetStats,S_IFIFO)||
+isFileType(targetStats,S_IFSOCK);
 linkTarget=binding.readlink(base,undefined);
 }
 resolvedLink=pathModule.resolve(previous,linkTarget);
@@ -3418,6 +3423,11 @@ function realpath(p, options, callback) {
 
 constseenLinks=newSafeMap();
 constknownHard=newSafeSet();
+// Whether the symlink this walk resolved last pointed at a pipe or a
+// socket, which is where the walk stops. It cannot be read back from the
+// shared stat buffer, which holds the last stat made anywhere in the
+// process rather than the last one made here.
+letreachedPipeOrSocket=false;
 
 // Current character position in p
 letpos;
@@ -3466,8 +3476,7 @@ function realpath(p, options, callback) {
 
 // Continue if not a symlink, break if a pipe/socket
 if(knownHard.has(base)){
-if(isFileType(statValues,S_IFIFO)||
-isFileType(statValues,S_IFSOCK)){
+if(reachedPipeOrSocket){
 returncallback(null,encodeRealpathResult(p,options));
 }
 returnprocess.nextTick(LOOP);
@@ -3497,9 +3506,11 @@ function realpath(p, options, callback) {
 returngotTarget(null,seenLinks.get(id));
 }
 }
-fs.stat(base,(err)=>{
+fs.stat(base,(err,targetStats)=>{
 if(err)returncallback(err);
 
+reachedPipeOrSocket=targetStats.isFIFO()||targetStats.isSocket();
+
 fs.readlink(base,(err,target)=>{
 if(!isWindows)seenLinks.set(id,target);
 gotTarget(err,target);

test/parallel/test-fs-realpath-async-stale-stat-values.js

@@ -0,0 +1,62 @@
+'use strict';
+
+// The async realpath() reads the shared stat buffer the same way realpathSync()
+// did, to decide whether the walk has reached a pipe or a socket. The walk's
+// own fs.stat() does leave the right value there, but it is not read until
+// after fs.readlink() and a process.nextTick(), and any stat completing in that
+// window replaces it.
+//
+// Truncating the walk only costs something when a second symlink follows the
+// one being resolved, so the path used here has two.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constreal=tmpdir.resolve('real');
+constpkg=tmpdir.resolve('pkg');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(real);
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(real,'index.js'),'');
+fs.symlinkSync(path.join('..','real'),path.join(pkg,'sub'));
+fs.symlinkSync('pkg',tmpdir.resolve('link'));
+execFileSync('mkfifo',[fifo]);
+
+constthroughLinks=tmpdir.resolve('link','sub','index.js');
+constexpected=path.join(real,'index.js');
+
+// Keep stats of the FIFO completing for as long as the walk runs, so that one
+// of them lands in the buffer during the window.
+letsettled=false;
+(functionstatFifo(){
+if(settled)return;
+fs.stat(fifo,statFifo);
+})();
+
+leterror;
+letresolvedPath;
+
+fs.realpath(throughLinks,common.mustCall((err,resolved)=>{
+settled=true;
+error=err;
+resolvedPath=resolved;
+}));
+
+// Asserted on exit rather than in the callback. An assertion that fails inside
+// this callback is lost: it does not reach an `uncaughtException` handler and
+// the process still exits 0, so the test would pass over the bug it covers.
+process.on('exit',()=>{
+assert.ifError(error);
+assert.strictEqual(resolvedPath,expected);
+});

test/parallel/test-fs-realpath-stale-stat-values.js

@@ -0,0 +1,73 @@
+// Flags: --expose-internals
+'use strict';
+
+// Resolving a path must not depend on what was stat'ed before it.
+//
+// While walking a path, realpath skips the components it already knows are
+// real, and in that branch it consulted the shared stat buffer to decide
+// whether the walk had reached a pipe or a socket. That buffer holds the result
+// of the last stat made anywhere in the process, so an unrelated stat of a FIFO
+// made the walk stop early and hand back the path with its symlinks unresolved.
+// The unresolved path is then cached, so every later resolution repeats it.
+
+constcommon=require('../common');
+
+if(common.isWindows)
+common.skip('no mkfifo on Windows');
+
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+const{ execFileSync }=require('child_process');
+const{ realpathCacheKey }=require('internal/fs/utils');
+consttmpdir=require('../common/tmpdir');
+
+tmpdir.refresh();
+
+constpkg=tmpdir.resolve('pkg');
+constlink=tmpdir.resolve('pkg-link');
+constfifo=tmpdir.resolve('fifo');
+
+fs.mkdirSync(pkg);
+fs.writeFileSync(path.join(pkg,'index.js'),'module.exports = {};\n');
+fs.writeFileSync(tmpdir.resolve('warm.js'),'module.exports = {};\n');
+fs.symlinkSync('pkg',link);
+execFileSync('mkfifo',[fifo]);
+
+constthroughLink=path.join(link,'index.js');
+constthroughReal=path.join(pkg,'index.js');
+
+// The walk only skips a component once something has established it as real. A
+// cache carrying the ancestors is that state, and it is the state the module
+// loader's own cache is in after it has resolved anything else under the
+// directory.
+functionancestorCache(){
+constcache=newMap();
+letdir='';
+for(constpartoftmpdir.path.split(path.sep).slice(1)){
+dir+=path.sep+part;
+cache.set(dir,dir);
+}
+returncache;
+}
+
+fs.statSync(path.join(pkg,'index.js'));
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+fs.statSync(fifo);
+assert.strictEqual(
+fs.realpathSync(throughLink,{[realpathCacheKey]: ancestorCache()}),
+throughReal,
+);
+
+// What the stale read costs through the module loader, whose cache puts the
+// walk in that same state: the symlink stays unresolved, so the file is loaded
+// a second time under a second name.
+require(tmpdir.resolve('warm.js'));
+fs.statSync(fifo);
+
+assert.strictEqual(require.resolve(throughLink),throughReal);
+assert.strictEqual(require(throughLink),require(throughReal));