diff --git a/src/quick-lint-js/io/file-canonical.cpp b/src/quick-lint-js/io/file-canonical.cpp index 9eee6132d9..52134ea3a9 100644 --- a/src/quick-lint-js/io/file-canonical.cpp +++ b/src/quick-lint-js/io/file-canonical.cpp @@ -425,6 +425,9 @@ class Path_Canonicalizer_Base { path_to_process_ = path_to_process_.substr(next_component_index); } + // TODO(strager): Have canonicalize_path accept an allocator. + Monotonic_Allocator allocator_{"Path_Canonicalizer_Base"}; + Canonicalize_Observer *observer_; Path_String_View original_path_; @@ -583,51 +586,15 @@ class Windows_Path_Canonicalizer quick_lint_js::Result process_start_of_path() { - std::wstring temp(path_to_process_); - - // The PathCch functions only support '\' as a directory separator. Convert - // all '/'s into '\'s. - for (wchar_t &c : temp) { - if (c == L'/') { - c = L'\\'; - } - } - - wchar_t *root_end; - HRESULT result = ::PathCchSkipRoot(temp.data(), &root_end); - switch (result) { - case S_OK: - // Path is absolute. - QLJS_ASSERT(root_end != temp.data()); - - path_to_process_ = path_to_process_.substr(root_end - temp.data()); - skip_to_next_component(); - - // Drop '\' from 'C:\' if present. - if (root_end[-1] == L'\\') { - --root_end; - } - canonical_.assign(temp.data(), root_end); - - need_root_slash_ = true; - break; - - case HRESULT_FROM_WIN32(ERROR_INVALID_PARAMETER): { - // Path is invalid or is relative. Assume that it is relative. - quick_lint_js::Result r = load_cwd(); - if (!r.ok()) { - return failed_result(Canonicalizing_Path_IO_Error{ - .canonicalizing_path = Path_String(this->path_to_process_), - .io_error = r.error(), - }); - } - break; - } - - default: - QLJS_UNIMPLEMENTED(); - break; - } + // @@@ what about empty path? + + // FIXME(strager): Do we need to copy (std::wstring) to add the null + // terminator? + Simplified_Path simplified_path = simplify_path_and_make_absolute( + &this->allocator_, std::wstring(path_to_process_).c_str()); + this->canonical_ = simplified_path.root; + this->path_to_process_ = simplified_path.relative; + this->need_root_slash_ = true; return {}; } diff --git a/test/test-file-canonical.cpp b/test/test-file-canonical.cpp index 363ee78b18..2b994425aa 100644 --- a/test/test-file-canonical.cpp +++ b/test/test-file-canonical.cpp @@ -27,10 +27,10 @@ #include #if defined(_WIN32) -// TODO(strager): This should be 1, not 0. Windows allows you to access -// 'c:\file.txt\.', for example. -#define QLJS_FILE_PATH_ALLOWS_FOLLOWING_COMPONENTS 0 +// Windows allows you to access 'c:\file.txt\.', for example. +#define QLJS_FILE_PATH_ALLOWS_FOLLOWING_COMPONENTS 1 #else +// POSIX does not allow you to access '/file.txt/.'. #define QLJS_FILE_PATH_ALLOWS_FOLLOWING_COMPONENTS 0 #endif @@ -262,7 +262,7 @@ TEST_F(Test_File_Canonical, canonical_path_removes_trailing_dot_component) { } TEST_F(Test_File_Canonical, - canonical_path_fails_with_dot_component_after_regular_file) { + canonical_path_fails_with_dot_and_component_after_regular_file) { std::string temp_dir = this->make_temporary_directory(); write_file_or_exit(temp_dir + "/just-a-file", u8""_sv); @@ -281,6 +281,36 @@ TEST_F(Test_File_Canonical, #endif } +#if QLJS_FILE_PATH_ALLOWS_FOLLOWING_COMPONENTS +TEST_F(Test_File_Canonical, canonical_path_removes_dot_after_regular_file) { + std::string temp_dir = this->make_temporary_directory(); + write_file_or_exit(temp_dir + "/just-a-file", u8""_sv); + + std::string input_path = temp_dir + "/just-a-file/."; + Result canonical = + canonicalize_path(input_path); + ASSERT_TRUE(canonical.ok()) << canonical.error().to_string(); + + EXPECT_THAT(std::string(canonical->path()), Not(::testing::EndsWith("/."))); + EXPECT_THAT(std::string(canonical->path()), Not(::testing::EndsWith("\\."))); + EXPECT_SAME_FILE(canonical->path(), input_path); +} +#else +TEST_F(Test_File_Canonical, canonical_path_fails_with_dot_after_regular_file) { + std::string temp_dir = this->make_temporary_directory(); + write_file_or_exit(temp_dir + "/just-a-file", u8""_sv); + + std::string input_path = temp_dir + "/just-a-file/."; + Result canonical = + canonicalize_path(input_path); + ASSERT_FALSE(canonical.ok()); + EXPECT_EQ(canonical.error().input_path, input_path); + EXPECT_THAT(canonical.error().canonicalizing_path, + ::testing::EndsWith("just-a-file")); + EXPECT_EQ(canonical.error().io_error.error, ENOTDIR); +} +#endif + TEST_F(Test_File_Canonical, canonical_path_removes_dot_components_after_missing_path) { std::string temp_dir = this->make_temporary_directory(); @@ -301,28 +331,42 @@ TEST_F(Test_File_Canonical, EXPECT_THAT(std::string(canonical->path()), Not(HasSubstr("\\.\\"))); } -// TODO(strager): This test is wrong if -// QLJS_FILE_PATH_ALLOWS_FOLLOWING_COMPONENTS. +#if QLJS_FILE_PATH_ALLOWS_FOLLOWING_COMPONENTS +TEST_F(Test_File_Canonical, + canonical_path_simplifies_dot_dot_component_after_regular_file) { + std::string temp_dir = this->make_temporary_directory(); + write_file_or_exit(temp_dir + "/just-a-file", u8""_sv); + write_file_or_exit(temp_dir + "/other.txt", u8""_sv); + + std::string input_path = temp_dir + "/just-a-file/../other.txt"; + Result canonical = + canonicalize_path(input_path); + ASSERT_TRUE(canonical.ok()) << canonical.error().to_string(); + + EXPECT_THAT(std::string(canonical->path()), Not(HasSubstr("/../"))); + EXPECT_THAT(std::string(canonical->path()), Not(HasSubstr("\\..\\"))); + EXPECT_THAT(std::string(canonical->path()), Not(HasSubstr("/just-a-file/"))); + EXPECT_THAT(std::string(canonical->path()), + Not(HasSubstr("\\just-a-file\\"))); + EXPECT_SAME_FILE(canonical->path(), temp_dir + "/other.txt"); +} +#else TEST_F(Test_File_Canonical, canonical_path_fails_with_dot_dot_component_after_regular_file) { std::string temp_dir = this->make_temporary_directory(); write_file_or_exit(temp_dir + "/just-a-file", u8""_sv); write_file_or_exit(temp_dir + "/other.txt", u8""_sv); - std::string input_path = temp_dir + "/just-a-file/../other.text"; + std::string input_path = temp_dir + "/just-a-file/../other.txt"; Result canonical = canonicalize_path(input_path); ASSERT_FALSE(canonical.ok()); EXPECT_EQ(canonical.error().input_path, input_path); EXPECT_THAT(canonical.error().canonicalizing_path, ::testing::EndsWith("just-a-file")); -#if QLJS_HAVE_WINDOWS_H - EXPECT_EQ(canonical.error().io_error.error, ERROR_DIRECTORY); -#endif -#if QLJS_HAVE_UNISTD_H EXPECT_EQ(canonical.error().io_error.error, ENOTDIR); -#endif } +#endif #if QLJS_FILE_PATH_ALLOWS_FOLLOWING_COMPONENTS TEST_F(Test_File_Canonical, @@ -335,8 +379,8 @@ TEST_F(Test_File_Canonical, canonicalize_path(input_path); ASSERT_TRUE(canonical.ok()) << canonical.error().to_string(); - EXPECT_FALSE(ends_with(canonical->path(), "/..")) << canonical.path(); - EXPECT_FALSE(ends_with(canonical->path(), "\\..")) << canonical.path(); + EXPECT_THAT(std::string(canonical->path()), Not(::testing::EndsWith("/."))); + EXPECT_THAT(std::string(canonical->path()), Not(::testing::EndsWith("\\."))); EXPECT_THAT(std::string(canonical->path()), Not(HasSubstr("fake-subdir"))); EXPECT_SAME_FILE(canonical->path(), temp_dir + "/just-a-file"); } @@ -353,12 +397,7 @@ TEST_F(Test_File_Canonical, EXPECT_EQ(canonical.error().input_path, input_path); EXPECT_THAT(canonical.error().canonicalizing_path, ::testing::EndsWith("just-a-file")); -#if QLJS_HAVE_WINDOWS_H - EXPECT_EQ(canonical.error().io_error.error, ERROR_DIRECTORY); -#endif -#if QLJS_HAVE_UNISTD_H EXPECT_EQ(canonical.error().io_error.error, ENOTDIR); -#endif } #endif @@ -372,8 +411,8 @@ TEST_F(Test_File_Canonical, canonical_path_with_dot_after_regular_file) { canonicalize_path(input_path); ASSERT_TRUE(canonical.ok()) << canonical.error().to_string(); - EXPECT_FALSE(ends_with(canonical->path(), "/.")) << canonical.path(); - EXPECT_FALSE(ends_with(canonical->path(), "\\.")) << canonical.path(); + EXPECT_THAT(std::string(canonical->path()), Not(::testing::EndsWith("/."))); + EXPECT_THAT(std::string(canonical->path()), Not(::testing::EndsWith("\\."))); EXPECT_SAME_FILE(canonical->path(), temp_dir + "/just-a-file"); } #else @@ -389,12 +428,7 @@ TEST_F(Test_File_Canonical, EXPECT_EQ(canonical.error().input_path, input_path); EXPECT_THAT(canonical.error().canonicalizing_path, ::testing::EndsWith("just-a-file")); -#if QLJS_HAVE_WINDOWS_H - EXPECT_EQ(canonical.error().io_error.error, ERROR_DIRECTORY); -#endif -#if QLJS_HAVE_UNISTD_H EXPECT_EQ(canonical.error().io_error.error, ENOTDIR); -#endif } #endif @@ -518,6 +552,23 @@ TEST_F(Test_File_Canonical, } } +#if QLJS_FILE_PATH_ALLOWS_FOLLOWING_COMPONENTS +TEST_F(Test_File_Canonical, + canonical_path_folds_dot_dot_components_after_missing_path) { + std::string temp_dir = this->make_temporary_directory(); + Result temp_dir_canonical = + canonicalize_path(temp_dir); + ASSERT_TRUE(temp_dir_canonical.ok()) + << temp_dir_canonical.error().to_string(); + write_file_or_exit(temp_dir + "/real.js", u8""_sv); + + Result canonical = + canonicalize_path(temp_dir + "/does-not-exist/../real.js"); + ASSERT_TRUE(canonical.ok()) << canonical.error().to_string(); + + EXPECT_SAME_FILE(canonical->path(), temp_dir + "/real.js"); +} +#else TEST_F(Test_File_Canonical, canonical_path_keeps_dot_dot_components_after_missing_path) { std::string temp_dir = this->make_temporary_directory(); @@ -538,6 +589,7 @@ TEST_F(Test_File_Canonical, ".." QLJS_PREFERRED_PATH_DIRECTORY_SEPARATOR + "real.js"); EXPECT_TRUE(canonical->have_missing_components()); } +#endif TEST_F(Test_File_Canonical, canonical_path_makes_relative_paths_absolute) { std::string temp_dir = this->make_temporary_directory();