Modify

Ticket #5652 (closed Bugs: fixed)

Opened 3 years ago

Last modified 2 years ago

recursive_directory_iterator fails on cyclic symbolic links

Reported by: Geurt Vos <geurt.vos@…> Owned by: bemandawes
Milestone: Boost 1.49.0 Component: filesystem
Version: Boost 1.46.1 Severity: Problem
Keywords: Cc: macbishop@…

Description

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

Attachments

bug5652.diff Download (815 bytes) - added by Daniel Aarno <macbishop@…> 3 years ago.
Fix for bug 5652: Check if file is symbolic link before stat:ing it during recursion (an not the other way around)
bug5652-perf-opt.diff Download (833 bytes) - added by Daniel Aarno <macbishop@…> 3 years ago.
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

Change History

Changed 3 years ago by 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)

comment:1 follow-up: ↓ 4 Changed 3 years ago by 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.

comment:2 Changed 3 years ago by Daniel Aarno <macbishop@…>

  • Cc macbishop@… added

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.

Changed 3 years ago by 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

comment:3 Changed 2 years ago by benjamin.kircher@…

Just walked into this with 1.48. +1

comment:4 in reply to: ↑ 1 ; follow-up: ↓ 5 Changed 2 years ago by bemandawes

  • Status changed from new to assigned

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

comment:5 in reply to: ↑ 4 Changed 2 years ago by 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.

comment:6 Changed 2 years ago by bemandawes

  • Status changed from assigned to closed
  • Resolution set to fixed
  • Milestone changed from To Be Determined to Boost 1.49.0

Fixed by changeset 76556.

Thanks, all!

--Beman

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.