From e41deab27a80304a3c86fd2315cfd572e4c47905 Mon Sep 17 00:00:00 2001 From: Laszlo Csomor Date: Fri, 18 Jan 2019 12:28:07 +0100 Subject: [PATCH] Windows: fix JNI's DeletePath function DeletePath now verifies that the input path is Windows-style (i.e. uses backslashes) and absolute, and adds the `\\?\` prefix to the path. Doing so ensures it can delete any file (including degenerately-named ones like `c:\foo\nul`). Fixes https://github.com/bazelbuild/bazel/issues/7173 --- .../windows/jni/WindowsFileOperations.java | 2 +- src/main/native/windows/file.cc | 50 ++++++++++++++++++- src/main/native/windows/file.h | 2 + src/test/native/windows/file_test.cc | 26 ++++++++++ 4 files changed, 77 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java index 8401d743f633fc..741533ed4d0570 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java +++ b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java @@ -163,7 +163,7 @@ public static void createJunction(String name, String target) throws IOException public static boolean deletePath(String path) throws IOException { WindowsJniLoader.loadJni(); String[] error = new String[] {null}; - int result = nativeDeletePath(asLongPath(path), error); + int result = nativeDeletePath(asLongPath(path.replace('/', '\\')), error); switch (result) { case DELETE_PATH_SUCCESS: return true; diff --git a/src/main/native/windows/file.cc b/src/main/native/windows/file.cc index eb4a0a3767bc2d..e2a31d521d07c8 100644 --- a/src/main/native/windows/file.cc +++ b/src/main/native/windows/file.cc @@ -15,6 +15,7 @@ #include // uint8_t #include +#include #include #include #include @@ -38,6 +39,32 @@ wstring RemoveUncPrefixMaybe(const wstring& path) { return bazel::windows::HasUncPrefix(path.c_str()) ? path.substr(4) : path; } +bool HasDriveSpecifierPrefix(const wstring& p) { + if (HasUncPrefix(p.c_str())) { + return p.size() >= 7 && iswalpha(p[4]) && p[5] == ':' && p[6] == '\\'; + } else { + return p.size() >= 3 && iswalpha(p[0]) && p[1] == ':' && p[2] == '\\'; + } +} + +bool IsAbsoluteNormalizedWindowsPath(const wstring& p) { + if (p.empty()) { + return false; + } + if (IsDevNull(p.c_str())) { + return true; + } + if (p.find_first_of('/') != wstring::npos) { + return false; + } + + return HasDriveSpecifierPrefix(p) && + p.find(L".\\") != 0 && p.find(L"\\.\\") == wstring::npos && + p.find(L"\\.") != p.size() - 2 && + p.find(L"..\\") != 0 && p.find(L"\\..\\") == wstring::npos && + p.find(L"\\..") != p.size() - 3; +} + static wstring uint32asHexString(uint32_t value) { WCHAR attr_str[8]; for (int i = 0; i < 8; ++i) { @@ -365,7 +392,18 @@ int CreateJunction(const wstring& junction_name, const wstring& junction_target, } int DeletePath(const wstring& path, wstring* error) { - const wchar_t* wpath = path.c_str(); + if (!IsAbsoluteNormalizedWindowsPath(path)) { + if (error) { + *error = MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"DeletePath", path, + L"expected an absolute Windows path"); + } + return DeletePathResult::kError; + } + + const std::wstring winpath(AddUncPrefixMaybe(path)); + const wchar_t* wpath = winpath.c_str(); + if (!DeleteFileW(wpath)) { DWORD err = GetLastError(); if (err == ERROR_SHARING_VIOLATION) { @@ -435,7 +473,7 @@ int DeletePath(const wstring& path, wstring* error) { } return DeletePathResult::kError; } - } else { + } else if (attr & FILE_ATTRIBUTE_READONLY) { // It's a file and it's probably read-only. // Make it writable then try deleting it again. attr &= ~FILE_ATTRIBUTE_READONLY; @@ -470,6 +508,14 @@ int DeletePath(const wstring& path, wstring* error) { } return DeletePathResult::kError; } + } else { + if (error) { + *error = MakeErrorMessage(WSTR(__FILE__), __LINE__, + (std::wstring(L"Unkown error, winpath=[") + + winpath + L"]").c_str(), + path, err); + } + return DeletePathResult::kError; } } return DeletePathResult::kSuccess; diff --git a/src/main/native/windows/file.h b/src/main/native/windows/file.h index b850902f216a13..05a327434e11c0 100644 --- a/src/main/native/windows/file.h +++ b/src/main/native/windows/file.h @@ -45,6 +45,8 @@ std::wstring AddUncPrefixMaybe(const std::wstring& path); std::wstring RemoveUncPrefixMaybe(const std::wstring& path); +bool IsAbsoluteNormalizedWindowsPath(const std::wstring& p); + // Keep in sync with j.c.g.devtools.build.lib.windows.WindowsFileOperations enum { IS_JUNCTION_YES = 0, diff --git a/src/test/native/windows/file_test.cc b/src/test/native/windows/file_test.cc index 4792aaae61179e..5919a37376b87b 100644 --- a/src/test/native/windows/file_test.cc +++ b/src/test/native/windows/file_test.cc @@ -48,6 +48,32 @@ class WindowsFileOperationsTest : public ::testing::Test { void TearDown() override { DeleteAllUnder(GetTestTmpDirW()); } }; +TEST_F(WindowsFileOperationsTest, TestIsAbsoluteWindowsStylePath) { + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"")); + EXPECT_TRUE(IsAbsoluteNormalizedWindowsPath(L"NUL")); + EXPECT_TRUE(IsAbsoluteNormalizedWindowsPath(L"nul")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"c")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"\\\\?\\c")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"c:")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"\\\\?\\c:")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"c:/")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"\\\\?\\c:/")); + EXPECT_TRUE(IsAbsoluteNormalizedWindowsPath(L"c:\\")); + EXPECT_TRUE(IsAbsoluteNormalizedWindowsPath(L"\\\\?\\c:\\")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"c:\\foo/bar")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"\\\\?\\c:\\foo/bar")); + EXPECT_TRUE(IsAbsoluteNormalizedWindowsPath(L"c:\\foo\\bar")); + EXPECT_TRUE(IsAbsoluteNormalizedWindowsPath(L"\\\\?\\c:\\foo\\bar")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"foo")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"foo\\bar")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"c:\\foo\\.")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"\\\\?\\c:\\foo\\.")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"c:\\foo\\.\\bar")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"\\\\?\\c:\\foo\\.\\bar")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"c:\\foo\\..\\bar")); + EXPECT_FALSE(IsAbsoluteNormalizedWindowsPath(L"\\\\?\\c:\\foo\\..\\bar")); +} + TEST_F(WindowsFileOperationsTest, TestCreateJunction) { wstring tmp(kUncPrefix + GetTestTmpDirW()); wstring target(tmp + L"\\junc_target");