Opened 11 years ago

Closed 8 years ago

#6821 closed Bugs (fixed)

recursive_directory_iterator: increment fails with 'access denied'

Reported by: Gary Sanders <lex21@…> Owned by: Beman Dawes
Milestone: To Be Determined Component: filesystem
Version: Boost 1.55.0 Severity: Showstopper
Keywords: Cc:

Description

This issue is found on Windows 7 x64, Win32 binary built with Visual Studio 2010. See attachment for sample program.

Recursive iteration fails when iterating a directory that contains a child directory for which the user does not have permission to access. The attached program illustrates this issue, assuming that the user has at least one directory that the current users does not have permission to access:

boost::filesystem::directory_iterator::construct: Access is denied: 
"c:\$Recycle.Bin\S-1-5-20"

The exception is caused by FindFirstFileW() returning ERROR_ACCESS_DENIED. This function is located in dir_itr_first(), filesystem/v3/src/operations.cpp, line 2000.

Intuitively this iteration should work and any directories for which the user does not have permission to access should be ignored. Though, perhaps there are at least two use cases for this behaviour when encountering a directory for which the user does not have permission to access:

  • Iteration is not expected to fail with an 'access denied' error, so if it encounters this situation throw an error. (The current behaviour.)
  • It is unknown if the directory being recursively iterated contains one or more directories for which the user does not have access. Therefore, these directories should be ignored and processing allowed to continue.

One solution might be to allow a flag to be passed in the constructor that would define which of the two behaviours to invoke.

Attachments (3)

fs_bug.cpp (278 bytes ) - added by Gary Sanders <lex21@…> 11 years ago.
source code
diff.txt (638 bytes ) - added by Gary Sanders <lex21@…> 10 years ago.
access_denied.patch (618 bytes ) - added by Gary Sanders <lex21@…> 10 years ago.
svn patch file

Download all attachments as: .zip

Change History (10)

by Gary Sanders <lex21@…>, 11 years ago

Attachment: fs_bug.cpp added

source code

by Gary Sanders <lex21@…>, 10 years ago

Attachment: diff.txt added

comment:1 by Gary Sanders <lex21@…>, 10 years ago

Attached is a proposed one line code change that will fix the issue described without changing or introducing functionality.

The diff is created from operations.cpp, trunk rev 78220.

by Gary Sanders <lex21@…>, 10 years ago

Attachment: access_denied.patch added

svn patch file

comment:2 by Beman Dawes, 10 years ago

Status: newassigned

You suggestion of adding an option to treat permission denied as an empty directory seems to me to be the best resolution, since the desired behavior is very application dependent.

I've implemented and tested that for Windows, and am satisfied with the results. This work has been stashed until 1.50.0 is out the door, when I'll resume work on it.

Thanks,

--Beman

comment:3 by Jesse Jarrell <jjarrell@…>, 10 years ago

The proposed fix only addresses when the first entry is a directory and returns an access denied code. The POSIX code handles this situation nicely with the iterator being set to EOF. However, if there are more entries you get an internal error.

Here is the test directory structure I used to reproduce this as user anonymous:

drwxrwxr-x 5 anonymous anonymous 4096 Aug 28 20:45 ./
drwxrwxr-x 8 anonymous anonymous 4096 Aug 28 20:14 ../
drwx------ 2 root      root      4096 Aug 28 15:30 root_noaccess/
drwxrwxr-x 2 root      anonymous 4096 Aug 28 15:28 root_subdir/
drwxrwxr-x 2 anonymous anonymous 4096 Aug 28 20:14 xfiles/

#include <boost/config/warning_disable.hpp>
#include <boost/filesystem.hpp>
#include <iostream>
#include <iterator>

using namespace std;

int main(int argc, char* argv[]) {
	boost::system::error_code dir_error;

	for ( boost::filesystem::recursive_directory_iterator end, dir(".", dir_error); dir != end; dir.increment(dir_error) ) {
		if (dir_error.value()) {
			cerr << "Error accessing file: " << dir_error.message() << endl;
		}else{
			cout << dir->path() << endl;
		}
	}
	return 0;
}

Output:

anonymous@ubuntu:~/perm_test$ ./test_perms 
"./root_noaccess"
Error accessing file: Permission denied
***** Internal Program Error - assertion (m_imp.get()) failed in boost::filesystem::directory_entry& boost::filesystem::directory_iterator::dereference() const:
/usr/local/include/boost/filesystem/operations.hpp(714): attempt to dereference end iterator
Aborted (core dumped)

The expected outcome would be for the iterator to continue on to root_subdir since the program has no access to the directory "./root_noaccess".

comment:4 by Jesse Jarrell <jjarrell@…>, 10 years ago

Found ticket #5403 that has more details and a workaround for the problems with increment.

comment:5 by Claudio Bley, 10 years ago

Actually, this is a regression in regard to v2.

In v2 you could use no_push() in order to skip a directory after an error occurred.

With v3, the directory_iterator at the top of the stack is invalid (ie. an end iterator) if an error occurs. So, you cannot dereference it, and you cannot use any other method (e.g. no_push) on it, otherwise std::abort will be called.

Simple fix: just don't push an end iterator to m_stack.

--- C:\Users\Claudio\src\boost.svn\boost\filesystem\operations.hpp      2012-07-24 08:43:28 +0200
+++ boost\filesystem\operations.hpp     2013-03-25 17:21:39 +0100
@@ -791,9 +791,11 @@
             m_stack.push(directory_iterator(m_stack.top()->path()));
           else
           {
-            m_stack.push(directory_iterator(m_stack.top()->path(), *ec));
+            directory_iterator next = directory_iterator(m_stack.top()->path(), *ec);
             if (*ec)
               return;
+            else
+              m_stack.push(next);
           }
           if (m_stack.top() != directory_iterator())
           {

comment:6 by rose.branston@…, 8 years ago

Severity: ProblemShowstopper
Version: Boost 1.49.0Boost 1.55.0

I have encountered the same problem on ubuntu except I don't get the "Permission Denied" message or an exception thrown, it just aborts.

comment:7 by Beman Dawes, 8 years ago

Resolution: fixed
Status: assignedclosed

recursive_directory_iterator::increment implementation code has been reorganized, adding an invariant that progress is always made even if an error is reported by exception or error_code. Thanks to Claudio Bley for his pull request, which was incorporated into the reorganized code.

Tickets 5403 and 6821 have been closed. Please open a new issue and supply test cases if you continue to experience problems.

Note: See TracTickets for help on using tickets.