[PATCH v7 0/2] MR8459: Draft: urlmon: Add PersistentZoneIdentifier implementation
Implementation PersistentZoneIdentifer object without any writing and reading zone information. OS Windows uses NTFS alternative data stream Zone.Identifer to write and read zone information. Also due to lack of reading zone information some test cases fails. -- v7: urlmon: Add PersistentZoneIdentifier implementation urlmon: Add PersistentZoneIdentifier test cases https://gitlab.winehq.org/wine/wine/-/merge_requests/8459
From: Mike Kozelkov <augenzi(a)etersoft.ru> --- dlls/urlmon/tests/Makefile.in | 3 +- dlls/urlmon/tests/zone_id.c | 580 ++++++++++++++++++++++++++++++++++ 2 files changed, 582 insertions(+), 1 deletion(-) create mode 100644 dlls/urlmon/tests/zone_id.c diff --git a/dlls/urlmon/tests/Makefile.in b/dlls/urlmon/tests/Makefile.in index 797a3434e1f..ed3cacd02fd 100644 --- a/dlls/urlmon/tests/Makefile.in +++ b/dlls/urlmon/tests/Makefile.in @@ -8,4 +8,5 @@ SOURCES = \ sec_mgr.c \ stream.c \ uri.c \ - url.c + url.c \ + zone_id.c diff --git a/dlls/urlmon/tests/zone_id.c b/dlls/urlmon/tests/zone_id.c new file mode 100644 index 00000000000..a4723805ab8 --- /dev/null +++ b/dlls/urlmon/tests/zone_id.c @@ -0,0 +1,580 @@ +#define COBJMACROS +#define CONST_VTABLE + +#include <wine/test.h> +#include "winbase.h" + +#include "urlmon.h" + +#define URLZONE_CUSTOM1 URLZONE_UNTRUSTED + 1 +#define URLZONE_CUSTOM2 URLZONE_USER_MAX + 1 + +#define TMP_PATH_MAX 2 * MAX_PATH + +typedef struct _zone_id_op_test { + URLZONE id; + HRESULT hres; +} zone_id_op_test; + +typedef struct _zone_id_test { + zone_id_op_test set; + zone_id_op_test get; +} zone_id_test; + +static const zone_id_test zone_id_tests[] = { + { + { URLZONE_INVALID, E_INVALIDARG }, + { URLZONE_UNTRUSTED, E_ACCESSDENIED } + }, + { + { URLZONE_LOCAL_MACHINE, S_OK }, + { URLZONE_LOCAL_MACHINE, S_OK } + }, + { + { URLZONE_INTRANET, S_OK }, + { URLZONE_INTRANET, S_OK } + }, + { + { URLZONE_TRUSTED, S_OK }, + { URLZONE_TRUSTED, S_OK } + }, + { + { URLZONE_INTERNET, S_OK }, + { URLZONE_INTERNET, S_OK } + }, + { + { URLZONE_UNTRUSTED, S_OK }, + { URLZONE_UNTRUSTED, S_OK } + }, + { + { URLZONE_CUSTOM1, E_INVALIDARG }, + { URLZONE_UNTRUSTED, E_ACCESSDENIED } + }, + { + { URLZONE_USER_MIN, E_INVALIDARG }, + { URLZONE_UNTRUSTED, E_ACCESSDENIED } + }, + { + { URLZONE_USER_MAX, E_INVALIDARG }, + { URLZONE_UNTRUSTED, E_ACCESSDENIED } + }, + { + { URLZONE_CUSTOM2, E_INVALIDARG }, + { URLZONE_UNTRUSTED, E_ACCESSDENIED } + } +}; + +typedef struct _get_cur_file_test { + LPCWSTR name; + HRESULT hres; +} get_cur_file_test; + +typedef struct load_file_test { + LPCWSTR name; + DWORD mode; + HRESULT hres; +} load_file_test; + +typedef struct _save_file_test { + LPCWSTR name; + BOOL remember; + HRESULT hres; +} save_file_test; + +typedef struct _persist_file_test { + zone_id_op_test set; + HRESULT is_dirty_before_save; + save_file_test save; + HRESULT is_dirty_after_save; + get_cur_file_test cur_file; + HRESULT is_dirty_before_load; + load_file_test load; + HRESULT is_dirty_after_load; + zone_id_op_test get; +} persist_file_test; + +static const persist_file_test persist_file_tests[] = { + { + { URLZONE_UNTRUSTED, S_OK }, + S_OK, + { NULL, FALSE, E_INVALIDARG }, + S_OK, + { NULL, E_NOTIMPL }, + S_OK, + { NULL, 0, E_INVALIDARG }, + S_OK, + { URLZONE_UNTRUSTED, __HRESULT_FROM_WIN32(ERROR_NOT_FOUND) } + }, + { + { URLZONE_INTERNET, S_OK }, + S_OK, + { L"00000000-0000-0000-0000-000000000000", FALSE, S_OK }, + S_OK, + { NULL, E_NOTIMPL }, + S_OK, + { L"00000000-0000-0000-0000-000000000000", STGM_READWRITE | STGM_SHARE_DENY_NONE, S_OK }, + S_OK, + { URLZONE_INTERNET, S_OK } + }, + { + { URLZONE_LOCAL_MACHINE, S_OK }, + S_OK, + { L"00000000-0000-0000-0000-000000000000", TRUE, S_OK }, + S_OK, + { NULL, E_NOTIMPL }, + S_OK, + { L"00000000-0000-0000-0000-000000000000", STGM_READWRITE | STGM_SHARE_DENY_NONE, S_OK }, + S_OK, + { URLZONE_LOCAL_MACHINE, S_OK } + }, + { + { URLZONE_TRUSTED, S_OK }, + S_OK, + { NULL, FALSE, S_OK }, + S_OK, + { NULL, E_NOTIMPL }, + S_OK, + { L"00000000-0000-0000-0000-000000000000", STGM_READWRITE | STGM_SHARE_DENY_NONE, S_OK }, + S_OK, + { URLZONE_TRUSTED, S_OK } + }, + { + { URLZONE_INTRANET, S_OK }, + S_OK, + { NULL, TRUE, S_OK }, + S_OK, + { NULL, E_NOTIMPL }, + S_OK, + { L"00000000-0000-0000-0000-000000000000", STGM_READWRITE | STGM_SHARE_DENY_NONE, S_OK }, + S_OK, + { URLZONE_INTRANET, S_OK } + }, + { + { URLZONE_INTERNET, S_OK }, + S_OK, + { L"11111111-1111-1111-1111-111111111111", FALSE, S_OK }, + S_OK, + { NULL, E_NOTIMPL }, + S_OK, + { L"11111111-1111-1111-1111-111111111111", STGM_READWRITE | STGM_SHARE_DENY_NONE, S_OK }, + S_OK, + { URLZONE_INTERNET, S_OK } + }, + { + { URLZONE_LOCAL_MACHINE, S_OK }, + S_OK, + { L"00000000-0000-0000-0000-000000000000", TRUE, S_OK }, + S_OK, + { NULL, E_NOTIMPL }, + S_OK, + { NULL, STGM_READ | STGM_SHARE_EXCLUSIVE, __HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND) }, + S_OK, + { URLZONE_UNTRUSTED, __HRESULT_FROM_WIN32(ERROR_NOT_FOUND) } + }, + { + { URLZONE_LOCAL_MACHINE, S_OK }, + S_OK, + { L"00000000-0000-0000-0000-000000000000", TRUE, S_OK }, + S_OK, + { NULL, E_NOTIMPL }, + S_OK, + { L"00000000-0000-0000-0000-000000000000", STGM_READ | STGM_SHARE_EXCLUSIVE, S_OK }, + S_OK, + { URLZONE_LOCAL_MACHINE, S_OK } + }, +}; + +static void test_uninitialized(void) +{ + const char uninit_tests[] = "uninitialized:"; + const zone_id_op_test get = { URLZONE_UNTRUSTED, HRESULT_FROM_WIN32(ERROR_NOT_FOUND) }; + const zone_id_op_test set = { URLZONE_INTERNET, S_OK }; + const get_cur_file_test get_cur_file = { NULL, E_NOTIMPL }; + const load_file_test load = { NULL, STGM_READ, E_INVALIDARG }; + const save_file_test save = { NULL, FALSE, E_INVALIDARG }; + IUnknown *unk; + IZoneIdentifier *zone_id; + IPersistFile *persist_file; + HRESULT hres; + DWORD zone; + CLSID clsid; + LPWSTR file_name; + + hres = CoCreateInstance(&CLSID_PersistentZoneIdentifier, NULL, + CLSCTX_INPROC_SERVER, &IID_IUnknown, (void**)&unk); + if (hres != S_OK) + { + skip("%s failed to obtain IUnknown iface: 0x%08lx\n", uninit_tests, hres); + return; + } + + hres = IUnknown_QueryInterface(unk, &IID_IZoneIdentifier, (void**)&zone_id); + if (hres != S_OK) + { + skip("%s failed to obtain IZoneIdentifier iface: 0x%08lx\n", uninit_tests, hres); + return; + } + + hres = IUnknown_QueryInterface(unk, &IID_IPersistFile, (void**)&persist_file); + if (hres != S_OK) + { + skip("%s failed to obtain IPersistFile iface: 0x%08lx\n", uninit_tests, hres); + return; + } + + hres = IZoneIdentifier_GetId(zone_id, &zone); + ok(hres == get.hres, "%s unexpected get zone id result: 0x%08lx, expected result: 0x%08lx\n", + uninit_tests, hres, get.hres); + ok(zone == get.id, "%s unexpected default zone: 0x%08lx, expected zone: 0x%08x\n", + uninit_tests, zone, get.id); + + hres = IPersistFile_GetClassID(persist_file, &clsid); + ok(hres == S_OK, "%s unexpected get class id result: 0x%08lx, expected result: 0x%08lx\n", + uninit_tests, hres, S_OK); + ok(IsEqualCLSID(&clsid, &CLSID_PersistentZoneIdentifier), + "%s unexpected class id: %s, expected class id: %s\n", + uninit_tests, debugstr_guid(&clsid), debugstr_guid(&CLSID_PersistentZoneIdentifier)); + + hres = IPersistFile_GetCurFile(persist_file, &file_name); + ok(hres == get_cur_file.hres, + "%s unexpected get current file result: 0x%08lx, expected result: 0x%08lx\n", + uninit_tests, hres, get_cur_file.hres); + ok(!lstrcmpW(file_name, get_cur_file.name), + "%s unexpected current file name: %s, expected file name: %s\n", + uninit_tests, debugstr_w(file_name), debugstr_w(get_cur_file.name)); + if (hres == S_OK || hres == S_FALSE) + CoTaskMemFree(file_name); + + hres = IPersistFile_IsDirty(persist_file); + ok(hres == S_OK, "%s unexpected change object state: 0x%08lx, expected state: 0x%08lx\n", + uninit_tests, hres, S_OK); + + hres = IZoneIdentifier_SetId(zone_id, set.id); + ok(hres == set.hres, "%s unexpected set zone id result: 0x%08lx, expected result: 0x%08lx\n", + uninit_tests, hres, set.hres); + + hres = IPersistFile_IsDirty(persist_file); + ok(hres == S_OK, + "%s unexpected change object state after set zone: 0x%08lx, expected state: 0x%08lx\n", + uninit_tests, hres, S_OK); + + hres = IPersistFile_Save(persist_file, save.name, save.remember); + ok(hres == save.hres, "%s unexpected save result: 0x%08lx, expected result: 0x%08lx\n", + uninit_tests, hres, save.hres); + + hres = IPersistFile_Load(persist_file, load.name, load.mode); + ok(hres == load.hres, "%s unexpected load result: 0x%08lx, expected result: 0x%08lx\n", + uninit_tests, hres, load.hres); +} + +static void test_IZoneIdentifier(void) +{ + const char zone_tests[] = "zone_identifier:"; + const zone_id_op_test get_removed = { URLZONE_UNTRUSTED, HRESULT_FROM_WIN32(ERROR_NOT_FOUND)}; + IUnknown *unk; + IZoneIdentifier *zone_id; + zone_id_op_test set; + zone_id_op_test get; + HRESULT hres; + DWORD i; + DWORD zone; + + hres = CoCreateInstance(&CLSID_PersistentZoneIdentifier, NULL, + CLSCTX_INPROC_SERVER, &IID_IUnknown, (void**)&unk); + if (hres != S_OK) + { + skip("%s failed to obtain IUnknown iface: 0x%08lx\n", zone_tests, hres); + return; + } + + hres = IUnknown_QueryInterface(unk, &IID_IZoneIdentifier, (void**)&zone_id); + if (hres != S_OK) + { + skip("%s failed to obtain IZoneIdentifier iface: 0x%08lx\n", zone_tests, hres); + return; + } + + for(i = 0; i < ARRAY_SIZE(zone_id_tests); ++i) + { + set = zone_id_tests[i].set; + get = zone_id_tests[i].get; + + hres = IZoneIdentifier_SetId(zone_id, set.id); + ok(hres == set.hres, + "%s unexpected set zone id result: 0x%08lx, expected result: 0x%08lx\n", + zone_tests, hres, set.hres); + + hres = IZoneIdentifier_GetId(zone_id, &zone); + ok(hres == get.hres, "%s unexpected get zone id result: 0x%08lx, expected result: 0x%08lx\n", + zone_tests, hres, get.hres); + ok(zone == get.id, + "%s unexpected zone id: 0x%08lx, expected zone: 0x%08x\n", zone_tests, zone, get.id); + } + + hres = IZoneIdentifier_Remove(zone_id); + ok(hres == S_OK, + "%s unexpected remove zone id result: 0x%08lx, expected result: 0x%08lx\n", + zone_tests, hres, S_OK); + + hres = IZoneIdentifier_GetId(zone_id, &zone); + ok(hres == get_removed.hres, + "%s unexpected get zone id result after remove: 0x%08lx, expected result: 0x%08lx\n", + zone_tests, hres, get_removed.hres); + ok(zone == get_removed.id, + "%s unexpected zone id after remove: 0x%08lx, expected zone: 0x%08x\n", + zone_tests, zone, get_removed.id); +} + +static void remove_dir(const WCHAR* dir_path) +{ + WCHAR files_mask[TMP_PATH_MAX]; + WCHAR file_path[TMP_PATH_MAX]; + HANDLE hfiles; + WIN32_FIND_DATAW file_data; + + wsprintfW(files_mask, L"%s\\*-*-*-*", dir_path); + + hfiles = FindFirstFileW(files_mask, &file_data); + if (hfiles != INVALID_HANDLE_VALUE) + do { + wsprintfW(file_path, L"%s\\%s", dir_path, file_data.cFileName); + DeleteFileW(file_path); + } while(FindNextFileW(hfiles, &file_data)); + + RemoveDirectoryW(dir_path); +} + +static void test_IPersistFile(void) +{ + const char file_tests[] = "persist_file:"; + IUnknown *unk; + IZoneIdentifier *zone_id_save; + IZoneIdentifier *zone_id_load; + IPersistFile *persist_file_save; + IPersistFile *persist_file_load; + HRESULT hres; + WCHAR tmp_dir[TMP_PATH_MAX]; + WCHAR sub_dir [] = L"PersistentZoneIdentifier"; + WCHAR tmp_file_path[TMP_PATH_MAX]; + WCHAR *file_path; + HANDLE hfile; + DWORD zone; + LPWSTR file_name; + DWORD i; + persist_file_test test; + + hres = CoCreateInstance(&CLSID_PersistentZoneIdentifier, NULL, CLSCTX_INPROC_SERVER, + &IID_IUnknown, (void**)&unk); + if (hres != S_OK) + { + skip("%s failed to obtain IUnknown save iface: 0x%08lx\n", file_tests, hres); + return; + } + + hres = IUnknown_QueryInterface(unk, &IID_IZoneIdentifier, (void**)&zone_id_save); + if (hres != S_OK) + { + skip("%s failed to obtain IZoneIdentifier save iface: 0x%08lx\n", file_tests, hres); + return; + } + + hres = IUnknown_QueryInterface(unk, &IID_IPersistFile, (void**)&persist_file_save); + if (hres != S_OK) + { + skip("%s failed to obtain IPersistFile save iface: 0x%08lx\n", file_tests, hres); + return; + } + + hres = CoCreateInstance(&CLSID_PersistentZoneIdentifier, NULL, + CLSCTX_INPROC_SERVER, &IID_IUnknown, (void**)&unk); + if (hres != S_OK) + { + skip("%s failed to obtain IUnknown load iface: 0x%08lx\n", file_tests, hres); + return; + } + + hres = IUnknown_QueryInterface(unk, &IID_IZoneIdentifier, (void**)&zone_id_load); + if (hres != S_OK) + { + skip("%s failed to obtain IZoneIdentifier load iface: 0x%08lx\n", file_tests, hres); + return; + } + + hres = IUnknown_QueryInterface(unk, &IID_IPersistFile, (void**)&persist_file_load); + if (hres != S_OK) + { + skip("%s failed to obtain IPersistFile load iface: 0x%08lx\n", file_tests, hres); + return; + } + + hres = IZoneIdentifier_Remove(zone_id_save); + ok(hres == S_OK, + "%s unexpected remove zone id result: 0x%08lx, expected result: 0x%08lx\n", file_tests, + hres, S_OK); + + if (!GetTempPathW(sizeof(tmp_dir), tmp_dir)) + { + skip("%s cannot get temporary directory path\n", file_tests); + return; + } + + if (lstrcatW(tmp_dir, sub_dir) != tmp_dir) + { + skip("%s annot get temporary subdirectory path\n", file_tests); + return; + } + + if (!CreateDirectoryW(tmp_dir, NULL)) + { + skip("%s cannot create temporary directory\n", file_tests); + return; + } + + for (i = 0; i < ARRAY_SIZE(persist_file_tests); ++i) { + file_name = NULL; + file_path = NULL; + + test = persist_file_tests[i]; + hres = IZoneIdentifier_SetId(zone_id_save, test.set.id); + ok(hres == test.set.hres, + "%s unexpected set zone id result: 0x%08lx, expected result: 0x%08lx\n", + file_tests, hres, test.set.hres); + + hres = IPersistFile_IsDirty(persist_file_save); + ok(hres == test.is_dirty_before_save, + "%s unexpected change object state before save: 0x%08lx, expected state: 0x%08lx\n", + file_tests, hres, test.is_dirty_before_save); + + if (test.save.name) + { + wsprintfW(tmp_file_path, L"%s\\%s", tmp_dir, test.save.name); + file_path = tmp_file_path; + } + else + file_path = NULL; + + if (file_path) + { + hfile = CreateFileW(file_path, GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); + if (hfile == INVALID_HANDLE_VALUE) + { + skip("%s failed to create file %s, error: 0x%08lx\n", + file_tests, debugstr_w(file_path), GetLastError()); + continue; + } + CloseHandle(hfile); + } + + hres = IPersistFile_Save(persist_file_save, file_path, test.save.remember); + ok(hres == test.save.hres, + "%s unexpected save result: 0x%08lx, expected result: 0x%08lx\n", + file_tests, hres, test.save.hres); + + hres = IPersistFile_IsDirty(persist_file_save); + ok(hres == test.is_dirty_after_save, + "%s unexpected change object state after save: 0x%08lx, expected state: 0x%08lx\n", + file_tests, hres, test.is_dirty_after_save); + + if (test.cur_file.name && test.cur_file.hres == S_OK) + { + wsprintfW(tmp_file_path, L"%s\\%s", tmp_dir, test.cur_file.name); + file_path = tmp_file_path; + } + else + file_path = NULL; + + hres = IPersistFile_GetCurFile(persist_file_save, &file_name); + ok(hres == test.cur_file.hres, + "%s unexpected get current file result: 0x%08lx, expected result: 0x%08lx\n", + file_tests, hres, test.cur_file.hres); + ok(!lstrcmpW(file_name, file_path), + "%s unexpected current file name: %s, expected file name: %s\n", + file_tests, debugstr_w(file_name), debugstr_w(file_path)); + + if (hres == S_OK || hres == S_FALSE) + CoTaskMemFree(file_name); + + hres = IZoneIdentifier_Remove(zone_id_load); + ok(hres == S_OK, + "%s unexpected remove zone id result: 0x%08lx, expected result: 0x%08lx\n", + file_tests, hres, S_OK); + + hres = IPersistFile_IsDirty(persist_file_load); + ok(hres == test.is_dirty_before_load, + "%s unexpected change object state before load: 0x%08lx, expected state: 0x%08lx\n", + file_tests, hres, test.is_dirty_before_load); + + if (test.load.name) + { + wsprintfW(tmp_file_path, L"%s\\%s", tmp_dir, test.load.name); + file_path = tmp_file_path; + } + else + file_path = NULL; + + hres = IPersistFile_Load(persist_file_load, file_path, test.load.mode); + ok(hres == test.load.hres, + "%s unexpected load result: 0x%08lx, expected result: 0x%08lx\n", + file_tests, hres, test.load.hres); + + hres = IPersistFile_Load(persist_file_save, file_path, test.load.mode); + + hres = IPersistFile_IsDirty(persist_file_load); + ok(hres == test.is_dirty_after_load, + "%s unexpected change object state after load: 0x%08lx, expected state: 0x%08lx\n", + file_tests, hres, test.is_dirty_after_load); + + hres = IZoneIdentifier_GetId(zone_id_load, &zone); + + /* GetId checks after expected Load falure shouldn't be marked as todo_wine */ + if (test.get.hres == __HRESULT_FROM_WIN32(ERROR_NOT_FOUND)) + { + ok(hres == test.get.hres, + "%s unexpected get zone id result after load: 0x%08lx, expected result: 0x%08lx\n", + file_tests, hres, test.get.hres); + ok(zone == test.get.id, + "%s unexpected zone id after load: 0x%08lx, expected zone: 0x%08x\n", + file_tests, zone, test.get.id); + } + else + { + todo_wine ok(hres == test.get.hres, + "%s unexpected get zone id result after load: 0x%08lx, expected result: 0x%08lx\n", + file_tests, hres, test.get.hres); + todo_wine ok(zone == test.get.id, + "%s unexpected zone id after load: 0x%08lx, expected zone: 0x%08x\n", + file_tests, zone, test.get.id); + } + + if (test.save.name) + { + wsprintfW(tmp_file_path, L"%s\\%s", tmp_dir, test.save.name); + file_path = tmp_file_path; + } + else + file_path = NULL; + + if (file_path) + DeleteFileW(file_path); + + } + + remove_dir(tmp_dir); + +} + +START_TEST(zone_id) +{ + if (CoInitializeEx(NULL, COINIT_MULTITHREADED) == RPC_E_CHANGED_MODE) + { + skip("init: failed to initilize COM"); + return; + } + + test_uninitialized(); + test_IZoneIdentifier(); + test_IPersistFile(); + + CoUninitialize(); +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8459
From: Mike Kozelkov <augenzi(a)etersoft.ru> --- dlls/urlmon/zone_id.c | 154 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 137 insertions(+), 17 deletions(-) diff --git a/dlls/urlmon/zone_id.c b/dlls/urlmon/zone_id.c index 43712fadbf7..e07c0b856b6 100644 --- a/dlls/urlmon/zone_id.c +++ b/dlls/urlmon/zone_id.c @@ -27,6 +27,13 @@ typedef struct { IPersistFile IPersistFile_iface; IZoneIdentifier IZoneIdentifier_iface; + BOOL is_dirty; + LPWSTR save_file_name; + LPWSTR load_file_name; + + URLZONE zone; + BOOL is_zone_removed; + IUnknown *outer; LONG ref; @@ -147,16 +154,21 @@ static HRESULT WINAPI PZIPersistFile_GetClassID(IPersistFile *iface, CLSID *clsi { PersistentZoneIdentifier *This = impl_from_IPersistFile(iface); - FIXME("(%p, %p) not implemented\n", This, clsid); + TRACE("(%p, %p)\n", This, clsid); - return E_NOTIMPL; + *clsid = CLSID_PersistentZoneIdentifier; + + return S_OK; } static HRESULT WINAPI PZIPersistFile_GetCurFile(IPersistFile *iface, LPOLESTR *file_name) { PersistentZoneIdentifier *This = impl_from_IPersistFile(iface); - FIXME("(%p, %p) not implemented\n", This, file_name); + /* GetCurFile isn't implemented in Windows*/ + TRACE("(%p, %p)\n", This, file_name); + + *file_name = NULL; return E_NOTIMPL; } @@ -165,36 +177,102 @@ static HRESULT WINAPI PZIPersistFile_IsDirty(IPersistFile *iface) { PersistentZoneIdentifier *This = impl_from_IPersistFile(iface); - FIXME("(%p) not implemented\n", This); + TRACE("(%p)\n", This); - return E_NOTIMPL; + return S_OK; } static HRESULT WINAPI PZIPersistFile_Load(IPersistFile *iface, LPCOLESTR file_name, DWORD mode) { PersistentZoneIdentifier *This = impl_from_IPersistFile(iface); + HANDLE hfile; - FIXME("(%p, %s, 0x%08lx) not implemented\n", This, debugstr_w(file_name), mode); + FIXME("(%p, %s, 0x%08lx) semi-stub\n", This, debugstr_w(file_name), mode); - return E_NOTIMPL; + if (!file_name) + { + if (!This->load_file_name) + return E_INVALIDARG; + + file_name = This->load_file_name; + } + + /* FIXME: Read zone information from Zone.Identifier NTFS alternate data stream */ + + hfile = CreateFileW(file_name, GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + if (hfile == INVALID_HANDLE_VALUE) + return HRESULT_FROM_WIN32(GetLastError()); + + CloseHandle(hfile); + + if (lstrcmpW(This->load_file_name, file_name)) + { + if (This->load_file_name) + free(This->load_file_name); + + This->load_file_name = wcsdup(file_name); + } + + return S_OK; } static HRESULT WINAPI PZIPersistFile_Save(IPersistFile *iface, LPCOLESTR file_name, BOOL remember) { PersistentZoneIdentifier *This = impl_from_IPersistFile(iface); + WCHAR *new_file_name; + HANDLE hfile; - FIXME("(%p, %s, %d) not implemented\n", This, debugstr_w(file_name), remember); + FIXME("(%p, %s, %d) semi-stub\n", This, debugstr_w(file_name), remember); - return E_NOTIMPL; + if (!file_name) + { + if (!This->save_file_name) + return E_INVALIDARG; + + file_name = This->save_file_name; + remember = FALSE; + } + + /* FIXME: Write zone information to Zone.Identifier NTFS alternate data stream */ + + hfile = CreateFileW(file_name, GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_ALWAYS, + FILE_ATTRIBUTE_NORMAL, NULL); + if (hfile == INVALID_HANDLE_VALUE) + return HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND); + + CloseHandle(hfile); + + if (!lstrcmpW(This->save_file_name, file_name)) + This->is_dirty = FALSE; + + if (remember) + { + new_file_name = wcsdup(file_name); + if (!new_file_name) + return E_OUTOFMEMORY; + + if (This->save_file_name) + free(This->save_file_name); + + This->save_file_name = new_file_name; + This->is_dirty = FALSE; + } + + return S_OK; } static HRESULT WINAPI PZIPersistFile_SaveCompleted(IPersistFile* iface, LPCOLESTR pszFileName) { PersistentZoneIdentifier *This = impl_from_IPersistFile(iface); - FIXME("(%p, %p) not implemented\n", This, pszFileName); + FIXME("(%p, %p) semi-stub \n", This, pszFileName); - return E_NOTIMPL; + /* FIXME: Add a notification to the object that it can write to its file */ + + return S_OK; } static const IPersistFileVtbl PZIPersistFileVtbl = { @@ -236,31 +314,66 @@ static ULONG WINAPI PZIZoneId_Release(IZoneIdentifier *iface) return IUnknown_Release(This->outer); } + +static BOOL is_trusted_zone(URLZONE zone) +{ + switch (zone) + { + case URLZONE_LOCAL_MACHINE: + case URLZONE_INTRANET: + case URLZONE_TRUSTED: + case URLZONE_INTERNET: + case URLZONE_UNTRUSTED: + return TRUE; + default: + return FALSE; + } +} + + static HRESULT WINAPI PZIZoneId_GetId(IZoneIdentifier* iface, DWORD* pdwZone) { PersistentZoneIdentifier *This = impl_from_IZoneIdentifier(iface); - FIXME("(%p, %p) not implemented\n", This, pdwZone); + TRACE("(%p, %p)\n", This, pdwZone); - return E_NOTIMPL; + if (This->is_zone_removed) + { + *pdwZone = URLZONE_UNTRUSTED; + return HRESULT_FROM_WIN32(ERROR_NOT_FOUND); + } else if (!is_trusted_zone(This->zone)) + { + *pdwZone = URLZONE_UNTRUSTED; + return E_ACCESSDENIED; + } else + { + *pdwZone = This->zone; + return S_OK; + } } static HRESULT WINAPI PZIZoneId_Remove(IZoneIdentifier* iface) { PersistentZoneIdentifier *This = impl_from_IZoneIdentifier(iface); - FIXME("(%p) not implemented\n", This); + TRACE("(%p)\n", This); - return E_NOTIMPL; + This->zone = URLZONE_LOCAL_MACHINE; + This->is_zone_removed = TRUE; + + return S_OK; } static HRESULT WINAPI PZIZoneId_SetId(IZoneIdentifier* iface, DWORD dwZone) { PersistentZoneIdentifier *This = impl_from_IZoneIdentifier(iface); - FIXME("(%p, 0x%08lx) not implemented\n", This, dwZone); + TRACE("(%p, 0x%08lx)\n", This, dwZone); - return E_NOTIMPL; + This->zone = dwZone; + This->is_zone_removed = FALSE; + + return is_trusted_zone(This->zone) ? S_OK : E_INVALIDARG; } static const IZoneIdentifierVtbl PZIZoneIdVtbl = { @@ -290,6 +403,13 @@ HRESULT PersistentZoneIdentifier_Construct(IUnknown *outer, LPVOID *ppobj) ret->IPersistFile_iface.lpVtbl = &PZIPersistFileVtbl; ret->IZoneIdentifier_iface.lpVtbl = &PZIZoneIdVtbl; + ret->save_file_name = NULL; + ret->load_file_name = NULL; + ret->is_dirty = FALSE; + + ret->zone = URLZONE_INVALID; + ret->is_zone_removed = TRUE; + ret->ref = 1; ret->outer = outer ? outer : &ret->IUnknown_inner; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8459
@jacek Could you please do a review? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8459#note_109087
Jacek Caban (@jacek) commented about dlls/urlmon/tests/zone_id.c:
+#define COBJMACROS +#define CONST_VTABLE + +#include <wine/test.h> +#include "winbase.h" + +#include "urlmon.h" Please use an existing file for tests, like `sec_mgr.c`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8459#note_109713
Jacek Caban (@jacek) commented about dlls/urlmon/tests/zone_id.c:
+ +typedef struct load_file_test { + LPCWSTR name; + DWORD mode; + HRESULT hres; +} load_file_test; + +typedef struct _save_file_test { + LPCWSTR name; + BOOL remember; + HRESULT hres; +} save_file_test; + +typedef struct _persist_file_test { + zone_id_op_test set; + HRESULT is_dirty_before_save; All those `is_dirty_*` do not seem very useful if they are always `S_OK` anyway. You may just as well just use `S_OK` in the test code.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8459#note_109714
Jacek Caban (@jacek) commented about dlls/urlmon/zone_id.c:
+ if (!file_name) + { + if (!This->save_file_name) + return E_INVALIDARG; + + file_name = This->save_file_name; + remember = FALSE; + } + + /* FIXME: Write zone information to Zone.Identifier NTFS alternate data stream */ + + hfile = CreateFileW(file_name, GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_ALWAYS, + FILE_ATTRIBUTE_NORMAL, NULL); + if (hfile == INVALID_HANDLE_VALUE) + return HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND); Should it be `return HRESULT_FROM_WIN32(GetLastError());`?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8459#note_109718
Jacek Caban (@jacek) commented about dlls/urlmon/tests/zone_id.c:
+ { NULL, E_NOTIMPL }, + S_OK, + { L"00000000-0000-0000-0000-000000000000", STGM_READ | STGM_SHARE_EXCLUSIVE, S_OK }, + S_OK, + { URLZONE_LOCAL_MACHINE, S_OK } + }, +}; + +static void test_uninitialized(void) +{ + const char uninit_tests[] = "uninitialized:"; + const zone_id_op_test get = { URLZONE_UNTRUSTED, HRESULT_FROM_WIN32(ERROR_NOT_FOUND) }; + const zone_id_op_test set = { URLZONE_INTERNET, S_OK }; + const get_cur_file_test get_cur_file = { NULL, E_NOTIMPL }; + const load_file_test load = { NULL, STGM_READ, E_INVALIDARG }; + const save_file_test save = { NULL, FALSE, E_INVALIDARG }; These don’t really make things easier to follow, you could just use expected values directly in the `ok()` statements below. Structs like that are useful when testing multiple value sets, but for one-offs like this, I think they actually make the code harder to read.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8459#note_109715
Jacek Caban (@jacek) commented about dlls/urlmon/tests/zone_id.c:
+ const save_file_test save = { NULL, FALSE, E_INVALIDARG }; + IUnknown *unk; + IZoneIdentifier *zone_id; + IPersistFile *persist_file; + HRESULT hres; + DWORD zone; + CLSID clsid; + LPWSTR file_name; + + hres = CoCreateInstance(&CLSID_PersistentZoneIdentifier, NULL, + CLSCTX_INPROC_SERVER, &IID_IUnknown, (void**)&unk); + if (hres != S_OK) + { + skip("%s failed to obtain IUnknown iface: 0x%08lx\n", uninit_tests, hres); + return; + } Are there any Windows versions that fail to create the zone identifier? If not, this should use `ok()` instead. If there are, it should be `win_skip()` to avoid unintentionally accepting such skips on Wine. The same applies to all other `skip()` calls in the patch.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8459#note_109716
Jacek Caban (@jacek) commented about dlls/urlmon/tests/zone_id.c:
+ hres = IPersistFile_Load(persist_file_load, file_path, test.load.mode); + ok(hres == test.load.hres, + "%s unexpected load result: 0x%08lx, expected result: 0x%08lx\n", + file_tests, hres, test.load.hres); + + hres = IPersistFile_Load(persist_file_save, file_path, test.load.mode); + + hres = IPersistFile_IsDirty(persist_file_load); + ok(hres == test.is_dirty_after_load, + "%s unexpected change object state after load: 0x%08lx, expected state: 0x%08lx\n", + file_tests, hres, test.is_dirty_after_load); + + hres = IZoneIdentifier_GetId(zone_id_load, &zone); + + /* GetId checks after expected Load falure shouldn't be marked as todo_wine */ + if (test.get.hres == __HRESULT_FROM_WIN32(ERROR_NOT_FOUND)) You could use `todo_wine_if(test.get.hres != __HRESULT_FROM_WIN32(ERROR_NOT_FOUND))`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8459#note_109717
Sorry for the delay. We generally expect tests to pass with each commit. As it stands, applying only the first patch would cause test failures. You could simply swap the order of the patches to avoid that. Alternatively, you could add `todo_wine`s in the first commit and remove them in the second. That approach is often better for bug fixes to show the exact problem being fixed, but in this case, swapping the commit order is probably the cleaner solution. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8459#note_109720
Jacek Caban (@jacek) commented about dlls/urlmon/zone_id.c:
+ + CloseHandle(hfile); + + if (!lstrcmpW(This->save_file_name, file_name)) + This->is_dirty = FALSE; + + if (remember) + { + new_file_name = wcsdup(file_name); + if (!new_file_name) + return E_OUTOFMEMORY; + + if (This->save_file_name) + free(This->save_file_name); + + This->save_file_name = new_file_name; This should be freed in `PZIUnk_Release`. Same for `save_file_name`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8459#note_109719
On Sun Jul 13 17:57:30 2025 +0000, Jacek Caban wrote:
Are there any Windows versions that fail to create the zone identifier? If not, this should use `ok()` instead. If there are, it should be `win_skip()` to avoid unintentionally accepting such skips on Wine. The same applies to all other `skip()` calls in the patch. Zone Identifier is suppoprted since Windows XP SP2, so I think `ok()` should be used.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8459#note_109740
participants (3)
-
Jacek Caban (＠jacek) -
Mike Kozelkov
-
Mike Kozelkov (＠augenzi)