Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

withFileTypes has different behavior in fs.readdir when reading symbolic directories #52663

Closed
kylo5aby opened this issue Apr 24, 2024 · 3 comments · Fixed by #55714
Closed
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@kylo5aby
Copy link
Contributor

Version

21.6.0

Platform

Linux 5.15.0-41-generic #44-Ubuntu x86_64 GNU/Linux

Subsystem

fs

What steps will reproduce the bug?

For the directory tree below

dir
├── dir1
│   └── 1.txt
└── dir1_s  -> dir1

When using fs.readdir or fs.readdirSync to read the contents of dir, there is a difference in the number of contents outputted by the two:

const res = fs.readdirSync('./dir', {recursive: true, withFileTypes: true});
console.log(res);
Output:
// [
//   Dirent {
//     name: 'dir1',
//     parentPath: './dir',
//     path: './dir',
//     [Symbol(type)]: 2
//   },
//   Dirent {
//     name: 'dir1_s',
//     parentPath: './dir',
//     path: './dir',
//     [Symbol(type)]: 3
//   },
//   Dirent {
//     name: '1.txt',
//     parentPath: 'dir/dir1',
//     path: 'dir/dir1',
//     [Symbol(type)]: 1
//   }
// ]
const res = fs.readdirSync('./dir', {recursive: true, withFileTypes: false});
console.log(res);
// Output:
// [ 'dir1', 'dir1_s', 'dir1/1.txt', 'dir1_s/1.txt' ]

So do fs.readdir.

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

Always

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

No response

What do you see instead?

when withFileTypes: true, the directory entry dir1_s/1.txt haven't been outputted. However, I did not find any description in the documentation regarding the difference in handling symbolic directories between the two.

Additional information

I found that the reason is because the two use different conditions to determine whether to continue traversal. When {withFileTypes:true}. it use

node/lib/fs.js

Line 1420 in 91dc8c9

if (dirent.isDirectory()) {

As described by class Dirent, type "directory" and "symbolic link" of dirent are mutually exclusive.

When {withFileTypes:false}, is use

node/lib/fs.js

Line 1428 in 91dc8c9

const stat = binding.internalModuleStat(resultPath);

Contrary to {withFileTypes:true}, "directory" and "symbolic link" are compatible, and then allows traversal to continue into 1.txt after encountering the symbolic directory dir1_s.

I'm not sure if this is work as expected. If it's not, I believe maintaining consistency with the results of {withFileTypes: false} would be preferable, aligning with the result of find -L [path].

@marco-ippolito marco-ippolito added the fs Issues and PRs related to the fs subsystem / file system. label Apr 24, 2024
@benjamingr
Copy link
Member

@nodejs/fs

@juanarbol
Copy link
Member

I'll take a look. Thanks for the extra information.

@juanarbol
Copy link
Member

juanarbol commented Nov 4, 2024

This was introduced by #48698. I'm still working on this one

juanarbol added a commit to juanarbol/node that referenced this issue Nov 4, 2024
juanarbol added a commit to juanarbol/node that referenced this issue Nov 14, 2024
juanarbol added a commit to juanarbol/node that referenced this issue Nov 14, 2024
juanarbol added a commit to juanarbol/node that referenced this issue Nov 17, 2024
Ceres6 pushed a commit to Ceres6/node that referenced this issue Nov 26, 2024
Fixes: nodejs#52663
Signed-off-by: Juan José Arboleda <[email protected]>
PR-URL: nodejs#55714
Reviewed-By: Ethan Arrowood <[email protected]>
Reviewed-By: James M Snell <[email protected]>
aduh95 pushed a commit that referenced this issue Nov 26, 2024
Fixes: #52663
Signed-off-by: Juan José Arboleda <[email protected]>
PR-URL: #55714
Reviewed-By: Ethan Arrowood <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants