diff --git a/deploy/windows/installer/ShopDBFlask.iss b/deploy/windows/installer/ShopDBFlask.iss index 598118e..e39dbfe 100644 --- a/deploy/windows/installer/ShopDBFlask.iss +++ b/deploy/windows/installer/ShopDBFlask.iss @@ -655,7 +655,7 @@ end; // If nothing is wrong the page collapses to a single reassuring statement. procedure RenderPreflight(Lines: TArrayOfString; var HasBlockers: Boolean); var - I, Y, Shown, Fails, Warns, Passes: Integer; + I, Y, Shown, Fails, Warns, Passes, RowHeight, MaxRows, Hidden: Integer; Parts: TArrayOfString; Check, Detail, Fix: String; begin @@ -663,6 +663,16 @@ begin Y := ScaleY(4); Shown := 0; + // How many rows actually FIT, measured, not guessed. The old cap of 6 bore no + // relation to the panel, which holds roughly three: at four or five notes the + // rows below were drawn past the bottom edge and silently vanished - taking + // with them the warning that the bundled MySQL collides on port 3306, which is + // exactly what the operator needs before the very next page. + RowHeight := ScaleY(46) + ScaleY(ROW_GAP); + if RowHeight < 1 then RowHeight := 1; + MaxRows := DetailPanel.Height div RowHeight; + if MaxRows < 1 then MaxRows := 1; + // Blockers first, then warnings - a second pass rather than one, so severity // ordering does not depend on the order the checks happen to run in. for I := 0 to GetArrayLength(Lines) - 1 do @@ -676,6 +686,9 @@ begin for I := 0 to GetArrayLength(Lines) - 1 do begin + // Blockers are capped too. Without this the FAIL loop drew every failure, + // however many, straight off the bottom of the panel. + if Shown >= MaxRows then Break; Parts := StringSplit(Lines[I], ['|'], stAll); if GetArrayLength(Parts) < 4 then Continue; if Parts[0] <> 'FAIL' then Continue; @@ -690,7 +703,7 @@ begin for I := 0 to GetArrayLength(Lines) - 1 do begin - if Shown >= 6 then Break; + if Shown >= MaxRows then Break; Parts := StringSplit(Lines[I], ['|'], stAll); if GetArrayLength(Parts) < 4 then Continue; if Parts[0] <> 'WARN' then Continue; @@ -727,8 +740,13 @@ begin + 'verified. Nothing needs your attention.'); end; - if (Shown >= 6) and (Warns > 6 - Fails) then - FooterText.Caption := 'Some notes are not shown. The full check is in the install log.' + // Count what was actually left out rather than inferring it from the old cap, + // which under-reported: with 2 failures and 5 warnings it claimed everything + // was shown while four rows had been dropped. + Hidden := (Fails + Warns) - Shown; + if Hidden > 0 then + FooterText.Caption := IntToStr(Hidden) + ' further item(s) are not shown here. ' + + 'All of them are in the install log.' else FooterText.Caption := IntToStr(Passes) + ' checks passed. Nothing has been changed on this server.'; end; @@ -837,10 +855,76 @@ begin Log('[shopdb] pre-fill base directory: ' + Result); end; +// Everything typed in the wizard ends up on a command line that +// CommandLineToArgvW parses, and reaches PowerShell parameters typed [int] or +// [string]. A blank, a space, or a trailing backslash therefore has to be caught +// HERE - by the time the script sees it, the arguments have already shifted. + +function IsValidPort(const S: String): Boolean; +var + I, N: Integer; +begin + Result := False; + if (S = '') or (Length(S) > 5) then Exit; + for I := 1 to Length(S) do + if (S[I] < '0') or (S[I] > '9') then Exit; // also rejects spaces and signs + N := StrToIntDef(S, -1); + Result := (N >= 1) and (N <= 65535); +end; + +function IsAsciiOnly(const S: String): Boolean; +var + I: Integer; +begin + Result := True; + for I := 1 to Length(S) do + if Ord(S[I]) > 126 then + begin + Result := False; + Exit; + end; +end; + +// A path that ends in a backslash - a drive root such as D:\ - closes the +// argument with \" , which CommandLineToArgvW reads as an ESCAPED quote. The +// argument never terminates and every argument after it shifts one place. +// Doubling the backslash inside the quotes is the documented way out. +function QuotePathArg(const P: String): String; +begin + if (P <> '') and (P[Length(P)] = '\') then + Result := '"' + P + '\' + '"' + else + Result := '"' + P + '"'; +end; + function NextButtonClick(CurPageID: Integer): Boolean; begin Result := True; + // A drive root is refused outright rather than quoted around. Uninstall + // removes the application directory recursively, so accepting D:\ here would + // mean uninstalling ShopDB-Flask wipes the whole drive. + if CurPageID = wpSelectDir then + begin + if Length(WizardDirValue) <= 3 then + begin + MsgBox('Choose a folder rather than a whole drive.' + #13#10#13#10 + + 'Removing ShopDB-Flask deletes the folder it was installed into, ' + + 'so installing to a drive root would delete everything on that ' + + 'drive when it is uninstalled.', mbError, MB_OK); + Result := False; + end; + end; + + if CurPageID = SitePage.ID then + begin + if not IsValidPort(SitePage.Values[1]) then + begin + MsgBox('Enter the port as a number between 1 and 65535.', mbError, MB_OK); + Result := False; + end; + end; + if CurPageID = wpWelcome then RunPreflight; @@ -872,6 +956,13 @@ begin begin MsgBox('Enter the database host.', mbError, MB_OK); Result := False; + end + else if not IsValidPort(DbDetailsPage.Values[1]) then + begin + MsgBox('Enter the database port as a number between 1 and 65535.' + + #13#10#13#10 + 'MySQL uses 3306 unless it was changed.', + mbError, MB_OK); + Result := False; end; end; end; @@ -1056,22 +1147,31 @@ var Cmd: String; begin if not FileExists(LnkPath) then Exit; + // THREE quotes each side, not four. Four is an escaped quote twice over, which + // keeps the whole expression inside one Pascal literal: LnkPath was never + // interpolated, PowerShell got the bare word "LnkPath", failed to parse, and + // no shortcut ever received the elevation flag. Self-elevation hid it - the + // only symptom was a second window after an extra prompt. Cmd := '-NoProfile -ExecutionPolicy Bypass -Command "' - + '$p='''' + LnkPath + ''''; ' + + '$p=''' + LnkPath + '''; ' + '$b=[IO.File]::ReadAllBytes($p); ' + '$b[21]=$b[21] -bor 0x20; ' + '[IO.File]::WriteAllBytes($p,$b)"'; Exec(PowerShellPath, Cmd, '', SW_HIDE, ewWaitUntilTerminated, ResultCode); + if ResultCode <> 0 then + Log('[shopdb] could not mark ' + LnkPath + ' run-as-administrator (exit ' + + IntToStr(ResultCode) + '); the console will self-elevate instead'); end; function RunInstallStages: String; var ResultCode: Integer; PwFile, Args, Common: String; + PwLines: TArrayOfString; begin Result := ''; - Common := '-BundleRoot "' + ExpandConstant('{tmp}\shopdb-bundle') + '"' + - ' -AppRoot "' + ExpandConstant('{app}') + '"' + + Common := '-BundleRoot ' + QuotePathArg(ExpandConstant('{tmp}\shopdb-bundle')) + + ' -AppRoot ' + QuotePathArg(ExpandConstant('{app}')) + ' -SiteHost "' + SitePage.Values[0] + '"' + ' -SitePort ' + SitePage.Values[1] + ' -OnFailure never' + @@ -1110,7 +1210,13 @@ begin if DbCredsPage.Values[1] <> '' then begin PwFile := ExpandConstant('{tmp}\dbpw.txt'); - SaveStringToFile(PwFile, DbCredsPage.Values[1] + #13#10, False); + // UTF-8, no BOM. SaveStringToFile writes an AnsiString, but the installer + // reads this back with -Encoding UTF8 - so any non-ASCII character in the + // password came out mangled and the database rejected a password that was + // typed correctly, reported as "wrong username or password". + SetArrayLength(PwLines, 1); + PwLines[0] := DbCredsPage.Values[1]; + SaveStringsToUTF8FileWithoutBOM(PwFile, PwLines, False); end else PwFile := ''; diff --git a/deploy/windows/installer/shopdb-admin.ps1 b/deploy/windows/installer/shopdb-admin.ps1 index 3f24305..43733b7 100644 --- a/deploy/windows/installer/shopdb-admin.ps1 +++ b/deploy/windows/installer/shopdb-admin.ps1 @@ -314,10 +314,38 @@ function Stop-App { function Restart-App { Head 'Restarting' - # Recycle rather than stop/start: it drains existing requests instead of - # cutting them off, and it is what a config change actually needs. - & $AppCmd recycle apppool /apppool.name:$AppPool 2>&1 | ForEach-Object { Say " $_" } - Start-Sleep -Seconds 2 + # A recycle CANNOT start something that is stopped - it is a no-op on a + # stopped pool. Restarting after a failed upgrade, which is exactly when the + # pool is stopped, therefore did nothing and then reported "did not respond" + # in red, as though the application were broken. Start it first when needed. + Import-Module WebAdministration -ErrorAction SilentlyContinue + $started = $true + try { + if (Test-Path "IIS:\AppPools\$AppPool") { + $started = ((Get-Item "IIS:\AppPools\$AppPool").State -eq 'Started') + } + } catch { } + if (-not $started) { + Say ' the application pool is stopped; starting it' 'Yellow' + Start-WebAppPool -Name $AppPool -ErrorAction SilentlyContinue + Start-Sleep -Seconds 3 + } else { + # Recycle rather than stop/start: it drains existing requests instead of + # cutting them off, and it is what a config change actually needs. + & $AppCmd recycle apppool /apppool.name:$AppPool 2>&1 | ForEach-Object { Say " $_" } + if ($LASTEXITCODE -ne 0) { Say " recycle reported exit $LASTEXITCODE" 'Yellow' } + Start-Sleep -Seconds 2 + } + + # A stopped SITE answers nothing however healthy the pool is. + try { + $siteNow = Get-Website -Name $SiteName -ErrorAction SilentlyContinue + if ($siteNow -and $siteNow.State -ne 'Started') { + Say ' the site is stopped; starting it' 'Yellow' + Start-Website -Name $SiteName -ErrorAction SilentlyContinue + Start-Sleep -Seconds 2 + } + } catch { } try { $r = Invoke-WebRequest -Uri ((Get-Deployment).BaseUrl + '/') -UseBasicParsing -TimeoutSec 30 Say (" back up (HTTP {0})" -f $r.StatusCode) 'Green' diff --git a/deploy/windows/installer/shopdb-install.ps1 b/deploy/windows/installer/shopdb-install.ps1 index 062e447..bf57df2 100644 --- a/deploy/windows/installer/shopdb-install.ps1 +++ b/deploy/windows/installer/shopdb-install.ps1 @@ -301,6 +301,22 @@ function New-Secret { return [Convert]::ToBase64String($b).Replace('+','-').Replace('/','_').TrimEnd('=') } +function Resolve-IisPath { + # IIS stores physicalPath as it was typed, so it can carry %SystemDrive%-style + # variables and a trailing backslash. A raw string compare therefore misses + # paths that ARE ours - on a hand-built deployment that means stage 4 fails to + # recognise its own application, and the uninstaller fails to recognise what + # it may remove. + param([string] $Path) + if (-not $Path) { return '' } + return [System.Environment]::ExpandEnvironmentVariables($Path).TrimEnd('\').ToLowerInvariant() +} + +function Test-SamePath { + param([string] $A, [string] $B) + return (Resolve-IisPath $A) -eq (Resolve-IisPath $B) +} + # Paths derived once. $Py = Join-Path $AppRoot 'venv\Scripts\python.exe' $Flask = Join-Path $AppRoot 'venv\Scripts\flask.exe' @@ -960,7 +976,6 @@ FLUSH PRIVILEGES; $rootPass ) -Encoding UTF8 Write-Log "MySQL root password written to $rootFile (Administrators and SYSTEM only)" 'OK' - Write-Log 'MYSQLROOTFILE:' + $rootFile Write-Log 'stage 0 complete' 'OK' } @@ -1084,16 +1099,42 @@ what is on this server, or restore a backup taken before the upgrade. # Uninstall FIRST, while the code is still importable - the # registry entry outlives the directory otherwise, and the app # then fails to load a plugin it still believes is installed. + $deregistered = $false try { if (Test-Path $Flask) { Push-Location $AppRoot - $env:FLASK_APP = 'shopdb' - & $Flask plugin uninstall $dir.Name 2>&1 | - ForEach-Object { Write-Log " $_" } - Pop-Location + try { + $env:FLASK_APP = 'shopdb' + # --yes is REQUIRED: the command carries a click + # confirmation_option, which aborts with exit 1 + # when there is no console to answer the prompt. + # Without it this deregistration could never once + # have succeeded. + # + # EAP bracket for the reason documented on + # Invoke-Native: '2>&1' under EAP 'Stop' turns any + # stderr line into a terminating error, which the + # catch below then downgraded to a WARN while the + # code was deleted regardless. + $prevEap = $ErrorActionPreference + $ErrorActionPreference = 'Continue' + try { + & $Flask plugin uninstall $dir.Name --yes 2>&1 | + ForEach-Object { Write-Log " $_" } + $deregistered = ($LASTEXITCODE -eq 0) + } finally { $ErrorActionPreference = $prevEap } + } finally { Pop-Location } } } catch { Write-Log " could not deregister $($dir.Name): $($_.Exception.Message)" 'WARN' } - Remove-Item $dir.FullName -Recurse -Force -ErrorAction SilentlyContinue + # Delete the code ONLY once the registry entry is really gone. + # Deleting it anyway left the application loading a plugin + # whose directory no longer existed - the exact failure the + # uninstall-first ordering above exists to avoid. + if ($deregistered) { + Remove-Item $dir.FullName -Recurse -Force -ErrorAction SilentlyContinue + } else { + Write-Log " left $($dir.Name) on disk: deregistration did not succeed" 'WARN' + } } } } @@ -1832,7 +1873,7 @@ has to come from the bundle either way. } else { foreach ($site in (Get-Website)) { foreach ($app in (Get-WebApplication -Site $site.Name -ErrorAction SilentlyContinue)) { - if ($app.PhysicalPath -eq $AppRoot) { + if (Test-SamePath $app.PhysicalPath $AppRoot) { $doomed += @{ Kind = 'app'; Name = $app.Path.Trim('/'); Site = $site.Name } } } @@ -1898,6 +1939,36 @@ If not, re-run matching how the application is published today: pass # METHOD A: its own site on $SitePort. if (Get-Website -Name $SiteName -ErrorAction SilentlyContinue) { Write-Log "site $SiteName already exists" 'OK' + # RECONCILE, do not just accept it. "Already exists" used to take the + # site however it was, so re-running with a different port left the + # binding alone while everything downstream - CORS_ORIGINS, the + # firewall rule, the shortcut, the stage 5 smoke test - used the NEW + # port. A working server was then reported as a failed install. + # + # Refuse rather than silently re-bind: moving where people reach the + # site is not something to do because a field defaulted. + if (-not $WhatIfOnly) { + $existingPorts = @() + $existingPath = '' + try { + $siteNow = Get-Website -Name $SiteName + $existingPath = [string] $siteNow.physicalPath + foreach ($b in $siteNow.bindings.Collection) { + if ($b.protocol -eq 'http' -and ($b.bindingInformation -match '^[^:]*:(\d+):')) { + $existingPorts += [int] $Matches[1] + } + } + } catch { } + if (($existingPorts.Count -gt 0) -and ($existingPorts -notcontains [int] $SitePort)) { + Fail ("site '{0}' is bound to port {1}, but this run was told to use {2}" -f ` + $SiteName, ($existingPorts -join ', '), $SitePort) ` + 'Re-run and enter the port the site already uses, or change the binding in IIS Manager first.' + } + if ($existingPath -and (-not (Test-SamePath $existingPath $AppRoot))) { + Fail ("site '{0}' serves {1}, not {2}" -f $SiteName, $existingPath, $AppRoot) ` + 'Something else owns that site name. Install into the directory it serves, or use a different site name.' + } + } } else { Write-Log "creating site $SiteName on port $SitePort" if (-not $WhatIfOnly) { @@ -2340,11 +2411,19 @@ function Invoke-Uninstall { $removed = $false foreach ($site in (Get-Website -ErrorAction SilentlyContinue)) { $app = Get-WebApplication -Site $site.Name -Name $mount -ErrorAction SilentlyContinue - if ($app) { - Remove-WebApplication -Site $site.Name -Name $mount -ErrorAction SilentlyContinue - Write-Log "removed application /$mount from site '$($site.Name)'" 'OK' - $removed = $true + if (-not $app) { continue } + # OWNERSHIP CHECK. Matching on the alias alone removed ANY application + # on the server that happened to share the name - a second instance, + # or an unrelated /shopdb under a different site - and -OnFailure + # never suppresses the typed confirmation, so it happened unattended. + if (-not (Test-SamePath $app.PhysicalPath $AppRoot)) { + Write-Log ("left /{0} under '{1}' alone: it serves {2}, not {3}" -f ` + $mount, $site.Name, $app.PhysicalPath, $AppRoot) 'WARN' + continue } + Remove-WebApplication -Site $site.Name -Name $mount -ErrorAction SilentlyContinue + Write-Log "removed application /$mount from site '$($site.Name)'" 'OK' + $removed = $true } if (-not $removed) { Write-Log "application /$mount not present" } } @@ -2441,6 +2520,20 @@ catch { if ($doRollback) { Invoke-Rollback } else { Write-Log 'rollback declined; partial install left in place' 'WARN' } } + # Stage 2 stops the pool on an upgrade so files can be replaced. If the run + # then failed, a site that was SERVING before this started is now off, and + # saying only "part-configured" left the operator to discover that from users. + # Not restarted automatically: after a failure in or past stage 3 the deployed + # code and the schema may no longer agree, and starting it would put a broken + # application back in front of people. Say it plainly instead. + $poolStopped = $false + try { $poolStopped = [bool] $script:PoolWasStopped } catch { } + if ($poolStopped) { + Write-Log "the application pool '$AppPool' was stopped for this upgrade and is STILL STOPPED" 'FAIL' + Write-Log ' the site was serving before this run and is now offline' 'FAIL' + Write-Log " once the cause above is fixed, re-run this installer, or start it by hand with:" 'FAIL' + Write-Log " Start-WebAppPool -Name '$AppPool'" 'FAIL' + } Write-Log "log: $script:LogPath" 'FAIL' exit 1 } diff --git a/deploy/windows/installer/shopdb-preflight.ps1 b/deploy/windows/installer/shopdb-preflight.ps1 index a3c6419..75559da 100644 --- a/deploy/windows/installer/shopdb-preflight.ps1 +++ b/deploy/windows/installer/shopdb-preflight.ps1 @@ -398,7 +398,7 @@ Invoke-Check 'MySQL' 'Backup client' { Add-Result 'MySQL' 'Backup client' 'PASS' "mysqldump found ($found)" } else { Add-Result 'MySQL' 'Backup client' 'WARN' 'mysqldump not found on this server' ` - 'Needed for the automatic pre-upgrade backup and for "shopdb-admin.ps1 backup". A first install works without it; upgrades will not be protected. Add mysqlclient\ to the installer bundle, or install the MySQL client on this server.' + 'Needed for the automatic pre-upgrade backup and for "shopdb-admin.ps1 backup". A first install works without it; upgrades will not be protected. It is not on this server yet - the installer stages its own copy, so this normally resolves itself during installation.' } }