From 55185413b1cad5a94ad2b4e7af9f4969a3666fd7 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Fri, 23 Jul 2021 15:32:00 -0700 Subject: [PATCH 01/11] Add validation to `$PSStyle` to reject printable text as ANSI escape sequence --- .../PowerShellCore_format_ps1xml.cs | 2 +- .../FormatAndOutput/common/PSStyle.cs | 222 ++++++++++++++++-- .../resources/PSStyleStrings.resx | 64 +++++ .../engine/Formatting/PSStyle.Tests.ps1 | 41 ++++ 4 files changed, 314 insertions(+), 15 deletions(-) create mode 100644 src/System.Management.Automation/resources/PSStyleStrings.resx diff --git a/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs b/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs index ddad10cfc5e..2d9cab7b0c2 100644 --- a/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs +++ b/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs @@ -2127,7 +2127,7 @@ private static IEnumerable ViewsOf_System_Management_Autom .AddItemScriptBlock(@"""$($_.ErrorAccent)$($_.ErrorAccent.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "ErrorAccent") .AddItemScriptBlock(@"""$($_.Error)$($_.Error.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Error") .AddItemScriptBlock(@"""$($_.Warning)$($_.Warning.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Warning") - .AddItemScriptBlock(@"""$($_.Verbose)$($_.Verbose.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.Verbose") + .AddItemScriptBlock(@"""$($_.Verbose)$($_.Verbose.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Verbose") .AddItemScriptBlock(@"""$($_.Debug)$($_.Debug.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Debug") .EndEntry() .EndList()); diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index be6eaef0e09..f646e206eb5 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System.Collections.Generic; +using System.Management.Automation.Internal; namespace System.Management.Automation { @@ -281,12 +282,43 @@ public sealed class ProgressConfiguration /// /// Gets or sets the style for progress bar. /// - public string Style { get; set; } = "\x1b[33;1m"; + public string Style + { + get + { + return _style; + } + + set + { + _style = ValidateNoContent(value); + } + } + + private string _style = "\x1b[33;1m"; /// /// Gets or sets the max width of the progress bar. /// - public int MaxWidth { get; set; } = 120; + public int MaxWidth + { + get + { + return _maxWidth; + } + + set + { + if (value < 0) + { + value = 0; + } + + _maxWidth = value; + } + } + + private int _maxWidth = 120; /// /// Gets or sets the view for progress bar. @@ -307,37 +339,128 @@ public sealed class FormattingData /// /// Gets or sets the accent style for formatting. /// - public string FormatAccent { get; set; } = "\x1b[32;1m"; + public string FormatAccent + { + get + { + return _formatAccent; + } + + set + { + _formatAccent = ValidateNoContent(value); + } + } + + private string _formatAccent = "\x1b[32;1m"; /// /// Gets or sets the style for table headers. /// - public string TableHeader { get; set; } = "\x1b[32;1m"; + public string TableHeader + { + get + { + return _tableHeader; + } + + set + { + _tableHeader = ValidateNoContent(value); + } + } + + private string _tableHeader = "\x1b[32;1m"; /// /// Gets or sets the accent style for errors. /// - public string ErrorAccent { get; set; } = "\x1b[36;1m"; + public string ErrorAccent + { + get + { + return _errorAccent; + } + + set + { + _errorAccent = ValidateNoContent(value); + } + } + + private string _errorAccent = "\x1b[36;1m"; /// /// Gets or sets the style for error messages. /// - public string Error { get; set; } = "\x1b[31;1m"; + public string Error + { + get + { + return _error; + } + + set + { + _error = ValidateNoContent(value); + } + } + + private string _error = "\x1b[31;1m"; /// /// Gets or sets the style for warning messages. /// - public string Warning { get; set; } = "\x1b[33;1m"; + public string Warning + { + get + { + return _warning; + } + + set + { + _warning = ValidateNoContent(value); + } + } + + private string _warning = "\x1b[33;1m"; /// /// Gets or sets the style for verbose messages. /// - public string Verbose { get; set; } = "\x1b[33;1m"; + public string Verbose + { + get + { + return _verbose; + } + + set + { + _verbose = ValidateNoContent(value); + } + } + + private string _verbose = "\x1b[33;1m"; /// /// Gets or sets the style for debug messages. /// - public string Debug { get; set; } = "\x1b[33;1m"; + public string Debug + { + get + { + return _debug; + } + + set + { + _debug = ValidateNoContent(value); + } + } + + private string _debug = "\x1b[33;1m"; } /// @@ -348,29 +471,89 @@ public sealed class FileInfoFormatting /// /// Gets or sets the style for directories. /// - public string Directory { get; set; } = "\x1b[44;1m"; + public string Directory + { + get + { + return _directory; + } + + set + { + _directory = ValidateNoContent(value); + } + } + + private string _directory = "\x1b[44;1m"; /// /// Gets or sets the style for symbolic links. /// - public string SymbolicLink { get; set; } = "\x1b[36;1m"; + public string SymbolicLink + { + get + { + return _symbolicLink; + } + + set + { + _symbolicLink = ValidateNoContent(value); + } + } + + private string _symbolicLink = "\x1b[36;1m"; /// /// Gets or sets the style for executables. /// - public string Executable { get; set; } = "\x1b[32;1m"; + public string Executable + { + get + { + return _executable; + } + + set + { + _executable = ValidateNoContent(value); + } + } + + private string _executable = "\x1b[32;1m"; + + /// + /// Custom dictionary handling validation of content. + /// + public class AnsiDictionary : Dictionary + { + /// + /// Constructor for AnsiDictionary. + /// + public AnsiDictionary() : base(StringComparer.OrdinalIgnoreCase){} + + /// + /// Addnew entry to dictionary. + /// + /// Key to add + /// ANSI string value to add + public new void Add(string key, string value) + { + base.Add(key, ValidateNoContent(value)); + } + } /// /// Gets the style for archive. /// - public Dictionary Extension { get; } + public AnsiDictionary Extension { get; } /// /// Initializes a new instance of the class. /// public FileInfoFormatting() { - Extension = new Dictionary(StringComparer.OrdinalIgnoreCase); + Extension = new AnsiDictionary(); // archives Extension.Add(".zip", "\x1b[31;1m"); @@ -516,6 +699,17 @@ private PSStyle() FileInfo = new FileInfoFormatting(); } + private static string ValidateNoContent(string text) + { + var decorartedString = new StringDecorated(text); + if (decorartedString.ContentLength > 0) + { + throw new InvalidOperationException(string.Format(PSStyleStrings.TextContainsContent, decorartedString.ToString(OutputRendering.PlainText))); + } + + return text; + } + /// /// Gets singleton instance. /// diff --git a/src/System.Management.Automation/resources/PSStyleStrings.resx b/src/System.Management.Automation/resources/PSStyleStrings.resx new file mode 100644 index 00000000000..3a5938dc317 --- /dev/null +++ b/src/System.Management.Automation/resources/PSStyleStrings.resx @@ -0,0 +1,64 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + The specified string contains printable content when it should only contain ANSI escape sequences: {0} + + diff --git a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 index f09fc27d1e8..40801fc201c 100644 --- a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 +++ b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 @@ -165,4 +165,45 @@ Describe 'Tests for $PSStyle automatic variable' { $PSStyle.Formatting.TableHeader = $old } } + + It 'Should fail if setting formatting contains printable characters: .' -TestCases @( + @{ Submember = 'Reset' } + @{ Submember = 'BlinkOff' } + @{ Submember = 'Blink' } + @{ Submember = 'BoldOff' } + @{ Submember = 'Bold' } + @{ Submember = 'HiddenOff' } + @{ Submember = 'Hidden' } + @{ Submember = 'ItalicOff' } + @{ Submember = 'Italic' } + @{ Submember = 'UnderlineOff' } + @{ Submember = 'Underline' } + @{ Submember = 'StrikethroughOff' } + @{ Submember = 'Strikethrough' } + @{ Member = 'Formatting'; Submember = 'FormatAccent' } + @{ Member = 'Formatting'; Submember = 'TableHeader' } + @{ Member = 'Formatting'; Submember = 'ErrorAccent' } + @{ Member = 'Formatting'; Submember = 'Error' } + @{ Member = 'Formatting'; Submember = 'Warning' } + @{ Member = 'Formatting'; Submember = 'Verbose' } + @{ Member = 'Formatting'; Submember = 'Debug' } + @{ Member = 'Progress'; Submember = 'Style' } + @{ Member = 'FileInfo'; Submember = 'Directory' } + @{ Member = 'FileInfo'; Submember = 'SymbolicLink' } + @{ Member = 'FileInfo'; Submember = 'Executable' } + @{ Member = 'FileInfo'; Submember = 'Hidden' } + ) { + param ($member, $submember) + + if ($null -ne $member) { + { $PSStyle.$member.$submember = $PSStyle.Foreground.Red + 'hello' } | Should -Throw + } + else { + { $PSStyle.$submember = $PSStyle.Foreground.Red + 'hello' } | Should -Throw + } + } + + It 'Should fail adding extension formatting with printable characters' { + { $PSStyle.FileInfo.Extension.Add('.md', 'hello') } | Should -Throw + } } From 5a5c0ee4819b52dae5b880ebd7bbd78b74d9a307 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Fri, 23 Jul 2021 15:37:08 -0700 Subject: [PATCH 02/11] fix codefactor --- .../FormatAndOutput/common/PSStyle.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index f646e206eb5..a39758c6bd3 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -528,15 +528,15 @@ public string Executable public class AnsiDictionary : Dictionary { /// - /// Constructor for AnsiDictionary. + /// Initializes a new instance of the class. /// - public AnsiDictionary() : base(StringComparer.OrdinalIgnoreCase){} + public AnsiDictionary() : base(StringComparer.OrdinalIgnoreCase) { } /// /// Addnew entry to dictionary. /// - /// Key to add - /// ANSI string value to add + /// Key to add. + /// ANSI string value to add. public new void Add(string key, string value) { base.Add(key, ValidateNoContent(value)); From 4465148029fbf4ab4665cc67a3b6890a257162a4 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Sat, 24 Jul 2021 10:30:47 -0700 Subject: [PATCH 03/11] address Ilya's feedback --- .../FormatAndOutput/common/PSStyle.cs | 4 ++-- .../resources/PSStyleStrings.resx | 3 +++ test/powershell/engine/Formatting/PSStyle.Tests.ps1 | 12 +++++++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index a39758c6bd3..d1371ced953 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -309,9 +309,9 @@ public int MaxWidth set { - if (value < 0) + if (value < 18) { - value = 0; + throw new ArgumentOutOfRangeException(nameof(MaxWidth), PSStyleStrings.ProgressWidthTooSmall); } _maxWidth = value; diff --git a/src/System.Management.Automation/resources/PSStyleStrings.resx b/src/System.Management.Automation/resources/PSStyleStrings.resx index 3a5938dc317..5fa1c2abb2e 100644 --- a/src/System.Management.Automation/resources/PSStyleStrings.resx +++ b/src/System.Management.Automation/resources/PSStyleStrings.resx @@ -61,4 +61,7 @@ The specified string contains printable content when it should only contain ANSI escape sequences: {0} + + The MaxWidth for the Progress rendering must be at least 18 to render correctly. + diff --git a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 index 40801fc201c..3a7c716bdcc 100644 --- a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 +++ b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 @@ -204,6 +204,16 @@ Describe 'Tests for $PSStyle automatic variable' { } It 'Should fail adding extension formatting with printable characters' { - { $PSStyle.FileInfo.Extension.Add('.md', 'hello') } | Should -Throw + { $PSStyle.FileInfo.Extension.Add('.md', 'hello') } | Should -Throw -ErrorId 'InvalidOperationException' + } + + It 'Should fail if MaxWidth is less than 18' { + $maxWidth = $PSStyle.Progress.MaxWidth + try { + { $PSStyle.Progress.MaxWidth = 17 } | Should -Throw -ErrorId 'ExceptionWhenSetting' + } + finally { + $PSStyle.Progress.MaxWidth = $maxWidth + } } } From 4b45d5c6341f1632873b3f1bf88d85465b8aec44 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Sat, 24 Jul 2021 12:17:57 -0700 Subject: [PATCH 04/11] use modern get/set syntax and add comment --- .../FormatAndOutput/common/PSStyle.cs | 130 ++++-------------- 1 file changed, 25 insertions(+), 105 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index d1371ced953..e13c873f2ec 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -284,15 +284,8 @@ public sealed class ProgressConfiguration /// public string Style { - get - { - return _style; - } - - set - { - _style = ValidateNoContent(value); - } + get => _style; + set => _style = ValidateNoContent(value); } private string _style = "\x1b[33;1m"; @@ -302,13 +295,10 @@ public string Style /// public int MaxWidth { - get - { - return _maxWidth; - } - + get => _maxWidth; set { + // Width less than 18 does not render correctly due to the different parts of the progress bar. if (value < 18) { throw new ArgumentOutOfRangeException(nameof(MaxWidth), PSStyleStrings.ProgressWidthTooSmall); @@ -341,15 +331,8 @@ public sealed class FormattingData /// public string FormatAccent { - get - { - return _formatAccent; - } - - set - { - _formatAccent = ValidateNoContent(value); - } + get => _formatAccent; + set => _formatAccent = ValidateNoContent(value); } private string _formatAccent = "\x1b[32;1m"; @@ -359,15 +342,8 @@ public string FormatAccent /// public string TableHeader { - get - { - return _tableHeader; - } - - set - { - _tableHeader = ValidateNoContent(value); - } + get => _tableHeader; + set => _tableHeader = ValidateNoContent(value); } private string _tableHeader = "\x1b[32;1m"; @@ -377,15 +353,8 @@ public string TableHeader /// public string ErrorAccent { - get - { - return _errorAccent; - } - - set - { - _errorAccent = ValidateNoContent(value); - } + get => _errorAccent; + set => _errorAccent = ValidateNoContent(value); } private string _errorAccent = "\x1b[36;1m"; @@ -395,15 +364,8 @@ public string ErrorAccent /// public string Error { - get - { - return _error; - } - - set - { - _error = ValidateNoContent(value); - } + get => _error; + set => _error = ValidateNoContent(value); } private string _error = "\x1b[31;1m"; @@ -413,15 +375,8 @@ public string Error /// public string Warning { - get - { - return _warning; - } - - set - { - _warning = ValidateNoContent(value); - } + get => _warning; + set => _warning = ValidateNoContent(value); } private string _warning = "\x1b[33;1m"; @@ -431,15 +386,8 @@ public string Warning /// public string Verbose { - get - { - return _verbose; - } - - set - { - _verbose = ValidateNoContent(value); - } + get => _verbose; + set => _verbose = ValidateNoContent(value); } private string _verbose = "\x1b[33;1m"; @@ -449,16 +397,9 @@ public string Verbose /// public string Debug { - get - { - return _debug; - } - - set - { - _debug = ValidateNoContent(value); - } - } + get => _debug; + set => _debug = ValidateNoContent(value); + } private string _debug = "\x1b[33;1m"; } @@ -473,15 +414,8 @@ public sealed class FileInfoFormatting /// public string Directory { - get - { - return _directory; - } - - set - { - _directory = ValidateNoContent(value); - } + get => _directory; + set => _directory = ValidateNoContent(value); } private string _directory = "\x1b[44;1m"; @@ -491,15 +425,8 @@ public string Directory /// public string SymbolicLink { - get - { - return _symbolicLink; - } - - set - { - _symbolicLink = ValidateNoContent(value); - } + get => _symbolicLink; + set => _symbolicLink = ValidateNoContent(value); } private string _symbolicLink = "\x1b[36;1m"; @@ -509,15 +436,8 @@ public string SymbolicLink /// public string Executable { - get - { - return _executable; - } - - set - { - _executable = ValidateNoContent(value); - } + get => _executable; + set => _executable = ValidateNoContent(value); } private string _executable = "\x1b[32;1m"; From 0604327ee9e217eb247522a5465e8e87f23a8a4f Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Sat, 24 Jul 2021 19:29:12 -0700 Subject: [PATCH 05/11] add support for Add() and Remove() extensions without the priod prefix --- .../FormatAndOutput/common/PSStyle.cs | 43 ++++++++++++++----- .../engine/Formatting/PSStyle.Tests.ps1 | 23 ++++++++++ 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index e13c873f2ec..3fce9116b59 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -443,37 +443,58 @@ public string Executable private string _executable = "\x1b[32;1m"; /// - /// Custom dictionary handling validation of content. + /// Custom dictionary handling validation of extension and content. /// - public class AnsiDictionary : Dictionary + public class FileExtensionDictionary : Dictionary { /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// - public AnsiDictionary() : base(StringComparer.OrdinalIgnoreCase) { } + public FileExtensionDictionary() : base(StringComparer.OrdinalIgnoreCase) { } /// - /// Addnew entry to dictionary. + /// Add new extension and decoration to dictionary. /// - /// Key to add. - /// ANSI string value to add. - public new void Add(string key, string value) + /// Extension to add. + /// ANSI string value to add. + public new void Add(string extension, string decoration) { - base.Add(key, ValidateNoContent(value)); + // if extension doesn't start with a dot, add one + if (!extension.StartsWith(".")) + { + extension = "." + extension; + } + + base.Add(extension, ValidateNoContent(decoration)); + } + + /// + /// Remove an extension from dictionary. + /// + /// Extension to remove. + public new void Remove(string extension) + { + // if extension doesn't start with a dot, add one + if (!extension.StartsWith(".")) + { + extension = "." + extension; + } + + base.Remove(extension); } } /// /// Gets the style for archive. /// - public AnsiDictionary Extension { get; } + public FileExtensionDictionary Extension { get; } /// /// Initializes a new instance of the class. /// public FileInfoFormatting() { - Extension = new AnsiDictionary(); + Extension = new FileExtensionDictionary(); // archives Extension.Add(".zip", "\x1b[31;1m"); diff --git a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 index 3a7c716bdcc..ef6dfb17646 100644 --- a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 +++ b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 @@ -207,8 +207,31 @@ Describe 'Tests for $PSStyle automatic variable' { { $PSStyle.FileInfo.Extension.Add('.md', 'hello') } | Should -Throw -ErrorId 'InvalidOperationException' } + It 'Should add and remove extension: ' -TestCases @( + @{ extension = '.myext' } + @{ extension = 'myext' } + ) { + param ($extension) + + if ($extension.StartsWith('.')) { + $fullExtension = $extension + } + else { + $fullExtension = '.' + $extension + } + + $PSStyle.FileInfo.Extension.Keys | Should -Not -Contain $fullextension + $PSStyle.FileInfo.Extension.Add($extension, $PSStyle.Foreground.Blue) + + $PSStyle.FileInfo.Extension[$fullextension] | Should -Be $PSStyle.Foreground.Blue + $PSStyle.FileInfo.Extension.Remove($extension) + $PSStyle.FileInfo.Extension.Keys | Should -Not -Contain $fullextension + } + It 'Should fail if MaxWidth is less than 18' { $maxWidth = $PSStyle.Progress.MaxWidth + + # minimum allowed width is 18 as less than that doesn't render correctly try { { $PSStyle.Progress.MaxWidth = 17 } | Should -Throw -ErrorId 'ExceptionWhenSetting' } From a671e7bc8be2511801e9c7f3d80c2870f75103ac Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Sun, 25 Jul 2021 22:28:40 -0700 Subject: [PATCH 06/11] update validation of adding and removing extension to require period prefix --- .../FormatAndOutput/common/PSStyle.cs | 8 ++--- .../resources/PSStyleStrings.resx | 3 ++ .../engine/Formatting/PSStyle.Tests.ps1 | 29 +++++++++---------- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index 3fce9116b59..d5c11770f64 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -445,7 +445,7 @@ public string Executable /// /// Custom dictionary handling validation of extension and content. /// - public class FileExtensionDictionary : Dictionary + public sealed class FileExtensionDictionary : Dictionary { /// /// Initializes a new instance of the class. @@ -459,10 +459,9 @@ public FileExtensionDictionary() : base(StringComparer.OrdinalIgnoreCase) { } /// ANSI string value to add. public new void Add(string extension, string decoration) { - // if extension doesn't start with a dot, add one if (!extension.StartsWith(".")) { - extension = "." + extension; + throw new ArgumentException(PSStyleStrings.ExtensionNotStartingWithPeriod); } base.Add(extension, ValidateNoContent(decoration)); @@ -474,10 +473,9 @@ public FileExtensionDictionary() : base(StringComparer.OrdinalIgnoreCase) { } /// Extension to remove. public new void Remove(string extension) { - // if extension doesn't start with a dot, add one if (!extension.StartsWith(".")) { - extension = "." + extension; + throw new ArgumentException(PSStyleStrings.ExtensionNotStartingWithPeriod); } base.Remove(extension); diff --git a/src/System.Management.Automation/resources/PSStyleStrings.resx b/src/System.Management.Automation/resources/PSStyleStrings.resx index 5fa1c2abb2e..bc7acb1c200 100644 --- a/src/System.Management.Automation/resources/PSStyleStrings.resx +++ b/src/System.Management.Automation/resources/PSStyleStrings.resx @@ -64,4 +64,7 @@ The MaxWidth for the Progress rendering must be at least 18 to render correctly. + + When adding or removing extensions, the extension must start with a period. + diff --git a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 index ef6dfb17646..98d998753d6 100644 --- a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 +++ b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 @@ -207,25 +207,22 @@ Describe 'Tests for $PSStyle automatic variable' { { $PSStyle.FileInfo.Extension.Add('.md', 'hello') } | Should -Throw -ErrorId 'InvalidOperationException' } - It 'Should add and remove extension: ' -TestCases @( - @{ extension = '.myext' } - @{ extension = 'myext' } - ) { - param ($extension) - - if ($extension.StartsWith('.')) { - $fullExtension = $extension - } - else { - $fullExtension = '.' + $extension - } - - $PSStyle.FileInfo.Extension.Keys | Should -Not -Contain $fullextension + It 'Should add and remove extension' { + $extension = '.mytest' + $PSStyle.FileInfo.Extension.Keys | Should -Not -Contain $extension $PSStyle.FileInfo.Extension.Add($extension, $PSStyle.Foreground.Blue) - $PSStyle.FileInfo.Extension[$fullextension] | Should -Be $PSStyle.Foreground.Blue + $PSStyle.FileInfo.Extension[$extension] | Should -Be $PSStyle.Foreground.Blue $PSStyle.FileInfo.Extension.Remove($extension) - $PSStyle.FileInfo.Extension.Keys | Should -Not -Contain $fullextension + $PSStyle.FileInfo.Extension.Keys | Should -Not -Contain $extension + } + + It 'Should fail to add extension does not start with a period' { + { $PSStyle.FileInfo.Extension.Add('mytest', $PSStyle.Foreground.Blue) } | Should -Throw -ErrorId 'ArgumentException' + } + + It 'Should fail to remove extension does not start with a period' { + { $PSStyle.FileInfo.Extension.Remove('zip') } | Should -Throw -ErrorId 'ArgumentException' } It 'Should fail if MaxWidth is less than 18' { From c7af849052711f974125059245ec0e83493b3691 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 26 Jul 2021 07:56:21 -0700 Subject: [PATCH 07/11] use char overload for StartsWith() --- .../FormatAndOutput/common/PSStyle.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index d5c11770f64..8d1ae8d6fd1 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -459,7 +459,7 @@ public FileExtensionDictionary() : base(StringComparer.OrdinalIgnoreCase) { } /// ANSI string value to add. public new void Add(string extension, string decoration) { - if (!extension.StartsWith(".")) + if (!extension.StartsWith('.')) { throw new ArgumentException(PSStyleStrings.ExtensionNotStartingWithPeriod); } @@ -473,7 +473,7 @@ public FileExtensionDictionary() : base(StringComparer.OrdinalIgnoreCase) { } /// Extension to remove. public new void Remove(string extension) { - if (!extension.StartsWith(".")) + if (!extension.StartsWith('.')) { throw new ArgumentException(PSStyleStrings.ExtensionNotStartingWithPeriod); } From 8ea054d71dae0afad3101839af1ed3221315e1ad Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 26 Jul 2021 12:31:47 -0700 Subject: [PATCH 08/11] change FileExtensionDictionary to encapsulate Dictionary privately --- .../FormatAndOutput/common/PSStyle.cs | 89 +++++++++++++++---- .../engine/Formatting/PSStyle.Tests.ps1 | 2 +- 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index 8d1ae8d6fd1..9213b27f131 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Collections; using System.Collections.Generic; using System.Management.Automation.Internal; @@ -445,40 +446,94 @@ public string Executable /// /// Custom dictionary handling validation of extension and content. /// - public sealed class FileExtensionDictionary : Dictionary + public sealed class FileExtensionDictionary : IEnumerable { - /// - /// Initializes a new instance of the class. - /// - public FileExtensionDictionary() : base(StringComparer.OrdinalIgnoreCase) { } + private static string ValidateExtension(string extension) + { + if (!extension.StartsWith('.')) + { + throw new ArgumentException(PSStyleStrings.ExtensionNotStartingWithPeriod); + } + + return extension; + } + + private readonly Dictionary _extensionDictionary = new(); /// /// Add new extension and decoration to dictionary. /// /// Extension to add. /// ANSI string value to add. - public new void Add(string extension, string decoration) + public void Add(string extension, string decoration) { - if (!extension.StartsWith('.')) - { - throw new ArgumentException(PSStyleStrings.ExtensionNotStartingWithPeriod); - } - - base.Add(extension, ValidateNoContent(decoration)); + _extensionDictionary.Add(ValidateExtension(extension), ValidateNoContent(decoration)); } /// /// Remove an extension from dictionary. /// /// Extension to remove. - public new void Remove(string extension) + public void Remove(string extension) { - if (!extension.StartsWith('.')) + _extensionDictionary.Remove(ValidateExtension(extension)); + } + + /// + /// Clear the dictionary. + /// + public void Clear() + { + _extensionDictionary.Clear(); + } + + /// + /// Gets the enumerator for the dictionary. + /// + /// The enumerator for the dictionary. + public IEnumerator GetEnumerator() + { + return _extensionDictionary.GetEnumerator(); + } + + /// + /// Gets or sets the decoration by specified extension. + /// + /// Extension to get decoration for. + /// The decoration for specified extension. + public string this[string extension] + { + get { - throw new ArgumentException(PSStyleStrings.ExtensionNotStartingWithPeriod); + return _extensionDictionary[ValidateExtension(extension)]; } - base.Remove(extension); + set + { + _extensionDictionary[ValidateExtension(extension)] = ValidateNoContent(value); + } + } + + /// + /// Gets whether the dictionary contains the specified extension. + /// + /// Extension to check for. + /// True if the dictionary contains the specified extension, otherwise false. + public bool ContainsKey(string extension) + { + return _extensionDictionary.ContainsKey(ValidateExtension(extension)); + } + + /// + /// Gets the extensions for the dictionary. + /// + /// The extensions for the dictionary. + public IEnumerable Keys + { + get + { + return _extensionDictionary.Keys; + } } } @@ -643,7 +698,7 @@ private static string ValidateNoContent(string text) var decorartedString = new StringDecorated(text); if (decorartedString.ContentLength > 0) { - throw new InvalidOperationException(string.Format(PSStyleStrings.TextContainsContent, decorartedString.ToString(OutputRendering.PlainText))); + throw new ArgumentException(string.Format(PSStyleStrings.TextContainsContent, decorartedString.ToString(OutputRendering.PlainText))); } return text; diff --git a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 index 98d998753d6..b4d966b8b21 100644 --- a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 +++ b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 @@ -204,7 +204,7 @@ Describe 'Tests for $PSStyle automatic variable' { } It 'Should fail adding extension formatting with printable characters' { - { $PSStyle.FileInfo.Extension.Add('.md', 'hello') } | Should -Throw -ErrorId 'InvalidOperationException' + { $PSStyle.FileInfo.Extension.Add('.md', 'hello') } | Should -Throw -ErrorId 'ArgumentException' } It 'Should add and remove extension' { From baf55befa1bca2aa5710d6c6197def985010cae4 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 26 Jul 2021 14:21:34 -0700 Subject: [PATCH 09/11] fix dictionary to be case-insensitive and validate extension for ContainsKey() --- .../FormatAndOutput/common/PSStyle.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index 9213b27f131..35299a10eac 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -458,7 +458,7 @@ private static string ValidateExtension(string extension) return extension; } - private readonly Dictionary _extensionDictionary = new(); + private readonly Dictionary _extensionDictionary = new(StringComparer.OrdinalIgnoreCase); /// /// Add new extension and decoration to dictionary. @@ -521,7 +521,7 @@ public string this[string extension] /// True if the dictionary contains the specified extension, otherwise false. public bool ContainsKey(string extension) { - return _extensionDictionary.ContainsKey(ValidateExtension(extension)); + return _extensionDictionary.ContainsKey(extension); } /// From 49b204bc9c08cfbb19f31ac8e26ce1eb9315dbc6 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 26 Jul 2021 14:24:36 -0700 Subject: [PATCH 10/11] don't have dictionary implement IEnumerable --- .../FormatAndOutput/common/PSStyle.cs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index 35299a10eac..0ba9a938b80 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -446,7 +446,7 @@ public string Executable /// /// Custom dictionary handling validation of extension and content. /// - public sealed class FileExtensionDictionary : IEnumerable + public sealed class FileExtensionDictionary { private static string ValidateExtension(string extension) { @@ -487,15 +487,6 @@ public void Clear() _extensionDictionary.Clear(); } - /// - /// Gets the enumerator for the dictionary. - /// - /// The enumerator for the dictionary. - public IEnumerator GetEnumerator() - { - return _extensionDictionary.GetEnumerator(); - } - /// /// Gets or sets the decoration by specified extension. /// From e745c86dfcdaeacfcd2af1671681a61f89444e52 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 26 Jul 2021 20:22:54 -0700 Subject: [PATCH 11/11] add validation to ContainsKey() --- .../FormatAndOutput/common/PSStyle.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index 0ba9a938b80..8efb46e40c7 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -512,7 +512,12 @@ public string this[string extension] /// True if the dictionary contains the specified extension, otherwise false. public bool ContainsKey(string extension) { - return _extensionDictionary.ContainsKey(extension); + if (string.IsNullOrEmpty(extension)) + { + return false; + } + + return _extensionDictionary.ContainsKey(ValidateExtension(extension)); } ///