Skip to content

Commit b3398bc

Browse files
pryrtpryrt
committed
CollectionInterface: improve error handling to prevent crashes
1 parent 1be52b6 commit b3398bc

File tree

3 files changed

+78
-23
lines changed

3 files changed

+78
-23
lines changed

‎src/Classes/CollectionInterfaceClass.cpp‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
CollectionInterface::CollectionInterface(HWND hwndNpp) {
77
_hwndNPP = hwndNpp;
88
_populateNppDirs();
9-
getListsFromJson();
9+
_areListsPopulated = getListsFromJson();
1010
};
1111

1212
void CollectionInterface::_populateNppDirs(void) {
@@ -228,8 +228,9 @@ std::string CollectionInterface::_xml_unentity(const std::string& text)
228228
}
229229
#pragma warning ( pop )
230230

231-
void CollectionInterface::getListsFromJson(void)
231+
bool CollectionInterface::getListsFromJson(void)
232232
{
233+
bool didThemeFail = false;
233234
auto string2wstring = [](std::string str) {
234235
if (str.empty()) return std::wstring();
235236
int wsz = MultiByteToWideChar(CP_UTF8, 0, str.c_str(), static_cast<int>(str.size()), NULL, 0);
@@ -242,9 +243,22 @@ void CollectionInterface::getListsFromJson(void)
242243
// Process Theme JSON
243244
////////////////////////////////
244245
std::vector<char> vcThemeJSON = downloadFileInMemory(L"https://raw.githubusercontent.com/notepad-plus-plus/nppThemes/master/themes/.toc.json");
245-
if ((vcThemeJSON.size() < 2) || (vcThemeJSON[0] != L'[')) {
246-
// issue#13: don't try to parse if there is no data to parse
247-
::MessageBox(_hwndNPP, L"Problems downloading Themes Collection information.\nYou might want to check your internet connection.", L"Download Problems", MB_ICONWARNING);
246+
if (vcThemeJSON.empty()) {
247+
// issue#13: do not continue if there's internet/connection problems
248+
return false; // nothing downloaded, so want to know to close the download-dialog to avoid annoying user with useless empty listbox
249+
}
250+
else if (vcThemeJSON[0] != L'[') {
251+
// related to issue#13: if downloadFileInMemory returns "404 Not Found" or similar, don't try to parse as JSON.
252+
// easiest check: if the JSON isn't the expected [...] JSON array, don't continue with the _theme_;
253+
// however, can still move to the UDL section, because that might still work, and since UDL is the primary purpose, it's probably worth it if UDL is working even if Themes aren't.
254+
std::string msg = "Cannot interpret Themes Collection information:\n\n";
255+
msg += vcThemeJSON.data();
256+
if (msg.size() > 100) {
257+
msg.resize(100);
258+
msg += "\n...";
259+
}
260+
::MessageBoxA(_hwndNPP, msg.c_str(), "CollectionInterface: Download Problems", MB_ICONWARNING);
261+
didThemeFail = true;
248262
}
249263
else {
250264
try
@@ -259,20 +273,35 @@ void CollectionInterface::getListsFromJson(void)
259273
catch (nlohmann::json::exception& e) {
260274
std::string msg = std::string("JSON Error in Theme data: ") + e.what();
261275
::MessageBoxA(_hwndNPP, msg.c_str(), "CollectionInterface: JSON Error", MB_ICONERROR);
276+
didThemeFail = true;
262277
}
263278
catch (std::exception& e) {
264279
std::string msg = std::string("Unrecognized Error in Theme data: ") + e.what();
265280
::MessageBoxA(_hwndNPP, msg.c_str(), "CollectionInterface: Unrecognized Error", MB_ICONERROR);
281+
didThemeFail = true;
266282
}
267283
}
268284

269285
////////////////////////////////
270286
// Process UDL JSON
271287
////////////////////////////////
272288
std::vector<char> vcUdlJSON = downloadFileInMemory(L"https://raw.githubusercontent.com/notepad-plus-plus/userDefinedLanguages/refs/heads/master/udl-list.json");
273-
if ((vcUdlJSON.size() < 2) || (vcUdlJSON[0] != L'{')) {
274-
// issue#13: don't try to parse if there is no data to parse
275-
::MessageBox(_hwndNPP, L"Problems downloading UDL Collection information.\nYou might want to check your internet connection.", L"Download Problems", MB_ICONWARNING);
289+
if (vcUdlJSON.empty()) {
290+
// issue#13: do not continue if there's internet/connection problems
291+
return false; // nothing downloaded, so want to know to close the download-dialog to avoid annoying user with useless empty listbox
292+
}
293+
else if (vcUdlJSON[0] != L'{') {
294+
// related to issue#13: if downloadFileInMemory returns "404 Not Found" or similar, don't try to parse as JSON.
295+
// easiest check: if the JSON isn't the expected [...] JSON array, don't continue with the _theme_;
296+
std::string msg = "Cannot interpret UDL Collection information:\n\n";
297+
msg += vcUdlJSON.data();
298+
if (msg.size() > 100) {
299+
msg.resize(100);
300+
msg += "\n...";
301+
}
302+
if (!didThemeFail)
303+
::MessageBoxA(_hwndNPP, msg.c_str(), "CollectionInterface: Download Problems", MB_ICONWARNING);
304+
return false; // without UDL info, it's not worth displaying the Download Dialog
276305
}
277306
else {
278307
try
@@ -366,14 +395,17 @@ void CollectionInterface::getListsFromJson(void)
366395
catch (nlohmann::json::exception& e) {
367396
std::string msg = std::string("JSON Error in UDL data: ") + e.what();
368397
::MessageBoxA(_hwndNPP, msg.c_str(), "CollectionInterface: JSON Error", MB_ICONERROR);
398+
return false; // without UDL info, it's not worth displaying the Download Dialog
369399
}
370400
catch (std::exception& e) {
371401
std::string msg = std::string("Unrecognized Error in UDL data: ") + e.what();
372402
::MessageBoxA(_hwndNPP, msg.c_str(), "CollectionInterface: Unrecognized Error", MB_ICONERROR);
403+
return false; // without UDL info, it's not worth displaying the Download Dialog
373404
}
374405
}
375406

376-
return;
407+
// if it makes it here, there is enough data to be worth displaying the dialog
408+
return true;
377409
}
378410

379411
std::wstring& CollectionInterface::_wsDeleteTrailingNulls(std::wstring& str)

‎src/Classes/CollectionInterfaceClass.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class CollectionInterface {
3232
//bool downloadFileToDisk(const std::wstring& url, const std::string& path);
3333
//bool downloadFileToDisk(const std::string& url, const std::wstring& path);
3434
bool downloadFileToDisk(const std::wstring& url, const std::wstring& path);
35-
void getListsFromJson(void);
35+
bool getListsFromJson(void);
3636

3737
// getter methods
3838
std::wstring nppCfgDir(void) { return _nppCfgDir; };
@@ -46,6 +46,7 @@ class CollectionInterface {
4646
bool isFunctionListDirWritable(void) { return _is_dir_writable(_nppCfgFunctionListDir); };
4747
bool isAutoCompletionDirWritable(void) { return _is_dir_writable(_nppCfgAutoCompletionDir); };
4848
bool isThemesDirWritable(void) { return _is_dir_writable(_nppCfgThemesDir); };
49+
bool areListsPopulated(void) { return _areListsPopulated; };
4950

5051
// if the chosen directory isn't writable, need to be able to use a directory that _is_ writable
5152
// as a TempDir, and then will need to use runas to copy from the TempDir to the real dir.
@@ -69,4 +70,5 @@ class CollectionInterface {
6970
_nppCfgThemesDir;
7071

7172
HWND _hwndNPP;
73+
bool _areListsPopulated;
7274
};

‎src/Dialogs/CollectionInterfaceDialog.cpp‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -97,15 +97,22 @@ INT_PTR CALLBACK ciDlgProc(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam
9797
Edit_SetText(GetDlgItem(hwndDlg, IDC_CI_PROGRESSLBL), L"READY");
9898

9999
pobjCI = new CollectionInterface(hParent);
100-
//pobjCI->getListsFromJson();
100+
if (!pobjCI->areListsPopulated()) {
101+
EndDialog(hwndDlg, 0);
102+
DestroyWindow(hwndDlg);
103+
g_hwndCIDlg = nullptr;
104+
delete pobjCI;
105+
pobjCI = NULL;
106+
return true;
107+
}
101108
_populate_file_cbx(hwndDlg, pobjCI->mapUDL, pobjCI->mapDISPLAY);
102109

103110
// Dark Mode Subclass and Theme: needs to go _after_ all the controls have been initialized
104111
LRESULT nppVersion = ::SendMessage(nppData._nppHandle, NPPM_GETNPPVERSION, 1, 0); // HIWORD(nppVersion) = major version; LOWORD(nppVersion) = zero-padded minor (so 8|500 will come after 8|410)
105112
LRESULT darkdialogVersion = MAKELONG(540, 8); // NPPM_GETDARKMODECOLORS requires 8.4.1 and NPPM_DARKMODESUBCLASSANDTHEME requires 8.5.4
106113
LRESULT localsubclassVersion = MAKELONG(810, 8); // from 8.540 to 8.810 (at least), need to do local subclassing because of tab control
107114
g_IsDarkMode = (bool)::SendMessage(nppData._nppHandle, NPPM_ISDARKMODEENABLED, 0, 0);
108-
if (g_IsDarkMode && (nppVersion>=darkdialogVersion)) {
115+
if (g_IsDarkMode && (nppVersion >= darkdialogVersion)) {
109116
::SendMessage(nppData._nppHandle, NPPM_GETDARKMODECOLORS, sizeof(NppDarkMode::Colors), reinterpret_cast<LPARAM>(&myColors));
110117
myBrushes.change(myColors);
111118
myPens.change(myColors);
@@ -219,13 +226,16 @@ INT_PTR CALLBACK ciDlgProc(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam
219226
{
220227
std::wstring wsCategory = _get_tab_category_wstr(hwndDlg, IDC_CI_TABCTRL);
221228
LRESULT selectedFileIndex = ::SendDlgItemMessage(hwndDlg, IDC_CI_COMBO_FILE, LB_GETCURSEL, 0, 0);
222-
switch (selectedFileIndex) {
223-
case CB_ERR:
224-
::MessageBox(NULL, L"Could not understand FILE combobox; sorry", L"Download Error", MB_ICONERROR);
225-
return true;
229+
if (selectedFileIndex == CB_ERR) {
230+
::MessageBox(NULL, L"Could not understand name selection; sorry", L"Download Error", MB_ICONERROR);
231+
return true;
226232
}
227233

228234
LRESULT needFileLen = ::SendDlgItemMessage(hwndDlg, IDC_CI_COMBO_FILE, LB_GETTEXTLEN, selectedFileIndex, 0);
235+
if (needFileLen == LB_ERR) {
236+
::MessageBox(NULL, L"Could not understand name selection; sorry", L"Download Error", MB_ICONERROR);
237+
return true;
238+
}
229239
std::wstring wsFilename(needFileLen, 0);
230240
::SendDlgItemMessage(hwndDlg, IDC_CI_COMBO_FILE, LB_GETTEXT, selectedFileIndex, reinterpret_cast<LPARAM>(wsFilename.data()));
231241

@@ -334,10 +344,12 @@ INT_PTR CALLBACK ciDlgProc(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam
334344
}
335345

336346
// update progress bar
337-
::SendDlgItemMessage(hwndDlg, IDC_CI_PROGRESSBAR, PBM_SETPOS, 100 * count / total, 0);
338347
wchar_t wcDLPCT[256];
339348
swprintf_s(wcDLPCT, L"Downloading %d%%", 100 * count / total);
340-
Edit_SetText(GetDlgItem(hwndDlg, IDC_CI_PROGRESSLBL), wcDLPCT);
349+
if (didDownload) {
350+
::SendDlgItemMessage(hwndDlg, IDC_CI_PROGRESSBAR, PBM_SETPOS, 100 * count / total, 0);
351+
Edit_SetText(GetDlgItem(hwndDlg, IDC_CI_PROGRESSLBL), wcDLPCT);
352+
}
341353

342354
// also download AC and FL, if applicable
343355
std::vector<std::wstring> xtra = { L"AC", L"FL" };
@@ -375,15 +387,24 @@ INT_PTR CALLBACK ciDlgProc(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam
375387
}
376388
}
377389
// update progress bar
378-
::SendDlgItemMessage(hwndDlg, IDC_CI_PROGRESSBAR, PBM_SETPOS, 100 * count / total, 0);
379390
swprintf_s(wcDLPCT, L"Downloading %d%%", 100 * count / total);
380-
Edit_SetText(GetDlgItem(hwndDlg, IDC_CI_PROGRESSLBL), wcDLPCT);
391+
if (didDownload) {
392+
::SendDlgItemMessage(hwndDlg, IDC_CI_PROGRESSBAR, PBM_SETPOS, 100 * count / total, 0);
393+
Edit_SetText(GetDlgItem(hwndDlg, IDC_CI_PROGRESSLBL), wcDLPCT);
394+
}
381395
}
382396

383397
// Final update of progress bar: 100%
384-
::SendDlgItemMessage(hwndDlg, IDC_CI_PROGRESSBAR, PBM_SETPOS, 100, 0);
385-
swprintf_s(wcDLPCT, L"Downloading %d%% [DONE]", 100);
386-
Edit_SetText(GetDlgItem(hwndDlg, IDC_CI_PROGRESSLBL), wcDLPCT);
398+
if (didDownload) {
399+
::SendDlgItemMessage(hwndDlg, IDC_CI_PROGRESSBAR, PBM_SETPOS, 100, 0);
400+
swprintf_s(wcDLPCT, L"Downloading %d%% [DONE]", 100);
401+
Edit_SetText(GetDlgItem(hwndDlg, IDC_CI_PROGRESSLBL), wcDLPCT);
402+
}
403+
else {
404+
::SendDlgItemMessage(hwndDlg, IDC_CI_PROGRESSBAR, PBM_SETPOS, 0, 0);
405+
swprintf_s(wcDLPCT, L"Nothing to Download. [DONE]");
406+
Edit_SetText(GetDlgItem(hwndDlg, IDC_CI_PROGRESSLBL), wcDLPCT);
407+
}
387408
}
388409
return true;
389410
case IDC_CI_HELPBTN:

0 commit comments

Comments
 (0)