Skip to content

Fix false positives by filesystem::equivalent on file systems with transient file IDs #5130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 5, 2024
Merged

Fix false positives by filesystem::equivalent on file systems with transient file IDs #5130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
50d5426
Fix false positives by `filesystem::equivalent` on file systems with …
muellerj2 Nov 24, 2024
a87b43f
restore __std_fs_get_file_id()
muellerj2 Nov 24, 2024
34368ed
Mark definitions as `[[nodiscard]]`.
StephanTLavavej Dec 2, 2024
edf968c
Add top-level const.
StephanTLavavej Dec 2, 2024
4878205
`_Last_error` is `__std_win_error::_Success` at the end.
StephanTLavavej Dec 2, 2024
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
31 changes: 7 additions & 24 deletions stl/inc/filesystem
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3431,38 +3431,21 @@ namespace filesystem {
return _STD filesystem::copy_file(_From, _To, copy_options::none);
}

_NODISCARD inline pair<__std_win_error, bool> _Equivalent(
const wchar_t* const _Lhs, const wchar_t* const _Rhs) noexcept {
__std_fs_file_id _Left_id;
__std_fs_file_id _Right_id;
auto _Last_error = __std_fs_get_file_id(&_Left_id, _Lhs);
if (_Last_error != __std_win_error::_Success) {
return {_Last_error, false};
}

_Last_error = __std_fs_get_file_id(&_Right_id, _Rhs);
if (_Last_error != __std_win_error::_Success) {
return {_Last_error, false};
}

return {__std_win_error::_Success, _CSTD memcmp(&_Left_id, &_Right_id, sizeof(__std_fs_file_id)) == 0};
}

_EXPORT_STD _NODISCARD inline bool equivalent(const path& _Lhs, const path& _Rhs) {
// test if the paths _Lhs and _Rhs refer to the same file
const auto _Result = _Equivalent(_Lhs.c_str(), _Rhs.c_str());
if (_Result.first != __std_win_error::_Success) {
_Throw_fs_error("equivalent", _Result.first, _Lhs, _Rhs);
const auto _Result = __std_fs_equivalent(_Lhs.c_str(), _Rhs.c_str());
if (_Result._Error != __std_win_error::_Success) {
_Throw_fs_error("equivalent", _Result._Error, _Lhs, _Rhs);
}

return _Result.second;
return _Result._Equivalent;
}

_EXPORT_STD _NODISCARD inline bool equivalent(const path& _Lhs, const path& _Rhs, error_code& _Ec) noexcept {
// test if the paths _Lhs and _Rhs refer to the same file
const auto _Result = _Equivalent(_Lhs.c_str(), _Rhs.c_str());
_Ec = _Make_ec(_Result.first);
return _Result.second;
const auto _Result = __std_fs_equivalent(_Lhs.c_str(), _Rhs.c_str());
_Ec = _Make_ec(_Result._Error);
return _Result._Equivalent;
}

_EXPORT_STD _NODISCARD inline file_status status(const path& _Path);
Expand Down
14 changes: 7 additions & 7 deletions stl/inc/xfilesystem_abi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,6 @@ struct __std_fs_convert_result {
__std_win_error _Err;
};

struct __std_fs_file_id { // typedef struct _FILE_ID_INFO {
unsigned long long _Volume_serial_number; // ULONGLONG VolumeSerialNumber;
unsigned char _Id[16]; // FILE_ID_128 FileId;
}; // } FILE_ID_INFO, ...;

enum class __std_fs_copy_options {
_None = 0x0,

Expand Down Expand Up @@ -303,8 +298,13 @@ _NODISCARD __std_fs_convert_result __stdcall __std_fs_convert_wide_to_narrow_rep
_In_ __std_code_page _Code_page, _In_reads_(_Input_len) const wchar_t* _Input_str, _In_ int _Input_len,
_Out_writes_opt_(_Output_len) char* _Output_str, _In_ int _Output_len) noexcept;

_NODISCARD _Success_(return == __std_win_error::_Success) __std_win_error
__stdcall __std_fs_get_file_id(_Out_ __std_fs_file_id* _Id, _In_z_ const wchar_t* _Path) noexcept;
struct __std_fs_equivalent_result {
bool _Equivalent;
__std_win_error _Error;
};

_NODISCARD __std_fs_equivalent_result __stdcall __std_fs_equivalent(
_In_z_ const wchar_t* _Path1, _In_z_ const wchar_t* _Path2) noexcept;

_NODISCARD __std_win_error __stdcall __std_fs_set_last_write_time(
_In_ long long _Last_write_filetime, _In_z_ const wchar_t* _Path) noexcept;
Expand Down
39 changes: 23 additions & 16 deletions stl/src/filesys.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,22 +320,26 @@ _FS_DLL space_info __CLRCALL_PURE_OR_CDECL _Statvfs(const wchar_t* _Fname) noexc

_FS_DLL int __CLRCALL_PURE_OR_CDECL _Equivalent(
const wchar_t* _Fname1, const wchar_t* _Fname2) noexcept { // test for equivalent file names
// See GH-3571: File IDs are only guaranteed to be unique and stable while handles remain open
#ifdef _CRT_APP
_FILE_ID_INFO _Info1 = {0};
_FILE_ID_INFO _Info2 = {0};
bool _Ok1 = false;
bool _Ok2 = false;

HANDLE _Handle = _FilesysOpenFile(_Fname1, FILE_READ_ATTRIBUTES, FILE_FLAG_BACKUP_SEMANTICS);
if (_Handle != INVALID_HANDLE_VALUE) { // get file1 info
_Ok1 = GetFileInformationByHandleEx(_Handle, FileIdInfo, &_Info1, sizeof(_Info1)) != 0;
CloseHandle(_Handle);
HANDLE _Handle1 = _FilesysOpenFile(_Fname1, FILE_READ_ATTRIBUTES, FILE_FLAG_BACKUP_SEMANTICS);
if (_Handle1 != INVALID_HANDLE_VALUE) { // get file1 info
_Ok1 = GetFileInformationByHandleEx(_Handle1, FileIdInfo, &_Info1, sizeof(_Info1)) != 0;
}

_Handle = _FilesysOpenFile(_Fname2, FILE_READ_ATTRIBUTES, FILE_FLAG_BACKUP_SEMANTICS);
if (_Handle != INVALID_HANDLE_VALUE) { // get file2 info
_Ok2 = GetFileInformationByHandleEx(_Handle, FileIdInfo, &_Info2, sizeof(_Info2)) != 0;
CloseHandle(_Handle);
HANDLE _Handle2 = _FilesysOpenFile(_Fname2, FILE_READ_ATTRIBUTES, FILE_FLAG_BACKUP_SEMANTICS);
if (_Handle2 != INVALID_HANDLE_VALUE) { // get file2 info
_Ok2 = GetFileInformationByHandleEx(_Handle2, FileIdInfo, &_Info2, sizeof(_Info2)) != 0;
CloseHandle(_Handle2);
}

if (_Handle1 != INVALID_HANDLE_VALUE) {
CloseHandle(_Handle1);
}

if (!_Ok1 && !_Ok2) {
Expand All @@ -351,16 +355,19 @@ _FS_DLL int __CLRCALL_PURE_OR_CDECL _Equivalent(
bool _Ok1 = false;
bool _Ok2 = false;

HANDLE _Handle = _FilesysOpenFile(_Fname1, FILE_READ_ATTRIBUTES, FILE_FLAG_BACKUP_SEMANTICS);
if (_Handle != INVALID_HANDLE_VALUE) { // get file1 info
_Ok1 = GetFileInformationByHandle(_Handle, &_Info1) != 0;
CloseHandle(_Handle);
HANDLE _Handle1 = _FilesysOpenFile(_Fname1, FILE_READ_ATTRIBUTES, FILE_FLAG_BACKUP_SEMANTICS);
if (_Handle1 != INVALID_HANDLE_VALUE) { // get file1 info
_Ok1 = GetFileInformationByHandle(_Handle1, &_Info1) != 0;
}

HANDLE _Handle2 = _FilesysOpenFile(_Fname2, FILE_READ_ATTRIBUTES, FILE_FLAG_BACKUP_SEMANTICS);
if (_Handle2 != INVALID_HANDLE_VALUE) { // get file2 info
_Ok2 = GetFileInformationByHandle(_Handle2, &_Info2) != 0;
CloseHandle(_Handle2);
}

_Handle = _FilesysOpenFile(_Fname2, FILE_READ_ATTRIBUTES, FILE_FLAG_BACKUP_SEMANTICS);
if (_Handle != INVALID_HANDLE_VALUE) { // get file2 info
_Ok2 = GetFileInformationByHandle(_Handle, &_Info2) != 0;
CloseHandle(_Handle);
if (_Handle1 != INVALID_HANDLE_VALUE) {
CloseHandle(_Handle1);
}

if (!_Ok1 && !_Ok2) {
Expand Down
39 changes: 38 additions & 1 deletion stl/src/filesystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,13 @@ void __stdcall __std_fs_directory_iterator_close(_In_ const __std_fs_dir_handle
return __vcp_Copyfile(_Source, _Target, /* _Fail_if_exists = */ false);
}

_Success_(return == __std_win_error::_Success) __std_win_error
struct __std_fs_file_id { // typedef struct _FILE_ID_INFO {
unsigned long long _Volume_serial_number; // ULONGLONG VolumeSerialNumber;
unsigned char _Id[16]; // FILE_ID_128 FileId;
}; // } FILE_ID_INFO, ...;

// TRANSITION, ABI: preserved for binary compatibility
[[nodiscard]] _Success_(return == __std_win_error::_Success) __std_win_error
__stdcall __std_fs_get_file_id(_Out_ __std_fs_file_id* const _Id, _In_z_ const wchar_t* const _Path) noexcept {
__std_win_error _Last_error;
const _STD _Fs_file _Handle(
Expand All @@ -448,6 +454,37 @@ _Success_(return == __std_win_error::_Success) __std_win_error
return _Get_file_id_by_handle(_Handle._Get(), reinterpret_cast<FILE_ID_INFO*>(_Id));
}

[[nodiscard]] __std_fs_equivalent_result __stdcall __std_fs_equivalent(
_In_z_ const wchar_t* const _Left_path, _In_z_ const wchar_t* const _Right_path) noexcept {
// See GH-3571: File IDs are only guaranteed to be unique and stable while handles remain open
__std_win_error _Last_error;
const _STD _Fs_file _Left_handle(
_Left_path, __std_access_rights::_File_read_attributes, __std_fs_file_flags::_Backup_semantics, &_Last_error);
if (_Last_error != __std_win_error::_Success) {
return {false, _Last_error};
}

FILE_ID_INFO _Left_info;
_Last_error = _Get_file_id_by_handle(_Left_handle._Get(), &_Left_info);
if (_Last_error != __std_win_error::_Success) {
return {false, _Last_error};
}

const _STD _Fs_file _Right_handle(
_Right_path, __std_access_rights::_File_read_attributes, __std_fs_file_flags::_Backup_semantics, &_Last_error);
if (_Last_error != __std_win_error::_Success) {
return {false, _Last_error};
}

FILE_ID_INFO _Right_info;
_Last_error = _Get_file_id_by_handle(_Right_handle._Get(), &_Right_info);
if (_Last_error != __std_win_error::_Success) {
return {false, _Last_error};
}

return {_CSTD memcmp(&_Left_info, &_Right_info, sizeof(FILE_ID_INFO)) == 0, __std_win_error::_Success};
}

[[nodiscard]] __std_win_error __stdcall __std_fs_create_directory_symbolic_link(
_In_z_ const wchar_t* const _Symlink_file_name, _In_z_ const wchar_t* const _Target_file_name) noexcept {
return _Create_symlink(_Symlink_file_name, _Target_file_name, SYMBOLIC_LINK_FLAG_DIRECTORY);
Expand Down