diff --git a/NTFSSecurity/ParameterConversion.cs b/NTFSSecurity/ParameterConversion.cs index 11ee2fa..e6a4a01 100644 --- a/NTFSSecurity/ParameterConversion.cs +++ b/NTFSSecurity/ParameterConversion.cs @@ -82,8 +82,9 @@ namespace NTFSSecurity return string.Format("The value for parameter -{0} is null. {1}", parameterName, DescribeValidValues(typeof(T))); } - // How many collections that hold one collection each are opened before the argument counts as a list: enough - // for any real argument, and a stop for a collection that holds itself. + // The most levels of nested collections that the conversion looks into, counting the argument itself: a value + // that is wrapped in more of them counts as a list. Enough for any real argument, and a stop for a collection + // that holds itself. private const int MaxNesting = 32; // The one value that the conversion takes from the argument of an enumeration that is not a flags enumeration: @@ -151,8 +152,23 @@ namespace NTFSSecurity } // The value as a message names it: a collection of one element by that element (also when it is a collection - // itself), a collection of several elements by its first ten, anything else by its text. + // itself), a collection of several elements by its first ten, anything else by its text. The message is built + // inside a catch block, so this method must not fail whatever the text or the enumeration of a value does; a + // value that cannot be formatted is named by its type. private static string Describe(object value) + { + try + { + return DescribeValue(value); + } + catch (Exception) + { + var unwrapped = value is PSObject ? ((PSObject)value).BaseObject : value; + return unwrapped == null ? string.Empty : unwrapped.GetType().FullName; + } + } + + private static string DescribeValue(object value) { var current = value; @@ -168,36 +184,29 @@ namespace NTFSSecurity var shown = new System.Collections.Generic.List(); var more = false; - try + foreach (var item in collection) { - foreach (var item in collection) - { - if (shown.Count == 10) - { - more = true; - break; - } - - shown.Add(item); - } - - if (shown.Count == 1 && !more) + if (shown.Count == 10) { - current = shown[0]; - continue; + more = true; + break; } - if (shown.Count == 0) - { - return string.Format("{0}", current); - } + shown.Add(item); + } - return string.Join(", ", shown.ConvertAll(item => string.Format("{0}", item))) + (more ? ", ..." : string.Empty); + if (shown.Count == 1 && !more) + { + current = shown[0]; + continue; } - catch (Exception) + + if (shown.Count == 0) { - return string.Format("{0}", value); + return string.Format("{0}", current); } + + return string.Join(", ", shown.ConvertAll(item => string.Format("{0}", item))) + (more ? ", ..." : string.Empty); } return string.Format("{0}", value); diff --git a/Tests/ParameterSets.Access.Tests.ps1 b/Tests/ParameterSets.Access.Tests.ps1 index 4a0a2c6..5693ee4 100644 --- a/Tests/ParameterSets.Access.Tests.ps1 +++ b/Tests/ParameterSets.Access.Tests.ps1 @@ -1346,6 +1346,10 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF $accessDeepScope = , $accessDeepScope } + # The message of the error names the value, and building it must not fail when the text of an element does. + $accessThrowingElement = [pscustomobject]@{ Unrelated = 1 } + $accessThrowingElement | Add-Member -MemberType ScriptMethod -Name ToString -Value { throw 'ToString throws' } -Force + $accessBadShapes = @( @{ Shape = 'a misspelled name'; Value = 'Typoo'; ValueText = 'Typoo' } @{ Shape = 'a blank string'; Value = ''; ValueText = $null } @@ -1353,6 +1357,7 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF @{ Shape = 'an undefined number'; Value = 99; ValueText = '99' } @{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null } @{ Shape = 'an array of one misspelled name'; Value = @('Typoo'); ValueText = 'Typoo' } + @{ Shape = 'an array of one object whose ToString throws'; Value = @($accessThrowingElement); ValueText = $null } ) $accessTypeBadShapes = @( @{ Shape = 'a misspelled name'; Value = 'Denyy'; ValueText = 'Denyy' } @@ -1361,6 +1366,7 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF @{ Shape = 'an undefined number'; Value = 99; ValueText = '99' } @{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null } @{ Shape = 'an array of one misspelled name'; Value = @('Denyy'); ValueText = 'Denyy' } + @{ Shape = 'an array of one object whose ToString throws'; Value = @($accessThrowingElement); ValueText = $null } @{ Shape = 'a comma list of two names'; Value = 'Allow,Deny'; ValueText = 'Allow,Deny' } @{ Shape = 'an array of two names'; Value = @('Allow', 'Deny'); ValueText = 'Allow, Deny' } @{ Shape = 'an array of one comma list'; Value = @('Allow,Deny'); ValueText = 'Allow,Deny' } @@ -1382,7 +1388,7 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF @{ Shape = 'an array nested 40 levels deep'; Value = $accessDeepScope; ValueText = $null } ) - # The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (13 + 3 * 6 + 6) = 74 rows. + # The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (14 + 3 * 7 + 6) = 82 rows. $accessInvalidParameterCases = foreach ($cmdlet in 'Add-NTFSAccess', 'Remove-NTFSAccess') { foreach ($shapeCase in $accessTypeBadShapes) { @{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Shape = $shapeCase.Shape; Value = $shapeCase.Value; ValueText = $shapeCase.ValueText } @@ -1397,6 +1403,15 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF } } + # The message names a list by its first ten elements: ten are named in full, eleven end with an ellipsis. The + # elements of a flags parameter are converted one by one, so these numbers fail there too (4 is not a flag). + $accessInvalidParameterCases += @( + @{ Cmdlet = 'Add-NTFSAccess'; Parameter = 'AccessType'; Shape = 'ten numbers'; Value = 1..10; ValueText = '1, 2, 3, 4, 5, 6, 7, 8, 9, 10' } + @{ Cmdlet = 'Add-NTFSAccess'; Parameter = 'AccessType'; Shape = 'eleven numbers'; Value = 1..11; ValueText = '1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...' } + @{ Cmdlet = 'Add-NTFSAccess'; Parameter = 'InheritanceFlags'; Shape = 'ten numbers'; Value = 1..10; ValueText = '1, 2, 3, 4, 5, 6, 7, 8, 9, 10' } + @{ Cmdlet = 'Add-NTFSAccess'; Parameter = 'InheritanceFlags'; Shape = 'eleven numbers'; Value = 1..11; ValueText = '1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...' } + ) + $accessSdInvalidParameterCases = foreach ($cmdlet in 'Add-NTFSAccess', 'Remove-NTFSAccess') { @{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = 'Denyy'; ValueText = 'Denyy' } @{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'Typoo'; ValueText = 'Typoo' } @@ -1424,6 +1439,8 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF @{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = [System.Collections.Generic.HashSet[string]]::new([string[]]@('ThisFolderAndSubfolders', 'ThisFolderAndFiles')) } @{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = $accessDeepAccessType } @{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = $accessDeepScope } + @{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = @($accessThrowingElement) } + @{ Cmdlet = $cmdlet; Parameter = 'InheritanceFlags'; Value = @($accessThrowingElement) } } # A named $null (a literal $null, a variable, an alias, splatting, or $PSDefaultParameterValues) must @@ -1722,6 +1739,39 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF } } + # The conversion reads at most two elements of a sequence to know that it is a list, and the message names ten, so a + # sequence that is long (or has no end) is not read to its end. + Context 'A long sequence of values' { + It 'Should read only the first elements of a long sequence given as -AccessType, and name its first ten in the error' -ForEach @( + @{ Mode = 'named' } + @{ Mode = 'piped' } + ) { + $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'LongSequence' -Directory + $before = (Get-Acl -LiteralPath $folder).Sddl + $counter = @{ Reads = 0 } + $count = { $counter.Reads++; 'Allow' }.GetNewClosure() + $sequence = [System.Linq.Enumerable]::Select([int[]] (1..100000), [Func[int, string]] $count) + + $message = $null + if ($Mode -eq 'named') { + try { + Add-NTFSAccess -Path $folder -Account 'S-1-1-0' -AccessRights ReadData -AccessType $sequence -ErrorAction Stop + } + catch { + $message = $_.Exception.Message + } + } + else { + [pscustomobject]@{ Path = $folder; Account = 'S-1-1-0'; AccessRights = 'ReadData'; AccessType = $sequence } | Add-NTFSAccess -ErrorVariable pipelineErrors -ErrorAction SilentlyContinue + $message = @($pipelineErrors)[0].Exception.Message + } + + $message | Should -Match ('The value ''' + ('Allow, ' * 10) + '\.\.\.''') + $counter.Reads | Should -BeLessThan 50 + (Get-Acl -LiteralPath $folder).Sddl | Should -BeExactly $before + } + } + # A typed parameter ends a named $null (for example a variable that misspells a property, such as -AccessType # $rule.AccessTpye) with a terminating ParameterBindingFailed; the object-typed parameter accepts it, so the # cmdlet must not keep the default silently. A named $null is reported the same way as a bad named value. diff --git a/Tests/ParameterSets.Audit.Tests.ps1 b/Tests/ParameterSets.Audit.Tests.ps1 index 5207d18..6c4aab4 100644 --- a/Tests/ParameterSets.Audit.Tests.ps1 +++ b/Tests/ParameterSets.Audit.Tests.ps1 @@ -1011,6 +1011,10 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF $auditDeepScope = , $auditDeepScope } + # The message of the error names the value, and building it must not fail when the text of an element does. + $auditThrowingElement = [pscustomobject]@{ Unrelated = 1 } + $auditThrowingElement | Add-Member -MemberType ScriptMethod -Name ToString -Value { throw 'ToString throws' } -Force + $auditBadShapes = @( @{ Shape = 'a misspelled name'; Value = 'Typoo'; ValueText = 'Typoo' } @{ Shape = 'a blank string'; Value = ''; ValueText = $null } @@ -1018,6 +1022,7 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF @{ Shape = 'an undefined number'; Value = 99; ValueText = '99' } @{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null } @{ Shape = 'an array of one misspelled name'; Value = @('Typoo'); ValueText = 'Typoo' } + @{ Shape = 'an array of one object whose ToString throws'; Value = @($auditThrowingElement); ValueText = $null } ) $auditFlagsBadShapes = @( @{ Shape = 'a misspelled name'; Value = 'Failurre'; ValueText = 'Failurre' } @@ -1026,9 +1031,10 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF @{ Shape = 'an undefined number'; Value = 99; ValueText = '99' } @{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null } @{ Shape = 'an array of one misspelled name'; Value = @('Failurre'); ValueText = 'Failurre' } + @{ Shape = 'an array of one object whose ToString throws'; Value = @($auditThrowingElement); ValueText = $null } ) - # The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (6 + 3 * 6 + 6) = 60 rows. + # The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (7 + 3 * 7 + 6) = 68 rows. # AppliesTo names one value, not a combination: PowerShell combines the numbers of a list bit by bit, so that one # of the values wins silently or a scope that the caller did not name is chosen. AuditFlags and the two flags # parameters take a list. Every kind of collection counts, also a collection of one element that holds a list. @@ -1077,6 +1083,8 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF @{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = @('ThisFolderAndSubfolders,ThisFolderAndFiles') } @{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = [System.Collections.Generic.HashSet[string]]::new([string[]]@('ThisFolderAndSubfolders', 'ThisFolderAndFiles')) } @{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = $auditDeepScope } + @{ Cmdlet = $cmdlet; Parameter = 'AuditFlags'; Value = @($auditThrowingElement) } + @{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = @($auditThrowingElement) } } # A named $null (a literal $null, a variable, splatting, or $PSDefaultParameterValues) must stay a