Skip to content

Commit

Permalink
Windows: fix JNI's DeletePath function
Browse files Browse the repository at this point in the history
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 bazelbuild#7173

Closes bazelbuild#7176.

PiperOrigin-RevId: 229913623
  • Loading branch information
laszlocsomor authored and weixiao-huang committed Jan 31, 2019
1 parent e77a6d3 commit 84d9e5f
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
48 changes: 46 additions & 2 deletions src/main/native/windows/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,31 @@ 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) {
Expand Down Expand Up @@ -365,7 +390,17 @@ 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) {
Expand Down Expand Up @@ -435,7 +470,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;
Expand Down Expand Up @@ -470,6 +505,15 @@ int DeletePath(const wstring& path, wstring* error) {
}
return DeletePathResult::kError;
}
} else {
if (error) {
*error = MakeErrorMessage(
WSTR(__FILE__), __LINE__,
(std::wstring(L"Unknown error, winpath=[") + winpath + L"]")
.c_str(),
path, err);
}
return DeletePathResult::kError;
}
}
return DeletePathResult::kSuccess;
Expand Down
2 changes: 2 additions & 0 deletions src/main/native/windows/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
26 changes: 26 additions & 0 deletions src/test/native/windows/file_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 84d9e5f

Please sign in to comment.