From 7c9dc303f8aa49dcedd83e480744168bd7b656bc Mon Sep 17 00:00:00 2001 From: patricktsullivan <93329749+patricktsullivan@users.noreply.github.com> Date: Sun, 27 Sep 2026 03:50:05 -0400 Subject: [PATCH 1/7] [Game] Make 'Put top cards on stack until...' history case-sensitive (#7358) --- .../src/game_graphics/dialogs/dlg_move_top_cards_until.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cockatrice/src/game_graphics/dialogs/dlg_move_top_cards_until.cpp b/cockatrice/src/game_graphics/dialogs/dlg_move_top_cards_until.cpp index 2b54452ac..41c907163 100644 --- a/cockatrice/src/game_graphics/dialogs/dlg_move_top_cards_until.cpp +++ b/cockatrice/src/game_graphics/dialogs/dlg_move_top_cards_until.cpp @@ -1,5 +1,6 @@ #include "dlg_move_top_cards_until.h" +#include #include #include #include @@ -19,6 +20,7 @@ DlgMoveTopCardsUntil::DlgMoveTopCardsUntil(QWidget *parent, const MoveTopCardsUn exprComboBox = new QComboBox(this); exprComboBox->setFocus(); exprComboBox->setEditable(true); + exprComboBox->completer()->setCaseSensitivity(Qt::CaseSensitive); exprComboBox->setInsertPolicy(QComboBox::InsertAtTop); exprComboBox->insertItems(0, options.exprs); exprLabel->setBuddy(exprComboBox); From 28d684f8aa30b82a4689290a8a981deb9b152aaa Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 27 Sep 2026 20:05:41 +0200 Subject: [PATCH 2/7] [Windows] Close running instances and purge stale runtime DLLs during update (#7308) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Windows] Close running instances and purge stale runtime DLLs during update * [Windows] Scope running-instance handling to install dir and fix NSIS build - Drop !include nsExec.nsh: nsExec is a plugin DLL, not a header, so makensis aborts before NSIS can build the installer. - Match processes by image name and executable path under $INSTDIR via PowerShell, then close (WM_CLOSE) and force-stop only those PIDs, so an unrelated oracle.exe (Oracle DB) is never killed on a silent /R update. - Gate the stale-runtime-DLL purge on $INSTDIR\cockatrice.exe existing, so a first-time install can't recursively delete an unrelated Plugins directory. * [Windows] Make process detection rely on an explicit PowerShell exit code * [Windows] Fix exit-code detection in the uninstaller process check * [Windows] Pass the install dir to PowerShell without quoting pitfalls * [Windows] Fail closed when a process path is unreadable * [Windows] Only pre-check running instances for /R updates * [Windows] Wait on a wall-clock 60 s deadline instead of a loop counter * [Windows] Verify the force close and abort if the process survived * [Windows] Defer silent updates instead of force-killing the client * [Windows] Purge OpenSSL 3 DLLs on portable upgrades and broaden the stale-DLL guard * [Windows] Replace the GetTickCount macro with a direct System::Call * [Windows] Only close running instances whose path is readable CloseMatchingApps reused the fail-closed detection filter, which counts a process whose Path cannot be read (Access denied) as a match. The installer runs elevated, so CloseMainWindow() would then be delivered to an unrelated oracle.exe / servatrice.exe belonging to some other program, silently closing a third party application during a Cockatrice update. The set of processes we act on now has to be the strict one: a readable path under $INSTDIR. Unreadable paths are simply not ours. * [Windows] Match running instances by a readable path under the install dir The Path read in the Where-Object filter raises "Access is denied" for processes the installer cannot open, and both branches of the try/catch turned that into a match. Together with mapping every non-zero nsExec result to "running" this made the guard a hard stop rather than a fail-closed one: a machine with an Oracle database service (or a servatrice service) running as SYSTEM always has a process named oracle / servatrice whose path cannot be read, so every silent update aborted after 60 s and every interactive install re-asked for Retry indefinitely. A process whose path cannot be read is not one of ours - the updater is launched by the application it updates and therefore runs in the same user session, and an interactive install is elevated and can read the paths of the user's own processes. Both the detection and the close path now require a readable path under $INSTDIR, which is what the comment above the helpers already claimed. * [Windows] Distinguish "could not determine" from "definitely running" Mapping every non-zero nsExec result to "running" folds nsExec's "error" return (powershell.exe could not be started at all - removed, blocked by policy, refused by antivirus) into "Cockatrice is running". That state can never clear: closing applications does not help, so silent installs aborted after the full 60 s wait and interactive installs could not get past the Retry/Cancel prompt. IsAppRunning now reports three states (0 not running, 1 running, 2 could not determine) and a new IsProcessLookupAvailable preflight checks that powershell.exe can run at all. A broken PowerShell is reported to the user once - with a message box, since a silent install would otherwise show nothing - and the install continues instead of being blocked forever. Only a definite "running" is now waited for. * [Windows] Force-stop the console applications after the grace period Servatrice and Oracle are console applications, so CloseMainWindow() returns false and does nothing for them. Every silent update therefore waited the full 60 s and then aborted, and every interactive install looped on the Retry prompt, for any user running Servatrice out of the install directory. They now get a second shutdown path: once the grace period is over they are force-stopped by PID with the same strict install-dir filter, since they hold no unsaved user data. Cockatrice keeps the graceful-only treatment - a force-kill there could destroy an unsaved deck - and still aborts instead. * [Windows] Tell the user when a silent install or uninstall is aborted On the /R path the installer runs with SetSilent and SetAutoClose, so the DetailPrint before Abort went nowhere: the user clicks Help -> Check for Client Updates, the download finishes, Cockatrice closes its window, and nothing else happens - no update and no error, because the client launches the installer detached and never looks at its exit code. Every silent abort path now shows a message box telling the user which application is still open and that they should close it and run the installer again. The same applies to the silent uninstall, where the only output is a log file nobody reads. * [Windows] Treat leftover runtime files as an install marker for the purge The guard on the stale-DLL purge only looked for the three executables and uninstall.exe, so it skipped precisely the case the purge exists for: a previous failed update whose uninstaller removed what it could and silently skipped the Qt DLLs that a running instance had locked. None of the four markers is left in that directory, and the stale DLLs then survive into the new install. A leftover Qt6Core.dll (what we ship today) or the qt.conf written by a portable install now count as an install marker as well. * [Windows] Run the section instance check before any file is deleted Call EnsureAppsNotRunning sat after the portable-upgrade block, which had already deleted the old cockatrice.exe, Qt*.dll and plugins directory. An interactive install ("Portable mode" pointing at an existing portable folder) has $ReinstallMode = 0, so the .onInit guard is skipped and locked files were silently skipped - exactly the mixed-Qt-DLL state this change is about. The check now runs right after SetOutPath, before the first Delete/RMDir in the section. * [Windows] Guard the old install before the old uninstaller deletes files The interactive upgrade path removes the previous installation long before the install section: componentsPagePre runs the old UninstallString on the components page, i.e. before the directory page and before any call of ours can check anything. Every uninstaller shipped before this change has no un.EnsureAppsNotRunning, so for exactly the versions affected by #1576 the old files were deleted while cockatrice.exe was still running and the new installer had no say in it. componentsPagePre now runs the same check before both ExecWait calls. At that point $INSTDIR is still only the default, not the directory the old install lives in, so the new EnsureOldInstallNotRunning guards the location recorded in the registry instead. The helpers now work on a $GuardDir the caller picks; the normal entry points keep seeding it with $INSTDIR. * [Windows] Do not abort the section check past the point of no return "nothing has been replaced yet, so aborting just defers the update" only holds for the guard in .onInit. On the /R path AutoUninstallIfNeeded has already run the old uninstaller, which RMDir /r's the whole install directory, and the purge below it deletes the runtime files - so an Abort from the section would leave the user with the old version uninstalled and the new one not installed at all, which is strictly worse than the bug this change fixes. The ordering now guarantees the check already passed: .onInit guards the /R path before the uninstall, so the section only repeats it where that is not already true, i.e. for interactive installs, where nothing of this installer has been deleted yet and an Abort is the correct outcome. As a safety net for whatever the purge could not remove (a still locked file, or a check that could not run at all), HasStaleRuntimeFiles reports surviving runtime files with a message box - also on a silent update, where there is no other way to let the user know the install came out incomplete. * [Windows] Only block the in-app updater with a message box The message box on the silent abort would also fire for a plain /S install, which is exactly the unattended case: a package manager (or a CI job) running the installer with a leftover Cockatrice process would now hang forever on a dialog nobody can see instead of aborting. The box is therefore limited to /R, the path the in-app updater takes, where the user just clicked Help -> Check for Client Updates, the client launches the installer detached and never inspects its exit code, and the window is about to disappear - without a message, the update would silently not happen. Every silent abort also sets an error level now, so an unattended caller can see that the run failed instead of having to read the log. The silent uninstaller reports through log and exit code for the same reason: it is run unattended by the /R updater itself (QuietUninstallString) and by package managers. The installer that invoked it tells the user about the incomplete result itself. * [Windows] Treat an unset guard directory as undeterminable $GuardDir is set by the three entry points into the check, but it is now what decides whether a process counts as one of ours, and an empty string would match every process that merely shares an image name - the same false positive the strict path filter exists to avoid. Report it as "could not determine" instead of running the check, so a future caller that forgets to set it warns the user rather than closing or killing a third party process. --------- Co-authored-by: Lukas Brübach --- cmake/NSIS.template.in | 551 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 549 insertions(+), 2 deletions(-) diff --git a/cmake/NSIS.template.in b/cmake/NSIS.template.in index dd45f0039..c214a333e 100644 --- a/cmake/NSIS.template.in +++ b/cmake/NSIS.template.in @@ -12,6 +12,9 @@ Var NormalDestDir Var PortableDestDir Var PortableMode Var ReinstallMode +Var AllowForceClose +Var GuardDir +Var OldUninstallString !include LogicLib.nsh !include FileFunc.nsh @@ -131,6 +134,20 @@ ${EndIf} ; Now that $PortableMode reflects reality, commit InstDir into the correct slot Call SetModeDestinationFromInstdir +; Make sure no application instance is still running (and holding file locks) +; before the previous version is uninstalled. On a silent update (/R /S) the +; running processes are asked to close gracefully and waited for; Cockatrice +; itself is never force-killed (data loss, e.g. an unsaved deck) so the update +; is aborted instead, while the console applications Oracle and Servatrice are +; force-stopped by PID once the grace period is over. On interactive installs +; the user is prompted to close them instead. +; Only needed this early for /R, where AutoUninstallIfNeeded runs below; for +; interactive installs the section re-checks after the directory page, when +; $INSTDIR is the directory the user actually picked. +${If} $ReinstallMode = 1 + Call EnsureAppsNotRunning +${EndIf} + ${If} $ReinstallMode = 1 ${AndIf} $PortableMode = 0 Call AutoUninstallIfNeeded @@ -144,6 +161,9 @@ ${If} ${NSIS_IS_64_BIT} == 1 SetRegView 64 ${EndIf} +; Ensure no application instance is still running before removing files. +Call un.EnsureAppsNotRunning + FunctionEnd Function RequireAdmin @@ -199,6 +219,473 @@ ${EndIf} FunctionEnd +; --- Running instance handling --- +; Cockatrice, Oracle and Servatrice must not be running while files are +; replaced or deleted. A still-running process holds locks on its .exe and +; Qt runtime DLLs, so a silent upgrade could otherwise end up with a mix of +; old and new Qt DLLs next to the new executable, failing with +; "The procedure entry point X could not be located in the dynamic link +; library ...Qt6Network.dll" on the next start. +; +; On a silent update (/R /S) matching processes are asked to close gracefully +; (WM_CLOSE) and waited for up to 60 s. Cockatrice is never force-stopped: it +; may be holding an unsaved deck or showing a "really leave this game?" +; confirmation, so if it refuses to close the install is aborted - nothing has +; been replaced yet, so the update is simply deferred. Oracle and Servatrice +; are console applications without a main window, so CloseMainWindow() does +; nothing for them and waiting cannot help; they hold no unsaved user data and +; are force-stopped by PID once the grace period is over. On interactive +; installs and uninstalls the user is prompted to close them instead. +; +; Processes are matched by image name AND by their executable path living +; under the guarded directory ($GuardDir, normally $INSTDIR), so unrelated +; processes that merely share an image name (e.g. the Oracle DB instance +; "oracle.exe") are never touched. + +; usage: set $R2 to the base image name (without extension, e.g. "cockatrice", +; as accepted by Get-Process -Name), call this, result in $R0 +; (1 = running from $GuardDir, 0 = not running, 2 = could not determine) +Function IsAppRunning + ; Every caller has to set $GuardDir; an empty one would make the path check + ; below match every process that merely shares the image name, which is the + ; opposite of what this guard is for. + ${If} $GuardDir == "" + StrCpy $R0 2 + Return + ${EndIf} + ; Pass the directory to guard to PowerShell via an environment variable so + ; it never has to be embedded into the -Command string: apostrophes in a + ; path would terminate the command early, and the path in a -like pattern + ; would treat [ and ] as wildcard characters. + System::Call 'kernel32::SetEnvironmentVariable(t, t) i ("COCKATRICE_GUARD_DIR", "$GuardDir")' + ; Exit the process explicitly so nsExec's pop reflects whether a matching + ; process was found: powershell.exe exits 0 whether or not the pipeline + ; produced objects. 0 = not running, 1 = running, anything else = could not + ; be determined (see IsProcessLookupAvailable). + ; A process is only a match when its path is readable and lives under + ; $GuardDir. A process we cannot inspect is not one of ours: the updater is + ; always started by the application it updates, so it runs in the same user + ; session, and an interactive install is elevated and can read the paths of + ; the user's own processes. Assuming a match instead would be a hard stop + ; rather than a fail-closed guard - an Oracle database instance running as + ; SYSTEM has a process named "oracle" whose path raises Access denied, and + ; that would make every silent update abort and every interactive install + ; loop on the Retry prompt with no way out but Cancel. + nsExec::ExecToLog 'powershell -NoProfile -Command "if (Get-Process -Name $\'$R2$\' -ErrorAction SilentlyContinue | Where-Object { try { $$p = $$_.Path; if ($$p) { $$p.StartsWith($$env:COCKATRICE_GUARD_DIR, [StringComparison]::OrdinalIgnoreCase) } else { $$false } } catch { $$false } } | Select-Object -First 1) { exit 1 } else { exit 0 }"' + Pop $R0 + ${If} $R0 == "0" + StrCpy $R0 0 + ${ElseIf} $R0 == "1" + StrCpy $R0 1 + ${Else} + ; powershell.exe could not be run at all (nsExec pushes the string + ; "error"), or died with an unexpected code. That is "could not + ; determine", which has to stay distinguishable from "definitely + ; running": a state that can never clear would block the installer + ; forever. IsProcessLookupAvailable warns about that case once. + StrCpy $R0 2 + ${EndIf} +FunctionEnd + +; Sets $R0 to 1 if PowerShell can be started at all, 0 if it cannot. The +; running instance check is built on top of it, so a machine where +; powershell.exe is missing, blocked by policy or refused by antivirus is +; reported once instead of being reported as "Cockatrice is running" on every +; single check. +Function IsProcessLookupAvailable + nsExec::ExecToLog 'powershell -NoProfile -Command "exit 0"' + Pop $R0 + ${If} $R0 == "0" + StrCpy $R0 1 + ${Else} + StrCpy $R0 0 + ${EndIf} +FunctionEnd + +Function CloseMatchingApps + ; gracefully ask every matching instance to close (sends WM_CLOSE) + ; + ; The filter here is deliberately the strict one: a process is only asked to + ; close when its path is readable and lives under $GuardDir. Treating a + ; process whose path cannot be read (an unrelated oracle.exe / servatrice.exe, + ; or one owned by another account running as SYSTEM) as a match would make + ; the installer close a third party application, and it runs elevated, so + ; CloseMainWindow() would succeed on it. + System::Call 'kernel32::SetEnvironmentVariable(t, t) i ("COCKATRICE_GUARD_DIR", "$GuardDir")' + nsExec::ExecToLog 'powershell -NoProfile -Command "Get-Process -Name $\'$R2$\' -ErrorAction SilentlyContinue | Where-Object { try { $$p = $$_.Path; if ($$p) { $$p.StartsWith($$env:COCKATRICE_GUARD_DIR, [StringComparison]::OrdinalIgnoreCase) } else { $$false } } catch { $$false } } | ForEach-Object { $$null = $$_.CloseMainWindow() }"' + Pop $R3 +FunctionEnd + +Function ForceCloseApps + ; stop the matching instances by PID once the grace period is over. Only + ; used for the console applications ($AllowForceClose), which have no window + ; to close and hold no unsaved user data. Same strict filter as above. + System::Call 'kernel32::SetEnvironmentVariable(t, t) i ("COCKATRICE_GUARD_DIR", "$GuardDir")' + nsExec::ExecToLog 'powershell -NoProfile -Command "Get-Process -Name $\'$R2$\' -ErrorAction SilentlyContinue | Where-Object { try { $$p = $$_.Path; if ($$p) { $$p.StartsWith($$env:COCKATRICE_GUARD_DIR, [StringComparison]::OrdinalIgnoreCase) } else { $$false } } catch { $$false } } | ForEach-Object { Stop-Process -Id $$_.Id -Force -ErrorAction SilentlyContinue }"' + Pop $R3 +FunctionEnd + +Function WaitForAppToClose + ; usage: set $R1 to the display name (e.g. "cockatrice.exe") and $R2 to the + ; base image name (e.g. "cockatrice") + Call IsAppRunning + ; Only a definite "running" is worth waiting for. "Not running" is done, and + ; "could not determine" must not keep the installer waiting on an answer it + ; will never get - EnsureAppsNotRunning has warned about that case already. + ${If} $R0 != 1 + Return + ${EndIf} + + ${If} ${Silent} + ; ask the application to close gracefully (WM_CLOSE), then wait for it to exit + DetailPrint "Closing $R1 ..." + Call CloseMatchingApps + ; wall-clock deadline: each poll has to spawn a fresh powershell.exe, so + ; a bare loop counter would not bound the real wait time to 60 s. + System::Call 'kernel32::GetTickCount()i.s' + Pop $R8 + ck_wait_loop: + Sleep 500 + Call IsAppRunning + ${If} $R0 = 0 + DetailPrint "$R1 closed." + Return + ${EndIf} + ${If} $R0 != 1 + Return + ${EndIf} + System::Call 'kernel32::GetTickCount()i.s' + Pop $R9 + IntOp $R9 $R9 - $R8 + ${If} $R9 < 60000 + Goto ck_wait_loop + ${EndIf} + ; The grace period is over. Cockatrice is never force-stopped: it may be + ; showing a "really leave this game?" confirmation or holding an unsaved + ; deck, and nothing has been replaced yet, so aborting just defers the + ; update and preserves the user's data. + ${If} $AllowForceClose = 0 + DetailPrint "$R1 is still running after 60 s; aborting the update so that no data is lost." + ; The in-app updater runs this installer silent and detached and + ; never looks at its exit code, so a plain Abort would leave the user + ; with no update and no explanation. A /S install without /R on the + ; other hand is unattended - by a package manager, for instance - + ; and a message box there would block forever, so that case only + ; reports through the log and the exit code. + ${If} $ReinstallMode = 1 + MessageBox MB_OK|MB_ICONEXCLAMATION \ + "$R1 did not close within 60 seconds, so the installation was aborted to avoid losing unsaved data.$\r$\n$\r$\nPlease close $R1 and run the installer again." + ${EndIf} + SetErrorLevel 1 + Abort + ${EndIf} + ; Oracle and Servatrice are console applications: CloseMainWindow() + ; returned false for them and waiting longer cannot help. They hold no + ; unsaved user data, so stop them by PID. + DetailPrint "Force-stopping $R1 ..." + Call ForceCloseApps + Sleep 2000 + Call IsAppRunning + ${If} $R0 = 0 + DetailPrint "$R1 closed." + Return + ${EndIf} + ${If} $R0 != 1 + Return + ${EndIf} + DetailPrint "$R1 is still running after 60 s; aborting the update." + ${If} $ReinstallMode = 1 + MessageBox MB_OK|MB_ICONEXCLAMATION \ + "$R1 could not be stopped, so the installation was aborted.$\r$\n$\r$\nPlease close $R1 and run the installer again." + ${EndIf} + SetErrorLevel 1 + Abort + ${Else} + ck_wait_prompt: + MessageBox MB_RETRYCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON1 \ + "$R1 is still running.$\r$\n$\r$\nPlease close it, then click Retry.$\r$\nClick Cancel to abort." \ + IDCANCEL ck_abort_install + Call IsAppRunning + ${If} $R0 = 0 + Return + ${EndIf} + ${If} $R0 != 1 + Return + ${EndIf} + Goto ck_wait_prompt + ck_abort_install: + Abort + ${EndIf} +FunctionEnd + +Function EnsureAppsNotRunning + ; guards the install directory + StrCpy $GuardDir $INSTDIR + Call GuardAppsNotRunning +FunctionEnd + +Function EnsureOldInstallNotRunning + ; Guards the directory of a *previous* installation, as recorded in the + ; registry. Needed by componentsPagePre, which runs the old uninstaller + ; before the directory page, so $INSTDIR is still the default there and not + ; the directory the old install lives in. Every uninstaller shipped before + ; this change had no running instance guard of its own, so the old one would + ; delete files while an instance of the application is still holding them. + ReadRegStr $R3 HKLM "SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\Cockatrice" "InstallLocation" + ${If} $R3 == "" + ; no usable record of where the old install is; $INSTDIR is the default + ; install directory at this point, which is where it usually is anyway + DetailPrint "Could not read the previous installation directory from the registry; checking $INSTDIR instead." + StrCpy $R3 $INSTDIR + ${EndIf} + StrCpy $GuardDir $R3 + Call GuardAppsNotRunning +FunctionEnd + +Function GuardAppsNotRunning + ; usage: set $GuardDir to the directory to guard, then call this + Call IsProcessLookupAvailable + ${If} $R0 = 0 + ; Without PowerShell we cannot tell whether a running instance is + ; holding locks on the files we are about to replace. Warn once and + ; continue: refusing to install would leave the user with an installer + ; that can never complete, which is worse than the locked-file failure + ; this check exists to prevent. MessageBox is shown even in a silent + ; install - the user has to be told to close the applications by hand. + DetailPrint "Could not start powershell.exe; skipping the running instance check. Close Cockatrice, Oracle and Servatrice manually if the installation reports a problem." + MessageBox MB_OK|MB_ICONEXCLAMATION "This installer could not check whether Cockatrice is still running, because powershell.exe could not be started.$\r$\n$\r$\nPlease close Cockatrice, Oracle and Servatrice manually before continuing." + Return + ${EndIf} + StrCpy $R1 "cockatrice.exe" + StrCpy $R2 "cockatrice" + ; a GUI application with unsaved user data: graceful close only + StrCpy $AllowForceClose 0 + Call WaitForAppToClose + StrCpy $R1 "oracle.exe" + StrCpy $R2 "oracle" + StrCpy $AllowForceClose 1 + Call WaitForAppToClose + StrCpy $R1 "servatrice.exe" + StrCpy $R2 "servatrice" + StrCpy $AllowForceClose 1 + Call WaitForAppToClose +FunctionEnd + +; Sets $R0 to 1 if $INSTDIR looks like an existing Cockatrice install. Used to +; guard the stale-runtime-DLL purge so a first-time install into a pre-existing +; directory with an unrelated "Plugins" folder is never touched, while an +; install that only ever held e.g. servatrice.exe is still purged. +Function IsExistingCockatriceInstall + StrCpy $R0 0 + ${If} ${FileExists} "$INSTDIR\cockatrice.exe" + StrCpy $R0 1 + ${EndIf} + ${If} ${FileExists} "$INSTDIR\oracle.exe" + StrCpy $R0 1 + ${EndIf} + ${If} ${FileExists} "$INSTDIR\servatrice.exe" + StrCpy $R0 1 + ${EndIf} + ${If} ${FileExists} "$INSTDIR\uninstall.exe" + StrCpy $R0 1 + ${EndIf} + ; A leftover runtime DLL counts as an install as well. A previously failed + ; update can leave a directory in which the old uninstaller removed the + ; executables and uninstall.exe but silently skipped the DLLs its running + ; instance had locked - which is exactly the broken install the purge is + ; meant to heal, and without these markers none of the files above would be + ; there to prove that it is one. + ${If} ${FileExists} "$INSTDIR\Qt6Core.dll" + StrCpy $R0 1 + ${EndIf} + ${If} ${FileExists} "$INSTDIR\qt.conf" + StrCpy $R0 1 + ${EndIf} +FunctionEnd + +; Sets $R0 to 1 if a runtime file of a previous version is still present after +; the purge. NSIS silently skips whatever it cannot delete, so one surviving DLL +; is enough to prove that something still holds a lock on the install +; directory. The purge works on wildcards, which the file existence checks do +; not, so this looks for files that are certainly part of the payload. +Function HasStaleRuntimeFiles + StrCpy $R0 0 + ${If} ${FileExists} "$INSTDIR\Qt6Core.dll" + StrCpy $R0 1 + ${EndIf} + ${If} ${FileExists} "$INSTDIR\Qt6Network.dll" + StrCpy $R0 1 + ${EndIf} +FunctionEnd + +; Uninstaller copies of the same routines (the uninstaller gets its own +; function set compiled in, it cannot call the installer functions). +Function un.IsAppRunning + ; An unset $GuardDir would match every process that merely shares the image + ; name, see IsAppRunning in the installer. + ${If} $GuardDir == "" + StrCpy $R0 2 + Return + ${EndIf} + ; Pass the directory to guard to PowerShell via an environment variable so + ; it never has to be embedded into the -Command string: apostrophes in a + ; path would terminate the command early, and the path in a -like pattern + ; would treat [ and ] as wildcard characters. + System::Call 'kernel32::SetEnvironmentVariable(t, t) i ("COCKATRICE_GUARD_DIR", "$GuardDir")' + ; Exit the process explicitly so nsExec's pop reflects whether a matching + ; process was found: powershell.exe exits 0 whether or not the pipeline + ; produced objects. 0 = not running, 1 = running, anything else = could not + ; be determined (see un.IsProcessLookupAvailable). + ; Strict match: a readable path under $GuardDir, same as in the installer. + nsExec::ExecToLog 'powershell -NoProfile -Command "if (Get-Process -Name $\'$R2$\' -ErrorAction SilentlyContinue | Where-Object { try { $$p = $$_.Path; if ($$p) { $$p.StartsWith($$env:COCKATRICE_GUARD_DIR, [StringComparison]::OrdinalIgnoreCase) } else { $$false } } catch { $$false } } | Select-Object -First 1) { exit 1 } else { exit 0 }"' + Pop $R0 + ${If} $R0 == "0" + StrCpy $R0 0 + ${ElseIf} $R0 == "1" + StrCpy $R0 1 + ${Else} + ; powershell.exe could not be run at all (nsExec pushes the string + ; "error"), or died with an unexpected code. That is "could not + ; determine", which has to stay distinguishable from "definitely + ; running": a state that can never clear would block the uninstaller + ; forever. un.IsProcessLookupAvailable warns about that case once. + StrCpy $R0 2 + ${EndIf} +FunctionEnd + +Function un.IsProcessLookupAvailable + nsExec::ExecToLog 'powershell -NoProfile -Command "exit 0"' + Pop $R0 + ${If} $R0 == "0" + StrCpy $R0 1 + ${Else} + StrCpy $R0 0 + ${EndIf} +FunctionEnd + +Function un.CloseMatchingApps + ; gracefully ask every matching instance to close (sends WM_CLOSE) + ; + ; Strict filter, same as in the installer: never close a process whose path + ; cannot be read, it is not necessarily one of ours. + System::Call 'kernel32::SetEnvironmentVariable(t, t) i ("COCKATRICE_GUARD_DIR", "$GuardDir")' + nsExec::ExecToLog 'powershell -NoProfile -Command "Get-Process -Name $\'$R2$\' -ErrorAction SilentlyContinue | Where-Object { try { $$p = $$_.Path; if ($$p) { $$p.StartsWith($$env:COCKATRICE_GUARD_DIR, [StringComparison]::OrdinalIgnoreCase) } else { $$false } } catch { $$false } } | ForEach-Object { $$null = $$_.CloseMainWindow() }"' + Pop $R3 +FunctionEnd + +Function un.ForceCloseApps + ; stop the matching instances by PID once the grace period is over, see + ; ForceCloseApps in the installer. + System::Call 'kernel32::SetEnvironmentVariable(t, t) i ("COCKATRICE_GUARD_DIR", "$GuardDir")' + nsExec::ExecToLog 'powershell -NoProfile -Command "Get-Process -Name $\'$R2$\' -ErrorAction SilentlyContinue | Where-Object { try { $$p = $$_.Path; if ($$p) { $$p.StartsWith($$env:COCKATRICE_GUARD_DIR, [StringComparison]::OrdinalIgnoreCase) } else { $$false } } catch { $$false } } | ForEach-Object { Stop-Process -Id $$_.Id -Force -ErrorAction SilentlyContinue }"' + Pop $R3 +FunctionEnd + +Function un.WaitForAppToClose + Call un.IsAppRunning + ; Only a definite "running" is worth waiting for, see WaitForAppToClose. + ${If} $R0 != 1 + Return + ${EndIf} + + ${If} ${Silent} + DetailPrint "Closing $R1 ..." + Call un.CloseMatchingApps + ; wall-clock deadline: each poll has to spawn a fresh powershell.exe, so + ; a bare loop counter would not bound the real wait time to 60 s. + System::Call 'kernel32::GetTickCount()i.s' + Pop $R8 + un_ck_wait_loop: + Sleep 500 + Call un.IsAppRunning + ${If} $R0 = 0 + DetailPrint "$R1 closed." + Return + ${EndIf} + ${If} $R0 != 1 + Return + ${EndIf} + System::Call 'kernel32::GetTickCount()i.s' + Pop $R9 + IntOp $R9 $R9 - $R8 + ${If} $R9 < 60000 + Goto un_ck_wait_loop + ${EndIf} + ; The grace period is over. Cockatrice is never force-stopped, see + ; WaitForAppToClose in the installer. A silent uninstall only reports + ; through the log and its exit code: it is also run unattended (the /R + ; updater uses it via QuietUninstallString, package managers use it + ; directly), and a message box would block there. The installer that ran + ; us reports the incomplete result to the user itself. + ${If} $AllowForceClose = 0 + DetailPrint "$R1 is still running after 60 s; aborting so that no data is lost." + SetErrorLevel 1 + Abort + ${EndIf} + DetailPrint "Force-stopping $R1 ..." + Call un.ForceCloseApps + Sleep 2000 + Call un.IsAppRunning + ${If} $R0 = 0 + DetailPrint "$R1 closed." + Return + ${EndIf} + ${If} $R0 != 1 + Return + ${EndIf} + DetailPrint "$R1 is still running after 60 s; aborting." + SetErrorLevel 1 + Abort + ${Else} + un_ck_wait_prompt: + MessageBox MB_RETRYCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON1 \ + "$R1 is still running.$\r$\n$\r$\nPlease close it, then click Retry.$\r$\nClick Cancel to abort." \ + IDCANCEL un_ck_abort_install + Call un.IsAppRunning + ${If} $R0 = 0 + Return + ${EndIf} + ${If} $R0 != 1 + Return + ${EndIf} + Goto un_ck_wait_prompt + un_ck_abort_install: + Abort + ${EndIf} +FunctionEnd + +Function un.EnsureAppsNotRunning + ; guards the install directory; the uninstaller only ever has to check this one + StrCpy $GuardDir $INSTDIR + Call un.GuardAppsNotRunning +FunctionEnd + +Function un.GuardAppsNotRunning + Call un.IsProcessLookupAvailable + ${If} $R0 = 0 + ; Same reasoning as in the installer: without PowerShell we cannot tell + ; whether a running instance still holds file locks, and refusing to + ; uninstall would leave the user with an uninstaller that never + ; completes. Warn once and continue. + DetailPrint "Could not start powershell.exe; skipping the running instance check. Close Cockatrice, Oracle and Servatrice manually if the uninstall reports a problem." + MessageBox MB_OK|MB_ICONEXCLAMATION "This uninstaller could not check whether Cockatrice is still running, because powershell.exe could not be started.$\r$\n$\r$\nPlease close Cockatrice, Oracle and Servatrice manually before continuing." + Return + ${EndIf} + StrCpy $R1 "cockatrice.exe" + StrCpy $R2 "cockatrice" + ; a GUI application with unsaved user data: graceful close only + StrCpy $AllowForceClose 0 + Call un.WaitForAppToClose + StrCpy $R1 "oracle.exe" + StrCpy $R2 "oracle" + StrCpy $AllowForceClose 1 + Call un.WaitForAppToClose + StrCpy $R1 "servatrice.exe" + StrCpy $R2 "servatrice" + StrCpy $AllowForceClose 1 + Call un.WaitForAppToClose +FunctionEnd + Function PortableModePageCreate ${If} $ReinstallMode = 1 @@ -258,8 +745,15 @@ ${If} $PortableMode = 0 ${EndIf} uninst32: + ; the old uninstaller is run here, before the directory page, and every + ; uninstaller predating this change has no guard of its own: make sure no + ; instance of the application is still running out of the *old* install + ; directory before letting it delete anything. The uninstall string is + ; stashed away first, the check works on $R0. + StrCpy $OldUninstallString $R0 + Call EnsureOldInstallNotRunning ClearErrors - ExecWait "$R0" + ExecWait "$OldUninstallString" done32: @@ -278,8 +772,10 @@ ${If} $PortableMode = 0 ${EndIf} uninst64: + StrCpy $OldUninstallString $R0 + Call EnsureOldInstallNotRunning ClearErrors - ExecWait "$R0" + ExecWait "$OldUninstallString" done64: ${EndIf} @@ -294,6 +790,26 @@ Section "Application" SecApplication SetShellVarContext all SetOutPath "$INSTDIR" +; No application instance may be running while any file below is deleted or +; replaced: both upgrade blocks start by removing the old executables and +; runtime DLLs, and NSIS silently skips whatever is locked, which is how an +; install ends up with a mix of old and new Qt DLLs next to the new executable +; ("The procedure entry point X could not be located in the dynamic link +; library ...Qt6Network.dll"). So the check has to come before the first +; Delete/RMDir and not after it - an interactive install into an existing +; portable directory with cockatrice.exe still running would otherwise delete +; the old files under its feet. +; +; It is skipped on the /R path: .onInit already ran it before +; AutoUninstallIfNeeded removed the previous installation, so by the time this +; section starts the old files are gone and a second Abort here would leave the +; user with the old version uninstalled and the new one not installed at all, +; which is worse than the locked files being prevented. The leftover runtime +; files are reported at the end of the purge instead. +${If} $ReinstallMode = 0 + Call EnsureAppsNotRunning +${EndIf} + ${If} $PortableMode = 1 ${AndIf} ${FileExists} "$INSTDIR\portable.dat" ; upgrade portable mode @@ -308,6 +824,8 @@ ${AndIf} ${FileExists} "$INSTDIR\portable.dat" Delete "$INSTDIR\Qt*.dll" Delete "$INSTDIR\libmysql.dll" Delete "$INSTDIR\icu*.dll" + Delete "$INSTDIR\libcrypto*.dll" + Delete "$INSTDIR\libssl*.dll" Delete "$INSTDIR\libeay32.dll" Delete "$INSTDIR\ssleay32.dll" Delete "$INSTDIR\qt.conf" @@ -318,6 +836,35 @@ ${AndIf} ${FileExists} "$INSTDIR\portable.dat" RMDir "$INSTDIR" ${EndIf} +; Remove any runtime DLLs left over from older versions, so a mismatched +; Qt/OpenSSL set cannot survive next to the new executables. +Call IsExistingCockatriceInstall +${If} $PortableMode = 0 +${AndIf} $R0 = 1 + RMDir /r "$INSTDIR\Plugins" + Delete "$INSTDIR\Qt*.dll" + Delete "$INSTDIR\libcrypto*.dll" + Delete "$INSTDIR\libssl*.dll" + Delete "$INSTDIR\zlib*.dll" + Delete "$INSTDIR\libmysql.dll" + Delete "$INSTDIR\icu*.dll" + Delete "$INSTDIR\libeay32.dll" + Delete "$INSTDIR\ssleay32.dll" +${EndIf} + +; Whatever was locked during the purge - by an instance that started while the +; installer was running, or because the running instance check could not be +; performed at all - is still on disk, and the new executables are about to be +; written next to it. That mix is exactly what makes the next start fail with +; "The procedure entry point X could not be located in the dynamic link library +; ...Qt6Network.dll", so say so instead of leaving it to the user to find out. +Call HasStaleRuntimeFiles +${If} $R0 = 1 + DetailPrint "Files of a previous version are still present; the installation may be incomplete." + MessageBox MB_OK|MB_ICONEXCLAMATION \ + "The installation could not remove all files of a previous version of Cockatrice, most likely because it is still running or was started as administrator.$\r$\n$\r$\nPlease close Cockatrice, Oracle and Servatrice and run the installer again." +${EndIf} + @CPACK_NSIS_EXTRA_PREINSTALL_COMMANDS@ @CPACK_NSIS_FULL_INSTALL@ From fa8220df3c63f61f41effdfe6ad88213a6545f3d Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 27 Sep 2026 20:05:41 +0200 Subject: [PATCH 3/7] [Client] Exit deterministically when launching the update installer (#7309) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Client] Exit deterministically when launching the update installer * [Client] Only quit when the main window close is accepted Wait for MainWindow::close() to be accepted before quitting after the update installer is launched. When the close is vetoed (a running card DB update, open games, or an unsaved deck), keep running and tell the user the installer is already waiting, instead of exiting over their answer. Also fix the comment so it does not claim settings are saved on the vetoed path. * [Client] Only quit on update when the shutdown actually ran MainWindow::closeEvent has a static re-entrancy guard that returns early on a second close event, leaving it in its default accepted state. DlgUpdate reached from a nested event loop while a shutdown prompt was up could then get close() == true with no shutdown work done (no settings flush, no tab shutdown) and tear down the process behind an unanswered prompt. Add MainWindow::closeForUpdate(), which reports false when a close is already in progress or was vetoed, and gate the update-exit on it. * [Client] Use QCoreApplication::exit() to leave on update QCoreApplication::quit() only asks the application to quit and can be interrupted if a top-level window refuses its close event. On the update path the process must actually leave so its Qt DLLs are unlocked before the installer starts replacing files - apply the exit(0) variant both on the accepted-close path and the fallback where no MainWindow was ever closed. * [Client] Warn that the update installer force-closes within a minute Telling the user Cockatrice just 'stays open for now' is misleading: the installer launched by #7308 waits only about 60 s for a graceful close before it force-terminates the process, which would lose unsaved work. State the deadline and tell the user to save and close before then. * [Client] Warn that a vetoed update is cancelled, not forced The warning on the veto path promised the opposite of what the installer does: WaitForAppToClose in #7308 never force-closes cockatrice.exe ($AllowForceClose is 0), it aborts the update after 60 s so no unsaved data is lost. A user who is told Cockatrice will be terminated in a minute hurries to close it, only to find the update already given up. State the real outcome instead: the installer waits about a minute for a close, and the update is cancelled if that does not happen. * [Client] Warn about a waiting installer without blocking it CloseMatchingApps in #7308 sends the WM_CLOSE once and the wait loop only polls IsAppRunning afterwards, but QMessageBox::warning is application modal and QGuiApplicationPrivate::processCloseEvent drops spontaneous close events for windows blocked by a modal widget. The veto path therefore swallowed the installer's one and only request, so even a user who resolved the blocker and closed the app still ended in the 60 s timeout. Move the warning into warnInstallerIsWaiting() and show it modeless, so the main window stays able to answer the installer while the message is up. * [Client] Close the parent on the no-MainWindow update fallback The fallback branch exits the process without closing anything, so it skips the settings flush and tab shutdown this change exists to get in front of the installer - the opposite of the fix if a call site ever stops passing a MainWindow. Close the parent widget first, like the code before this change did, and only then leave unconditionally: the parent is the one that knows how to shut down gracefully, and the deterministic exit is what the installer needs. * Add override. --------- Co-authored-by: Lukas Brübach --- .../interface/widgets/dialogs/dlg_update.cpp | 61 ++++++++++++++++++- .../interface/widgets/dialogs/dlg_update.h | 1 + cockatrice/src/interface/window_main.cpp | 13 +++- cockatrice/src/interface/window_main.h | 8 +++ 4 files changed, 80 insertions(+), 3 deletions(-) diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp index 7cf58d3e0..c84a46528 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp @@ -5,15 +5,25 @@ #include "../client/network/update/client/release_channel.h" #include "../interface/window_main.h" +#include #include +#include +#include #include #include #include #include +#include #include #include #include +// Executable that, when it sits next to the downloaded update installer, is installed instead of +// it. A packager shipping a custom build - or someone testing one - can drop the file there and +// have Cockatrice run it rather than the official installer, without the release channel having to +// host an installer for that build. It is a drop-in and is run with the same arguments. +static const QString UPDATE_INSTALLER_OVERRIDE = "Cockatrice-Update-Override.exe"; + DlgUpdate::DlgUpdate(QWidget *parent) : QDialog(parent) { @@ -203,6 +213,24 @@ void DlgUpdate::setLabel(const QString &newText) statusLabel->setText(newText); } +void DlgUpdate::warnInstallerIsWaiting() +{ + // Modeless on purpose: the installer asks the application to close exactly once (see + // CloseMatchingApps in NSIS.template.in) and only polls afterwards, while Qt drops spontaneous + // close events - a WM_CLOSE from the installer - for windows that are blocked by a modal + // widget (QGuiApplicationPrivate::processCloseEvent). An application modal message box here + // would therefore swallow the installer's one and only request for the rest of its wait. + auto *notification = new QMessageBox(QMessageBox::Warning, tr("Update"), + tr("The update installer is already running and waits about a " + "minute for Cockatrice to close. Cockatrice is still busy, so " + "save your work and close it before then. Otherwise the " + "installer gives up and the update is cancelled."), + QMessageBox::Ok, parentWidget()); + notification->setWindowModality(Qt::NonModal); + notification->setAttribute(Qt::WA_DeleteOnClose); + notification->show(); +} + void DlgUpdate::updateCheckError(const QString &errorString) { setLabel(tr("Error")); @@ -224,6 +252,12 @@ void DlgUpdate::downloadSuccessful(const QUrl &filepath) QString installerPath = filepath.toLocalFile(); + const QString overridePath = QDir(QFileInfo(installerPath).absolutePath()).filePath(UPDATE_INSTALLER_OVERRIDE); + if (QFileInfo::exists(overridePath)) { + qCInfo(DlgUpdateLog) << "Installing the update installer override instead of the download:" << overridePath; + installerPath = overridePath; + } + QString appDir = QDir::toNativeSeparators(QCoreApplication::applicationDirPath()); QProcess process; process.setProgram(installerPath); @@ -240,8 +274,31 @@ void DlgUpdate::downloadSuccessful(const QUrl &filepath) // Try to open the installer. If it opens, quit Cockatrice if (process.startDetached()) { - QMetaObject::invokeMethod(static_cast(parent()), "close", Qt::QueuedConnection); - qCInfo(DlgUpdateLog) << "Opened downloaded update file successfully - closing Cockatrice"; + qCInfo(DlgUpdateLog) << "Opened update installer successfully - closing Cockatrice"; + // Close the main window synchronously so file locks are released before the NSIS installer + // (already launched) starts replacing files. This also flushes settings and shuts down the + // tabs, but only when the close is actually accepted: MainWindow may veto it for a running + // card DB update, an open game, or an unsaved deck, and closeForUpdate() also reports a + // close already in progress (reached from a nested event loop while a shutdown prompt is + // up). Only quit when the shutdown really ran - otherwise keep running so the user can + // resolve the blocker, and warn them that the installer only waits about a minute + // before it gives up and cancels the update. + if (auto *window = qobject_cast(parent())) { + if (window->closeForUpdate()) { + QTimer::singleShot(0, qApp, [] { QCoreApplication::exit(0); }); + } else { + warnInstallerIsWaiting(); + } + } else { + // Not a MainWindow, so no faithful close can be requested - but the parent still gets + // its close() call, which is what the code before this change did and lets it flush + // settings and shut down its tabs. Leaving is then unconditional: the installer is + // already running against a process that still holds locks on the files it replaces. + if (auto *widget = parentWidget()) { + widget->close(); + } + QTimer::singleShot(0, qApp, [] { QCoreApplication::exit(0); }); + } close(); } else { setLabel(tr("Error")); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_update.h b/cockatrice/src/interface/widgets/dialogs/dlg_update.h index 7bd0020d5..2cef46e70 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_update.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_update.h @@ -42,6 +42,7 @@ private: void addStopDownloadAndRemoveOthers(bool enable); void beginUpdateCheck(); void setLabel(const QString &text); + void warnInstallerIsWaiting(); QLabel *statusLabel, *descriptionLabel; QProgressBar *progress; QPushButton *manualDownload, *gotoDownload, *ok, *stopDownload; diff --git a/cockatrice/src/interface/window_main.cpp b/cockatrice/src/interface/window_main.cpp index ae967312d..77cc85130 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -871,7 +871,6 @@ void MainWindow::actShow() void MainWindow::closeEvent(QCloseEvent *event) { // workaround Qt bug where closeEvent gets called twice - static bool bClosingDown = false; if (bClosingDown) { return; } @@ -900,6 +899,18 @@ void MainWindow::closeEvent(QCloseEvent *event) tabSupervisor->deleteLater(); } +bool MainWindow::closeForUpdate() +{ + // A shutdown is already being handled (e.g. this is reached from a nested event loop while + // closeEvent() is blocked on a user prompt). close() would hit the re-entrancy guard and return + // true without any shutdown having happened, so report faithfully instead: the caller must keep + // the process alive until the user resolves whatever is blocking the close. + if (bClosingDown) { + return false; + } + return close(); +} + void MainWindow::changeEvent(QEvent *event) { if (event->type() == QEvent::LanguageChange) { diff --git a/cockatrice/src/interface/window_main.h b/cockatrice/src/interface/window_main.h index fc7980d16..1c4dde300 100644 --- a/cockatrice/src/interface/window_main.h +++ b/cockatrice/src/interface/window_main.h @@ -167,6 +167,7 @@ private: LagMonitor lagMonitor; ///< watches the main thread for event loop stalls LatencyStatusWidget *latencyStatus = nullptr; ///< status bar widget with live round-trip stats and history graph bool bHasActivated, askedForDbUpdater; + bool bClosingDown = false; ///< guards closeEvent() against re-entrancy bool skipStartupAutoConnect = false; bool startupAutoConnectAttempted = false; bool firstRunWizardActive = false; @@ -205,6 +206,13 @@ public: return tabSupervisor; } + /** + * @brief Closes the window so an update installer can replace the running binaries. + * Returns true only if the shutdown actually ran (settings flushed, tabs shut down); + * false if the close was vetoed by the user or is already in progress. + */ + bool closeForUpdate(); + protected: void closeEvent(QCloseEvent *event) override; void changeEvent(QEvent *event) override; From 2632f8a040f1ba0df0d81d6008ed899f8b52d2fd Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 27 Sep 2026 21:32:49 +0200 Subject: [PATCH 4/7] [Client/Server] Keep non-deck files out of the deck storage (#7352) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The local deck and replay browsers list every file in the folder, so a stray file (e.g. a PNG screenshot of a deck) can be parsed as a garbage plaintext deck, opened in the editor, and uploaded. The server also accepts oversized deck payloads with only silent truncation. Hide files that are not in a supported deck/replay format in the two QFileSystemModel views (directories stay visible), skip them when opening, reject them when uploading, and make the server reject deck uploads larger than MAX_FILE_LENGTH instead of truncating them. Co-authored-by: Lukas Brübach --- .../widgets/tabs/tab_deck_storage.cpp | 18 ++++++++++++++++++ .../src/interface/widgets/tabs/tab_replays.cpp | 13 +++++++++++++ servatrice/src/serversocketinterface.cpp | 4 ++++ 3 files changed, 35 insertions(+) diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index 62769d0e3..41ebb17a2 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -49,6 +50,12 @@ namespace // How long to wait after the last visibility change before reading back the // Public/Private column, in milliseconds. constexpr int VISIBILITY_REFRESH_DELAY = 500; + +// Whether the file's name matches one of the deck formats Cockatrice can load. +bool isSupportedDeckFile(const QString &filePath) +{ + return QDir::match(DeckLoader::ACCEPTED_FILE_EXTENSIONS, QFileInfo(filePath).fileName()); +} } // namespace TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, @@ -58,6 +65,8 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, { localDirModel = new QFileSystemModel(this); localDirModel->setRootPath(SettingsCache::instance().paths().getDeckPath()); + localDirModel->setNameFilters(DeckLoader::ACCEPTED_FILE_EXTENSIONS); + localDirModel->setNameFilterDisables(false); localDirModel->sort(0, Qt::AscendingOrder); localDirView = new QTreeView; @@ -312,6 +321,10 @@ void TabDeckStorage::actOpenLocalDeck() } QString filePath = localDirModel->filePath(curLeft); + if (!isSupportedDeckFile(filePath)) { + continue; + } + std::optional deckOpt = DeckLoader::loadFromFile(filePath, DeckFileFormat::Cockatrice, true); if (!deckOpt) { continue; @@ -376,6 +389,11 @@ void TabDeckStorage::actUpload() void TabDeckStorage::uploadDeck(const QString &filePath, const QString &targetPath) { + if (!isSupportedDeckFile(filePath)) { + QMessageBox::critical(this, tr("Error"), tr("Invalid deck file")); + return; + } + QFile deckFile(filePath); QFileInfo deckFileInfo(deckFile); diff --git a/cockatrice/src/interface/widgets/tabs/tab_replays.cpp b/cockatrice/src/interface/widgets/tabs/tab_replays.cpp index ac4b2cbe9..e26bea2a3 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_replays.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_replays.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -34,6 +35,12 @@ inline Q_LOGGING_CATEGORY(TabReplaysLog, "replays_tab"); +namespace +{ +// File name filters for the local replay files Cockatrice can load. +const QStringList REPLAY_FILE_NAME_FILTERS = {"*.cor"}; +} // namespace + TabReplays::TabReplays(TabSupervisor *_tabSupervisor, AbstractClient *_client, const ServerInfo_User *currentUserInfo) : Tab(_tabSupervisor), client(_client) { @@ -62,6 +69,8 @@ QGroupBox *TabReplays::createLeftLayout() { localDirModel = new QFileSystemModel(this); localDirModel->setRootPath(SettingsCache::instance().paths().getReplaysPath()); + localDirModel->setNameFilters(REPLAY_FILE_NAME_FILTERS); + localDirModel->setNameFilterDisables(false); localDirModel->sort(0, Qt::AscendingOrder); localDirView = new QTreeView; @@ -263,6 +272,10 @@ void TabReplays::actOpenLocalReplay() } QString filePath = localDirModel->filePath(curLeft); + if (!QDir::match(REPLAY_FILE_NAME_FILTERS, QFileInfo(filePath).fileName())) { + continue; + } + QFile f(filePath); if (!f.open(QIODevice::ReadOnly)) { continue; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 9c78c48f3..a8d13675d 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -982,6 +982,10 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman return Response::RespInvalidData; } + if (cmd.deck_list().size() > static_cast(MAX_FILE_LENGTH)) { + return Response::RespInvalidData; + } + sqlInterface->checkSql(); QString deckStr = fileFromStdString(cmd.deck_list()); From 1365f3cb492c788e1ae3a3927e9fb00dcf7f9033 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 27 Sep 2026 21:38:50 +0200 Subject: [PATCH 5/7] [Avatar] Cap user avatar upload resolution at 1024x1024 (#7353) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Avatar] Cap user avatar upload resolution at 1024x1024 * Remove stylesheet. --------- Co-authored-by: Lukas Brübach --- .../src/interface/widgets/dialogs/dlg_edit_avatar.cpp | 8 ++++++++ .../src/interface/widgets/dialogs/dlg_edit_avatar.h | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_edit_avatar.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_edit_avatar.cpp index b5710b208..120e20f3e 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_edit_avatar.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_edit_avatar.cpp @@ -19,6 +19,9 @@ DlgEditAvatar::DlgEditAvatar(QWidget *parent) : QDialog(parent), image() textLabel = new QLabel(tr("To change your avatar, choose a new image.\nTo remove your current avatar, confirm " "without choosing a new image.")); + hintLabel = new QLabel( + tr("Images are automatically downscaled to at most %1x%1 pixels.").arg(QString::number(MAX_AVATAR_DIMENSION))); + browseButton = new QPushButton(tr("Browse...")); connect(browseButton, &QPushButton::clicked, this, &DlgEditAvatar::actBrowse); @@ -26,6 +29,7 @@ DlgEditAvatar::DlgEditAvatar(QWidget *parent) : QDialog(parent), image() grid->addWidget(imageLabel, 0, 0, 1, 2); grid->addWidget(textLabel, 1, 0); grid->addWidget(browseButton, 1, 1); + grid->addWidget(hintLabel, 2, 0, 1, 2); auto *buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); connect(buttonBox, &QDialogButtonBox::accepted, this, &DlgEditAvatar::actOk); @@ -72,6 +76,10 @@ QByteArray DlgEditAvatar::getImage() return QByteArray(); } + if (image.width() > MAX_AVATAR_DIMENSION || image.height() > MAX_AVATAR_DIMENSION) { + image = image.scaled(MAX_AVATAR_DIMENSION, MAX_AVATAR_DIMENSION, Qt::KeepAspectRatio, Qt::SmoothTransformation); + } + for (;;) { QByteArray ba; QBuffer buffer(&ba); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_edit_avatar.h b/cockatrice/src/interface/widgets/dialogs/dlg_edit_avatar.h index d635ac2a8..9ac774b24 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_edit_avatar.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_edit_avatar.h @@ -29,8 +29,9 @@ private slots: void actBrowse(); private: + static constexpr int MAX_AVATAR_DIMENSION = 1024; QImage image; - QLabel *textLabel, *imageLabel; + QLabel *textLabel, *imageLabel, *hintLabel; QPushButton *browseButton; }; From 6816c580cf06f373d7906254809cc9792e633218 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:59:20 +0200 Subject: [PATCH 6/7] Bump doc/doxygen/theme from `d52eafe` to `46483f1` (#7371) --- doc/doxygen/theme | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/doxygen/theme b/doc/doxygen/theme index d52eafe3e..46483f1e5 160000 --- a/doc/doxygen/theme +++ b/doc/doxygen/theme @@ -1 +1 @@ -Subproject commit d52eafe3e9303399fda15661f3d7bb8fe3d7eabc +Subproject commit 46483f1e5a70ffb9ecd3b82d0a1cd1b24edf13da From 61e01990c4b14912627c4771ea22755e1ba9db1f Mon Sep 17 00:00:00 2001 From: tooomm Date: Fri, 2 Oct 2026 03:41:29 +0200 Subject: [PATCH 7/7] [CI] Use Doxygen v1.18.0 (#7374) * Bump doxygen version to v1.18.0 * Update Doxyfile config to v1.18.0 * bump batch size * bump more --- .github/workflows/documentation-build.yml | 2 +- Doxyfile | 97 ++++++++++++++++++----- 2 files changed, 77 insertions(+), 22 deletions(-) diff --git a/.github/workflows/documentation-build.yml b/.github/workflows/documentation-build.yml index 419cbfbfb..f13b964cd 100644 --- a/.github/workflows/documentation-build.yml +++ b/.github/workflows/documentation-build.yml @@ -41,7 +41,7 @@ jobs: - name: "Install Doxygen" uses: ssciwr/doxygen-install@v2 with: - version: "1.16.1" + version: "1.18.0" - name: "Update Doxygen Configuration" run: | diff --git a/Doxyfile b/Doxyfile index 9b05682cc..d6f0af2d2 100644 --- a/Doxyfile +++ b/Doxyfile @@ -1,4 +1,4 @@ -# Doxyfile 1.16.1 +# Doxyfile 1.18.0 # This file describes the settings to be used by the documentation system # Doxygen (www.doxygen.org) for a project. @@ -11,7 +11,7 @@ # TAG = value [value, ...] # For lists, items can also be appended using: # TAG += value [value, ...] -# Values that contain spaces should be placed between quotes (\" \"). +# Values that contain spaces should be placed between quotes (" "). # # Note: # @@ -81,7 +81,7 @@ OUTPUT_DIRECTORY = docs # and will distribute the generated files over these directories. Enabling this # option can be useful when feeding Doxygen a huge amount of source files, where # putting all generated files in the same directory would otherwise cause -# performance problems for the file system. Adapt CREATE_SUBDIRS_LEVEL to +# performance problems for the file system. Adjust CREATE_SUBDIRS_LEVEL to # control the number of sub-directories. # The default value is: NO. @@ -802,7 +802,7 @@ GENERATE_REQUIREMENTS = YES # The REQ_TRACEABILITY_INFO tag controls if traceability information is shown on # the requirements page (only relevant when using \requirement comment blocks). -# The setting NO will disable the traceablility information altogether. The +# The setting NO will disable the traceability information altogether. The # setting UNSATISFIED_ONLY will show a list of requirements that are missing a # satisfies relation (through the command: \satisfies). Similarly the setting # UNVERIFIED_ONLY will show a list of requirements that are missing a verifies @@ -855,8 +855,8 @@ SHOW_NAMESPACES = YES # The FILE_VERSION_FILTER tag can be used to specify a program or script that # Doxygen should invoke to get the current version for each file (typically from # the version control system). Doxygen will invoke the program by executing (via -# popen()) the command command input-file, where command is the value of the -# FILE_VERSION_FILTER tag, and input-file is the name of an input file provided +# popen()) the command command input_file, where command is the value of the +# FILE_VERSION_FILTER tag, and input_file is the name of an input file provided # by Doxygen. Whatever the program writes to standard output is used as the file # version. For an example see the documentation. @@ -1643,8 +1643,8 @@ DOCSET_PUBLISHER_NAME = Publisher # a.o. the download links, offline (the HTML help workshop was already many # years in maintenance mode). You can download the HTML help workshop from the # web archives at Installation executable (see: -# http://web.archive.org/web/20160201063255/http://download.microsoft.com/downlo -# ad/0/A/9/0A939EF6-E31C-430F-A3DF-DFAE7960D564/htmlhelp.exe). +# http://web.archive.org/web/20160201063255/https://download.microsoft.com/downl +# oad/0/A/9/0A939EF6-E31C-430F-A3DF-DFAE7960D564/htmlhelp.exe). # # The HTML Help Workshop contains a compiler that can convert all HTML output # generated by Doxygen into a single compiled HTML file (.chm). Compiled HTML @@ -1951,7 +1951,7 @@ MATHJAX_FORMAT = HTML-CSS # output directory using the MATHJAX_RELPATH option. For Mathjax version 2 the # destination directory should contain the MathJax.js script. For instance, if # the mathjax directory is located at the same level as the HTML output -# directory, then MATHJAX_RELPATH should be ../mathjax.s For Mathjax versions 3 +# directory, then MATHJAX_RELPATH should be ../mathjax. For Mathjax versions 3 # and 4 the destination directory should contain the tex-.js script # (where is either chtml or svg). The default value points to the # MathJax Content Delivery Network so you can quickly see the result without @@ -2172,9 +2172,8 @@ EXTRA_PACKAGES = # # Note: Only use a user-defined header if you know what you are doing! # Note: The header is subject to change so you typically have to regenerate the -# default header when upgrading to a newer version of Doxygen. The following -# commands have a special meaning inside the header (and footer): For a -# description of the possible markers and block names see the documentation. +# default header when upgrading to a newer version of Doxygen. For a description +# of the possible markers and block names see the documentation. # This tag requires that the tag GENERATE_LATEX is set to YES. LATEX_HEADER = @@ -2657,6 +2656,19 @@ HAVE_DOT = YES DOT_NUM_THREADS = 0 +# The DOT_BATCH_SIZE specifies the number of dot graphs Doxygen is allowed to +# compile in a single invocation of dot. When set to 1 Doxygen will invoke dot +# for each graph separately, which can cause significant process creation +# overhead especially on systems with many CPU cores. Together with +# DOT_NUM_THREADS this setting can be used to optimise the dot processing speed +# for a particular system. Doxygen will try to give each thread a balanced batch +# of work. If the total number of graphs to process exceeds DOT_NUM_THREADS * +# DOT_BATCH_SIZE then additional batches will be created for dot to process. +# Minimum value: 1, maximum value: 1000, default value: 50. +# This tag requires that the tag HAVE_DOT is set to YES. + +DOT_BATCH_SIZE = 500 + # DOT_COMMON_ATTR is common attributes for nodes, edges and labels of # subgraphs. When you want a differently looking font in the dot files that # Doxygen generates you can specify fontname, fontcolor and fontsize attributes. @@ -2948,6 +2960,58 @@ PLANTUML_INCLUDE_PATH = PLANTUMLFILE_DIRS = +# When using Mermaid diagrams with CLI rendering, the MERMAID_PATH tag should be +# used to specify the directory where the mmdc (Mermaid CLI) executable can be +# found. If left blank, CLI-based rendering is disabled. For HTML output, +# client-side rendering via JavaScript is used by default and does not require +# mmdc. For LaTeX/PDF output, mmdc is required to pre-generate images. Doxygen +# will generate a warning when CLI rendering is needed but mmdc is not +# available. + +MERMAID_PATH = + +# When using Mermaid diagrams, the MERMAID_CONFIG_FILE tag can be used to +# specify a JSON configuration file for the Mermaid CLI tool (mmdc). This file +# can contain theme settings and other Mermaid configuration options. + +MERMAID_CONFIG_FILE = + +# The MERMAID_RENDER_MODE tag selects how Mermaid diagrams are rendered. +# Possible values are: AUTO (use client-side rendering for HTML and mmdc for +# LaTeX/PDF and other formats. If MERMAID_PATH is not set, non-HTML diagrams +# will produce a warning), CLI (use the mmdc tool to pre-generate images +# (requires Node.js and mermaid-js/mermaid-cli). Works for all output formats) +# and CLIENT_SIDE (embed mermaid.js in HTML output for client-side rendering. +# Does not require mmdc but only works for HTML output). +# The default value is: AUTO. + +MERMAID_RENDER_MODE = AUTO + +# The MERMAID_JS_URL tag specifies the URL to load mermaid.js from when using +# client-side rendering (MERMAID_RENDER_MODE is CLIENT_SIDE or AUTO). The +# default points to the latest Mermaid v11 release on the jsDelivr CDN. +# +# The default CDN URL requires internet access when viewing the generated +# documentation. For offline use, download mermaid.esm.min.mjs and set this to a +# relative path, or use MERMAID_RENDER_MODE=CLI to pre-generate images instead. +# Examples: +# - Latest v11 (default): +# https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.esm.min.mjs +# - Pinned version: +# https://cdn.jsdelivr.net/npm/mermaid@11.3.0/dist/mermaid.esm.min.mjs +# - Local copy: ./mermaid.esm.min.mjs (user must place file in HTML output +# directory) +# The default value is: +# https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.esm.min.mjs. + +MERMAID_JS_URL = https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.esm.min.mjs + +# The MERMAIDFILE_DIRS tag can be used to specify one or more directories that +# contain Mermaid files that are included in the documentation (see the +# \mermaidfile command). + +MERMAIDFILE_DIRS = + # The DOT_GRAPH_MAX_NODES tag can be used to set the maximum number of nodes # that will be shown in the graph. If the number of nodes in a graph becomes # larger than this value, Doxygen will truncate the graph, which is visualized @@ -2972,15 +3036,6 @@ DOT_GRAPH_MAX_NODES = 50 MAX_DOT_GRAPH_DEPTH = 0 -# Set the DOT_MULTI_TARGETS tag to YES to allow dot to generate multiple output -# files in one run (i.e. multiple -o and -T options on the command line). This -# makes dot run faster, but since only newer versions of dot (>1.8.10) support -# this, this feature is disabled by default. -# The default value is: NO. -# This tag requires that the tag HAVE_DOT is set to YES. - -DOT_MULTI_TARGETS = NO - # If the GENERATE_LEGEND tag is set to YES Doxygen will generate a legend page # explaining the meaning of the various boxes and arrows in the dot generated # graphs.