From 5d16e6bd00d1e75be52a319bd8ad969a333d1e0f Mon Sep 17 00:00:00 2001
From: Andrey Semashev <andrey.semashev@gmail.com>
Date: Tue, 18 Jun 2024 21:03:51 +0300
Subject: [PATCH] Fixed file_size and is_empty for symlinks on Windows.
 Reworked is_empty.

GetFileAttributesExW that was used to implement file_size and is_empty
on Windows returns information about the symlink rather than the file
the symlink refers to. Fix this by opening the file and using
GetFileInformationByHandle to obtain the file size and attributes.

Additionally, reworked is_empty implementation to reuse the file handle
(and fd on POSIX systems) to create the directory iterator if the
operation is invoked on a directory. On POSIX systems, implement a
more lightweight version of is_empty_directory when readdir is safe
to use. Reusing the file handle/fd improves protection against
filesystem races, when the file that is being tested by is_empty
is initially a directory and then, when we create a directory
iterator, it is not.

Fixes https://github.com/boostorg/filesystem/issues/313.
---
 doc/release_history.html |   7 ++
 src/directory.cpp        |  94 +++++++++++++++++++++-
 src/operations.cpp       | 164 +++++++++++++++++++++++++++------------
 test/operations_test.cpp | 101 ++++++++++++++++++++++++
 4 files changed, 315 insertions(+), 51 deletions(-)

diff --git a/doc/release_history.html b/doc/release_history.html
index b6cceddb3..b3410fc13 100644
--- a/doc/release_history.html
+++ b/doc/release_history.html
@@ -40,6 +40,13 @@
   </tr>
 </table>
 
+<h2>1.86.0</h2>
+<ul>
+  <li><code>is_empty</code> operation is now better protected against concurrent filesystem modifications.</li>
+  <li>On POSIX systems, <code>is_empty</code> now indicates error if invoked on a file other than a regular file or a directory.</li>
+  <li>On Windows, fixed <code>file_size</code> and <code>is_empty</code> operating on symlinks rather than the files the symlinks refer to. (<a href="https://github.com/boostorg/filesystem/issues/313">#313</a>)</li>
+</ul>
+
 <h2>1.85.0</h2>
 <ul>
   <li><code>path::generic_path</code> and <code>path::generic_string</code> methods now remove duplicate directory separators in the returned paths.</li>
diff --git a/src/directory.cpp b/src/directory.cpp
index 0cf50254f..56a974a76 100644
--- a/src/directory.cpp
+++ b/src/directory.cpp
@@ -39,6 +39,7 @@
 #include <unistd.h>
 #include <fcntl.h>
 
+#include <memory>
 #include <boost/scope/unique_fd.hpp>
 
 #if defined(_POSIX_THREAD_SAFE_FUNCTIONS) && (_POSIX_THREAD_SAFE_FUNCTIONS >= 0) && defined(_SC_THREAD_SAFE_FUNCTIONS) && \
@@ -668,8 +669,8 @@ extra_data_format g_extra_data_format = file_directory_information_format;
  * Must be large enough to accommodate at least one FILE_DIRECTORY_INFORMATION or *_DIR_INFO struct and one filename.
  * NTFS, VFAT, exFAT and ReFS support filenames up to 255 UTF-16/UCS-2 characters. (For ReFS, there is no information
  * on the on-disk format, and it is possible that it supports longer filenames, up to 32768 UTF-16/UCS-2 characters.)
- * The buffer cannot be larger than 64k, because up to Windows 8.1, NtQueryDirectoryFile and GetFileInformationByHandleEx
- * fail with ERROR_INVALID_PARAMETER when trying to retrieve the filenames from a network share.
+ * The buffer cannot be larger than 64k, otherwise up to Windows 8.1, NtQueryDirectoryFile and GetFileInformationByHandleEx
+ * fail with ERROR_INVALID_PARAMETER when trying to retrieve filenames from a network share.
  */
 BOOST_CONSTEXPR_OR_CONST std::size_t dir_itr_extra_size = 65536u;
 
@@ -1087,7 +1088,94 @@ BOOST_CONSTEXPR_OR_CONST err_t not_found_error_code = ERROR_PATH_NOT_FOUND;
 
 } // namespace
 
-#if defined(BOOST_WINDOWS_API)
+#if defined(BOOST_POSIX_API)
+
+//! Tests if the directory is empty
+bool is_empty_directory(boost::scope::unique_fd&& fd, path const& p, error_code* ec)
+{
+#if !defined(BOOST_FILESYSTEM_USE_READDIR_R)
+    // Use a more optimal implementation without the overhead of constructing the iterator state
+
+    struct closedir_deleter
+    {
+        using result_type = void;
+        result_type operator() (DIR* dir) const noexcept
+        {
+            ::closedir(dir);
+        }
+    };
+
+    int err;
+
+#if defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW)
+    std::unique_ptr< DIR, closedir_deleter > dir(::fdopendir(fd.get()));
+    if (BOOST_UNLIKELY(!dir))
+    {
+        err = errno;
+    fail:
+        emit_error(err, p, ec, "boost::filesystem::is_empty");
+        return false;
+    }
+
+    // At this point fd will be closed by closedir
+    fd.release();
+#else // defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW)
+    std::unique_ptr< DIR, closedir_deleter > dir(::opendir(p.c_str()));
+    if (BOOST_UNLIKELY(!dir))
+    {
+        err = errno;
+    fail:
+        emit_error(err, p, ec, "boost::filesystem::is_empty");
+        return false;
+    }
+#endif // defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW)
+
+    while (true)
+    {
+        errno = 0;
+        struct dirent* const ent = ::readdir(dir.get());
+        if (!ent)
+        {
+            err = errno;
+            if (err != 0)
+                goto fail;
+
+            return true;
+        }
+
+        // Skip dot and dot-dot entries
+        if (!(ent->d_name[0] == path::dot
+            && (ent->d_name[1] == static_cast< path::string_type::value_type >('\0') ||
+                (ent->d_name[1] == path::dot && ent->d_name[2] == static_cast< path::string_type::value_type >('\0')))))
+        {
+            return false;
+        }
+    }
+
+#else // !defined(BOOST_FILESYSTEM_USE_READDIR_R)
+
+    filesystem::directory_iterator itr;
+#if defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW)
+    filesystem::detail::directory_iterator_params params{ std::move(fd) };
+    filesystem::detail::directory_iterator_construct(itr, p, directory_options::none, &params, ec);
+#else
+    filesystem::detail::directory_iterator_construct(itr, p, directory_options::none, nullptr, ec);
+#endif
+    return itr == filesystem::directory_iterator();
+
+#endif // !defined(BOOST_FILESYSTEM_USE_READDIR_R)
+}
+
+#else // BOOST_WINDOWS_API
+
+//! Tests if the directory is empty
+bool is_empty_directory(unique_handle&& h, path const& p, error_code* ec)
+{
+    filesystem::directory_iterator itr;
+    filesystem::detail::directory_iterator_params params{ h.get(), false };
+    filesystem::detail::directory_iterator_construct(itr, p, directory_options::none, &params, ec);
+    return itr == filesystem::directory_iterator();
+}
 
 //! Initializes directory iterator implementation
 void init_directory_iterator_impl() noexcept
diff --git a/src/operations.cpp b/src/operations.cpp
index 1744f1ecb..25fb1ceea 100644
--- a/src/operations.cpp
+++ b/src/operations.cpp
@@ -28,6 +28,7 @@
 #include <limits>
 #include <memory>
 #include <string>
+#include <utility>
 #include <cstddef>
 #include <cstdlib> // for malloc, free
 #include <cstring>
@@ -182,6 +183,7 @@ using boost::system::system_category;
 // At least Mac OS X 10.6 and older doesn't support O_CLOEXEC
 #ifndef O_CLOEXEC
 #define O_CLOEXEC 0
+#define BOOST_FILESYSTEM_NO_O_CLOEXEC
 #endif
 
 #if defined(_POSIX_SYNCHRONIZED_IO) && _POSIX_SYNCHRONIZED_IO > 0
@@ -289,15 +291,6 @@ BOOST_CONSTEXPR_OR_CONST unsigned int symloop_max =
 
 //  general helpers  -----------------------------------------------------------------//
 
-bool is_empty_directory(path const& p, error_code* ec)
-{
-    fs::directory_iterator itr;
-    detail::directory_iterator_construct(itr, p, directory_options::none, nullptr, ec);
-    return itr == fs::directory_iterator();
-}
-
-bool not_found_error(int errval) noexcept; // forward declaration
-
 #ifdef BOOST_POSIX_API
 
 //--------------------------------------------------------------------------------------//
@@ -2435,6 +2428,7 @@ inline path convert_nt_path_to_win32_path(const wchar_t* nt_path, std::size_t si
 #endif // defined(BOOST_POSIX_API)
 
 } // unnamed namespace
+
 } // namespace detail
 
 //--------------------------------------------------------------------------------------//
@@ -3932,29 +3926,35 @@ uintmax_t file_size(path const& p, error_code* ec)
 
 #if defined(BOOST_FILESYSTEM_USE_STATX)
     struct ::statx path_stat;
+    int err;
     if (BOOST_UNLIKELY(invoke_statx(AT_FDCWD, p.c_str(), AT_NO_AUTOMOUNT, STATX_TYPE | STATX_SIZE, &path_stat) < 0))
     {
-        emit_error(errno, p, ec, "boost::filesystem::file_size");
+        err = errno;
+    fail:
+        emit_error(err, p, ec, "boost::filesystem::file_size");
         return static_cast< uintmax_t >(-1);
     }
 
     if (BOOST_UNLIKELY((path_stat.stx_mask & (STATX_TYPE | STATX_SIZE)) != (STATX_TYPE | STATX_SIZE) || !S_ISREG(path_stat.stx_mode)))
     {
-        emit_error(BOOST_ERROR_NOT_SUPPORTED, p, ec, "boost::filesystem::file_size");
-        return static_cast< uintmax_t >(-1);
+        err = BOOST_ERROR_NOT_SUPPORTED;
+        goto fail;
     }
 #else
     struct ::stat path_stat;
+    int err;
     if (BOOST_UNLIKELY(::stat(p.c_str(), &path_stat) < 0))
     {
-        emit_error(errno, p, ec, "boost::filesystem::file_size");
+        err = errno;
+    fail:
+        emit_error(err, p, ec, "boost::filesystem::file_size");
         return static_cast< uintmax_t >(-1);
     }
 
     if (BOOST_UNLIKELY(!S_ISREG(path_stat.st_mode)))
     {
-        emit_error(BOOST_ERROR_NOT_SUPPORTED, p, ec, "boost::filesystem::file_size");
-        return static_cast< uintmax_t >(-1);
+        err = BOOST_ERROR_NOT_SUPPORTED;
+        goto fail;
     }
 #endif
 
@@ -3964,23 +3964,35 @@ uintmax_t file_size(path const& p, error_code* ec)
 
     // assume uintmax_t is 64-bits on all Windows compilers
 
-    WIN32_FILE_ATTRIBUTE_DATA fad;
+    unique_handle h(create_file_handle(
+        p.c_str(),
+        FILE_READ_ATTRIBUTES,
+        FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
+        nullptr,
+        OPEN_EXISTING,
+        FILE_FLAG_BACKUP_SEMANTICS));
 
-    if (BOOST_UNLIKELY(!::GetFileAttributesExW(p.c_str(), ::GetFileExInfoStandard, &fad)))
+    DWORD err;
+    if (BOOST_UNLIKELY(!h))
     {
-        emit_error(BOOST_ERRNO, p, ec, "boost::filesystem::file_size");
+    fail_errno:
+        err = BOOST_ERRNO;
+    fail:
+        emit_error(err, p, ec, "boost::filesystem::file_size");
         return static_cast< uintmax_t >(-1);
     }
 
-    if (BOOST_UNLIKELY((fad.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0))
+    BY_HANDLE_FILE_INFORMATION info;
+    if (BOOST_UNLIKELY(!::GetFileInformationByHandle(h.get(), &info)))
+        goto fail_errno;
+
+    if (BOOST_UNLIKELY((info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0u))
     {
-        emit_error(ERROR_NOT_SUPPORTED, p, ec, "boost::filesystem::file_size");
-        return static_cast< uintmax_t >(-1);
+        err = ERROR_NOT_SUPPORTED;
+        goto fail;
     }
 
-    return (static_cast< uintmax_t >(fad.nFileSizeHigh)
-            << (sizeof(fad.nFileSizeLow) * 8u)) |
-        fad.nFileSizeLow;
+    return (static_cast< uintmax_t >(info.nFileSizeHigh) << 32u) | info.nFileSizeLow;
 
 #endif // defined(BOOST_POSIX_API)
 }
@@ -4057,6 +4069,18 @@ path initial_path(error_code* ec)
     return init_path;
 }
 
+//! Tests if the directory is empty. Implemented in directory.cpp.
+bool is_empty_directory
+(
+#if defined(BOOST_POSIX_API)
+    boost::scope::unique_fd&& fd,
+#else
+    unique_handle&& h,
+#endif
+    path const& p,
+    error_code* ec
+);
+
 BOOST_FILESYSTEM_DECL
 bool is_empty(path const& p, system::error_code* ec)
 {
@@ -4065,49 +4089,93 @@ bool is_empty(path const& p, system::error_code* ec)
 
 #if defined(BOOST_POSIX_API)
 
-#if defined(BOOST_FILESYSTEM_USE_STATX)
-    struct ::statx path_stat;
-    if (BOOST_UNLIKELY(invoke_statx(AT_FDCWD, p.c_str(), AT_NO_AUTOMOUNT, STATX_TYPE | STATX_SIZE, &path_stat) < 0))
+    boost::scope::unique_fd file;
+    int err = 0;
+    while (true)
     {
-        emit_error(errno, p, ec, "boost::filesystem::is_empty");
-        return false;
+        file.reset(::open(p.c_str(), O_RDONLY | O_CLOEXEC));
+        if (BOOST_UNLIKELY(!file))
+        {
+            err = errno;
+            if (err == EINTR)
+                continue;
+
+        fail:
+            emit_error(err, p, ec, "boost::filesystem::is_empty");
+            return false;
+        }
+
+        break;
     }
 
-    if (BOOST_UNLIKELY((path_stat.stx_mask & STATX_TYPE) != STATX_TYPE))
+#if defined(BOOST_FILESYSTEM_NO_O_CLOEXEC) && defined(FD_CLOEXEC)
+    if (BOOST_UNLIKELY(::fcntl(file.get(), F_SETFD, FD_CLOEXEC) < 0))
     {
-    fail_unsupported:
-        emit_error(BOOST_ERROR_NOT_SUPPORTED, p, ec, "boost::filesystem::is_empty");
-        return false;
+        err = errno;
+        goto fail;
     }
+#endif
 
-    if (S_ISDIR(get_mode(path_stat)))
-        return is_empty_directory(p, ec);
-
-    if (BOOST_UNLIKELY((path_stat.stx_mask & STATX_SIZE) != STATX_SIZE))
-        goto fail_unsupported;
+#if defined(BOOST_FILESYSTEM_USE_STATX)
+    struct ::statx path_stat;
+    if (BOOST_UNLIKELY(invoke_statx(file.get(), "", AT_EMPTY_PATH | AT_NO_AUTOMOUNT, STATX_TYPE | STATX_SIZE, &path_stat) < 0))
+    {
+        err = errno;
+        goto fail;
+    }
 
-    return get_size(path_stat) == 0u;
+    if (BOOST_UNLIKELY((path_stat.stx_mask & (STATX_TYPE | STATX_SIZE)) != (STATX_TYPE | STATX_SIZE)))
+    {
+        err = BOOST_ERROR_NOT_SUPPORTED;
+        goto fail;
+    }
 #else
     struct ::stat path_stat;
-    if (BOOST_UNLIKELY(::stat(p.c_str(), &path_stat) < 0))
+    if (BOOST_UNLIKELY(::fstat(file.get(), &path_stat) < 0))
     {
-        emit_error(errno, p, ec, "boost::filesystem::is_empty");
-        return false;
+        err = errno;
+        goto fail;
     }
-
-    return S_ISDIR(get_mode(path_stat)) ? is_empty_directory(p, ec) : get_size(path_stat) == 0u;
 #endif
 
+    const mode_t mode = get_mode(path_stat);
+    if (S_ISDIR(mode))
+        return is_empty_directory(std::move(file), p, ec);
+
+    if (BOOST_UNLIKELY(!S_ISREG(mode)))
+    {
+        err = BOOST_ERROR_NOT_SUPPORTED;
+        goto fail;
+    }
+
+    return get_size(path_stat) == 0u;
+
 #else // defined(BOOST_POSIX_API)
 
-    WIN32_FILE_ATTRIBUTE_DATA fad;
-    if (BOOST_UNLIKELY(!::GetFileAttributesExW(p.c_str(), ::GetFileExInfoStandard, &fad)))
+    unique_handle h(create_file_handle(
+        p.c_str(),
+        FILE_READ_ATTRIBUTES | FILE_LIST_DIRECTORY,
+        FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
+        nullptr,
+        OPEN_EXISTING,
+        FILE_FLAG_BACKUP_SEMANTICS));
+
+    if (BOOST_UNLIKELY(!h))
     {
-        emit_error(BOOST_ERRNO, p, ec, "boost::filesystem::is_empty");
+    fail_errno:
+        const DWORD err = BOOST_ERRNO;
+        emit_error(err, p, ec, "boost::filesystem::is_empty");
         return false;
     }
 
-    return (fad.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ? is_empty_directory(p, ec) : (!fad.nFileSizeHigh && !fad.nFileSizeLow);
+    BY_HANDLE_FILE_INFORMATION info;
+    if (BOOST_UNLIKELY(!::GetFileInformationByHandle(h.get(), &info)))
+        goto fail_errno;
+
+    if ((info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0u)
+        return is_empty_directory(std::move(h), p, ec);
+
+    return (info.nFileSizeHigh | info.nFileSizeLow) == 0u;
 
 #endif // defined(BOOST_POSIX_API)
 }
diff --git a/test/operations_test.cpp b/test/operations_test.cpp
index 2e44121d0..56a14cdbc 100644
--- a/test/operations_test.cpp
+++ b/test/operations_test.cpp
@@ -2255,6 +2255,105 @@ void creation_time_tests(const fs::path& dirx)
     fs::remove(f1x);
 }
 
+//  symlink_file_size_tests  ---------------------------------------------------------//
+
+void symlink_file_size_tests()
+{
+    cout << "symlink_file_size_tests..." << endl;
+
+    // Most of these symlinks are already created in symlink_status_tests(), which is run before this test
+    fs::path dangling_sym(dir / "dangling-sym");
+    fs::path dangling_directory_sym(dir / "dangling-directory-sym");
+    fs::path sym_d1(dir / "sym-d1");
+    fs::path sym_f1(dir / "sym-f1");
+    fs::path sym_d1f1(d1 / "sym-d1f1");
+    fs::create_symlink(d1f1, sym_d1f1);
+
+    bool exception_thrown = false;
+    try
+    {
+        fs::file_size(dangling_sym);
+    }
+    catch (fs::filesystem_error&)
+    {
+        exception_thrown = true;
+    }
+    BOOST_TEST(exception_thrown);
+
+    exception_thrown = false;
+    try
+    {
+        fs::file_size(dangling_directory_sym);
+    }
+    catch (fs::filesystem_error&)
+    {
+        exception_thrown = true;
+    }
+    BOOST_TEST(exception_thrown);
+
+    exception_thrown = false;
+    try
+    {
+        fs::file_size(sym_d1);
+    }
+    catch (fs::filesystem_error&)
+    {
+        exception_thrown = true;
+    }
+    BOOST_TEST(exception_thrown);
+
+    boost::uintmax_t size = fs::file_size(sym_f1);
+    BOOST_TEST_EQ(size, 7u);
+
+    size = fs::file_size(sym_d1f1);
+    BOOST_TEST_EQ(size, 0u);
+}
+
+//  symlink_is_empty_tests  ----------------------------------------------------------//
+
+void symlink_is_empty_tests()
+{
+    cout << "symlink_file_size_tests..." << endl;
+
+    // These symlinks are already created in symlink_status_tests() and symlink_file_size_tests(), which are run before this test
+    fs::path dangling_sym(dir / "dangling-sym");
+    fs::path dangling_directory_sym(dir / "dangling-directory-sym");
+    fs::path sym_d1(dir / "sym-d1");
+    fs::path sym_f1(dir / "sym-f1");
+    fs::path sym_d1f1(d1 / "sym-d1f1");
+
+    bool exception_thrown = false;
+    try
+    {
+        fs::is_empty(dangling_sym);
+    }
+    catch (fs::filesystem_error&)
+    {
+        exception_thrown = true;
+    }
+    BOOST_TEST(exception_thrown);
+
+    exception_thrown = false;
+    try
+    {
+        fs::is_empty(dangling_directory_sym);
+    }
+    catch (fs::filesystem_error&)
+    {
+        exception_thrown = true;
+    }
+    BOOST_TEST(exception_thrown);
+
+    bool empty = fs::is_empty(sym_d1);
+    BOOST_TEST_EQ(empty, false);
+
+    empty = fs::is_empty(sym_f1);
+    BOOST_TEST_EQ(empty, false);
+
+    empty = fs::is_empty(sym_d1f1);
+    BOOST_TEST_EQ(empty, true);
+}
+
 //  write_time_tests  ----------------------------------------------------------------//
 
 void write_time_tests(const fs::path& dirx)
@@ -2848,6 +2947,8 @@ int cpp_main(int argc, char* argv[])
         copy_symlink_tests(f1, d1);
         canonical_symlink_tests();
         weakly_canonical_symlink_tests();
+        symlink_file_size_tests();
+        symlink_is_empty_tests();
     }
     iterator_status_tests(); // lots of cases by now, so a good time to test
                              //  dump_tree(dir);