From 34e3ca61b68a639083712981a72d4bba5b5f7716 Mon Sep 17 00:00:00 2001 From: lselden Date: Mon, 12 Jul 2021 23:07:49 -0400 Subject: [PATCH 1/5] Escape quotes and newlines with -UseQuotes AsNeeded Fix for issue #9284 - Escape fields that contain quotes and newlines, not just those that contain the delimiter --- .../commands/utility/CsvCommands.cs | 7 +++++-- .../ConvertTo-Csv.Tests.ps1 | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs index cbd9d4c36b4..0f3c1979edb 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs @@ -881,6 +881,7 @@ internal class ExportCsvHelper : IDisposable private readonly BaseCsvWritingCommand.QuoteKind _quoteKind; private readonly HashSet _quoteFields; private readonly StringBuilder _outputString; + private readonly char[] _escapeCharacters; /// /// Initializes a new instance of the class. @@ -894,6 +895,8 @@ internal ExportCsvHelper(char delimiter, BaseCsvWritingCommand.QuoteKind quoteKi _quoteKind = quoteKind; _quoteFields = quoteFields == null ? null : new HashSet(quoteFields, StringComparer.OrdinalIgnoreCase); _outputString = new StringBuilder(128); + // As per RFC 4180, fields with line breaks, double quotes (the escape character) or delimiter should be escaped + _escapeCharacters = new Char[] { '"', '\r', '\n', delimiter}; } // Name of properties to be written in CSV format @@ -967,7 +970,7 @@ internal string ConvertPropertyNamesCSV(IList propertyNames) AppendStringWithEscapeAlways(_outputString, propertyName); break; case BaseCsvWritingCommand.QuoteKind.AsNeeded: - if (propertyName.Contains(_delimiter)) + if (propertyName.IndexOfAny(_escapeCharacters) != -1) { AppendStringWithEscapeAlways(_outputString, propertyName); } @@ -1038,7 +1041,7 @@ internal string ConvertPSObjectToCSV(PSObject mshObject, IList propertyN AppendStringWithEscapeAlways(_outputString, value); break; case BaseCsvWritingCommand.QuoteKind.AsNeeded: - if (value != null && value.Contains(_delimiter)) + if (value != null && value.IndexOfAny(_escapeCharacters) != -1) { AppendStringWithEscapeAlways(_outputString, value); } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 index 494051507f7..a88863b9143 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 @@ -41,6 +41,7 @@ Describe "ConvertTo-Csv" -Tags "CI" { $Name = "Hello"; $Data = "World"; $testObject = [pscustomobject]@{ FirstColumn = $Name; SecondColumn = $Data } $testNullObject = [pscustomobject]@{ FirstColumn = $Name; SecondColumn = $null } + $testQuotesObject = [pscustomobject]@{ FirstColumn = "`"$Name`""; SecondColumn = "`nWorld"} } It "Should Be able to be called without error" { @@ -127,6 +128,11 @@ Describe "ConvertTo-Csv" -Tags "CI" { $result[0] | Should -BeExactly "`"FirstColumn`",`"SecondColumn`"" $result[1] | Should -BeExactly "`"Hello`"," + + $result = $testQuotesObject | ConvertTo-Csv -UseQuotes Always -Delimiter ',' + + $result[0] | Should -BeExactly "`"FirstColumn`",`"SecondColumn`"" + $result[1] | Should -BeExactly "`"`"`"Hello`"`"`",`"`nWorld`"" } It "UseQuotes Always is default" { @@ -146,6 +152,8 @@ Describe "ConvertTo-Csv" -Tags "CI" { $result[0] | Should -BeExactly "FirstColumn,SecondColumn" $result[1] | Should -BeExactly "Hello," + + $result = $testQuotesObject | ConvertTo-Csv -UseQuotes Never } It "UseQuotes AsNeeded" { @@ -158,6 +166,16 @@ Describe "ConvertTo-Csv" -Tags "CI" { $result[0] | Should -BeExactly "`"FirstColumn`"rSecondColumn" $result[1] | Should -BeExactly "Hellor" + + $result = $testQuotesObject | ConvertTo-Csv -UseQuotes AsNeeded -Delimiter ',' + + $result[0] | Should -BeExactly "FirstColumn,SecondColumn" + $result[1] | Should -BeExactly "`"`"`"Hello`"`"`",`"`nWorld`"" + + $result = $testQuotesObject | ConvertTo-Csv -UseQuotes AsNeeded -Delimiter 'e' + + $result[0] | Should -BeExactly "FirstColumne`"SecondColumn`"" + $result[1] | Should -BeExactly "`"`"`"Hello`"`"`"e`"`nWorld`"" } } } From c712df9199858563923f532c6305bf3232759053 Mon Sep 17 00:00:00 2001 From: lselden Date: Tue, 13 Jul 2021 12:01:47 -0400 Subject: [PATCH 2/5] add additional -UseQuotes AsNeeded tests Added additional tests for different character scenarios that should be handled --- .../ConvertTo-Csv.Tests.ps1 | 47 +++++++++++++------ 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 index a88863b9143..eac3778382c 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 @@ -41,7 +41,6 @@ Describe "ConvertTo-Csv" -Tags "CI" { $Name = "Hello"; $Data = "World"; $testObject = [pscustomobject]@{ FirstColumn = $Name; SecondColumn = $Data } $testNullObject = [pscustomobject]@{ FirstColumn = $Name; SecondColumn = $null } - $testQuotesObject = [pscustomobject]@{ FirstColumn = "`"$Name`""; SecondColumn = "`nWorld"} } It "Should Be able to be called without error" { @@ -128,11 +127,6 @@ Describe "ConvertTo-Csv" -Tags "CI" { $result[0] | Should -BeExactly "`"FirstColumn`",`"SecondColumn`"" $result[1] | Should -BeExactly "`"Hello`"," - - $result = $testQuotesObject | ConvertTo-Csv -UseQuotes Always -Delimiter ',' - - $result[0] | Should -BeExactly "`"FirstColumn`",`"SecondColumn`"" - $result[1] | Should -BeExactly "`"`"`"Hello`"`"`",`"`nWorld`"" } It "UseQuotes Always is default" { @@ -152,8 +146,6 @@ Describe "ConvertTo-Csv" -Tags "CI" { $result[0] | Should -BeExactly "FirstColumn,SecondColumn" $result[1] | Should -BeExactly "Hello," - - $result = $testQuotesObject | ConvertTo-Csv -UseQuotes Never } It "UseQuotes AsNeeded" { @@ -166,16 +158,43 @@ Describe "ConvertTo-Csv" -Tags "CI" { $result[0] | Should -BeExactly "`"FirstColumn`"rSecondColumn" $result[1] | Should -BeExactly "Hellor" + } - $result = $testQuotesObject | ConvertTo-Csv -UseQuotes AsNeeded -Delimiter ',' + It "UseQuotes AsNeeded Escapes Delimiters" { + $testDelimitersObject = [pscustomobject]@{ "FirstColumn" = "Hello,"; "Second,Column" = "World" }; - $result[0] | Should -BeExactly "FirstColumn,SecondColumn" - $result[1] | Should -BeExactly "`"`"`"Hello`"`"`",`"`nWorld`"" + $result = $testDelimitersObject | ConvertTo-Csv -UseQuotes AsNeeded -Delimiter ',' + + $result[0] | Should -BeExactly "FirstColumn,`"Second,Column`"" + $result[1] | Should -BeExactly "`"Hello,`",World" + + $result = $testDelimitersObject | ConvertTo-Csv -UseQuotes AsNeeded -Delimiter "r" - $result = $testQuotesObject | ConvertTo-Csv -UseQuotes AsNeeded -Delimiter 'e' + $result[0] | Should -BeExactly "`"FirstColumn`"rSecond,Column" + $result[1] | Should -BeExactly "Hello,r`"World`"" + } + It "UseQuotes AsNeeded Escapes Newlines" { + $testCRLFObject = [pscustomobject]@{ "First`r`nColumn" = "Hello`r`nWorld" }; + $testLFObject = [pscustomobject]@{ "First`nColumn" = "Hello`nWorld" }; + + $result = $testCRLFObject | ConvertTo-Csv -UseQuotes AsNeeded + + $result[0] | Should -BeExactly "`"First`r`nColumn`"" + $result[1] | Should -BeExactly "`"Hello`r`nWorld`"" + + $result = $testLFObject | ConvertTo-Csv -UseQuotes AsNeeded - $result[0] | Should -BeExactly "FirstColumne`"SecondColumn`"" - $result[1] | Should -BeExactly "`"`"`"Hello`"`"`"e`"`nWorld`"" + $result[0] | Should -BeExactly "`"First`nColumn`"" + $result[1] | Should -BeExactly "`"Hello`nWorld`"" } + It "UseQuotes AsNeeded Escapes Quotes" { + $testQuotesObject = [pscustomobject]@{ "First`"Column" = "`"Hello`" World" }; + + $result = $testQuotesObject | ConvertTo-Csv -UseQuotes AsNeeded + + $result[0] | Should -BeExactly "`"First`"`"Column`"" + $result[1] | Should -BeExactly "`"`"`"Hello`"`" World`"" + } + } } From 15200e4bba5e659e7692ac9899c1cee2ece9e41a Mon Sep 17 00:00:00 2001 From: lselden Date: Tue, 13 Jul 2021 12:24:49 -0400 Subject: [PATCH 3/5] Code Quality cleanup per codefactor review --- .../commands/utility/CsvCommands.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs index 0f3c1979edb..28839e9ab00 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs @@ -895,8 +895,9 @@ internal ExportCsvHelper(char delimiter, BaseCsvWritingCommand.QuoteKind quoteKi _quoteKind = quoteKind; _quoteFields = quoteFields == null ? null : new HashSet(quoteFields, StringComparer.OrdinalIgnoreCase); _outputString = new StringBuilder(128); + // As per RFC 4180, fields with line breaks, double quotes (the escape character) or delimiter should be escaped - _escapeCharacters = new Char[] { '"', '\r', '\n', delimiter}; + _escapeCharacters = new char[] { '"', '\r', '\n', delimiter }; } // Name of properties to be written in CSV format From 4832f43b91128e700b131a969c052e800ef6549f Mon Sep 17 00:00:00 2001 From: lselden Date: Tue, 13 Jul 2021 14:25:20 -0400 Subject: [PATCH 4/5] inline AsNeeded character detection Cast string as ReadOnlySpan to avoid allocation of char array --- .../commands/utility/CsvCommands.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs index 28839e9ab00..1ec99fc8113 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CsvCommands.cs @@ -881,7 +881,6 @@ internal class ExportCsvHelper : IDisposable private readonly BaseCsvWritingCommand.QuoteKind _quoteKind; private readonly HashSet _quoteFields; private readonly StringBuilder _outputString; - private readonly char[] _escapeCharacters; /// /// Initializes a new instance of the class. @@ -895,9 +894,6 @@ internal ExportCsvHelper(char delimiter, BaseCsvWritingCommand.QuoteKind quoteKi _quoteKind = quoteKind; _quoteFields = quoteFields == null ? null : new HashSet(quoteFields, StringComparer.OrdinalIgnoreCase); _outputString = new StringBuilder(128); - - // As per RFC 4180, fields with line breaks, double quotes (the escape character) or delimiter should be escaped - _escapeCharacters = new char[] { '"', '\r', '\n', delimiter }; } // Name of properties to be written in CSV format @@ -971,7 +967,8 @@ internal string ConvertPropertyNamesCSV(IList propertyNames) AppendStringWithEscapeAlways(_outputString, propertyName); break; case BaseCsvWritingCommand.QuoteKind.AsNeeded: - if (propertyName.IndexOfAny(_escapeCharacters) != -1) + + if (propertyName.AsSpan().IndexOfAny(_delimiter, '\n', '"') != -1) { AppendStringWithEscapeAlways(_outputString, propertyName); } @@ -1042,7 +1039,7 @@ internal string ConvertPSObjectToCSV(PSObject mshObject, IList propertyN AppendStringWithEscapeAlways(_outputString, value); break; case BaseCsvWritingCommand.QuoteKind.AsNeeded: - if (value != null && value.IndexOfAny(_escapeCharacters) != -1) + if (value != null && value.AsSpan().IndexOfAny(_delimiter, '\n', '"') != -1) { AppendStringWithEscapeAlways(_outputString, value); } From a9f296f793876aa6eecccd31843b5562b7645b53 Mon Sep 17 00:00:00 2001 From: lselden <2904993+lselden@users.noreply.github.com> Date: Wed, 14 Jul 2021 14:56:45 -0400 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Ilya --- .../Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 index eac3778382c..2a121a7c16c 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Csv.Tests.ps1 @@ -173,6 +173,7 @@ Describe "ConvertTo-Csv" -Tags "CI" { $result[0] | Should -BeExactly "`"FirstColumn`"rSecond,Column" $result[1] | Should -BeExactly "Hello,r`"World`"" } + It "UseQuotes AsNeeded Escapes Newlines" { $testCRLFObject = [pscustomobject]@{ "First`r`nColumn" = "Hello`r`nWorld" }; $testLFObject = [pscustomobject]@{ "First`nColumn" = "Hello`nWorld" }; @@ -187,6 +188,7 @@ Describe "ConvertTo-Csv" -Tags "CI" { $result[0] | Should -BeExactly "`"First`nColumn`"" $result[1] | Should -BeExactly "`"Hello`nWorld`"" } + It "UseQuotes AsNeeded Escapes Quotes" { $testQuotesObject = [pscustomobject]@{ "First`"Column" = "`"Hello`" World" };