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.

Sign up for GitHub

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

Jump to bottom

Resolving symlinks can result in an infinite loop #125

Closed
SuperchupuDev opened this issue Oct 3, 2024 · 6 comments · Fixed by #126
Closed

Resolving symlinks can result in an infinite loop #125

SuperchupuDev opened this issue Oct 3, 2024 · 6 comments · Fixed by #126

Comments

@SuperchupuDev
Copy link
Contributor

If there's a directory symlink somewhere and it points into a parent directory fdir will enter an infinite loop. Other libraries such as fast-glob seem to have an upper limit to avoid this, but I haven't looked at its handling yet
@SuperchupuDev SuperchupuDev mentioned this issue Oct 3, 2024
Closed
@thecodrr
Copy link
Owner

thecodrr commented Oct 4, 2024

Hm. I don't think this should happen at all. Setting an upper limit could work as a hotfix but we should add a proper fix that checks against this.

@thecodrr
Copy link
Owner

thecodrr commented Oct 4, 2024

I think I found a solution. Putting up a PR soon for testing.

@thecodrr
Copy link
Owner

thecodrr commented Oct 4, 2024 •
edited
Loading

@SuperchupuDev What would be the ideal behavior if we crawl /some/dir:

  mock({
    "/some/dir": {
      file: "somefile",
      fileSymlink: mock.symlink({
        path: "/some/dir",
      }),
      another: mock.symlink({ path: "/some/dir/fileSymlink" }),
      another2: mock.symlink({ path: "./fileSymlink" }),
    },
  });

Should we crawl /some/dir only once? Or should we crawl it 3 times (one time for each symlink)?

@thecodrr
Copy link
Owner

thecodrr commented Oct 4, 2024

I tested tree and it does this:

thecodrr@DESKTOP-DS3J1G9:~$ tree -ld test/
test/
├── symdir -> /home/thecodrr/test  [recursive, not followed]
├── symsymdir -> /home/thecodrr/test/symdir  [recursive, not followed]
└── symsymsymdir -> ./symdir  [recursive, not followed]

3 directories

Interesting:

tree -l test_parent/test/
test_parent/test/
├── file
└── sym -> /home/thecodrr/test_parent
    ├── randomchild
    └── test
        ├── file
        └── sym -> /home/thecodrr/test_parent  [recursive, not followed]

3 directories, 3 files

If there's a symlink somewhere deep inside that points to a parent of the directory you are crawling, it gets crawled and you get 2 sets of files. However, if there are 2 symlinks that point to the same parent directory, it bails out:

tree -l test_parent/test/
test_parent/test/
├── file
├── sym -> /home/thecodrr/test_parent
│   ├── randomchild
│   └── test
│       ├── file
│       ├── sym -> /home/thecodrr/test_parent  [recursive, not followed]
│       └── sym2 -> /home/thecodrr/test_parent  [recursive, not followed]
└── sym2 -> /home/thecodrr/test_parent  [recursive, not followed]

5 directories, 3 files

The general policy being that if there are more than one symlinks pointing at the same directory, it will bail out:

tree -l test_parent/test/
test_parent/test/
├── file
├── sym -> /home/thecodrr/test_parent
│   ├── randomchild
│   └── test
│       ├── file
│       ├── sym -> /home/thecodrr/test_parent  [recursive, not followed]
│       ├── sym2 -> /home/thecodrr/test_parent  [recursive, not followed]
│       ├── sym3 -> /home/thecodrr/test2
│       │   └── hello
│       └── sym4 -> /home/thecodrr/test2  [recursive, not followed]
├── sym2 -> /home/thecodrr/test_parent  [recursive, not followed]
├── sym3 -> /home/thecodrr/test2  [recursive, not followed]
└── sym4 -> /home/thecodrr/test2  [recursive, not followed]

9 directories, 4 files

@SuperchupuDev
Copy link
Contributor Author

@SuperchupuDev What would be the ideal behavior if we crawl /some/dir:

  mock({
    "/some/dir": {
      file: "somefile",
      fileSymlink: mock.symlink({
        path: "/some/dir",
      }),
      another: mock.symlink({ path: "/some/dir/fileSymlink" }),
      another2: mock.symlink({ path: "./fileSymlink" }),
    },
  });

Should we crawl /some/dir only once? Or should we crawl it 3 times (one time for each symlink)?

i mean the former sounds better for performance, but both sound like they would do the same in terms of what the user gets returned so i guess both work 🤷‍♀️ whatever you find better to implement

@SuperchupuDev SuperchupuDev mentioned this issue Oct 7, 2024
Closed
@SuperchupuDev
Copy link
Contributor Author

hi @thecodrr, any news on this? do you need help in implementing a fix?

@thecodrr thecodrr mentioned this issue Oct 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix recursive symlinks causing infinite loop thecodrr/fdir
2 participants
@thecodrr @SuperchupuDev