fix(config): Ensure manipulating config with UTF8 encoding (ScoopInstaller#4644)

Author: chawyehsu

CHANGELOG.md

@@ -18,6 +18,7 @@
 
 - **autoupdate:** Allow checksum file that contains whitespaces ([#4619](https://github.com/ScoopInstaller/Scoop/issues/4619))
 - **autoupdate:** Rename $response to $res ([#4706](https://github.com/ScoopInstaller/Scoop/issues/4706))
+- **config:** Ensure manipulating config with UTF8 encoding ([#4644](https://github.com/ScoopInstaller/Scoop/issues/4644))
 - **config:** Allow scoop config use Unicode characters ([#4631](https://github.com/ScoopInstaller/Scoop/issues/4631))
 - **config:** Fix `set_config` bugs ([#3681](https://github.com/ScoopInstaller/Scoop/issues/3681))
 - **current:** Remove 'current' while it's not a junction ([#4687](https://github.com/ScoopInstaller/Scoop/issues/4687))

lib/core.ps1

@@ -41,7 +41,10 @@ function load_cfg($file) {
     }
 
     try {
-        return (Get-Content $file -Raw | ConvertFrom-Json -ErrorAction Stop)
+        # ReadAllLines will detect the encoding of the file automatically
+        # Ref: https://docs.microsoft.com/en-us/dotnet/api/system.io.file.readalllines?view=netframework-4.5
+        $content = [System.IO.File]::ReadAllLines($file)
+        return ($content | ConvertFrom-Json -ErrorAction Stop)
     } catch {
         Write-Host "ERROR loading $file`: $($_.exception.message)"
     }
@@ -80,7 +83,8 @@ function set_config {
         $scoopConfig.PSObject.Properties.Remove($name)
     }
 
-    ConvertTo-Json $scoopConfig | Set-Content -Path $configFile -Encoding Default
+    # Save config with UTF8NoBOM encoding
+    ConvertTo-Json $scoopConfig | Out-UTF8File -FilePath $configFile
     return $scoopConfig
 }
 
@@ -1092,6 +1096,22 @@ function get_magic_bytes_pretty($file, $glue = ' ') {
     return (get_magic_bytes $file | ForEach-Object { $_.ToString('x2') }) -join $glue
 }
 
+function Out-UTF8File {
+    param(
+        [Parameter(Mandatory = $True, Position = 0)]
+        [Alias("Path")]
+        [String] $FilePath,
+        [Parameter(ValueFromPipeline = $True)]
+        [PSObject] $InputObject
+    )
+    process {
+        # Ref: https://stackoverflow.com/questions/5596982
+        # Performance Note: `WriteAllLines` throttles memory usage while
+        # `WriteAllText` needs to keep the complete string in memory.
+        [System.IO.File]::WriteAllLines($FilePath, $InputObject)
+    }
+}
+
 ##################
 # Core Bootstrap #
 ##################

lib/install.ps1

@@ -289,7 +289,9 @@ function dl_with_cache_aria2($app, $version, $manifest, $architecture, $dir, $co
     if (-not($download_finished)) {
         # write aria2 input file
         if ($urlstxt_content -ne '') {
-            Set-Content -Path $urlstxt $urlstxt_content
+            ensure $cachedir | Out-Null
+            # Write aria2 input-file with UTF8NoBOM encoding
+            $urlstxt_content | Out-UTF8File -FilePath $urlstxt
         }
 
         # build aria2 command
@@ -298,6 +300,10 @@ function dl_with_cache_aria2($app, $version, $manifest, $architecture, $dir, $co
         # handle aria2 console output
         Write-Host 'Starting download with aria2 ...'
 
+        # Set console output encoding to UTF8 for non-ASCII characters printing
+        $oriConsoleEncoding = [Console]::OutputEncoding
+        [Console]::OutputEncoding = New-Object System.Text.UTF8Encoding
+
         Invoke-Expression $aria2 | ForEach-Object {
             # Skip blank lines
             if ([String]::IsNullOrWhiteSpace($_)) { return }
@@ -333,6 +339,9 @@ function dl_with_cache_aria2($app, $version, $manifest, $architecture, $dir, $co
             Remove-Item $urlstxt -Force -ErrorAction SilentlyContinue
             Remove-Item "$($data.$url.source).aria2*" -Force -ErrorAction SilentlyContinue
         }
+
+        # Revert console encoding
+        [Console]::OutputEncoding = $oriConsoleEncoding
     }
 
     foreach ($url in $urls) {

test/Scoop-Config.Tests.ps1

@@ -2,80 +2,77 @@
 
 Describe 'config' -Tag 'Scoop' {
     BeforeAll {
-        $json = '{ "one": 1, "two": [ { "a": "a" }, "b", 2 ], "three": { "four": 4 }, "five": true, "six": false, "seven": "\/Date(1529917395805)\/", "eight": "2019-03-18T15:22:09.3930000+00:00" }'
+        $configFile = "$env:TEMP\ScoopTestFixtures\config.json"
+        if (Test-Path $configFile) {
+            Remove-Item -Path $configFile -Force
+        }
+        $unicode = [Regex]::Unescape('\u4f60\u597d\u3053\u3093\u306b\u3061\u306f') # 你好こんにちは
     }
 
-    It 'converts JSON to PSObject' {
-        $obj = ConvertFrom-Json $json
-
-        $obj.one | Should -BeExactly 1
-        $obj.two[0].a | Should -Be 'a'
-        $obj.two[1] | Should -Be 'b'
-        $obj.two[2] | Should -BeExactly 2
-        $obj.three.four | Should -BeExactly 4
-        $obj.five | Should -BeTrue
-        $obj.six | Should -BeFalse
-        $obj.seven | Should -BeOfType [System.DateTime]
-        if ($PSVersionTable.PSVersion.Major -lt 6) {
-            $obj.eight | Should -BeOfType [System.String]
-        } else {
-            $obj.eight | Should -BeOfType [System.DateTime]
-        }
+    BeforeEach {
+        $scoopConfig = $null
     }
 
-    It 'load_config should return PSObject' {
-        Mock Get-Content { $json }
-        Mock Test-Path { $true }
-        (load_cfg 'file') | Should -Not -BeNullOrEmpty
-        (load_cfg 'file') | Should -BeOfType [System.Management.Automation.PSObject]
-        (load_cfg 'file').one | Should -BeExactly 1
+    It 'load_cfg should return null if config file does not exist' {
+        load_cfg $configFile | Should -Be $null
     }
 
-    It 'get_config should return exactly the same values' {
-        $scoopConfig = ConvertFrom-Json $json
-        get_config 'does_not_exist' 'default' | Should -Be 'default'
+    It 'set_config should be able to save typed values correctly' {
+        # number
+        $scoopConfig = set_config 'one' 1
+        $scoopConfig.one | Should -BeExactly 1
 
-        get_config 'one' | Should -BeExactly 1
-        (get_config 'two')[0].a | Should -Be 'a'
-        (get_config 'two')[1] | Should -Be 'b'
-        (get_config 'two')[2] | Should -BeExactly 2
-        (get_config 'three').four | Should -BeExactly 4
-        get_config 'five' | Should -BeTrue
-        get_config 'six' | Should -BeFalse
-        get_config 'seven' | Should -BeOfType [System.DateTime]
-        if ($PSVersionTable.PSVersion.Major -lt 6) {
-            get_config 'eight' | Should -BeOfType [System.String]
-        } else {
-            get_config 'eight' | Should -BeOfType [System.DateTime]
-        }
-    }
+        # boolean
+        $scoopConfig = set_config 'two' $true
+        $scoopConfig.two | Should -BeTrue
+        $scoopConfig = set_config 'three' $false
+        $scoopConfig.three | Should -BeFalse
 
-    It 'set_config should create a new PSObject and ensure existing directory' {
-        $scoopConfig = $null
-        $configFile = "$PSScriptRoot\.scoop"
+        # underline key
+        $scoopConfig = set_config 'under_line' 'four'
+        $scoopConfig.under_line | Should -BeExactly 'four'
 
-        Mock ensure { $PSScriptRoot } -Verifiable -ParameterFilter { $dir -eq (Split-Path -Path $configFile) }
-        Mock Set-Content {} -Verifiable -ParameterFilter { $Path -eq $configFile }
-        Mock ConvertTo-Json { '' } -Verifiable -ParameterFilter { $InputObject -is [System.Management.Automation.PSObject] }
+        # string
+        $scoopConfig = set_config 'five' 'not null'
 
-        set_config 'does_not_exist' 'default'
+        # datetime
+        $scoopConfig = set_config 'time' ([System.DateTime]::Parse('2019-03-18T15:22:09.3930000+00:00', $null, [System.Globalization.DateTimeStyles]::AdjustToUniversal))
+        $scoopConfig.time | Should -BeOfType [System.DateTime]
 
-        Assert-VerifiableMock
+        # non-ASCII
+        $scoopConfig = set_config 'unicode' $unicode
+        $scoopConfig.unicode | Should -Be $unicode
     }
 
-    It "set_config should remove a value if set to `$null" {
-        $scoopConfig = New-Object PSObject
-        $scoopConfig | Add-Member -MemberType NoteProperty -Name 'should_be_removed' -Value 'a_value'
-        $scoopConfig | Add-Member -MemberType NoteProperty -Name 'should_stay' -Value 'another_value'
-        $configFile = "$PSScriptRoot\.scoop"
-
-        Mock Set-Content {} -Verifiable -ParameterFilter { $Path -eq $configFile }
-        Mock ConvertTo-Json { '' } -Verifiable -ParameterFilter { $InputObject -is [System.Management.Automation.PSObject] }
+    It 'load_cfg should return PSObject if config file exist' {
+        $scoopConfig = load_cfg $configFile
+        $scoopConfig | Should -Not -BeNullOrEmpty
+        $scoopConfig | Should -BeOfType [System.Management.Automation.PSObject]
+        $scoopConfig.one | Should -BeExactly 1
+        $scoopConfig.two | Should -BeTrue
+        $scoopConfig.three | Should -BeFalse
+        $scoopConfig.under_line | Should -BeExactly 'four'
+        $scoopConfig.five | Should -Be 'not null'
+        $scoopConfig.time | Should -BeOfType [System.DateTime]
+        $scoopConfig.time | Should -Be ([System.DateTime]::Parse('2019-03-18T15:22:09.3930000+00:00', $null, [System.Globalization.DateTimeStyles]::AdjustToUniversal))
+        $scoopConfig.unicode | Should -Be $unicode
+    }
 
-        $scoopConfig = set_config 'should_be_removed' $null
-        $scoopConfig.should_be_removed | Should -BeNullOrEmpty
-        $scoopConfig.should_stay | Should -Be 'another_value'
+    It 'get_config should return exactly the same values' {
+        $scoopConfig = load_cfg $configFile
+        (get_config 'one') | Should -BeExactly 1
+        (get_config 'two') | Should -BeTrue
+        (get_config 'three') | Should -BeFalse
+        (get_config 'under_line') | Should -BeExactly 'four'
+        (get_config 'five') | Should -Be 'not null'
+        (get_config 'time') | Should -BeOfType [System.DateTime]
+        (get_config 'time') | Should -Be ([System.DateTime]::Parse('2019-03-18T15:22:09.3930000+00:00', $null, [System.Globalization.DateTimeStyles]::AdjustToUniversal))
+        (get_config 'unicode') | Should -Be $unicode
+    }
 
-        Assert-VerifiableMock
+    It 'set_config should remove a value if being set to $null' {
+        $scoopConfig = load_cfg $configFile
+        $scoopConfig = set_config 'five' $null
+        $scoopConfig.five | Should -BeNullOrEmpty
     }
 }