Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
@strager
strager committed Feb 2, 2024
1 parent cb5f85b commit 4ad2772
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 71 deletions.
57 changes: 12 additions & 45 deletions src/quick-lint-js/io/file-canonical.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_;

Expand Down Expand Up @@ -583,51 +586,15 @@ class Windows_Path_Canonicalizer

quick_lint_js::Result<void, Canonicalizing_Path_IO_Error>
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<void, Windows_File_IO_Error> 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 {};
}
Expand Down
104 changes: 78 additions & 26 deletions test/test-file-canonical.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
#include <string>

#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

Expand Down Expand Up @@ -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);

Expand All @@ -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_Path_Result, Canonicalize_Path_IO_Error> 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_Path_Result, Canonicalize_Path_IO_Error> 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();
Expand All @@ -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_Path_Result, Canonicalize_Path_IO_Error> 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_Path_Result, Canonicalize_Path_IO_Error> 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,
Expand All @@ -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");
}
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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<Canonical_Path_Result, Canonicalize_Path_IO_Error> 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_Path_Result, Canonicalize_Path_IO_Error> 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();
Expand All @@ -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();
Expand Down

0 comments on commit 4ad2772

Please sign in to comment.