From 5a366cabf5dabc8e18b5c6bdce60c700f592a8ec 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. A note on the tests: we are missing some error code assertions, notably in the directory_entry_tests block that tests the missing file case and after some of the non-symlink status accessor calls in the dangling symlink block in directory_entry_symlink_tests. The reason is an unrelated issue that causes these functions to not report error for non-existing files when reading from the cache. Due to the structure of the tests, this is the case for many of these calls, and in v4, this would always be the case after construction, since the constructors write to the cache. This issue should be addressed separately. --- include/boost/filesystem/directory.hpp | 28 ++-- src/directory.cpp | 10 +- test/operations_test.cpp | 192 ++++++++++++++++++++++++- 3 files changed, 217 insertions(+), 13 deletions(-) diff --git a/include/boost/filesystem/directory.hpp b/include/boost/filesystem/directory.hpp index 7b383544d..782515b75 100644 --- a/include/boost/filesystem/directory.hpp +++ b/include/boost/filesystem/directory.hpp @@ -206,7 +206,7 @@ class directory_entry file_status status() const { if (!filesystem::status_known(m_status)) - refresh_impl(); + refresh_impl(refresh_mode::follow); return m_status; } @@ -215,14 +215,14 @@ class directory_entry ec.clear(); if (!filesystem::status_known(m_status)) - refresh_impl(&ec); + refresh_impl(&ec, refresh_mode::follow); return m_status; } file_status symlink_status() const { if (!filesystem::status_known(m_symlink_status)) - refresh_impl(); + refresh_impl(refresh_mode::no_follow); return m_symlink_status; } @@ -231,14 +231,14 @@ 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; } filesystem::file_type file_type() const { if (!filesystem::type_present(m_status)) - refresh_impl(); + refresh_impl(refresh_mode::follow); return m_status.type(); } @@ -247,14 +247,14 @@ class directory_entry ec.clear(); if (!filesystem::type_present(m_status)) - refresh_impl(&ec); + refresh_impl(&ec, refresh_mode::follow); return m_status.type(); } 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(); } @@ -263,7 +263,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(); } @@ -381,7 +381,17 @@ class directory_entry bool operator>=(directory_entry const& rhs) const { return m_path >= rhs.m_path; } 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..b7ba8e5ee 100644 --- a/src/directory.cpp +++ b/src/directory.cpp @@ -104,7 +104,9 @@ 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(); m_symlink_status = filesystem::file_status(); @@ -116,9 +118,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; + // Don't clobber ec after the symlink_status call + 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);