Regression in fs.readdirSync on AIX introduced in 18.17.0

Version

v18.17.0

Platform

AIX huritmdemo 3 7 00F70A434C00

Subsystem

fs

What steps will reproduce the bug?

Create a directory
Create a file under that directory whose name is exactly the same as the directory name
Create a second file under the directory whose name is not the same
Run the following script passing in the path to the folder

const fs = require('node:fs')
const path = require('node:path')

for (const entry of fs.readdirSync(process.argv[2], { withFileTypes: true  })) {
  console.log('Found entry', path.join(process.argv[2], entry.name))
  console.log('Is file?', entry.isFile())
  console.log('Is directory?', entry.isDirectory())
  console.log('')
}

How often does it reproduce? Is there a required condition?

This reproduce is stable and does not depend on the precise name of the file/folder.

What is the expected behavior? Why is that the expected behavior?

The expected behaviour is that the file with the same name as the folder is correctly reported as a file

-bash-5.1$ ls -ld SameNameForFileAndFolder
drwx------    2 will     staff           256 05 Sep 11:10 SameNameForFileAndFolder
-bash-5.1$ ls -l SameNameForFileAndFolder/
total 0
-rw-------    1 will     staff             0 05 Sep 11:10 AnotherEntry
-rw-------    1 will     staff             0 05 Sep 11:09 SameNameForFileAndFolder
-bash-5.1$ ./node-v18.16.1-aix-ppc64/bin/node recreate.js SameNameForFileAndFolder
Found entry SameNameForFileAndFolder/AnotherEntry
Is file? true
Is directory? false

Found entry SameNameForFileAndFolder/SameNameForFileAndFolder
Is file? true
Is directory? false

What do you see instead?

The actual behaviour is that the file with the same name as the folder is incorrectly reported to be a directory

-bash-5.1$ ls -ld SameNameForFileAndFolder
drwx------    2 will     staff           256 05 Sep 11:10 SameNameForFileAndFolder
-bash-5.1$ ls -l SameNameForFileAndFolder/
total 0
-rw-------    1 will     staff             0 05 Sep 11:10 AnotherEntry
-rw-------    1 will     staff             0 05 Sep 11:09 SameNameForFileAndFolder
-bash-5.1$ ./node-v18.17.1-aix-ppc64/bin/node recreate.js SameNameForFileAndFolder
Found entry SameNameForFileAndFolder/AnotherEntry
Is file? true
Is directory? false

Found entry SameNameForFileAndFolder/SameNameForFileAndFolder
Is file? false
Is directory? true

Additional information

No response

Looking at the changelog for 18.17.0 I would expect the root cause to be #41439 but my git bisect is still running...

git bisect confirms that #41439 introduced the issue, specifically commit 7273ef5.

@Ethan-Arrowood

If I revert this specific part of the commit

diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
index 23865845ba..7b88ae5574 100644
--- a/lib/internal/fs/utils.js
+++ b/lib/internal/fs/utils.js
@@ -232,7 +231,7 @@ function join(path, name) {
   }

   if (typeof path === 'string' && typeof name === 'string') {
-    return pathModule.basename(path) === name ? path : pathModule.join(path, name);
+    return pathModule.join(path, name);
   }

Then my issue seems to go away. @Ethan-Arrowood do you recall why this particular change was necessary? The Uint8Array branch of the function unconditionally joins the paths and does not do this conditional checking of the basename.

Well attempting to run the tests shows that there's probably a pretty good reason that this was introduced! 🙃

node:fs:1668
  handleErrorFromBinding(ctx);
  ^

Error: ENOTDIR: not a directory, lstat '/home/will/node/test/addons/.docbuildstamp/.docbuildstamp'
    at Object.lstatSync (node:fs:1668:3)
    at getDirent (node:internal/fs/utils:313:32)
    at Dir.processReadResult (node:internal/fs/dir:154:9)
    at req.oncomplete (node:internal/fs/dir:131:14) {
  errno: -20,
  syscall: 'lstat',
  code: 'ENOTDIR',
  path: '/home/will/node/test/addons/.docbuildstamp/.docbuildstamp'
}

Node.js v18.17.0
make[1]: *** [Makefile:413: test/addons/.buildstamp] Error 1
make: *** [Makefile:330: test] Error 2
-bash-5.1$

I introduced that so that recursive readdir would work. I think I remember AIX being a trouble platform to get it to play well. Look and see what's being returned from lower level calls (such as the bindings). There's a good chance AIX is leaving off valuable info about a file and so the code is making a best guess (and getting it wrong)

I've managed to recreate this issue on macOS/arm64 by forcing the ReadDir and ScanDir native bindings to always return a file type of UV_DIRENT_UNKNOWN, to mimic what happens on AIX. This triggers an extra branch in getDirent() where we need to lstat the file.

My hunch in #49499 (comment) on the suspicious behaviour in join being the root cause was correct, but there was knock on issues that needed to be addressed. Firstly I had to back-port 27cadf5 to the v18.x branch, and then I had to fix up readSyncRecursive and processReadResult in internal/fs/dir.js as they were assuming that the path member of a dirent is the path to the file itself rather than the path to the folder containing the file.

PR #49603 opened with the proposed fix.

fs: fix readdir and opendir recursive with unknown file types

Navigation Menu

Repository navigation

Regression in `fs.readdirSync` on AIX introduced in 18.17.0 #49499

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

Additional information

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Regression in fs.readdirSync on AIX introduced in 18.17.0 #49499

Description

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

Additional information

Activity

lux01 commented on Sep 5, 2023

lux01 commented on Sep 6, 2023

lux01 commented on Sep 7, 2023

lux01 commented on Sep 7, 2023

Ethan-Arrowood commented on Sep 7, 2023

lux01 commented on Sep 11, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Regression in `fs.readdirSync` on AIX introduced in 18.17.0 #49499