Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Fix Windows file checks of unicode paths #16105

Merged
merged 2 commits into from
Jan 30, 2020
Merged

Fix Windows file checks of unicode paths #16105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69ab0f5
Fix Windows file checks of unicode paths
stuartmorgan-g Jan 28, 2020
6545dbe
Merge branch 'master' into windows-unicode-file-exists
stuartmorgan-g Jan 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions fml/file_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "flutter/fml/build_config.h"
#include "flutter/fml/file.h"
#include "flutter/fml/mapping.h"
#include "flutter/fml/paths.h"
#include "flutter/fml/unique_fd.h"

static bool WriteStringToFile(const fml::UniqueFD& fd,
Expand Down Expand Up @@ -267,3 +268,31 @@ TEST(FileTest, EmptyMappingTest) {

ASSERT_TRUE(fml::UnlinkFile(dir.fd(), "my_contents"));
}

TEST(FileTest, FileTestsWork) {
fml::ScopedTemporaryDirectory dir;
ASSERT_TRUE(dir.fd().is_valid());
const char* filename = "some.txt";
auto fd =
fml::OpenFile(dir.fd(), filename, true, fml::FilePermission::kWrite);
ASSERT_TRUE(fd.is_valid());
fd.reset();
ASSERT_TRUE(fml::FileExists(dir.fd(), filename));
ASSERT_TRUE(
fml::IsFile(fml::paths::JoinPaths({dir.path(), filename}).c_str()));
ASSERT_TRUE(fml::UnlinkFile(dir.fd(), filename));
}

TEST(FileTest, FileTestsSupportsUnicode) {
fml::ScopedTemporaryDirectory dir;
ASSERT_TRUE(dir.fd().is_valid());
const char* filename = u8"äëïöüテスト☃";
auto fd =
fml::OpenFile(dir.fd(), filename, true, fml::FilePermission::kWrite);
ASSERT_TRUE(fd.is_valid());
fd.reset();
ASSERT_TRUE(fml::FileExists(dir.fd(), filename));
ASSERT_TRUE(
fml::IsFile(fml::paths::JoinPaths({dir.path(), filename}).c_str()));
ASSERT_TRUE(fml::UnlinkFile(dir.fd(), filename));
}
29 changes: 18 additions & 11 deletions fml/platform/win/file_win.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <Shlwapi.h>
#include <fcntl.h>
#include <limits.h>
#include <sys/stat.h>

#include <algorithm>
#include <sstream>
Expand All @@ -18,10 +17,6 @@
#include "flutter/fml/platform/win/errors_win.h"
#include "flutter/fml/platform/win/wstring_conversion.h"

#if defined(OS_WIN)
#define S_ISREG(m) (((m)&S_IFMT) == S_IFREG)
#endif

namespace fml {

static std::string GetFullHandlePath(const fml::UniqueFD& handle) {
Expand Down Expand Up @@ -80,6 +75,16 @@ static DWORD GetShareFlags(FilePermission permission) {
return FILE_SHARE_READ;
}

static DWORD GetFileAttributesForUtf8Path(const char* absolute_path) {
return ::GetFileAttributes(ConvertToWString(absolute_path).c_str());
}

static DWORD GetFileAttributesForUtf8Path(const fml::UniqueFD& base_directory,
const char* path) {
std::string full_path = GetFullHandlePath(base_directory) + "\\" + path;
return GetFileAttributesForUtf8Path(full_path.c_str());
}

std::string CreateTemporaryDirectory() {
// Get the system temporary directory.
auto temp_dir_container = GetTemporaryDirectoryPath();
Expand Down Expand Up @@ -253,16 +258,17 @@ bool IsDirectory(const fml::UniqueFD& directory) {
}

bool IsDirectory(const fml::UniqueFD& base_directory, const char* path) {
std::string full_path = GetFullHandlePath(base_directory) + "\\" + path;
return ::GetFileAttributes(ConvertToWString(full_path.c_str()).c_str()) &
return GetFileAttributesForUtf8Path(base_directory, path) &
FILE_ATTRIBUTE_DIRECTORY;
}

bool IsFile(const std::string& path) {
struct stat buf;
if (stat(path.c_str(), &buf) != 0)
DWORD attributes = GetFileAttributesForUtf8Path(path.c_str());
if (attributes == INVALID_FILE_ATTRIBUTES) {
return false;
return S_ISREG(buf.st_mode);
}
return !(attributes &
Copy link
Member

@chinmaygarde chinmaygarde Jan 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not follow symlinks?

Copy link
Contributor Author

@stuartmorgan-g stuartmorgan-g Jan 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't S_ISREG be false for symlinks?

(As noted in the PR description, it's hard to implement these functions when there's absolutely no documentation about what they are intended to mean. Or tests enforcing any specific semantics...)

(FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_REPARSE_POINT));
}

bool UnlinkDirectory(const char* path) {
Expand Down Expand Up @@ -323,7 +329,8 @@ bool TruncateFile(const fml::UniqueFD& file, size_t size) {
}

bool FileExists(const fml::UniqueFD& base_directory, const char* path) {
return IsFile(GetAbsolutePath(base_directory, path).c_str());
return GetFileAttributesForUtf8Path(base_directory, path) !=
INVALID_FILE_ATTRIBUTES;
}

bool WriteAtomically(const fml::UniqueFD& base_directory,
Expand Down