From 845b9498a1e2dd108a5417b0853aa1655bf07398 Mon Sep 17 00:00:00 2001 From: Ilya Date: Mon, 14 Dec 2020 10:29:59 +0500 Subject: [PATCH 1/7] Replace Newtonsoft.Json with System.Text.Json --- .../engine/InitialSessionState.cs | 4 + .../engine/PSConfiguration.cs | 155 ++++++++---------- test/powershell/Host/Startup.Tests.ps1 | 8 +- .../CompatiblePSEditions.Module.Tests.ps1 | 6 +- .../engine/Basic/DefaultCommands.Tests.ps1 | 2 +- .../engine/Basic/PropertyAccessor.Tests.ps1 | 8 +- test/xUnit/csharp/test_PSConfiguration.cs | 8 +- 7 files changed, 85 insertions(+), 106 deletions(-) diff --git a/src/System.Management.Automation/engine/InitialSessionState.cs b/src/System.Management.Automation/engine/InitialSessionState.cs index e75ca018f8e..36d2eb3bac1 100644 --- a/src/System.Management.Automation/engine/InitialSessionState.cs +++ b/src/System.Management.Automation/engine/InitialSessionState.cs @@ -43,6 +43,10 @@ internal static void Init() // * have high disk cost // We shouldn't create too many tasks. + Task.Run(() => + { + var jsonDocument = System.Text.Json.Nodes.JsonObject.Parse("{\"test\": \"test\"}"); + }); #if !UNIX // Amsi initialize can be a little slow Task.Run(() => AmsiUtils.WinScanContent(content: string.Empty, sourceMetadata: string.Empty, warmUp: true)); diff --git a/src/System.Management.Automation/engine/PSConfiguration.cs b/src/System.Management.Automation/engine/PSConfiguration.cs index 16273979a69..179437878b8 100644 --- a/src/System.Management.Automation/engine/PSConfiguration.cs +++ b/src/System.Management.Automation/engine/PSConfiguration.cs @@ -1,16 +1,15 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; using System.Collections.Generic; using System.IO; +#if UNIX using System.Management.Automation.Internal; -using System.Text; +#endif +using System.Text.Json; +using System.Text.Json.Nodes; using System.Threading; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; - namespace System.Management.Automation.Configuration { /// @@ -65,12 +64,10 @@ internal sealed class PowerShellConfig private readonly string perUserConfigFile; private readonly string perUserConfigDirectory; - // Note: JObject and JsonSerializer are thread safe. // Root Json objects corresponding to the configuration file for 'AllUsers' and 'CurrentUser' respectively. // They are used as a cache to avoid hitting the disk for every read operation. - private readonly JObject[] configRoots; - private readonly JObject emptyConfig; - private readonly JsonSerializer serializer; + private readonly JsonObject[] configScopeRoots; + private readonly JsonObject emptyConfig; /// /// Lock used to enable multiple concurrent readers and singular write locks within a single process. @@ -91,9 +88,8 @@ private PowerShellConfig() perUserConfigDirectory = Platform.ConfigDirectory; perUserConfigFile = Path.Combine(perUserConfigDirectory, ConfigFileName); - emptyConfig = new JObject(); - configRoots = new JObject[2]; - serializer = JsonSerializer.Create(new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.None, MaxDepth = 10 }); + emptyConfig = new JsonObject(); + configScopeRoots = new JsonObject[2]; fileLock = new ReaderWriterLockSlim(); } @@ -386,9 +382,9 @@ internal PSKeyword GetLogKeywords() private T ReadValueFromFile(ConfigScope scope, string key, T defaultValue = default) { string fileName = GetConfigFilePath(scope); - JObject configData = configRoots[(int)scope]; + var configScopeData = configScopeRoots[(int)scope]; - if (configData == null) + if (configScopeData is null) { if (File.Exists(fileName)) { @@ -398,9 +394,17 @@ private T ReadValueFromFile(ConfigScope scope, string key, T defaultValue = d fileLock.EnterReadLock(); using var stream = OpenFileStreamWithRetry(fileName, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); - using var jsonReader = new JsonTextReader(new StreamReader(stream)); + using var reader = new StreamReader(stream); + string jsonString = reader.ReadToEnd(); - configData = serializer.Deserialize(jsonReader) ?? emptyConfig; + if (!string.IsNullOrEmpty(jsonString)) + { + configScopeData = JsonNode.Parse(jsonString) as JsonObject; + } + else + { + configScopeData = emptyConfig; + } } catch (Exception exc) { @@ -413,20 +417,33 @@ private T ReadValueFromFile(ConfigScope scope, string key, T defaultValue = d } else { - configData = emptyConfig; + configScopeData = emptyConfig; } - // Set the configuration cache. - JObject originalValue = Interlocked.CompareExchange(ref configRoots[(int)scope], configData, null); + // Set the configuration cache only if another thread has not updated it. + var originalValue = Interlocked.CompareExchange(ref configScopeRoots[(int)scope], configScopeData, null); if (originalValue != null) { - configData = originalValue; + configScopeData = originalValue; } } - if (configData != emptyConfig && configData.TryGetValue(key, StringComparison.OrdinalIgnoreCase, out JToken jToken)) + if (!ReferenceEquals(configScopeData, emptyConfig)) { - return jToken.ToObject(serializer) ?? defaultValue; + var value = configScopeData[key]; + if (value is null) + { + return defaultValue; + } + + try + { + return JsonSerializer.Deserialize(value) ?? defaultValue; + } + catch (Exception exc) + { + throw PSTraceSource.NewInvalidOperationException(exc, PSConfigurationStrings.CanNotConfigurationFile, args: fileName); + } } return defaultValue; @@ -473,83 +490,41 @@ private void UpdateValueInFile(ConfigScope scope, string key, T value, bool a // Since multiple properties can be in a single file, replacement is required instead of overwrite if a file already exists. // Handling the read and write operations within a single FileStream prevents other processes from reading or writing the file while // the update is in progress. It also locks out readers during write operations. + // + using var stream = OpenFileStreamWithRetry(fileName, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); + using var reader = new StreamReader(stream); + string jsonString = reader.ReadToEnd(); - JObject jsonObject = null; - using FileStream fs = OpenFileStreamWithRetry(fileName, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); - - // UTF8, BOM detection, and bufferSize are the same as the basic stream constructor. - // The most important parameter here is the last one, which keeps underlying stream open after StreamReader is disposed - // so that it can be reused for the subsequent write operation. - using (StreamReader streamRdr = new StreamReader(fs, Encoding.UTF8, detectEncodingFromByteOrderMarks: true, bufferSize: 1024, leaveOpen: true)) - using (JsonTextReader jsonReader = new JsonTextReader(streamRdr)) + JsonObject configScopeData = null; + if (!string.IsNullOrEmpty(jsonString)) { - // Safely determines whether there is content to read from the file - bool isReadSuccess = jsonReader.Read(); - if (isReadSuccess) - { - // Read the stream into a root JObject for manipulation - jsonObject = serializer.Deserialize(jsonReader); - JProperty propertyToModify = jsonObject.Property(key); + configScopeData = configScopeData = JsonNode.Parse(jsonString) as JsonObject; + } - if (propertyToModify == null) - { - // The property doesn't exist, so add it - if (addValue) - { - jsonObject.Add(new JProperty(key, value)); - } - // else the property doesn't exist so there is nothing to remove - } - else - { - // The property exists - if (addValue) - { - propertyToModify.Replace(new JProperty(key, value)); - } - else - { - propertyToModify.Remove(); - } - } - } - else - { - // The file doesn't already exist and we want to write to it or it exists with no content. - // A new file will be created that contains only this value. - // If the file doesn't exist and a we don't want to write to it, no action is needed. - if (addValue) - { - jsonObject = new JObject(new JProperty(key, value)); - } - else - { - return; - } - } + if (configScopeData is null) + { + configScopeData = new JsonObject(); } + configScopeData[key] = JsonValue.Create(value); + + var options = new JsonSerializerOptions + { + WriteIndented = true + }; + + var jsonOutput = configScopeData.ToJsonString(options); + // Reset the stream position to the beginning so that the // changes to the file can be written to disk - fs.Seek(0, SeekOrigin.Begin); + stream.SetLength(0); // Update the file with new content - using (StreamWriter streamWriter = new StreamWriter(fs)) - using (JsonTextWriter jsonWriter = new JsonTextWriter(streamWriter)) - { - // The entire document exists within the root JObject. - // I just need to write that object to produce the document. - jsonObject.WriteTo(jsonWriter); - - // This trims the file if the file shrank. If the file grew, - // it is a no-op. The purpose is to trim extraneous characters - // from the file stream when the resultant JObject is smaller - // than the input JObject. - fs.SetLength(fs.Position); - } + using var streamWriter = new StreamWriter(stream); + streamWriter.Write(jsonOutput); // Refresh the configuration cache. - Interlocked.Exchange(ref configRoots[(int)scope], jsonObject); + Interlocked.Exchange(ref configScopeRoots[(int)scope], configScopeData); } finally { @@ -591,7 +566,7 @@ private void RemoveValueFromFile(ConfigScope scope, string key) } } - #region GroupPolicy Configs +#region GroupPolicy Configs /// /// The GroupPolicy related settings used in PowerShell are as follows in Registry: @@ -728,5 +703,5 @@ internal sealed class ProtectedEventLogging : PolicyBase public string[] EncryptionCertificate { get; set; } } - #endregion +#endregion } diff --git a/test/powershell/Host/Startup.Tests.ps1 b/test/powershell/Host/Startup.Tests.ps1 index ddaa2736a5c..728e406c923 100644 --- a/test/powershell/Host/Startup.Tests.ps1 +++ b/test/powershell/Host/Startup.Tests.ps1 @@ -10,7 +10,6 @@ Describe "Validate start of console host" -Tag CI { 'Microsoft.Win32.Primitives.dll' 'Microsoft.Win32.Registry.dll' 'netstandard.dll' - 'Newtonsoft.Json.dll' 'pwsh.dll' 'System.Collections.Concurrent.dll' 'System.Collections.dll' @@ -21,7 +20,6 @@ Describe "Validate start of console host" -Tag CI { 'System.Console.dll' 'System.Data.Common.dll' 'System.Diagnostics.Process.dll' - 'System.Diagnostics.TraceSource.dll' 'System.Diagnostics.Tracing.dll' 'System.IO.FileSystem.AccessControl.dll' 'System.IO.FileSystem.DriveInfo.dll' @@ -33,6 +31,7 @@ Describe "Validate start of console host" -Tag CI { 'System.Net.Mail.dll' 'System.Net.NetworkInformation.dll' 'System.Net.Primitives.dll' + 'System.Numerics.Vectors.dll' 'System.ObjectModel.dll' 'System.Private.CoreLib.dll' 'System.Private.Uri.dll' @@ -41,15 +40,16 @@ Describe "Validate start of console host" -Tag CI { 'System.Reflection.Emit.Lightweight.dll' 'System.Reflection.Primitives.dll' 'System.Runtime.dll' + 'System.Runtime.CompilerServices.Unsafe.dll' 'System.Runtime.InteropServices.dll' 'System.Runtime.Loader.dll' 'System.Runtime.Numerics.dll' - 'System.Runtime.Serialization.Formatters.dll' - 'System.Runtime.Serialization.Primitives.dll' 'System.Security.AccessControl.dll' 'System.Security.Cryptography.dll' 'System.Security.Principal.Windows.dll' 'System.Text.Encoding.Extensions.dll' + 'System.Text.Encodings.Web.dll' + 'System.Text.Json.dll' 'System.Text.RegularExpressions.dll' 'System.Threading.dll' 'System.Threading.Tasks.Parallel.dll' diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index 61f9f8e0f9f..ccfdfeb8f2e 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -508,7 +508,7 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "Feature" { It "Fails to import incompatible module if implicit WinCompat is disabled in config" { $LogPath = Join-Path $TestDrive (New-Guid).ToString() $ConfigPath = Join-Path $TestDrive 'powershell.config.json' - '{"DisableImplicitWinCompat" : "True"}' | Out-File -Force $ConfigPath + '{"DisableImplicitWinCompat" : true}' | Out-File -Force $ConfigPath & $pwsh -NoProfile -NonInteractive -settingsFile $ConfigPath -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName2" *> $LogPath $LogPath | Should -FileContentMatch 'cannot be loaded implicitly using the Windows Compatibility' } @@ -516,7 +516,7 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "Feature" { It "Fails to auto-import incompatible module during CommandDiscovery\ModuleAutoload if implicit WinCompat is Disabled in config" { $LogPath = Join-Path $TestDrive (New-Guid).ToString() $ConfigPath = Join-Path $TestDrive 'powershell.config.json' - '{"DisableImplicitWinCompat" : "True","Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned"}' | Out-File -Force $ConfigPath + '{"DisableImplicitWinCompat" : true,"Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned"}' | Out-File -Force $ConfigPath & $pwsh -NoProfile -NonInteractive -settingsFile $ConfigPath -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`'); Test-$ModuleName2" *> $LogPath $LogPath | Should -FileContentMatch 'not recognized as a name of a cmdlet' } @@ -524,7 +524,7 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "Feature" { It "Successfully auto-imports incompatible module during CommandDiscovery\ModuleAutoload if implicit WinCompat is Enabled in config" { $LogPath = Join-Path $TestDrive (New-Guid).ToString() $ConfigPath = Join-Path $TestDrive 'powershell.config.json' - '{"DisableImplicitWinCompat" : "False","Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned"}' | Out-File -Force $ConfigPath + '{"DisableImplicitWinCompat" : false,"Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned"}' | Out-File -Force $ConfigPath & $pwsh -NoProfile -NonInteractive -settingsFile $ConfigPath -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`'); Test-$ModuleName2" *> $LogPath $LogPath | Should -FileContentMatch 'True' } diff --git a/test/powershell/engine/Basic/DefaultCommands.Tests.ps1 b/test/powershell/engine/Basic/DefaultCommands.Tests.ps1 index bdcd5af0000..b551fe6e1a4 100644 --- a/test/powershell/engine/Basic/DefaultCommands.Tests.ps1 +++ b/test/powershell/engine/Basic/DefaultCommands.Tests.ps1 @@ -548,7 +548,7 @@ Describe "Verify approved aliases list" -Tags "CI" { # On Preview releases, Experimental Features may add new cmdlets/aliases, so we get cmdlets/aliases with features disabled if ($isPreview) { $emptyConfigPath = Join-Path -Path $TestDrive -ChildPath "test.config.json" - Set-Content -Path $emptyConfigPath -Value "" -Force -ErrorAction Stop + New-Item -Path $emptyConfigPath -ItemType file -Force -ErrorAction Stop | Out-Null $currentAliasList = & "$PSHOME/pwsh" -NoProfile -OutputFormat XML -SettingsFile $emptyConfigPath -Command $getAliases -args ($moduleList | ConvertTo-Json) $currentCmdletList = & "$PSHOME/pwsh" -NoProfile -OutputFormat XML -SettingsFile $emptyConfigPath -Command $getCommands -args ($moduleList | ConvertTo-Json) } diff --git a/test/powershell/engine/Basic/PropertyAccessor.Tests.ps1 b/test/powershell/engine/Basic/PropertyAccessor.Tests.ps1 index ac0c445a8a3..100181da8e4 100644 --- a/test/powershell/engine/Basic/PropertyAccessor.Tests.ps1 +++ b/test/powershell/engine/Basic/PropertyAccessor.Tests.ps1 @@ -42,7 +42,7 @@ Describe "User-Specific powershell.config.json Modifications" -Tags "CI" { BeforeEach { if ($IsNotSkipped) { - Set-Content -Path $userPropertiesFile -Value '{"Microsoft.PowerShell:ExecutionPolicy":"RemoteSigned"}' + Set-Content -Path $userPropertiesFile -Value '{ "Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned"}' } } @@ -83,15 +83,15 @@ Describe "User-Specific powershell.config.json Modifications" -Tags "CI" { } It "Verify Writes Update Properties" { - Get-Content -Path $userPropertiesFile | Should -Be '{"Microsoft.PowerShell:ExecutionPolicy":"RemoteSigned"}' + Get-Content -Path $userPropertiesFile | Should -Be '{ "Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned"}' Set-ExecutionPolicy -Scope CurrentUser -ExecutionPolicy Bypass - Get-Content -Path $userPropertiesFile | Should -Be '{"Microsoft.PowerShell:ExecutionPolicy":"Bypass"}' + (Get-Content -Path $userPropertiesFile -Raw) -replace "`r`n?|`n","" | Should -Be '{ "Microsoft.PowerShell:ExecutionPolicy": "Bypass"}' } It "Verify Writes Create the File if Not Present" { Remove-Item $userPropertiesFile -Force Test-Path $userPropertiesFile | Should -BeFalse Set-ExecutionPolicy -Scope CurrentUser -ExecutionPolicy Bypass - Get-Content -Path $userPropertiesFile | Should -Be '{"Microsoft.PowerShell:ExecutionPolicy":"Bypass"}' + (Get-Content -Path $userPropertiesFile -Raw) -replace "`r`n?|`n","" | Should -Be '{ "Microsoft.PowerShell:ExecutionPolicy": "Bypass"}' } } diff --git a/test/xUnit/csharp/test_PSConfiguration.cs b/test/xUnit/csharp/test_PSConfiguration.cs index 54e69da68f0..9a7fa52a8d3 100644 --- a/test/xUnit/csharp/test_PSConfiguration.cs +++ b/test/xUnit/csharp/test_PSConfiguration.cs @@ -7,9 +7,9 @@ using System.Management.Automation.Configuration; using System.Management.Automation.Internal; using System.Reflection; +using System.Text.Json.Nodes; using System.Threading; using Newtonsoft.Json; -using Newtonsoft.Json.Linq; using PSTests.Internal; using Xunit; @@ -99,7 +99,7 @@ public void Dispose() { Dispose(true); GC.SuppressFinalize(this); - } + } protected virtual void Dispose(bool disposing) { @@ -390,8 +390,8 @@ private static void CreateBrokenConfigFile(string fileName) internal void ForceReadingFromFile() { // Reset the cached roots. - FieldInfo roots = typeof(PowerShellConfig).GetField("configRoots", BindingFlags.NonPublic | BindingFlags.Instance); - JObject[] value = (JObject[])roots.GetValue(PowerShellConfig.Instance); + FieldInfo roots = typeof(PowerShellConfig).GetField("_configScopeRoots", BindingFlags.NonPublic | BindingFlags.Instance); + var value = (JsonObject[])roots.GetValue(PowerShellConfig.Instance); value[0] = null; value[1] = null; } From 2bab94428cabb6a19b2e78edffc86d462d1f3ca6 Mon Sep 17 00:00:00 2001 From: Ilya Date: Wed, 20 Oct 2021 22:47:20 +0500 Subject: [PATCH 2/7] nullable enable --- .../host/msh/ConsoleHost.cs | 2 +- .../engine/Modules/ModuleCmdletBase.cs | 2 +- .../engine/PSConfiguration.cs | 88 ++++++++++--------- 3 files changed, 49 insertions(+), 43 deletions(-) diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index 0d647a2ded1..0d1c3e413cf 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs @@ -310,7 +310,7 @@ internal static void ParseCommandLine(string[] args) } #endif - if (s_cpp.SettingsFile is not null) + if (!string.IsNullOrEmpty(s_cpp.SettingsFile)) { PowerShellConfig.Instance.SetSystemConfigFilePath(s_cpp.SettingsFile); } diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index dc5f500f59b..dd6d4a92a02 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -4532,7 +4532,7 @@ internal static ModuleLoggingGroupPolicyStatus GetModuleLoggingInformation(out I else if (moduleLogging.EnableModuleLogging == true) { status = ModuleLoggingGroupPolicyStatus.Enabled; - moduleNames = moduleLogging.ModuleNames; + moduleNames = moduleLogging.ModuleNames ?? Array.Empty(); } } diff --git a/src/System.Management.Automation/engine/PSConfiguration.cs b/src/System.Management.Automation/engine/PSConfiguration.cs index 179437878b8..bf4177d48ba 100644 --- a/src/System.Management.Automation/engine/PSConfiguration.cs +++ b/src/System.Management.Automation/engine/PSConfiguration.cs @@ -1,7 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +#nullable enable + using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.IO; #if UNIX using System.Management.Automation.Internal; @@ -66,8 +69,8 @@ internal sealed class PowerShellConfig // Root Json objects corresponding to the configuration file for 'AllUsers' and 'CurrentUser' respectively. // They are used as a cache to avoid hitting the disk for every read operation. - private readonly JsonObject[] configScopeRoots; - private readonly JsonObject emptyConfig; + private readonly JsonObject[] _configScopeRoots; + private readonly JsonObject _emptyConfig; /// /// Lock used to enable multiple concurrent readers and singular write locks within a single process. @@ -88,8 +91,8 @@ private PowerShellConfig() perUserConfigDirectory = Platform.ConfigDirectory; perUserConfigFile = Path.Combine(perUserConfigDirectory, ConfigFileName); - emptyConfig = new JsonObject(); - configScopeRoots = new JsonObject[2]; + _emptyConfig = new JsonObject(); + _configScopeRoots = new JsonObject[2]; fileLock = new ReaderWriterLockSlim(); } @@ -109,14 +112,15 @@ private string GetConfigFilePath(ConfigScope scope) /// internal void SetSystemConfigFilePath(string value) { - if (!string.IsNullOrEmpty(value) && !File.Exists(value)) + FileInfo info = new FileInfo(value); + + if (!info.Exists) { throw new FileNotFoundException(value); } - FileInfo info = new FileInfo(value); systemWideConfigFile = info.FullName; - systemWideConfigDirectory = info.Directory.FullName; + systemWideConfigDirectory = info.Directory!.FullName; } /// @@ -126,9 +130,9 @@ internal void SetSystemConfigFilePath(string value) /// /// Whether this is a system-wide or per-user setting. /// Value if found, null otherwise. The behavior matches ModuleIntrinsics.GetExpandedEnvironmentVariable(). - internal string GetModulePath(ConfigScope scope) + internal string? GetModulePath(ConfigScope scope) { - string modulePath = ReadValueFromFile(scope, Constants.PSModulePathEnvVar); + string? modulePath = ReadValueFromFile(scope, Constants.PSModulePathEnvVar); if (!string.IsNullOrEmpty(modulePath)) { modulePath = Environment.ExpandEnvironmentVariables(modulePath); @@ -151,10 +155,10 @@ internal string GetModulePath(ConfigScope scope) /// Whether this is a system-wide or per-user setting. /// The shell associated with this policy. Typically, it is "Microsoft.PowerShell". /// The execution policy if found. Null otherwise. - internal string GetExecutionPolicy(ConfigScope scope, string shellId) + internal string? GetExecutionPolicy(ConfigScope scope, string shellId) { string key = GetExecutionPolicySettingKey(shellId); - string execPolicy = ReadValueFromFile(scope, key); + string? execPolicy = ReadValueFromFile(scope, key); return string.IsNullOrEmpty(execPolicy) ? null : execPolicy; } @@ -222,13 +226,13 @@ internal bool IsImplicitWinCompatEnabled() return !settingValue; } - internal string[] GetWindowsPowerShellCompatibilityModuleDenyList() + internal string[]? GetWindowsPowerShellCompatibilityModuleDenyList() { return ReadValueFromFile(ConfigScope.CurrentUser, WindowsPowerShellCompatibilityModuleDenyListKey) ?? ReadValueFromFile(ConfigScope.AllUsers, WindowsPowerShellCompatibilityModuleDenyListKey); } - internal string[] GetWindowsPowerShellCompatibilityNoClobberModuleList() + internal string[]? GetWindowsPowerShellCompatibilityNoClobberModuleList() { return ReadValueFromFile(ConfigScope.CurrentUser, WindowsPowerShellCompatibilityNoClobberModuleListKey) ?? ReadValueFromFile(ConfigScope.AllUsers, WindowsPowerShellCompatibilityNoClobberModuleListKey); @@ -237,7 +241,7 @@ internal string[] GetWindowsPowerShellCompatibilityNoClobberModuleList() /// /// Corresponding settings of the original Group Policies. /// - internal PowerShellPolicies GetPowerShellPolicies(ConfigScope scope) + internal PowerShellPolicies? GetPowerShellPolicies(ConfigScope scope) { return ReadValueFromFile(scope, nameof(PowerShellPolicies)); } @@ -251,7 +255,7 @@ internal PowerShellPolicies GetPowerShellPolicies(ConfigScope scope) /// internal string GetSysLogIdentity() { - string identity = ReadValueFromFile(ConfigScope.AllUsers, "LogIdentity"); + string? identity = ReadValueFromFile(ConfigScope.AllUsers, "LogIdentity"); if (string.IsNullOrEmpty(identity) || identity.Equals(LogDefaultValue, StringComparison.OrdinalIgnoreCase)) @@ -270,7 +274,7 @@ internal string GetSysLogIdentity() /// internal PSLevel GetLogLevel() { - string levelName = ReadValueFromFile(ConfigScope.AllUsers, "LogLevel"); + string? levelName = ReadValueFromFile(ConfigScope.AllUsers, "LogLevel"); PSLevel level; if (string.IsNullOrEmpty(levelName) || @@ -301,7 +305,7 @@ internal PSLevel GetLogLevel() /// internal PSChannel GetLogChannels() { - string values = ReadValueFromFile(ConfigScope.AllUsers, "LogChannels"); + string? values = ReadValueFromFile(ConfigScope.AllUsers, "LogChannels"); PSChannel result = 0; if (!string.IsNullOrEmpty(values)) @@ -340,7 +344,7 @@ internal PSChannel GetLogChannels() /// internal PSKeyword GetLogKeywords() { - string values = ReadValueFromFile(ConfigScope.AllUsers, "LogKeywords"); + string? values = ReadValueFromFile(ConfigScope.AllUsers, "LogKeywords"); PSKeyword result = 0; if (!string.IsNullOrEmpty(values)) @@ -379,10 +383,11 @@ internal PSKeyword GetLogKeywords() /// The ConfigScope of the configuration file to update. /// The string key of the value. /// The default value to return if the key is not present. - private T ReadValueFromFile(ConfigScope scope, string key, T defaultValue = default) + [return: NotNullIfNotNull("defaultValue")] + private T? ReadValueFromFile(ConfigScope scope, string key, T? defaultValue = default) { string fileName = GetConfigFilePath(scope); - var configScopeData = configScopeRoots[(int)scope]; + var configScopeData = _configScopeRoots[(int)scope]; if (configScopeData is null) { @@ -401,9 +406,10 @@ private T ReadValueFromFile(ConfigScope scope, string key, T defaultValue = d { configScopeData = JsonNode.Parse(jsonString) as JsonObject; } - else + + if (configScopeData is null) { - configScopeData = emptyConfig; + configScopeData = _emptyConfig; } } catch (Exception exc) @@ -417,18 +423,18 @@ private T ReadValueFromFile(ConfigScope scope, string key, T defaultValue = d } else { - configScopeData = emptyConfig; + configScopeData = _emptyConfig; } // Set the configuration cache only if another thread has not updated it. - var originalValue = Interlocked.CompareExchange(ref configScopeRoots[(int)scope], configScopeData, null); + var originalValue = Interlocked.CompareExchange(ref _configScopeRoots[(int)scope], configScopeData, null); if (originalValue != null) { configScopeData = originalValue; } } - if (!ReferenceEquals(configScopeData, emptyConfig)) + if (!ReferenceEquals(configScopeData, _emptyConfig)) { var value = configScopeData[key]; if (value is null) @@ -480,7 +486,7 @@ private static FileStream OpenFileStreamWithRetry(string fullPath, FileMode mode /// The string key of the value. /// The value to set. /// Whether the key-value pair should be added to or removed from the file. - private void UpdateValueInFile(ConfigScope scope, string key, T value, bool addValue) + private void UpdateValueInFile(ConfigScope scope, string key, T? value, bool addValue) { try { @@ -495,7 +501,7 @@ private void UpdateValueInFile(ConfigScope scope, string key, T value, bool a using var reader = new StreamReader(stream); string jsonString = reader.ReadToEnd(); - JsonObject configScopeData = null; + JsonObject? configScopeData = null; if (!string.IsNullOrEmpty(jsonString)) { configScopeData = configScopeData = JsonNode.Parse(jsonString) as JsonObject; @@ -524,7 +530,7 @@ private void UpdateValueInFile(ConfigScope scope, string key, T value, bool a streamWriter.Write(jsonOutput); // Refresh the configuration cache. - Interlocked.Exchange(ref configScopeRoots[(int)scope], configScopeData); + Interlocked.Exchange(ref _configScopeRoots[(int)scope], configScopeData); } finally { @@ -614,19 +620,19 @@ private void RemoveValueFromFile(ConfigScope scope, string key) /// internal sealed class PowerShellPolicies { - public ScriptExecution ScriptExecution { get; set; } + public ScriptExecution? ScriptExecution { get; set; } - public ScriptBlockLogging ScriptBlockLogging { get; set; } + public ScriptBlockLogging? ScriptBlockLogging { get; set; } - public ModuleLogging ModuleLogging { get; set; } + public ModuleLogging? ModuleLogging { get; set; } - public ProtectedEventLogging ProtectedEventLogging { get; set; } + public ProtectedEventLogging? ProtectedEventLogging { get; set; } - public Transcription Transcription { get; set; } + public Transcription? Transcription { get; set; } - public UpdatableHelp UpdatableHelp { get; set; } + public UpdatableHelp? UpdatableHelp { get; set; } - public ConsoleSessionConfiguration ConsoleSessionConfiguration { get; set; } + public ConsoleSessionConfiguration? ConsoleSessionConfiguration { get; set; } } internal abstract class PolicyBase { } @@ -636,7 +642,7 @@ internal abstract class PolicyBase { } /// internal sealed class ScriptExecution : PolicyBase { - public string ExecutionPolicy { get; set; } + public string? ExecutionPolicy { get; set; } public bool? EnableScripts { get; set; } } @@ -658,7 +664,7 @@ internal sealed class ModuleLogging : PolicyBase { public bool? EnableModuleLogging { get; set; } - public string[] ModuleNames { get; set; } + public string[]? ModuleNames { get; set; } } /// @@ -670,7 +676,7 @@ internal sealed class Transcription : PolicyBase public bool? EnableInvocationHeader { get; set; } - public string OutputDirectory { get; set; } + public string? OutputDirectory { get; set; } } /// @@ -680,7 +686,7 @@ internal sealed class UpdatableHelp : PolicyBase { public bool? EnableUpdateHelpDefaultSourcePath { get; set; } - public string DefaultSourcePath { get; set; } + public string? DefaultSourcePath { get; set; } } /// @@ -690,7 +696,7 @@ internal sealed class ConsoleSessionConfiguration : PolicyBase { public bool? EnableConsoleSessionConfiguration { get; set; } - public string ConsoleSessionConfigurationName { get; set; } + public string? ConsoleSessionConfigurationName { get; set; } } /// @@ -700,7 +706,7 @@ internal sealed class ProtectedEventLogging : PolicyBase { public bool? EnableProtectedEventLogging { get; set; } - public string[] EncryptionCertificate { get; set; } + public string[]? EncryptionCertificate { get; set; } } #endregion From de53003611a34276fbe2c376ce797d6c757ba42a Mon Sep 17 00:00:00 2001 From: Ilya Date: Tue, 2 Aug 2022 10:36:38 +0500 Subject: [PATCH 3/7] Use compound assignment --- .../engine/PSConfiguration.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/System.Management.Automation/engine/PSConfiguration.cs b/src/System.Management.Automation/engine/PSConfiguration.cs index bf4177d48ba..d23ac6e4da8 100644 --- a/src/System.Management.Automation/engine/PSConfiguration.cs +++ b/src/System.Management.Automation/engine/PSConfiguration.cs @@ -407,10 +407,7 @@ internal PSKeyword GetLogKeywords() configScopeData = JsonNode.Parse(jsonString) as JsonObject; } - if (configScopeData is null) - { - configScopeData = _emptyConfig; - } + configScopeData ??= _emptyConfig; } catch (Exception exc) { @@ -507,10 +504,7 @@ private void UpdateValueInFile(ConfigScope scope, string key, T? value, bool configScopeData = configScopeData = JsonNode.Parse(jsonString) as JsonObject; } - if (configScopeData is null) - { - configScopeData = new JsonObject(); - } + configScopeData ??= new JsonObject(); configScopeData[key] = JsonValue.Create(value); From 8e102291fe492e0a1bfdf125ddde7d0438cd5166 Mon Sep 17 00:00:00 2001 From: Ilya Date: Tue, 2 Aug 2022 10:55:07 +0500 Subject: [PATCH 4/7] Fix typo --- src/System.Management.Automation/engine/InitialSessionState.cs | 2 +- src/System.Management.Automation/engine/PSConfiguration.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/InitialSessionState.cs b/src/System.Management.Automation/engine/InitialSessionState.cs index 36d2eb3bac1..6eab539f7d1 100644 --- a/src/System.Management.Automation/engine/InitialSessionState.cs +++ b/src/System.Management.Automation/engine/InitialSessionState.cs @@ -45,7 +45,7 @@ internal static void Init() // We shouldn't create too many tasks. Task.Run(() => { - var jsonDocument = System.Text.Json.Nodes.JsonObject.Parse("{\"test\": \"test\"}"); + var jsonDocument = System.Text.Json.Nodes.JsonObject.Parse("1"); }); #if !UNIX // Amsi initialize can be a little slow diff --git a/src/System.Management.Automation/engine/PSConfiguration.cs b/src/System.Management.Automation/engine/PSConfiguration.cs index d23ac6e4da8..53afd0b3e00 100644 --- a/src/System.Management.Automation/engine/PSConfiguration.cs +++ b/src/System.Management.Automation/engine/PSConfiguration.cs @@ -501,7 +501,7 @@ private void UpdateValueInFile(ConfigScope scope, string key, T? value, bool JsonObject? configScopeData = null; if (!string.IsNullOrEmpty(jsonString)) { - configScopeData = configScopeData = JsonNode.Parse(jsonString) as JsonObject; + configScopeData = JsonNode.Parse(jsonString) as JsonObject; } configScopeData ??= new JsonObject(); From fc597880979909b8ceb09e50198b7be8a3f026a6 Mon Sep 17 00:00:00 2001 From: Ilya Date: Tue, 2 Aug 2022 11:12:51 +0500 Subject: [PATCH 5/7] Fix startup test --- test/powershell/Host/Startup.Tests.ps1 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/powershell/Host/Startup.Tests.ps1 b/test/powershell/Host/Startup.Tests.ps1 index 728e406c923..2a32090cc01 100644 --- a/test/powershell/Host/Startup.Tests.ps1 +++ b/test/powershell/Host/Startup.Tests.ps1 @@ -20,6 +20,7 @@ Describe "Validate start of console host" -Tag CI { 'System.Console.dll' 'System.Data.Common.dll' 'System.Diagnostics.Process.dll' + 'System.Diagnostics.TraceSource.dll' 'System.Diagnostics.Tracing.dll' 'System.IO.FileSystem.AccessControl.dll' 'System.IO.FileSystem.DriveInfo.dll' @@ -40,8 +41,8 @@ Describe "Validate start of console host" -Tag CI { 'System.Reflection.Emit.Lightweight.dll' 'System.Reflection.Primitives.dll' 'System.Runtime.dll' - 'System.Runtime.CompilerServices.Unsafe.dll' 'System.Runtime.InteropServices.dll' + 'System.Runtime.Intrinsics.dll' 'System.Runtime.Loader.dll' 'System.Runtime.Numerics.dll' 'System.Security.AccessControl.dll' From 7bb07dad31994066296c0bdb02fb0b8867993f8f Mon Sep 17 00:00:00 2001 From: Ilya Date: Mon, 26 Sep 2022 22:54:54 +0500 Subject: [PATCH 6/7] Remove using --- src/System.Management.Automation/engine/PSConfiguration.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/System.Management.Automation/engine/PSConfiguration.cs b/src/System.Management.Automation/engine/PSConfiguration.cs index 53afd0b3e00..7c031a3518d 100644 --- a/src/System.Management.Automation/engine/PSConfiguration.cs +++ b/src/System.Management.Automation/engine/PSConfiguration.cs @@ -6,9 +6,6 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.IO; -#if UNIX -using System.Management.Automation.Internal; -#endif using System.Text.Json; using System.Text.Json.Nodes; using System.Threading; From e5424ac2ff053e8cb7ae2857b8cda14fdd57305b Mon Sep 17 00:00:00 2001 From: Ilya Date: Mon, 26 Sep 2022 23:14:30 +0500 Subject: [PATCH 7/7] Revert "Remove using" This reverts commit 7bb07dad31994066296c0bdb02fb0b8867993f8f. --- src/System.Management.Automation/engine/PSConfiguration.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/System.Management.Automation/engine/PSConfiguration.cs b/src/System.Management.Automation/engine/PSConfiguration.cs index 7c031a3518d..53afd0b3e00 100644 --- a/src/System.Management.Automation/engine/PSConfiguration.cs +++ b/src/System.Management.Automation/engine/PSConfiguration.cs @@ -6,6 +6,9 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.IO; +#if UNIX +using System.Management.Automation.Internal; +#endif using System.Text.Json; using System.Text.Json.Nodes; using System.Threading;