Skip to content

Commit 2b35bc8

Browse files
LastiqueLastique
committed
Check if the handle refers to a directory in dir_itr_create on Windows.
Also, make a more robust check whether the handle refers to a symlink in case if GetFileInformationByHandleEx is not available. If it is not a symlink, but some other type of a reparse point, continue processing it as if it is a regular directory. Also, in remove_all_impl, check whether creating a directory iterator fails due to the file not being a directory. Interpret this the same way as with ELOOP - the error indicates that the directory was replaced with some other kind of file between querying its type and creating the directory iterator. Retry the operation.
1 parent f803579 commit 2b35bc8

File tree

3 files changed

+79
-56
lines changed

3 files changed

+79
-56
lines changed

‎src/directory.cpp

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
402402

403403
boost::intrusive_ptr< detail::dir_itr_imp > pimpl(new (extra_size) detail::dir_itr_imp());
404404
if (BOOST_UNLIKELY(!pimpl))
405-
return make_error_code(boost::system::errc::not_enough_memory);
405+
return make_error_code(system::errc::not_enough_memory);
406406

407407
#if defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW)
408408
int flags = O_DIRECTORY | O_RDONLY | O_NONBLOCK | O_CLOEXEC;
@@ -755,7 +755,7 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
755755
{
756756
boost::intrusive_ptr< detail::dir_itr_imp > pimpl(new (dir_itr_extra_size) detail::dir_itr_imp());
757757
if (BOOST_UNLIKELY(!pimpl))
758-
return make_error_code(boost::system::errc::not_enough_memory);
758+
return make_error_code(system::errc::not_enough_memory);
759759

760760
DWORD flags = FILE_FLAG_BACKUP_SEMANTICS;
761761
if ((opts & static_cast< unsigned int >(directory_options::_detail_no_follow)) != 0u)
@@ -768,28 +768,37 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
768768
return error_code(error, system_category());
769769
}
770770

771-
if ((opts & static_cast< unsigned int >(directory_options::_detail_no_follow)) != 0u)
771+
GetFileInformationByHandleEx_t* get_file_information_by_handle_ex = filesystem::detail::atomic_load_relaxed(get_file_information_by_handle_ex_api);
772+
if (BOOST_LIKELY(get_file_information_by_handle_ex != NULL))
772773
{
773-
GetFileInformationByHandleEx_t* get_file_information_by_handle_ex = filesystem::detail::atomic_load_relaxed(get_file_information_by_handle_ex_api);
774-
if (BOOST_LIKELY(get_file_information_by_handle_ex != NULL))
775-
{
776-
file_attribute_tag_info info;
777-
BOOL res = get_file_information_by_handle_ex(h.handle, file_attribute_tag_info_class, &info, sizeof(info));
778-
if (BOOST_UNLIKELY(!res))
779-
goto return_last_error;
774+
file_attribute_tag_info info;
775+
BOOL res = get_file_information_by_handle_ex(h.handle, file_attribute_tag_info_class, &info, sizeof(info));
776+
if (BOOST_UNLIKELY(!res))
777+
goto return_last_error;
778+
779+
if (BOOST_UNLIKELY((info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0u))
780+
return make_error_code(system::errc::not_a_directory);
780781

782+
if ((opts & static_cast< unsigned int >(directory_options::_detail_no_follow)) != 0u)
783+
{
781784
if ((info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0u && is_reparse_point_tag_a_symlink(info.ReparseTag))
782-
return make_error_code(boost::system::errc::too_many_symbolic_link_levels);
785+
return make_error_code(system::errc::too_many_symbolic_link_levels);
783786
}
784-
else
785-
{
786-
BY_HANDLE_FILE_INFORMATION info;
787-
BOOL res = ::GetFileInformationByHandle(h.handle, &info);
788-
if (BOOST_UNLIKELY(!res))
789-
goto return_last_error;
787+
}
788+
else
789+
{
790+
BY_HANDLE_FILE_INFORMATION info;
791+
BOOL res = ::GetFileInformationByHandle(h.handle, &info);
792+
if (BOOST_UNLIKELY(!res))
793+
goto return_last_error;
790794

791-
if ((info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0u)
792-
return make_error_code(boost::system::errc::too_many_symbolic_link_levels);
795+
if (BOOST_UNLIKELY((info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0u))
796+
return make_error_code(system::errc::not_a_directory);
797+
798+
if ((opts & static_cast< unsigned int >(directory_options::_detail_no_follow)) != 0u)
799+
{
800+
if ((info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0u && is_reparse_point_a_symlink_ioctl(h.handle))
801+
return make_error_code(system::errc::too_many_symbolic_link_levels);
793802
}
794803
}
795804

@@ -798,7 +807,7 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
798807
{
799808
case file_id_extd_dir_info_format:
800809
{
801-
if (!filesystem::detail::atomic_load_relaxed(get_file_information_by_handle_ex_api)(h.handle, file_id_extd_directory_restart_info_class, extra_data, dir_itr_extra_size))
810+
if (!get_file_information_by_handle_ex(h.handle, file_id_extd_directory_restart_info_class, extra_data, dir_itr_extra_size))
802811
{
803812
DWORD error = ::GetLastError();
804813

@@ -832,7 +841,7 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
832841
case file_full_dir_info_format:
833842
fallback_to_file_full_dir_info_format:
834843
{
835-
if (!filesystem::detail::atomic_load_relaxed(get_file_information_by_handle_ex_api)(h.handle, file_full_directory_restart_info_class, extra_data, dir_itr_extra_size))
844+
if (!get_file_information_by_handle_ex(h.handle, file_full_directory_restart_info_class, extra_data, dir_itr_extra_size))
836845
{
837846
DWORD error = ::GetLastError();
838847

@@ -861,7 +870,7 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
861870
case file_id_both_dir_info_format:
862871
fallback_to_file_id_both_dir_info:
863872
{
864-
if (!filesystem::detail::atomic_load_relaxed(get_file_information_by_handle_ex_api)(h.handle, file_id_both_directory_restart_info_class, extra_data, dir_itr_extra_size))
873+
if (!get_file_information_by_handle_ex(h.handle, file_id_both_directory_restart_info_class, extra_data, dir_itr_extra_size))
865874
{
866875
DWORD error = ::GetLastError();
867876

@@ -968,7 +977,7 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
968977
{
969978
boost::intrusive_ptr< detail::dir_itr_imp > pimpl(new (static_cast< std::size_t >(0u)) detail::dir_itr_imp());
970979
if (BOOST_UNLIKELY(!pimpl))
971-
return make_error_code(boost::system::errc::not_enough_memory);
980+
return make_error_code(system::errc::not_enough_memory);
972981

973982
// use a form of search Sebastian Martel reports will work with Win98
974983
fs::path dirpath(dir);
@@ -1086,7 +1095,7 @@ void directory_iterator_construct(directory_iterator& it, path const& p, unsigne
10861095
if (!ec)
10871096
throw;
10881097

1089-
*ec = make_error_code(boost::system::errc::not_enough_memory);
1098+
*ec = make_error_code(system::errc::not_enough_memory);
10901099
it.m_imp.reset();
10911100
}
10921101
}
@@ -1143,7 +1152,7 @@ void directory_iterator_increment(directory_iterator& it, system::error_code* ec
11431152
throw;
11441153

11451154
it.m_imp.reset();
1146-
*ec = make_error_code(boost::system::errc::not_enough_memory);
1155+
*ec = make_error_code(system::errc::not_enough_memory);
11471156
}
11481157
}
11491158

‎src/operations.cpp

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ void init_directory_iterator_impl() BOOST_NOEXCEPT;
252252

253253
namespace {
254254

255-
// The number of retries remove_all should make if it detects that the directory it is about to enter has been replaced with a symlink
255+
// The number of retries remove_all should make if it detects that the directory it is about to enter has been replaced with a symlink or a regular file
256256
BOOST_CONSTEXPR_OR_CONST unsigned int remove_all_directory_replaced_retry_count = 5u;
257257

258258
#if defined(BOOST_POSIX_API)
@@ -876,6 +876,7 @@ inline bool remove_impl(path const& p, error_code* ec)
876876
//! remove_all() implementation
877877
uintmax_t remove_all_impl(path const& p, error_code* ec)
878878
{
879+
error_code dit_create_ec;
879880
for (unsigned int attempt = 0u; attempt < remove_all_directory_replaced_retry_count; ++attempt)
880881
{
881882
fs::file_type type;
@@ -900,23 +901,25 @@ uintmax_t remove_all_impl(path const& p, error_code* ec)
900901
if (type == fs::directory_file) // but not a directory symlink
901902
{
902903
fs::directory_iterator itr;
903-
error_code local_ec;
904-
fs::detail::directory_iterator_construct(itr, p, static_cast< unsigned int >(directory_options::_detail_no_follow), &local_ec);
904+
fs::detail::directory_iterator_construct(itr, p, static_cast< unsigned int >(directory_options::_detail_no_follow), &dit_create_ec);
905905
if (BOOST_UNLIKELY(!!local_ec))
906906
{
907+
if (dit_create_ec == error_code(ENOTDIR, system_category()))
908+
continue;
909+
907910
#if defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW)
908911
// If open(2) with O_NOFOLLOW fails with ELOOP, this means that either the path contains a loop
909912
// of symbolic links, or the last element of the path is a symbolic link. Given that lstat(2) above
910913
// did not fail, most likely it is the latter case. I.e. between the lstat above and this open call
911914
// the filesystem was modified so that the path no longer refers to a directory file (as opposed to a symlink).
912-
if (local_ec == error_code(ELOOP, system_category()))
915+
if (dit_create_ec == error_code(ELOOP, system_category()))
913916
continue;
914917
#endif // defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW)
915918

916919
if (!ec)
917-
BOOST_FILESYSTEM_THROW(filesystem_error("boost::filesystem::remove_all", p, local_ec));
920+
BOOST_FILESYSTEM_THROW(filesystem_error("boost::filesystem::remove_all", p, dit_create_ec));
918921

919-
*ec = local_ec;
922+
*ec = dit_create_ec;
920923
return static_cast< uintmax_t >(-1);
921924
}
922925

@@ -940,7 +943,10 @@ uintmax_t remove_all_impl(path const& p, error_code* ec)
940943
return count;
941944
}
942945

943-
emit_error(ELOOP, p, ec, "boost::filesystem::remove_all: path cannot be opened as a directory");
946+
if (!ec)
947+
BOOST_FILESYSTEM_THROW(filesystem_error("boost::filesystem::remove_all: path cannot be opened as a directory", p, dit_create_ec));
948+
949+
*ec = dit_create_ec;
944950
return static_cast< uintmax_t >(-1);
945951
}
946952

@@ -1063,13 +1069,29 @@ inline void to_FILETIME(std::time_t t, FILETIME& ft) BOOST_NOEXCEPT
10631069
ft.dwHighDateTime = static_cast< DWORD >(temp >> 32);
10641070
}
10651071

1072+
} // unnamed namespace
1073+
1074+
bool is_reparse_point_a_symlink_ioctl(HANDLE h)
1075+
{
1076+
boost::scoped_ptr< reparse_data_buffer_with_storage > buf(new reparse_data_buffer_with_storage);
1077+
1078+
// Query the reparse data
1079+
DWORD dwRetLen = 0u;
1080+
BOOL result = ::DeviceIoControl(h, FSCTL_GET_REPARSE_POINT, NULL, 0, buf.get(), sizeof(*buf), &dwRetLen, NULL);
1081+
if (BOOST_UNLIKELY(!result))
1082+
return false;
1083+
1084+
return is_reparse_point_tag_a_symlink(buf->rdb.ReparseTag);
1085+
}
1086+
1087+
namespace {
1088+
10661089
inline bool is_reparse_point_a_symlink(path const& p)
10671090
{
10681091
handle_wrapper h(create_file_handle(p, 0u, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT));
10691092
if (BOOST_UNLIKELY(h.handle == INVALID_HANDLE_VALUE))
10701093
return false;
10711094

1072-
ULONG reparse_point_tag;
10731095
GetFileInformationByHandleEx_t* get_file_information_by_handle_ex = filesystem::detail::atomic_load_relaxed(get_file_information_by_handle_ex_api);
10741096
if (BOOST_LIKELY(get_file_information_by_handle_ex != NULL))
10751097
{
@@ -1081,22 +1103,10 @@ inline bool is_reparse_point_a_symlink(path const& p)
10811103
if ((info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0u)
10821104
return false;
10831105

1084-
reparse_point_tag = info.ReparseTag;
1106+
return is_reparse_point_tag_a_symlink(info.ReparseTag);
10851107
}
1086-
else
1087-
{
1088-
boost::scoped_ptr< reparse_data_buffer_with_storage > buf(new reparse_data_buffer_with_storage);
10891108

1090-
// Query the reparse data
1091-
DWORD dwRetLen = 0u;
1092-
BOOL result = ::DeviceIoControl(h.handle, FSCTL_GET_REPARSE_POINT, NULL, 0, buf.get(), sizeof(*buf), &dwRetLen, NULL);
1093-
if (BOOST_UNLIKELY(!result))
1094-
return false;
1095-
1096-
reparse_point_tag = buf->rdb.ReparseTag;
1097-
}
1098-
1099-
return is_reparse_point_tag_a_symlink(reparse_point_tag);
1109+
return is_reparse_point_a_symlink_ioctl(h.handle);
11001110
}
11011111

11021112
inline std::size_t get_full_path_name(path const& src, std::size_t len, wchar_t* buf, wchar_t** p)
@@ -1220,6 +1230,7 @@ inline bool remove_impl(path const& p, error_code* ec)
12201230
//! remove_all() implementation
12211231
uintmax_t remove_all_impl(path const& p, error_code* ec)
12221232
{
1233+
error_code dit_create_ec;
12231234
for (unsigned int attempt = 0u; attempt < remove_all_directory_replaced_retry_count; ++attempt)
12241235
{
12251236
const DWORD attrs = ::GetFileAttributesW(p.c_str());
@@ -1254,17 +1265,19 @@ uintmax_t remove_all_impl(path const& p, error_code* ec)
12541265
if (recurse)
12551266
{
12561267
fs::directory_iterator itr;
1257-
error_code local_ec;
1258-
fs::detail::directory_iterator_construct(itr, p, static_cast< unsigned int >(directory_options::_detail_no_follow), &local_ec);
1259-
if (BOOST_UNLIKELY(!!local_ec))
1268+
fs::detail::directory_iterator_construct(itr, p, static_cast< unsigned int >(directory_options::_detail_no_follow), &dit_create_ec);
1269+
if (BOOST_UNLIKELY(!!dit_create_ec))
12601270
{
1261-
if (local_ec == make_error_condition(system::errc::too_many_symbolic_link_levels))
1271+
if (dit_create_ec == make_error_condition(system::errc::not_a_directory) ||
1272+
dit_create_ec == make_error_condition(system::errc::too_many_symbolic_link_levels))
1273+
{
12621274
continue;
1275+
}
12631276

12641277
if (!ec)
1265-
BOOST_FILESYSTEM_THROW(filesystem_error("boost::filesystem::remove_all", p, local_ec));
1278+
BOOST_FILESYSTEM_THROW(filesystem_error("boost::filesystem::remove_all", p, dit_create_ec));
12661279

1267-
*ec = local_ec;
1280+
*ec = dit_create_ec;
12681281
return static_cast< uintmax_t >(-1);
12691282
}
12701283

@@ -1288,11 +1301,10 @@ uintmax_t remove_all_impl(path const& p, error_code* ec)
12881301
return count;
12891302
}
12901303

1291-
error_code local_ec(make_error_code(system::errc::too_many_symbolic_link_levels));
12921304
if (!ec)
1293-
BOOST_FILESYSTEM_THROW(filesystem_error("boost::filesystem::remove_all: path cannot be opened as a directory", p, local_ec));
1305+
BOOST_FILESYSTEM_THROW(filesystem_error("boost::filesystem::remove_all: path cannot be opened as a directory", p, dit_create_ec));
12941306

1295-
*ec = local_ec;
1307+
*ec = dit_create_ec;
12961308
return static_cast< uintmax_t >(-1);
12971309
}
12981310

‎src/windows_tools.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ inline boost::filesystem::perms make_permissions(boost::filesystem::path const&
6262
return prms;
6363
}
6464

65+
bool is_reparse_point_a_symlink_ioctl(HANDLE h);
66+
6567
inline bool is_reparse_point_tag_a_symlink(ULONG reparse_point_tag)
6668
{
6769
return reparse_point_tag == IO_REPARSE_TAG_SYMLINK

0 commit comments

Comments
 (0)