From 76bf91a25fc996e7fb6f7207ce08b83a13d77a0e Mon Sep 17 00:00:00 2001 From: stm Date: Fri, 31 Jul 2026 15:38:21 +0200 Subject: [PATCH] Fix unprovoked symlink errors in directory_entry Since https://github.com/boostorg/filesystem/commit/d508d4950f7b51c84d2819a5b643b9406523027c, a number of functions in directory_entry's public interface call directory_entry::refresh_impl, which updates the cached file status comprehensively. On POSIX systems, for example, both lstat and stat are called for symlinks, and stat errors are reported to the caller. As a result, e.g. checking the status of a symlink with symlink_status produces an error if the symlink target is gone or if access to it is denied. In v4, this also affects the constructor overload that takes an error_code, i.e. constructing a directory_entry with a path to a broken symlink produces an error. Add a parameter to refresh_impl allowing to configure what the caller is interested in, thus avoiding false positives. --- include/boost/filesystem/directory.hpp | 36 +++-- src/directory.cpp | 13 +- test/operations_test.cpp | 192 ++++++++++++++++++++++++- 3 files changed, 223 insertions(+), 18 deletions(-) diff --git a/include/boost/filesystem/directory.hpp b/include/boost/filesystem/directory.hpp index 169c89d48..01edba186 100644 --- a/include/boost/filesystem/directory.hpp +++ b/include/boost/filesystem/directory.hpp @@ -500,8 +500,8 @@ class directory_entry * \brief Returns the file status. * * \effects - * For the cached file status `m_status`, if `!status_known(m_status)`, calls `refresh(ec)`. Then returns - * `m_status`. + * For the cached file status `m_status`, if `!status_known(m_status)`, refreshes the cache to update it. + * Then returns `m_status`. * * \note The implementation does not query the filesystem after the file status has been cached. * Filesystem changes after the file status has been cached will not be reflected in the result. @@ -515,7 +515,7 @@ class directory_entry ec.clear(); if (!filesystem::status_known(m_status)) - refresh_impl(&ec); + refresh_impl(&ec, refresh_mode::follow); return m_status; } @@ -523,7 +523,7 @@ class directory_entry file_status status() const { if (!filesystem::status_known(m_status)) - refresh_impl(); + refresh_impl(refresh_mode::follow); return m_status; } @@ -531,8 +531,8 @@ class directory_entry * \brief Returns the symlink file status. * * \effects - * For the cached symlink file status `m_symlink_status`, if `!status_known(m_symlink_status)`, calls - * `refresh(ec)`. Then returns `m_symlink_status`. + * For the cached symlink file status `m_symlink_status`, if `!status_known(m_symlink_status)`, refreshes + * the cache to update it. Then returns `m_symlink_status`. * * \note The implementation does not query the filesystem after the symlink file status has been cached. * Filesystem changes after the symlink file status has been cached will not be reflected in the result. @@ -546,7 +546,7 @@ class directory_entry ec.clear(); if (!filesystem::status_known(m_symlink_status)) - refresh_impl(&ec); + refresh_impl(&ec, refresh_mode::no_follow); return m_symlink_status; } @@ -554,7 +554,7 @@ class directory_entry file_status symlink_status() const { if (!filesystem::status_known(m_symlink_status)) - refresh_impl(); + refresh_impl(refresh_mode::no_follow); return m_symlink_status; } @@ -577,7 +577,7 @@ class directory_entry ec.clear(); if (!filesystem::type_present(m_status)) - refresh_impl(&ec); + refresh_impl(&ec, refresh_mode::follow); return m_status.type(); } @@ -585,7 +585,7 @@ class directory_entry filesystem::file_type file_type() const { if (!filesystem::type_present(m_status)) - refresh_impl(); + refresh_impl(refresh_mode::follow); return m_status.type(); } @@ -608,7 +608,7 @@ class directory_entry ec.clear(); if (!filesystem::type_present(m_symlink_status)) - refresh_impl(&ec); + refresh_impl(&ec, refresh_mode::no_follow); return m_symlink_status.type(); } @@ -616,7 +616,7 @@ class directory_entry filesystem::file_type symlink_file_type() const { if (!filesystem::type_present(m_symlink_status)) - refresh_impl(); + refresh_impl(refresh_mode::no_follow); return m_symlink_status.type(); } @@ -942,7 +942,17 @@ class directory_entry #if !defined(BOOST_FILESYSTEM_DOXYGEN) private: - BOOST_FILESYSTEM_DECL void refresh_impl(system::error_code* ec = nullptr) const; + enum class refresh_mode + { + no_follow, + follow, + follow_lenient + }; + + BOOST_FILESYSTEM_DECL void refresh_impl( + system::error_code* ec = nullptr, + refresh_mode mode = refresh_mode::follow_lenient) const; + void refresh_impl(refresh_mode mode) const { refresh_impl(nullptr, mode); } void assign_with_status(boost::filesystem::path&& p, file_status st, file_status symlink_st) { diff --git a/src/directory.cpp b/src/directory.cpp index 31147b289..76a0801ac 100644 --- a/src/directory.cpp +++ b/src/directory.cpp @@ -104,9 +104,12 @@ namespace filesystem { // // //--------------------------------------------------------------------------------------// -BOOST_FILESYSTEM_DECL void directory_entry::refresh_impl(system::error_code* ec) const +BOOST_FILESYSTEM_DECL void directory_entry::refresh_impl( + system::error_code* ec, + directory_entry::refresh_mode mode) const { - m_status = filesystem::file_status(); + if (mode != refresh_mode::no_follow) + m_status = filesystem::file_status(); m_symlink_status = filesystem::file_status(); m_symlink_status = detail::symlink_status(m_path, ec); @@ -116,9 +119,11 @@ BOOST_FILESYSTEM_DECL void directory_entry::refresh_impl(system::error_code* ec) // Also works if symlink_status fails - set m_status to status_error as well m_status = m_symlink_status; } - else + else if (mode != refresh_mode::no_follow) { - m_status = detail::status(m_path, ec); + system::error_code ec2; + // In follow_lenient mode, update m_status but suppress errors + m_status = detail::status(m_path, mode == refresh_mode::follow_lenient ? &ec2 : ec); } } diff --git a/test/operations_test.cpp b/test/operations_test.cpp index 2e9d9fe3b..b70edab4c 100644 --- a/test/operations_test.cpp +++ b/test/operations_test.cpp @@ -934,7 +934,7 @@ void create_symlink_tests() BOOST_TEST(!fs::is_other(stat)); } - error_code ec = error_code(); + error_code ec; fs::create_symlink("doesnotexist", "", ec); BOOST_TEST(ec); } @@ -2403,6 +2403,194 @@ void symlink_is_empty_tests() BOOST_TEST_EQ(empty, true); } +// directory_entry_tests ----------------------------------------------------// + +void directory_entry_tests() +{ + cout << "directory_entry_tests..." << endl; + + fs::path reg_file(dir / "reg-file"); + fs::path nonexistent_file(dir / "nonexistent-file"); + fs::remove(reg_file); + fs::remove(nonexistent_file); + create_file(reg_file); + error_code ec; + + fs::directory_entry reg_entry(reg_file); + fs::directory_entry nonexistent_entry(nonexistent_file); + + BOOST_TEST(reg_entry.exists()); + BOOST_TEST(reg_entry.exists(ec)); + BOOST_TEST(!ec); + BOOST_TEST_EQ(reg_entry.status().type(), fs::regular_file); + BOOST_TEST_EQ(reg_entry.status(ec).type(), fs::regular_file); + BOOST_TEST(!ec); + BOOST_TEST_EQ(reg_entry.symlink_status().type(), fs::regular_file); + BOOST_TEST_EQ(reg_entry.symlink_status(ec).type(), fs::regular_file); + BOOST_TEST(!ec); + BOOST_TEST_EQ(reg_entry.file_type(), fs::regular_file); + BOOST_TEST_EQ(reg_entry.file_type(ec), fs::regular_file); + BOOST_TEST(!ec); + BOOST_TEST_EQ(reg_entry.symlink_file_type(), fs::regular_file); + BOOST_TEST_EQ(reg_entry.symlink_file_type(ec), fs::regular_file); + BOOST_TEST(!ec); + + reg_entry.refresh(ec); + BOOST_TEST(!ec); + + // Make sure status() and symlink_status() hold the expected types + // after a call to refresh, too + BOOST_TEST_EQ(reg_entry.symlink_status().type(), fs::regular_file); + BOOST_TEST_EQ(reg_entry.status().type(), fs::regular_file); + +#if BOOST_FILESYSTEM_VERSION >= 4 + // ctor overload with error_code + { + ec.clear(); + fs::directory_entry reg_entry2(reg_file, ec); + BOOST_TEST(!ec); + BOOST_TEST(reg_entry2.path() == reg_file); + } + + // assign overload with error_code + ec.clear(); + reg_entry.assign(reg_file, ec); + BOOST_TEST(!ec); + BOOST_TEST(reg_entry.path() == reg_file); +#endif + + // Missing file + BOOST_TEST_EQ(nonexistent_entry.status().type(), fs::file_not_found); + BOOST_TEST_EQ(nonexistent_entry.status(ec).type(), fs::file_not_found); + BOOST_TEST_EQ(nonexistent_entry.symlink_status().type(), fs::file_not_found); + BOOST_TEST_EQ(nonexistent_entry.symlink_status(ec).type(), fs::file_not_found); + BOOST_TEST_EQ(nonexistent_entry.file_type(), fs::file_not_found); + BOOST_TEST_EQ(nonexistent_entry.file_type(ec), fs::file_not_found); + BOOST_TEST_EQ(nonexistent_entry.symlink_file_type(), fs::file_not_found); + BOOST_TEST_EQ(nonexistent_entry.symlink_file_type(ec), fs::file_not_found); + BOOST_TEST(!nonexistent_entry.exists(ec)); + BOOST_TEST(!nonexistent_entry.exists()); + +#if BOOST_FILESYSTEM_VERSION >= 4 + // ctor overload with error_code + { + ec.clear(); + fs::directory_entry nonexistent_entry2(nonexistent_file, ec); + BOOST_TEST(ec); + BOOST_TEST(nonexistent_entry2.path().empty()); + } + + // assign overload with error_code + ec.clear(); + nonexistent_entry.assign(nonexistent_file, ec); + BOOST_TEST(ec); + BOOST_TEST_EQ(nonexistent_entry.path(), nonexistent_file); +#endif + + fs::remove(reg_file); +} + +// directory_entry_symlink_tests --------------------------------------------// + +void directory_entry_symlink_tests() +{ + cout << "directory_entry_symlink_tests..." << endl; + + fs::path reg_file(dir / "reg-file"); + fs::path valid_sym(dir / "valid-sym"); + fs::path dangling_sym(dir / "dangling-sym"); + fs::remove(reg_file); + fs::remove(valid_sym); + fs::remove(dangling_sym); + create_file(reg_file); + fs::create_symlink(reg_file, valid_sym); + fs::create_symlink("does not exist", dangling_sym); + error_code ec; + + fs::directory_entry sym_entry(valid_sym); + fs::directory_entry dsym_entry(dangling_sym); + + // Valid symlink + BOOST_TEST(sym_entry.exists()); + BOOST_TEST(sym_entry.exists(ec)); + BOOST_TEST(!ec); + BOOST_TEST(sym_entry.is_symlink()); + BOOST_TEST(sym_entry.is_symlink(ec)); + BOOST_TEST(!ec); + BOOST_TEST_EQ(sym_entry.status().type(), fs::regular_file); + BOOST_TEST_EQ(sym_entry.status(ec).type(), fs::regular_file); + BOOST_TEST(!ec); + BOOST_TEST_EQ(sym_entry.symlink_status().type(), fs::symlink_file); + BOOST_TEST_EQ(sym_entry.symlink_status(ec).type(), fs::symlink_file); + BOOST_TEST(!ec); + BOOST_TEST_EQ(sym_entry.file_type(), fs::regular_file); + BOOST_TEST_EQ(sym_entry.file_type(ec), fs::regular_file); + BOOST_TEST(!ec); + BOOST_TEST_EQ(sym_entry.symlink_file_type(), fs::symlink_file); + BOOST_TEST_EQ(sym_entry.symlink_file_type(ec), fs::symlink_file); + BOOST_TEST(!ec); + + BOOST_TEST(!dsym_entry.exists()); + BOOST_TEST(!dsym_entry.exists(ec)); + ec.clear(); + BOOST_TEST(dsym_entry.is_symlink()); + BOOST_TEST(dsym_entry.is_symlink(ec)); + BOOST_TEST(!ec); + BOOST_TEST_EQ(dsym_entry.status().type(), fs::file_not_found); + BOOST_TEST_EQ(dsym_entry.status(ec).type(), fs::file_not_found); + ec.clear(); + BOOST_TEST_EQ(dsym_entry.symlink_status().type(), fs::symlink_file); + BOOST_TEST_EQ(dsym_entry.symlink_status(ec).type(), fs::symlink_file); + BOOST_TEST(!ec); + BOOST_TEST_EQ(dsym_entry.file_type(), fs::file_not_found); + BOOST_TEST_EQ(dsym_entry.file_type(ec), fs::file_not_found); + ec.clear(); + BOOST_TEST_EQ(dsym_entry.symlink_file_type(), fs::symlink_file); + BOOST_TEST_EQ(dsym_entry.symlink_file_type(ec), fs::symlink_file); + BOOST_TEST(!ec); + + sym_entry.refresh(ec); + BOOST_TEST(!ec); + dsym_entry.refresh(ec); + BOOST_TEST(!ec); + + // Make sure status() and symlink_status() hold the expected types + // after a call to refresh, too + BOOST_TEST_EQ(sym_entry.symlink_status().type(), fs::symlink_file); + BOOST_TEST_EQ(sym_entry.status().type(), fs::regular_file); + BOOST_TEST_EQ(dsym_entry.symlink_status().type(), fs::symlink_file); + BOOST_TEST_EQ(dsym_entry.status().type(), fs::file_not_found); + +#if BOOST_FILESYSTEM_VERSION >= 4 + // ctor overload with error_code + { + ec.clear(); + fs::directory_entry sym_entry2(valid_sym, ec); + BOOST_TEST(!ec); + BOOST_TEST(sym_entry2.path() == valid_sym); + + // In particular, shouldn't report an error with broken symlinks + fs::directory_entry dsym_entry2(dangling_sym, ec); + BOOST_TEST(!ec); + BOOST_TEST(dsym_entry2.path() == dangling_sym); + } + + ec.clear(); + sym_entry.assign(valid_sym, ec); + BOOST_TEST(!ec); + BOOST_TEST(sym_entry.path() == valid_sym); + + // In particular, shouldn't report an error with broken symlinks + dsym_entry.assign(dangling_sym, ec); + BOOST_TEST(!ec); + BOOST_TEST(dsym_entry.path() == dangling_sym); +#endif + + fs::remove(reg_file); + fs::remove(valid_sym); + fs::remove(dangling_sym); +} + // write_time_tests ----------------------------------------------------------------// void write_time_tests(const fs::path& dirx) @@ -3025,6 +3213,7 @@ int cpp_main(int argc, char* argv[]) weakly_canonical_basic_tests(); permissions_tests(); copy_file_tests(f1, d1); + directory_entry_tests(); if (create_symlink_ok) // only if symlinks supported { symlink_status_tests(); @@ -3033,6 +3222,7 @@ int cpp_main(int argc, char* argv[]) weakly_canonical_symlink_tests(); symlink_file_size_tests(); symlink_is_empty_tests(); + directory_entry_symlink_tests(); } iterator_status_tests(); // lots of cases by now, so a good time to test // dump_tree(dir);