recursive_directory_iterator fails on cyclic symbolic links

[Geurt Vos <geurt.vos@…>]

recursive_directory_iterator throws 'Too many levels of symbolic links' on bad, yet correct symbolic links. To reproduce on Linux:

$ ln -s mybadlink mybadlink

iterating through directory containing 'mybadlink' (with symlink_option::none) results in exception:

boost::filesystem::status: Too many levels of symbolic links: "./mybadlink"

Also tested on boost 1.47.0-beta1

[Daniel Aarno <macbishop@…>]

Fix for bug 5652: Check if file is symbolic link before stat:ing it during recursion (an not the other way around)

[Daniel Aarno <macbishop@…>]

I can confirm this issue and there is an easy fix. To reproduce and test the issue the following simple program that counts the files (recursively) in a directory can be used on a directory containing the "symlink to self" as described by Geurt.

#include <string> #include <iostream>

#include <boost/filesystem.hpp>

int ftw(const std::string &root) {

using namespace boost::filesystem; typedef recursive_directory_iterator walker; int count = 0; for (walker w(root); w != walker(); ++w)

++count;

return count;

}

int main (int argc, const char argv) {

int count = ftw(argv[1]); std::cout << count << std::endl;

}

The problem is in the increment method in recur_dir_itr_imp. It checks the file with is_directory, which will end up in a "stat" call on the file. Of course if the file is a symlink to itself stat will try to follow the link indefinitely (since it will try to deference the symlink until a real file is found). This causes stat to generate an ELOOP error and the exception is thrown.

The solution is to not check if the file is a directory if it is a symlink - unless of course we are following symlinks during recursion, in which case the exception should occur. We can test this by adding symlink_option::recurse as an option to the walker constructor in the above example.

[Daniel Aarno <macbishop@…>]

For efficiency reasons it would also be a good idea to first check for symlink_option::recurse and *then* for is_symlink, since if we are following symlinks we will not have to do the (expensive) call to lstat and we will only do an integer comparision.

[Daniel Aarno <macbishop@…>]

Performance optimized patch for bug 5652: Check if file is symbolic link before stat:ing it during recursion (an not the other way around) but first check if we are following symlinks

[benjamin.kircher@…]

Just walked into this with 1.48. +1

[Beman Dawes]

Replying to Daniel Aarno <macbishop@…>:

I can confirm this issue and there is an easy fix. To reproduce and test the issue the following simple program that counts the files (recursively) in a directory can be used on a directory containing the "symlink to self" as described by Geurt.

#include <string> #include <iostream>

#include <boost/filesystem.hpp>

int ftw(const std::string &root) {

using namespace boost::filesystem; typedef recursive_directory_iterator walker; int count = 0; for (walker w(root); w != walker(); ++w)

++count;

return count;

}

int main (int argc, const char argv) {

int count = ftw(argv[1]); std::cout << count << std::endl;

}

Thanks for the test program! Providing a test program is a very effective way to communicative exactly what problem you are having, and prevents me from chasing my tail around in circles.

On Windows the program throws with a message "The name of the file cannot be resolved by the system". I haven't looked at your patch yet, but wanted to thank you for the test case first,

--Beman

[macbishop@…]

Replying to bemandawes:

On Windows the program throws with a message "The name of the file cannot be resolved by the system". I haven't looked at your patch yet, but wanted to thank you for the test case first

Thanks for looking into this. To clarify, the test case takes as its first argument a path to a directory containing the recursive symlink, so you need to:

1) Setup a directory with a symlink that points to itself (not sure how to do it on windows) 2) Then run the test case, with the path to the directory given as the only argument, e.g. ./a.out thedir

Let me know if you have any questions or suggestions on the test-case or patches.

[Beman Dawes]

Fixed by changeset 76556.

Thanks, all!

--Beman