Browse Source

fix(access): build the message of a bad parameter value even when the text of the value throws

The message of the error is built inside the catch block of the conversion, and
Describe formatted the one element of a collection of one element outside of
any guard. An element whose ToString throws (a script method, for example)
ended the command with a terminating ExtendedTypeSystemException instead of
the InvalidParameterValue error for that item, and a piped command stopped at
that object, where the pages say that the error is reported for just that
item. Describe now falls back to the type name whatever the text or the
enumeration of the value does, for the five parameters of the four cmdlets
alike.

Tests, written first: 24 rows failed on the previous build in all four
configurations and none fails now (an array of one object whose ToString
throws, piped for every parameter and named for one parameter of each kind).
New rows also pin what the previous commit claimed but did not test: the
message names the first ten elements of a list and ends with an ellipsis from
the eleventh, and a sequence of 100,000 elements is read only up to its first
elements (a counting sequence, named and piped). A build without the two limits
fails 4 of them and a build with a limit of nine fails 6, in all four
configurations. The comment of the nesting limit now says exactly what it
counts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>
pull/131/head
Raimund Andree 2 weeks ago
parent
commit
ec58a18c28
  1. 29
      NTFSSecurity/ParameterConversion.cs
  2. 52
      Tests/ParameterSets.Access.Tests.ps1
  3. 10
      Tests/ParameterSets.Audit.Tests.ps1

29
NTFSSecurity/ParameterConversion.cs

@ -82,8 +82,9 @@ namespace NTFSSecurity
return string.Format("The value for parameter -{0} is null. {1}", parameterName, DescribeValidValues(typeof(T))); 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 // The most levels of nested collections that the conversion looks into, counting the argument itself: a value
// for any real argument, and a stop for a collection that holds itself. // 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; private const int MaxNesting = 32;
// The one value that the conversion takes from the argument of an enumeration that is not a flags enumeration: // 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 // 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) 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; var current = value;
@ -168,8 +184,6 @@ namespace NTFSSecurity
var shown = new System.Collections.Generic.List<object>(); var shown = new System.Collections.Generic.List<object>();
var more = false; var more = false;
try
{
foreach (var item in collection) foreach (var item in collection)
{ {
if (shown.Count == 10) if (shown.Count == 10)
@ -194,11 +208,6 @@ namespace NTFSSecurity
return string.Join(", ", shown.ConvertAll(item => string.Format("{0}", item))) + (more ? ", ..." : string.Empty); return string.Join(", ", shown.ConvertAll(item => string.Format("{0}", item))) + (more ? ", ..." : string.Empty);
} }
catch (Exception)
{
return string.Format("{0}", value);
}
}
return string.Format("{0}", value); return string.Format("{0}", value);
} }

52
Tests/ParameterSets.Access.Tests.ps1

@ -1346,6 +1346,10 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
$accessDeepScope = , $accessDeepScope $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 = @( $accessBadShapes = @(
@{ Shape = 'a misspelled name'; Value = 'Typoo'; ValueText = 'Typoo' } @{ Shape = 'a misspelled name'; Value = 'Typoo'; ValueText = 'Typoo' }
@{ Shape = 'a blank string'; Value = ''; ValueText = $null } @{ 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 = 'an undefined number'; Value = 99; ValueText = '99' }
@{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null } @{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null }
@{ Shape = 'an array of one misspelled name'; Value = @('Typoo'); ValueText = 'Typoo' } @{ 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 = @( $accessTypeBadShapes = @(
@{ Shape = 'a misspelled name'; Value = 'Denyy'; ValueText = 'Denyy' } @{ 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 = 'an undefined number'; Value = 99; ValueText = '99' }
@{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null } @{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null }
@{ Shape = 'an array of one misspelled name'; Value = @('Denyy'); ValueText = 'Denyy' } @{ 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 = '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 two names'; Value = @('Allow', 'Deny'); ValueText = 'Allow, Deny' }
@{ Shape = 'an array of one comma list'; 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 } @{ 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') { $accessInvalidParameterCases = foreach ($cmdlet in 'Add-NTFSAccess', 'Remove-NTFSAccess') {
foreach ($shapeCase in $accessTypeBadShapes) { foreach ($shapeCase in $accessTypeBadShapes) {
@{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Shape = $shapeCase.Shape; Value = $shapeCase.Value; ValueText = $shapeCase.ValueText } @{ 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') { $accessSdInvalidParameterCases = foreach ($cmdlet in 'Add-NTFSAccess', 'Remove-NTFSAccess') {
@{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = 'Denyy'; ValueText = 'Denyy' } @{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = 'Denyy'; ValueText = 'Denyy' }
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'Typoo'; ValueText = 'Typoo' } @{ 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 = 'AppliesTo'; Value = [System.Collections.Generic.HashSet[string]]::new([string[]]@('ThisFolderAndSubfolders', 'ThisFolderAndFiles')) }
@{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = $accessDeepAccessType } @{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = $accessDeepAccessType }
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = $accessDeepScope } @{ 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 # 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 <Mode> -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 # 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 # $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. # cmdlet must not keep the default silently. A named $null is reported the same way as a bad named value.

10
Tests/ParameterSets.Audit.Tests.ps1

@ -1011,6 +1011,10 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF
$auditDeepScope = , $auditDeepScope $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 = @( $auditBadShapes = @(
@{ Shape = 'a misspelled name'; Value = 'Typoo'; ValueText = 'Typoo' } @{ Shape = 'a misspelled name'; Value = 'Typoo'; ValueText = 'Typoo' }
@{ Shape = 'a blank string'; Value = ''; ValueText = $null } @{ 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 = 'an undefined number'; Value = 99; ValueText = '99' }
@{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null } @{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null }
@{ Shape = 'an array of one misspelled name'; Value = @('Typoo'); ValueText = 'Typoo' } @{ 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 = @( $auditFlagsBadShapes = @(
@{ Shape = 'a misspelled name'; Value = 'Failurre'; ValueText = 'Failurre' } @{ 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 = 'an undefined number'; Value = 99; ValueText = '99' }
@{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null } @{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null }
@{ Shape = 'an array of one misspelled name'; Value = @('Failurre'); ValueText = 'Failurre' } @{ 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 # 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 # 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. # 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 = @('ThisFolderAndSubfolders,ThisFolderAndFiles') }
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = [System.Collections.Generic.HashSet[string]]::new([string[]]@('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 = '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 # A named $null (a literal $null, a variable, splatting, or $PSDefaultParameterValues) must stay a

Loading…
Cancel
Save