fix(scoop-status): sanitize app names and import Get-UserAgent if missing
#6252
Conversation
BinToss
changed the title
Get-UserAgent if missing
|
|
|
Is there a conventional-changelog config I'm unaware of? Updating the changelog with |
lib/manifest.ps1
Outdated
| if (!(Get-Command Get-UserAgent -ErrorAction SilentlyContinue)) { | ||
| Import-Module "$PSScriptRoot/download.ps1" -Function 'Get-UserAgent' | ||
| } |
There was a problem hiding this comment.
Note: this if statement imports Get-UserAgent if a *suspected* race condition causes the function to be called before it's imported by a separate script. I don't know why so many of Scoop's scripts call functions without explicitly importing them beforehand. That's a refactor for another day.
libexec/scoop-status.ps1
Outdated
| [string[]] $appNames = Get-ChildItem $dir -Directory -Name -Exclude 'scoop' | Where-Object Length -GT 0 | ||
| if ($appNames.Length -eq 0) { return } | ||
|
|
||
| foreach ($app in $appNames) { |
There was a problem hiding this comment.
- fix: refactor
Where-Object name -NE 'scoop'toGet-ChildItem'sExcludeparameter. If the input object didn't have aNameparameter, an error would occur. This would happen whenGet-ChildItemreturns a zero-length array such as in the case of an empty or non-existent Global app dir. - fix: add
Where-Object Length -GT 0to exclude zero-length strings from$appNames. - fix: assign app names array to
[string[]] $appNames. Ensures the script halts if any other type is assigned to$appNamesand prevents unexpected behavior caused by a type mismatch. - style: change
ForEach-Objecttoforeach. A stylistic preference for the$appName in $appNamessyntax over$appName = $_.
… strings to `app_status` - Modify `Get-ChildItem ($dir = appsdir $global)` - Add `-Directory` switch; Get only `DirectoryInfo` items. - Add `-Name` switch; Return directories' names. - Exclude items named 'scoop'. - Use Where-Object to filter out zero-length names. - Assign to `[string[]] $appNames` to assure type safety. - Return if no app names found. - Change `ForEach-Object` to `foreach`. These changes improved PowerShell's ability to trace a failed call to Get-UserAgent. It had noted Where-Object or Get-ChildItem calls in scoop-status.ps1 as the cause despite the error being thrown by manifest.ps1's `url_manifest` function. The stack trace should instead be something like... at manifest.ps1:18:40 (function url_manifest) at manifest.ps1:74:24 (function manifest) at core.ps1:557:17 (function app_status) at scoop-status.ps1:65:19
…l, empty, or whitespace
…rethrown without changes Doing so can make debugging difficult.
Only one function is tested and only one of its results is tested.
BinToss
force-pushed
the
fix/scoop-status/Get-UserAgent
branch
from
3ce189a to
6c07782
Compare
|
While refactoring loops, I realized PowerShell can be too forgiving with This mistake was causing |
…c readability `NOT (a OR b OR c OR d)` is slightly easier to understand than `(NOT a) AND (NOT b) AND (NOT c) and (NOT d)`.
…ay is assigned to $appNames
This allows manifest.ps1 to import download.ps1 and call `get_proxy` when core.ps1's `get_config` is not in scope.
…iptblocks where `continue` would normally be used in `foreach` loops
Description
The first commit improves PowerShell's ability to trace a failed call to Get-UserAgent. It had noted Where-Object or Get-ChildItem calls in scoop-status.ps1 as the cause despite the error being thrown by manifest.ps1's
url_manifestfunction.Later commits improve error handling across the call chains and the last commit imports the
Get-UserAgentfunction when it isn't already imported.Motivation and Context
A lack of argument sanitation and an instance of bad error handling caused undefined behavior an error message that didn't make sense. As a result, the root cause was difficult to trace.
Resolves #6251, #6103
How Has This Been Tested?
I iterated changes and ran
scoop statusafter every iteration until the root causes were diagnosed and fixed. Unmodified scripts were not directory tested. As such, they may break if they relied on undefined behavior caused by a lack of argument sanitation.Checklist:
developbranch.