From 904798e968e9e7268e022606cf29bdf25599ec2c Mon Sep 17 00:00:00 2001 From: Matthew Glazar Date: Sat, 10 Feb 2024 23:57:56 -0500 Subject: [PATCH] feat(io): teach list_directory/delete_directory_recursive symlinks Teach some test-centric APIs about POSIX symlinks and Windows symlinks (but not other kinds of reparse points). This will make it easier to test symlink support in other parts of quick-lint-js, such as in canonicalize_path. --- src/quick-lint-js/io/file.cpp | 89 +++++++++++++ src/quick-lint-js/io/file.h | 27 ++++ src/quick-lint-js/io/temporary-directory.cpp | 57 ++++++-- src/quick-lint-js/io/temporary-directory.h | 14 +- test/quick-lint-js/file-matcher.h | 6 +- test/quick-lint-js/filesystem-test.cpp | 21 ++- test/test-temporary-directory.cpp | 133 ++++++++++++++++++- tools/test-typescript/main.cpp | 2 +- 8 files changed, 324 insertions(+), 25 deletions(-) diff --git a/src/quick-lint-js/io/file.cpp b/src/quick-lint-js/io/file.cpp index 0434ce06d4..20d5de399f 100644 --- a/src/quick-lint-js/io/file.cpp +++ b/src/quick-lint-js/io/file.cpp @@ -168,6 +168,16 @@ std::string Write_File_IO_Error::to_string() const { std::exit(1); } +std::string Symlink_IO_Error::to_string() const { + return "failed to create symlink to "s + this->target + " at " + this->path + + ": "s + this->io_error.to_string(); +} + +[[noreturn]] void Symlink_IO_Error::print_and_exit() const { + std::fprintf(stderr, "error: %s\n", this->to_string().c_str()); + std::exit(1); +} + bool operator==(const Read_File_IO_Error &lhs, const Read_File_IO_Error &rhs) { return lhs.path == rhs.path && lhs.io_error == rhs.io_error; } @@ -411,6 +421,85 @@ bool file_ids_equal(const ::FILE_ID_INFO &a, const ::FILE_ID_INFO &b) { std::memcmp(&b.FileId, &a.FileId, sizeof(b.FileId)) == 0; } #endif + +namespace { +Result create_posix_symbolic_link( + const char *path, const char *target, [[maybe_unused]] bool is_directory) { +#if defined(QLJS_FILE_POSIX) + int rc = ::symlink(target, path); + if (rc != 0) { + return failed_result(Symlink_IO_Error{ + .path = path, + .target = target, + .io_error = POSIX_File_IO_Error{errno}, + }); + } + return {}; +#elif defined(QLJS_FILE_WINDOWS) + std::optional wpath = mbstring_to_wstring(path); + std::optional wtarget = mbstring_to_wstring(target); + if (!wpath.has_value() || !wtarget.has_value()) { + return failed_result(Symlink_IO_Error{ + .path = path, + .target = target, + .io_error = Windows_File_IO_Error{ERROR_INVALID_PARAMETER}, + }); + } + + // TODO(strager): Ensure a relative target path creates relative symlinks, not + // absolute symlinks resolved to the current working directory. + // + // FIXME(strager): With SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE, + // ::CreateSymbolicLinkW can fail with ERROR_INVALID_PARAMETER or maybe + // something else. Need to test more Windows versions. + // + // NOTE(strager): ::CreateSymbolicLinkW fails with ERROR_PRIVILEGE_NOT_HELD + // (1314) if SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE is not set. + if (!::CreateSymbolicLinkW( + wpath->c_str(), wtarget->c_str(), + SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE | + (is_directory ? SYMBOLIC_LINK_FLAG_DIRECTORY : 0))) { + return failed_result(Symlink_IO_Error{ + .path = path, + .target = target, + .io_error = Windows_File_IO_Error{::GetLastError()}, + }); + } + + return {}; +#else +#error "Unknown platform" +#endif +} +} + +Result create_posix_directory_symbolic_link( + const char *path, const char *target) { + return create_posix_symbolic_link(path, target, /*is_directory=*/true); +} + +Result create_posix_file_symbolic_link( + const char *path, const char *target) { + return create_posix_symbolic_link(path, target, /*is_directory=*/false); +} + +void create_posix_directory_symbolic_link_or_exit(const char *path, + const char *target) { + Result result = + create_posix_directory_symbolic_link(path, target); + if (!result.ok()) { + result.error().print_and_exit(); + } +} + +void create_posix_file_symbolic_link_or_exit(const char *path, + const char *target) { + Result result = + create_posix_file_symbolic_link(path, target); + if (!result.ok()) { + result.error().print_and_exit(); + } +} } #endif diff --git a/src/quick-lint-js/io/file.h b/src/quick-lint-js/io/file.h index a273cab5ac..0a0be40638 100644 --- a/src/quick-lint-js/io/file.h +++ b/src/quick-lint-js/io/file.h @@ -46,6 +46,15 @@ struct Write_File_IO_Error { [[noreturn]] void print_and_exit() const; }; +struct Symlink_IO_Error { + std::string path; + std::string target; + Platform_File_IO_Error io_error; + + std::string to_string() const; + [[noreturn]] void print_and_exit() const; +}; + Result read_file(const std::string &path); Result read_file(const char *path); Result read_file(const char *path, @@ -77,6 +86,24 @@ Result open_file_for_writing( #if QLJS_HAVE_WINDOWS_H bool file_ids_equal(const ::FILE_ID_INFO &, const ::FILE_ID_INFO &); #endif + +// Create a POSIX/UNIX-style symbolic link. +// +// On POSIX platforms like Linux and macOS, calls ::symlink. +// create_posix_directory_symbolic_link and create_posix_file_symbolic_link +// behave identically. +// +// On Windows, calls ::CreateSymbolicLinkW with +// SYMBOLIC_LINK_FLAG_DIRECTORY (for create_posix_directory_symbolic_link) or +// without SYMBOLIC_LINK_FLAG_DIRECTORY (for create_posix_file_symbolic_link). +Result create_posix_directory_symbolic_link( + const char *path, const char *target); +Result create_posix_file_symbolic_link( + const char *path, const char *target); +void create_posix_directory_symbolic_link_or_exit(const char *path, + const char *target); +void create_posix_file_symbolic_link_or_exit(const char *path, + const char *target); } #endif diff --git a/src/quick-lint-js/io/temporary-directory.cpp b/src/quick-lint-js/io/temporary-directory.cpp index 7791c263d5..95f9358d3c 100644 --- a/src/quick-lint-js/io/temporary-directory.cpp +++ b/src/quick-lint-js/io/temporary-directory.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -354,7 +355,7 @@ Result list_directory( Result list_directory( const char *directory, - Temporary_Function_Ref visit_file) { + Temporary_Function_Ref visit_file) { #if QLJS_HAVE_WINDOWS_H return list_directory_raw(directory, [&](::WIN32_FIND_DATAW &entry) -> void { // TODO(strager): Reduce allocations. @@ -364,9 +365,17 @@ Result list_directory( QLJS_UNIMPLEMENTED(); } if (!is_dot_or_dot_dot(entry_name->c_str())) { - bool is_directory = (entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == - FILE_ATTRIBUTE_DIRECTORY; - visit_file(entry_name->c_str(), is_directory); + File_Type_Flags flags = File_Type_Flags::none; + if ((entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == + FILE_ATTRIBUTE_DIRECTORY) { + flags = enum_set_flags(flags, File_Type_Flags::is_directory); + } + if ((entry.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == + FILE_ATTRIBUTE_REPARSE_POINT) { + flags = enum_set_flags( + flags, File_Type_Flags::is_symbolic_link_or_reparse_point); + } + visit_file(entry_name->c_str(), flags); } }); #elif QLJS_HAVE_DIRENT_H @@ -375,8 +384,9 @@ Result list_directory( if (is_dot_or_dot_dot(entry->d_name)) { return; } - bool is_directory; - if (entry->d_type == DT_UNKNOWN) { + File_Type_Flags flags = File_Type_Flags::none; + switch (entry->d_type) { + case DT_UNKNOWN: { temp_path.clear(); temp_path += directory; temp_path += QLJS_PREFERRED_PATH_DIRECTORY_SEPARATOR; @@ -387,14 +397,31 @@ Result list_directory( if (errno == ENOENT) { return; } - is_directory = false; } else { - is_directory = S_ISDIR(s.st_mode); + if (S_ISDIR(s.st_mode)) { + flags = enum_set_flags(flags, File_Type_Flags::is_directory); + } + if (S_ISLNK(s.st_mode)) { + flags = enum_set_flags( + flags, File_Type_Flags::is_symbolic_link_or_reparse_point); + } } - } else { - is_directory = entry->d_type == DT_DIR; + break; + } + + case DT_DIR: + flags = enum_set_flags(flags, File_Type_Flags::is_directory); + break; + + case DT_LNK: + flags = enum_set_flags( + flags, File_Type_Flags::is_symbolic_link_or_reparse_point); + break; + + default: + break; } - visit_file(entry->d_name, is_directory); + visit_file(entry->d_name, flags); }); #else #error "Unsupported platform" @@ -419,14 +446,16 @@ void list_directory_recursively(const char *directory, std::size_t path_length = this->path.size(); auto visit_child = [&](const char *child_name, - bool is_directory) -> void { + File_Type_Flags flags) -> void { this->path.resize(path_length); this->path += QLJS_PREFERRED_PATH_DIRECTORY_SEPARATOR; this->path += child_name; - if (is_directory) { + if (enum_has_flags(flags, File_Type_Flags::is_directory) && + !enum_has_flags( + flags, File_Type_Flags::is_symbolic_link_or_reparse_point)) { this->recurse(depth + 1); } else { - this->visitor.visit_file(path); + this->visitor.visit_file(path, flags); } }; diff --git a/src/quick-lint-js/io/temporary-directory.h b/src/quick-lint-js/io/temporary-directory.h index f582822b83..fc5310d510 100644 --- a/src/quick-lint-js/io/temporary-directory.h +++ b/src/quick-lint-js/io/temporary-directory.h @@ -38,6 +38,13 @@ void create_directory_or_exit(const std::string& path); Result make_timestamped_directory( std::string_view parent_directory, const char* format); +enum class File_Type_Flags : std::uint8_t { + none = 0, + + is_directory = 1 << 0, + is_symbolic_link_or_reparse_point = 1 << 1, +}; + // Call visit_file for each child of the given directory. // // '.' and '..' are excluded. @@ -48,7 +55,7 @@ Result list_directory( Temporary_Function_Ref visit_file); Result list_directory( const char* directory, - Temporary_Function_Ref visit_file); + Temporary_Function_Ref visit_file); QLJS_WARNING_PUSH // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69210 @@ -61,7 +68,10 @@ class List_Directory_Visitor { // 'directory' given to list_directory_recursively. // // visit_file is not called for '.' or '..' entries. - virtual void visit_file(const std::string& path) = 0; + // + // On Windows, visit_file is called for directory symbolic links and directory + // reparse points. + virtual void visit_file(const std::string& path, File_Type_Flags flags) = 0; // Called before descending into a directory. virtual void visit_directory_pre(const std::string& path); diff --git a/test/quick-lint-js/file-matcher.h b/test/quick-lint-js/file-matcher.h index 4ed89f8f65..0c5966204f 100644 --- a/test/quick-lint-js/file-matcher.h +++ b/test/quick-lint-js/file-matcher.h @@ -127,14 +127,16 @@ inline ::testing::AssertionResult assert_same_file( return assert_same_file(lhs_expr, rhs_expr, lhs_path.c_str(), rhs_path); } +// Does not follow symlinks. inline ::testing::AssertionResult assert_file_does_not_exist(const char* expr, const char* path) { bool exists; #if QLJS_HAVE_STD_FILESYSTEM - exists = std::filesystem::exists(std::filesystem::path(path)); + exists = std::filesystem::exists( + std::filesystem::symlink_status(std::filesystem::path(path))); #elif QLJS_HAVE_SYS_STAT_H struct ::stat s; - if (::stat(path, &s) == 0) { + if (::lstat(path, &s) == 0) { exists = true; } else { switch (errno) { diff --git a/test/quick-lint-js/filesystem-test.cpp b/test/quick-lint-js/filesystem-test.cpp index 064c0f13b0..133d399344 100644 --- a/test/quick-lint-js/filesystem-test.cpp +++ b/test/quick-lint-js/filesystem-test.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -35,7 +36,8 @@ namespace quick_lint_js { void delete_directory_recursive(const std::string& path) { struct Delete_Visitor : public List_Directory_Visitor { - void visit_file(const std::string& path) override { + void visit_file(const std::string& path, + [[maybe_unused]] File_Type_Flags flags) override { #if QLJS_HAVE_UNISTD_H int rc = std::remove(path.c_str()); if (rc != 0) { @@ -50,9 +52,20 @@ void delete_directory_recursive(const std::string& path) { path.c_str()); return; } - if (!::DeleteFileW(wpath->c_str())) { - std::fprintf(stderr, "warning: failed to delete %s: %s\n", path.c_str(), - windows_error_message(::GetLastError()).c_str()); + if (enum_has_flags(flags, File_Type_Flags::is_directory)) { + QLJS_ASSERT(enum_has_flags( + flags, File_Type_Flags::is_symbolic_link_or_reparse_point)); + if (!::RemoveDirectoryW(wpath->c_str())) { + std::fprintf( + stderr, "warning: failed to delete directory symlink %s: %s\n", + path.c_str(), windows_error_message(::GetLastError()).c_str()); + } + } else { + if (!::DeleteFileW(wpath->c_str())) { + std::fprintf(stderr, "warning: failed to delete %s: %s\n", + path.c_str(), + windows_error_message(::GetLastError()).c_str()); + } } #else #error "Unsupported platform" diff --git a/test/test-temporary-directory.cpp b/test/test-temporary-directory.cpp index 3eb50036c9..9893b05986 100644 --- a/test/test-temporary-directory.cpp +++ b/test/test-temporary-directory.cpp @@ -15,12 +15,15 @@ #include #include #include +#include #include #if QLJS_HAVE_UNISTD_H #include #endif +using namespace std::literals::string_view_literals; + namespace quick_lint_js { namespace { #if QLJS_HAVE_UNISTD_H @@ -61,6 +64,34 @@ TEST(Test_Temporary_Directory, } #endif +TEST(Test_Temporary_Directory, + delete_directory_containing_symlink_to_missing_directory) { + std::string temp_dir = make_temporary_directory(); + std::string sub_dir = temp_dir + "/sub_dir"; + create_directory_or_exit(sub_dir); + create_posix_directory_symbolic_link_or_exit((sub_dir + "/linkdir").c_str(), + "doesnotexist"); + + delete_directory_recursive(sub_dir); + + EXPECT_FILE_DOES_NOT_EXIST(sub_dir + "/linkdir"); + EXPECT_FILE_DOES_NOT_EXIST(sub_dir); +} + +TEST(Test_Temporary_Directory, + delete_file_containing_symlink_to_missing_directory) { + std::string temp_dir = make_temporary_directory(); + std::string sub_dir = temp_dir + "/sub_dir"; + create_directory_or_exit(sub_dir); + create_posix_file_symbolic_link_or_exit((sub_dir + "/linkfile").c_str(), + "doesnotexist"); + + delete_directory_recursive(sub_dir); + + EXPECT_FILE_DOES_NOT_EXIST(sub_dir + "/linkfile"); + EXPECT_FILE_DOES_NOT_EXIST(sub_dir); +} + TEST(Test_Temporary_Directory, creating_directory_over_existing_directory_fails) { std::string temp_dir = make_temporary_directory(); @@ -204,7 +235,7 @@ TEST_F(Test_Directory, list_directory_recursively) { create_directory_or_exit(temp_dir + "/dir-c"); struct Test_Visitor final : public List_Directory_Visitor { - void visit_file(const std::string& path) override { + void visit_file(const std::string& path, File_Type_Flags) override { this->visited_files.push_back(path); } @@ -247,12 +278,110 @@ TEST_F(Test_Directory, list_directory_recursively) { #undef SEP } +TEST_F(Test_Directory, list_directory_with_symlinks) { + std::string temp_dir = this->make_temporary_directory(); + create_directory_or_exit(temp_dir + "/dir"); + write_file_or_exit(temp_dir + "/file", u8""_sv); + // clang-format off + create_posix_directory_symbolic_link_or_exit((temp_dir + "/dir-symlink").c_str(), "dir"); + create_posix_directory_symbolic_link_or_exit((temp_dir + "/missing-dir-symlink").c_str(), "missing-dir"); + create_posix_file_symbolic_link_or_exit((temp_dir + "/file-symlink").c_str(), "file"); + create_posix_file_symbolic_link_or_exit((temp_dir + "/missing-file-symlink").c_str(), "missing-file"); + // clang-format on + + std::vector visited_files; + list_directory( + temp_dir.c_str(), [&](const char* name, File_Type_Flags flags) -> void { + if (name == "dir-symlink"sv || name == "missing-dir-symlink"sv) { + EXPECT_TRUE(enum_has_flags( + flags, File_Type_Flags::is_symbolic_link_or_reparse_point)); +#if defined(_WIN32) + EXPECT_TRUE(enum_has_flags(flags, File_Type_Flags::is_directory)); +#else + EXPECT_FALSE(enum_has_flags(flags, File_Type_Flags::is_directory)); +#endif + } else if (name == "file-symlink"sv || + name == "missing-file-symlink"sv) { + EXPECT_TRUE(enum_has_flags( + flags, File_Type_Flags::is_symbolic_link_or_reparse_point)); + EXPECT_FALSE(enum_has_flags(flags, File_Type_Flags::is_directory)); + } + visited_files.emplace_back(name); + }); + + EXPECT_THAT(visited_files, ::testing::UnorderedElementsAreArray({ + "dir", + "file", + "dir-symlink", + "missing-dir-symlink", + "file-symlink", + "missing-file-symlink", + })); +} + +TEST_F(Test_Directory, list_directory_recursively_with_symlinks) { + std::string temp_dir = this->make_temporary_directory(); + create_directory_or_exit(temp_dir + "/dir"); + write_file_or_exit(temp_dir + "/file", u8""_sv); + // clang-format off + create_posix_directory_symbolic_link_or_exit((temp_dir + "/dir-symlink").c_str(), "dir"); + create_posix_directory_symbolic_link_or_exit((temp_dir + "/missing-dir-symlink").c_str(), "missing-dir"); + create_posix_file_symbolic_link_or_exit((temp_dir + "/file-symlink").c_str(), "file"); + create_posix_file_symbolic_link_or_exit((temp_dir + "/missing-file-symlink").c_str(), "missing-file"); + // clang-format on + + struct Test_Visitor final : public List_Directory_Visitor { + void visit_file(const std::string& path, File_Type_Flags flags) override { + SCOPED_TRACE(path); + std::string_view name = path_file_name(path); + this->visited_files.emplace_back(name); + + if (name == "dir-symlink" || name == "missing-dir-symlink") { + EXPECT_TRUE(enum_has_flags( + flags, File_Type_Flags::is_symbolic_link_or_reparse_point)); +#if defined(_WIN32) + EXPECT_TRUE(enum_has_flags(flags, File_Type_Flags::is_directory)); +#else + EXPECT_FALSE(enum_has_flags(flags, File_Type_Flags::is_directory)); +#endif + } else if (name == "file-symlink" || name == "missing-file-symlink") { + EXPECT_TRUE(enum_has_flags( + flags, File_Type_Flags::is_symbolic_link_or_reparse_point)); + EXPECT_FALSE(enum_has_flags(flags, File_Type_Flags::is_directory)); + } + } + + void visit_directory_pre(const std::string&) override {} + + void visit_directory_post(const std::string&) override {} + + void on_error(const Platform_File_IO_Error& error, + [[maybe_unused]] int depth) override { + ADD_FAILURE() << error.to_string(); + } + + std::vector visited_files; + }; + Test_Visitor visitor; + list_directory_recursively(temp_dir.c_str(), visitor); + + EXPECT_THAT(visitor.visited_files, ::testing::UnorderedElementsAreArray({ + "file", + "dir-symlink", + "missing-dir-symlink", + "file-symlink", + "missing-file-symlink", + })); +} + TEST_F(Test_Directory, list_directory_recursively_on_regular_file_fails) { std::string temp_dir = this->make_temporary_directory(); write_file_or_exit(temp_dir + "/testfile", u8""_sv); struct Test_Visitor final : public List_Directory_Visitor { - void visit_file(const std::string& path) override { ADD_FAILURE() << path; } + void visit_file(const std::string& path, File_Type_Flags) override { + ADD_FAILURE() << path; + } void on_error(const Platform_File_IO_Error& error, int depth) override { SCOPED_TRACE(error.to_string()); diff --git a/tools/test-typescript/main.cpp b/tools/test-typescript/main.cpp index ae12b83937..1e03164525 100644 --- a/tools/test-typescript/main.cpp +++ b/tools/test-typescript/main.cpp @@ -304,7 +304,7 @@ void process_test_case_directory_or_file( const char* root_path) : expected_results(expected_results), root_path(root_path) {} - void visit_file(const std::string& file_path) override { + void visit_file(const std::string& file_path, File_Type_Flags) override { if (ends_with(file_path, ".ts"sv) || ends_with(file_path, ".tsx"sv)) { process_test_case_file(this->expected_results, file_path.c_str()); }