From 84e580b67e5a407afdd669c3204ef97d115d8cb0 Mon Sep 17 00:00:00 2001 From: Katayama Hirofumi MZ Date: Sat, 1 Apr 2023 22:21:59 +0900 Subject: [PATCH] [REGEDIT] Fix ListView selection and finding (#5150) We will check the data size correctly, instead of 3 NUL byte appending hack. Add bSelectNone parameter to UpdateAddress and RefreshListView functions. If bSelectNone is TRUE, then select nothing of ListView. Fix item selection of ListView. Rename CompareData helper function as MatchData and improve it. Improve the search algorithm. If the item selection of ListView changed, scroll down to the item. Follow up to #5146. CORE-15986, CORE-18230 --- base/applications/regedit/childwnd.c | 4 +- base/applications/regedit/find.c | 94 ++++++++++++---------------- base/applications/regedit/framewnd.c | 21 ++++--- base/applications/regedit/listview.c | 8 ++- base/applications/regedit/main.h | 4 +- base/applications/regedit/treeview.c | 4 +- 6 files changed, 65 insertions(+), 70 deletions(-) diff --git a/base/applications/regedit/childwnd.c b/base/applications/regedit/childwnd.c index 19c4cea780f..7910dcefdaa 100644 --- a/base/applications/regedit/childwnd.c +++ b/base/applications/regedit/childwnd.c @@ -229,7 +229,7 @@ LRESULT CALLBACK AddressBarProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lPar } VOID -UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath) +UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath, BOOL bSelectNone) { LPCWSTR keyPath, rootName; LPWSTR fullPath; @@ -251,7 +251,7 @@ UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath) if (keyPath) { - RefreshListView(g_pChildWnd->hListWnd, hRootKey, keyPath); + RefreshListView(g_pChildWnd->hListWnd, hRootKey, keyPath, bSelectNone); rootName = get_root_key_name(hRootKey); cbFullPath = (wcslen(rootName) + 1 + wcslen(keyPath) + 1) * sizeof(WCHAR); fullPath = malloc(cbFullPath); diff --git a/base/applications/regedit/find.c b/base/applications/regedit/find.c index c970a53cf51..444f4a25952 100644 --- a/base/applications/regedit/find.c +++ b/base/applications/regedit/find.c @@ -87,41 +87,32 @@ static BOOL CompareName(LPCWSTR pszName1, LPCWSTR pszName2) } } -static BOOL -CompareData( - DWORD dwType, - LPCWSTR psz1, - LPCWSTR psz2) +/* Do not assume that pch1 is terminated with UNICODE_NULL */ +static BOOL MatchString(LPCWCH pch1, INT cch1, LPCWCH pch2, INT cch2) { - INT i, cch1 = wcslen(psz1), cch2 = wcslen(psz2); - if (dwType == REG_SZ || dwType == REG_EXPAND_SZ) - { - if (s_dwFlags & RSF_WHOLESTRING) - { - if (s_dwFlags & RSF_MATCHCASE) - return 2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, 0, - psz1, cch1, psz2, cch2); - else - return 2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, - NORM_IGNORECASE, psz1, cch1, psz2, cch2); - } + INT i; + DWORD dwNorm = ((s_dwFlags & RSF_MATCHCASE) ? NORM_IGNORECASE : 0); - for(i = 0; i <= cch1 - cch2; i++) - { - if (s_dwFlags & RSF_MATCHCASE) - { - if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, 0, - psz1 + i, cch2, psz2, cch2)) - return TRUE; - } - else - { - if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, - NORM_IGNORECASE, psz1 + i, cch2, psz2, cch2)) - return TRUE; - } - } + if (s_dwFlags & RSF_WHOLESTRING) + return 2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, dwNorm, pch1, cch1, pch2, cch2); + + if (cch1 < cch2) + return FALSE; + + for (i = 0; i <= cch1 - cch2; i++) + { + if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, dwNorm, pch1 + i, cch2, pch2, cch2)) + return TRUE; } + + return FALSE; +} + +static BOOL MatchData(DWORD dwType, LPCVOID pv1, DWORD cb1) +{ + if (dwType == REG_SZ || dwType == REG_EXPAND_SZ || dwType == REG_MULTI_SZ) + return MatchString(pv1, (INT)(cb1 / sizeof(WCHAR)), s_szFindWhat, lstrlenW(s_szFindWhat)); + return FALSE; } @@ -143,7 +134,7 @@ BOOL RegFindRecurse( LONG lResult; WCHAR szSubKey[MAX_PATH]; DWORD i, c, cb, type; - BOOL fPast = FALSE; + BOOL fPast; LPWSTR *ppszNames = NULL; LPBYTE pb = NULL; @@ -169,6 +160,7 @@ BOOL RegFindRecurse( goto err; ZeroMemory(ppszNames, c * sizeof(LPWSTR)); + /* Retrieve the value names associated with the current key */ for(i = 0; i < c; i++) { if (DoEvents()) @@ -192,10 +184,11 @@ BOOL RegFindRecurse( qsort(ppszNames, c, sizeof(LPWSTR), compare); - if (pszValueName == NULL) - pszValueName = ppszNames[0]; + /* If pszValueName is NULL, the function will search for all values within the key */ + fPast = (pszValueName == NULL); - for(i = 0; i < c; i++) + /* Search within the values */ + for (i = 0; i < c; i++) { if (DoEvents()) goto err; @@ -212,10 +205,7 @@ BOOL RegFindRecurse( CompareName(ppszNames[i], s_szFindWhat)) { *ppszFoundSubKey = _wcsdup(szSubKey); - if (ppszNames[i][0] == 0) - *ppszFoundValueName = NULL; - else - *ppszFoundValueName = _wcsdup(ppszNames[i]); + *ppszFoundValueName = _wcsdup(ppszNames[i]); goto success; } @@ -223,7 +213,7 @@ BOOL RegFindRecurse( NULL, &cb); if (lResult != ERROR_SUCCESS) goto err; - pb = malloc(cb + 3); /* To avoid buffer overrun, append 3 NULs */ + pb = malloc(cb); if (pb == NULL) goto err; lResult = RegQueryValueExW(hSubKey, ppszNames[i], NULL, &type, @@ -231,19 +221,10 @@ BOOL RegFindRecurse( if (lResult != ERROR_SUCCESS) goto err; - /* To avoid buffer overrun, append 3 NUL bytes. - NOTE: cb can be an odd number although UNICODE_NULL is two bytes. - Two bytes at odd position is not enough to avoid buffer overrun. */ - pb[cb] = pb[cb + 1] = pb[cb + 2] = 0; - - if ((s_dwFlags & RSF_LOOKATDATA) && - CompareData(type, (LPWSTR) pb, s_szFindWhat)) + if ((s_dwFlags & RSF_LOOKATDATA) && MatchData(type, pb, cb)) { *ppszFoundSubKey = _wcsdup(szSubKey); - if (ppszNames[i][0] == 0) - *ppszFoundValueName = NULL; - else - *ppszFoundValueName = _wcsdup(ppszNames[i]); + *ppszFoundValueName = _wcsdup(ppszNames[i]); goto success; } free(pb); @@ -258,6 +239,7 @@ BOOL RegFindRecurse( } ppszNames = NULL; + /* Retrieve the number of sub-keys */ lResult = RegQueryInfoKeyW(hSubKey, NULL, NULL, NULL, &c, NULL, NULL, NULL, NULL, NULL, NULL, NULL); if (lResult != ERROR_SUCCESS) @@ -267,6 +249,7 @@ BOOL RegFindRecurse( goto err; ZeroMemory(ppszNames, c * sizeof(LPWSTR)); + /* Retrieve the names of the sub-keys */ for(i = 0; i < c; i++) { if (DoEvents()) @@ -290,6 +273,7 @@ BOOL RegFindRecurse( qsort(ppszNames, c, sizeof(LPWSTR), compare); + /* Search within the sub-keys */ for(i = 0; i < c; i++) { if (DoEvents()) @@ -315,6 +299,7 @@ BOOL RegFindRecurse( goto success; } + /* Search within the value entries of the sub-key */ if (RegFindRecurse(hSubKey, ppszNames[i], NULL, ppszFoundSubKey, ppszFoundValueName)) { @@ -371,7 +356,7 @@ BOOL RegFindWalk( WCHAR szKeyName[MAX_PATH]; WCHAR szSubKey[MAX_PATH]; LPWSTR pch; - BOOL fPast; + BOOL fPast, fKeyMatched; LPWSTR *ppszNames = NULL; hBaseKey = *phKey; @@ -470,7 +455,8 @@ BOOL RegFindWalk( goto success; } - if (RegFindRecurse(hSubKey, ppszNames[i], NULL, + fKeyMatched = (_wcsicmp(ppszNames[i], szKeyName) == 0); + if (RegFindRecurse(hSubKey, ppszNames[i], (fKeyMatched ? pszValueName : NULL), ppszFoundSubKey, ppszFoundValueName)) { LPWSTR psz = *ppszFoundSubKey; diff --git a/base/applications/regedit/framewnd.c b/base/applications/regedit/framewnd.c index e3d695a6728..f886e0c7730 100644 --- a/base/applications/regedit/framewnd.c +++ b/base/applications/regedit/framewnd.c @@ -383,7 +383,7 @@ static BOOL LoadHive(HWND hWnd) /* refresh tree and list views */ RefreshTreeView(g_pChildWnd->hTreeWnd); pszKeyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hRootKey); - RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath); + RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath, TRUE); } else { @@ -421,7 +421,7 @@ static BOOL UnloadHive(HWND hWnd) /* refresh tree and list views */ RefreshTreeView(g_pChildWnd->hTreeWnd); pszKeyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hRootKey); - RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath); + RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath, TRUE); } else { @@ -518,7 +518,7 @@ static BOOL ImportRegistryFile(HWND hWnd) /* refresh tree and list views */ RefreshTreeView(g_pChildWnd->hTreeWnd); pszKeyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hKeyRoot); - RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, pszKeyPath); + RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, pszKeyPath, TRUE); return bRet; } @@ -877,7 +877,7 @@ static BOOL CreateNewValue(HKEY hRootKey, LPCWSTR pszKeyPath, DWORD dwType) return FALSE; } - RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath); + RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath, TRUE); /* locate the newly added value, and get ready to rename it */ memset(&lvfi, 0, sizeof(lvfi)); @@ -885,7 +885,12 @@ static BOOL CreateNewValue(HKEY hRootKey, LPCWSTR pszKeyPath, DWORD dwType) lvfi.psz = szNewValue; iIndex = ListView_FindItem(g_pChildWnd->hListWnd, -1, &lvfi); if (iIndex >= 0) + { + ListView_SetItemState(g_pChildWnd->hListWnd, iIndex, + LVIS_SELECTED | LVIS_FOCUSED, LVIS_SELECTED | LVIS_FOCUSED); + ListView_EnsureVisible(g_pChildWnd->hListWnd, iIndex, FALSE); (void)ListView_EditLabel(g_pChildWnd->hListWnd, iIndex); + } return TRUE; } @@ -1128,11 +1133,11 @@ static BOOL _CmdWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) { case ID_EDIT_MODIFY: if (valueName && ModifyValue(hWnd, hKey, valueName, FALSE)) - RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath); + RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, FALSE); break; case ID_EDIT_MODIFY_BIN: if (valueName && ModifyValue(hWnd, hKey, valueName, TRUE)) - RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath); + RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, FALSE); break; case ID_EDIT_RENAME: if (GetFocus() == g_pChildWnd->hListWnd) @@ -1180,7 +1185,7 @@ static BOOL _CmdWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) item = ni; } - RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath); + RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, FALSE); if(errs > 0) { LoadStringW(hInst, IDS_ERR_DELVAL_CAPTION, caption, ARRAY_SIZE(caption)); @@ -1243,7 +1248,7 @@ static BOOL _CmdWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) case ID_VIEW_REFRESH: RefreshTreeView(g_pChildWnd->hTreeWnd); keyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hKeyRoot); - RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath); + RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, TRUE); break; /*case ID_OPTIONS_TOOLBAR:*/ /* toggle_child(hWnd, LOWORD(wParam), hToolBar);*/ diff --git a/base/applications/regedit/listview.c b/base/applications/regedit/listview.c index ce0d5408794..346ed784afa 100644 --- a/base/applications/regedit/listview.c +++ b/base/applications/regedit/listview.c @@ -91,7 +91,7 @@ VOID SetValueName(HWND hwndLV, LPCWSTR pszValueName) { ListView_SetItemState(hwndLV, i, 0, LVIS_FOCUSED | LVIS_SELECTED); } - if (pszValueName == NULL) + if (pszValueName == NULL || pszValueName[0] == 0) i = 0; else { @@ -101,6 +101,7 @@ VOID SetValueName(HWND hwndLV, LPCWSTR pszValueName) } ListView_SetItemState(hwndLV, i, LVIS_FOCUSED | LVIS_SELECTED, LVIS_FOCUSED | LVIS_SELECTED); + ListView_EnsureVisible(hwndLV, i, FALSE); iListViewSelect = i; } @@ -668,7 +669,7 @@ void DestroyListView(HWND hwndLV) } -BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath) +BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath, BOOL bSelectNone) { DWORD max_sub_key_len; DWORD max_val_name_len; @@ -738,6 +739,9 @@ BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath) { ListView_SetItemState(hwndLV, i, 0, LVIS_FOCUSED | LVIS_SELECTED); } + + if (bSelectNone) + iListViewSelect = -1; ListView_SetItemState(hwndLV, iListViewSelect, LVIS_FOCUSED | LVIS_SELECTED, LVIS_FOCUSED | LVIS_SELECTED); diff --git a/base/applications/regedit/main.h b/base/applications/regedit/main.h index 7c8b864626b..bb257e792cc 100644 --- a/base/applications/regedit/main.h +++ b/base/applications/regedit/main.h @@ -97,7 +97,7 @@ void ShowAboutBox(HWND hWnd); LRESULT CALLBACK ChildWndProc(HWND, UINT, WPARAM, LPARAM); void ResizeWnd(int cx, int cy); LPCWSTR get_root_key_name(HKEY hRootKey); -VOID UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath); +VOID UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath, BOOL bSelectNone); /* edit.c */ BOOL ModifyValue(HWND hwnd, HKEY hKey, LPCWSTR valueName, BOOL EditBin); @@ -125,7 +125,7 @@ BOOL ExportRegistryFile(HWND hWnd); /* listview.c */ HWND CreateListView(HWND hwndParent, HMENU id, INT cx); -BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath); +BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath, BOOL bSelectNone); WCHAR *GetValueName(HWND hwndLV, int iStartAt); BOOL ListWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result); BOOL TreeWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result); diff --git a/base/applications/regedit/treeview.c b/base/applications/regedit/treeview.c index ee26a04538a..28222432fea 100644 --- a/base/applications/regedit/treeview.c +++ b/base/applications/regedit/treeview.c @@ -643,7 +643,7 @@ BOOL TreeWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result) /* Get the parent of the current item */ HTREEITEM hParentItem = TreeView_GetParent(g_pChildWnd->hTreeWnd, pnmtv->itemNew.hItem); - UpdateAddress(pnmtv->itemNew.hItem, NULL, NULL); + UpdateAddress(pnmtv->itemNew.hItem, NULL, NULL, TRUE); /* Disable the Permissions menu item for 'My Computer' */ EnableMenuItem(hMenuFrame, ID_EDIT_PERMISSIONS, MF_BYCOMMAND | (hParentItem ? MF_ENABLED : MF_GRAYED)); @@ -722,7 +722,7 @@ BOOL TreeWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result) lResult = FALSE; } else - UpdateAddress(ptvdi->item.hItem, hRootKey, szBuffer); + UpdateAddress(ptvdi->item.hItem, hRootKey, szBuffer, FALSE); } *Result = lResult; }