Browse Source

fix(access): reject a list of any kind of collection for -AccessType and -AppliesTo

The rejection of a list for the enumerations that are not flags
enumerations looked only at a string with a comma and at an IList of more
than one element. A collection that is not an IList (a HashSet, a Queue, the
keys of a hashtable, a LINQ sequence), a collection of one element that
holds a comma list (@('Allow,Deny'), such as a JSON array of one string), and
a collection that holds a collection of several elements still reached
LanguagePrimitives.ConvertTo, which added up the numbers: -AppliesTo
$scopes.Keys created a ThisFolderAndFiles entry that nobody named.

ParameterConversion takes the one value out of the argument now: a wrapped
PSObject counts as what it wraps, a collection of one element converts as
its element (also when that element is a collection of one element), and a
collection of several elements or an element with a comma is the list that is
rejected. A dictionary, an empty collection, and a collection that cannot be
enumerated keep going to the conversion, which reports them as before. A
blank value inside a collection of one element is reported as blank.

The tests add the shapes to the bad-value tables of the four cmdlets (32
rows, red before the change and green after it in four configurations) and
pin that a list of one name and a hash set of one name stay valid. The
valid-value matrices of rc8 and this build still differ in the nine list rows
only, and a probe of 50 shapes shows every collection of several values
rejected and every single value accepted, in both editions.

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
e39aeed1b2
  1. 97
      NTFSSecurity/ParameterConversion.cs
  2. 20
      Tests/ParameterSets.Access.Tests.ps1
  3. 12
      Tests/ParameterSets.Audit.Tests.ps1

97
NTFSSecurity/ParameterConversion.cs

@ -18,8 +18,9 @@ namespace NTFSSecurity
/// Converts the raw value of a parameter to T, the way the PowerShell binder converts an argument for a
/// parameter of that type: names in any case, a comma-separated list for a flags enumeration, a number, or
/// an enumeration instance. An enumeration that is not a flags enumeration takes one value: PowerShell would
/// add up the numbers of a list for it, which selects a value that the caller did not name, so a list is
/// rejected for it.
/// add up the numbers of a list for it, so that one of the values wins silently or a value that the caller
/// did not name is chosen, so a list is rejected for it, in whatever kind of collection it comes. A
/// collection of one element converts as that element.
/// </summary>
/// <param name="value">
/// The raw value of the parameter. Not null: the caller keeps the default of the parameter for null instead
@ -35,16 +36,12 @@ namespace NTFSSecurity
/// </exception>
internal static T Convert<T>(object value, string parameterName)
{
var text = value as string;
if (text != null && string.IsNullOrWhiteSpace(text))
{
throw new ArgumentException(string.Format(
"The value for parameter -{0} is blank. {1}", parameterName, DescribeValidValues(typeof(T))));
}
var convertible = value;
if (!typeof(T).IsDefined(typeof(FlagsAttribute), false))
{
var list = DescribeList(value);
string list;
convertible = TakeOneValue(value, out list);
if (list != null)
{
throw new ArgumentException(string.Format(
@ -53,9 +50,16 @@ namespace NTFSSecurity
}
}
var text = convertible as string;
if (text != null && string.IsNullOrWhiteSpace(text))
{
throw new ArgumentException(string.Format(
"The value for parameter -{0} is blank. {1}", parameterName, DescribeValidValues(typeof(T))));
}
try
{
return (T)LanguagePrimitives.ConvertTo(value, typeof(T));
return (T)LanguagePrimitives.ConvertTo(convertible, typeof(T));
}
catch (Exception ex)
{
@ -78,32 +82,65 @@ namespace NTFSSecurity
return string.Format("The value for parameter -{0} is null. {1}", parameterName, DescribeValidValues(typeof(T)));
}
// The text of a list that the conversion would add up, for the message: a string that holds a comma, or a
// collection of more than one element (a wrapped PSObject counts as what it wraps). Null for any other value,
// such as a single name or a collection of one element, which convert to that one value.
private static string DescribeList(object value)
// The one value that the conversion takes from the argument of an enumeration that is not a flags enumeration:
// the argument itself, or the only element of a collection of any kind (a wrapped PSObject counts as what it
// wraps, and a collection that holds a collection is opened too). When the argument holds more than one
// value, list is its text for the message: a string that holds a comma, a collection of several elements, or
// a collection of one element that holds such a list; the returned value is then not used. A dictionary, an
// empty collection, or a collection that cannot be enumerated is returned as it is, for the conversion to
// reject or convert with its own error.
private static object TakeOneValue(object value, out string list)
{
var unwrapped = value is PSObject ? ((PSObject)value).BaseObject : value;
list = null;
var current = value;
var text = unwrapped as string;
if (text != null)
while (true)
{
return text.IndexOf(',') >= 0 ? text : null;
}
var unwrapped = current is PSObject ? ((PSObject)current).BaseObject : current;
var list = unwrapped as IList;
if (list == null || list.Count < 2)
{
return null;
}
var text = unwrapped as string;
if (text != null)
{
if (text.IndexOf(',') >= 0)
{
list = text;
}
var items = new string[list.Count];
for (var i = 0; i < items.Length; i++)
{
items[i] = string.Format("{0}", list[i]);
}
return unwrapped;
}
var collection = unwrapped as IEnumerable;
if (collection == null || unwrapped is IDictionary)
{
return unwrapped;
}
var items = new System.Collections.Generic.List<object>();
try
{
foreach (var item in collection)
{
items.Add(item);
}
}
catch (Exception)
{
return value;
}
if (items.Count == 1)
{
current = items[0];
continue;
}
return string.Join(", ", items);
if (items.Count > 1)
{
list = string.Join(", ", items.ConvertAll(item => string.Format("{0}", item)));
}
return value;
}
}
// The valid names of the enumeration, in declaration order, with a note for a flags enumeration that a

20
Tests/ParameterSets.Access.Tests.ps1

@ -1352,16 +1352,24 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
@{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; 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' }
@{ Shape = 'an array that holds an array of two names'; Value = , @('Allow', 'Deny'); ValueText = 'Allow, Deny' }
@{ Shape = 'a hash set of two names'; Value = [System.Collections.Generic.HashSet[string]]::new([string[]]@('Allow', 'Deny')); ValueText = $null }
@{ Shape = 'the keys of a hashtable'; Value = (@{ Allow = 1; Deny = 2 }).Keys; ValueText = $null }
)
# AccessType and AppliesTo name one value each, not a combination: PowerShell would add the numbers of a list, and
# an entry for a scope that the caller did not name would follow.
# AccessType and AppliesTo name one value each, not a combination: PowerShell would add the numbers of a list, so that
# one of the values wins silently or, for AppliesTo, a scope that the caller did not name is chosen. Every kind of
# collection counts, also a collection of one element that holds a list.
$accessAppliesToListShapes = @(
@{ Shape = 'a comma list of two names'; Value = 'ThisFolderAndSubfolders,ThisFolderAndFiles'; ValueText = 'ThisFolderAndSubfolders,ThisFolderAndFiles' }
@{ Shape = 'an array of two names'; Value = @('ThisFolderAndSubfolders', 'ThisFolderAndFiles'); ValueText = 'ThisFolderAndSubfolders, ThisFolderAndFiles' }
@{ Shape = 'an array of one comma list'; Value = @('ThisFolderAndSubfolders,ThisFolderAndFiles'); ValueText = 'ThisFolderAndSubfolders,ThisFolderAndFiles' }
@{ Shape = 'a hash set of two names'; Value = [System.Collections.Generic.HashSet[string]]::new([string[]]@('ThisFolderAndSubfolders', 'ThisFolderAndFiles')); ValueText = $null }
@{ Shape = 'the keys of a hashtable'; Value = (@{ ThisFolderAndSubfolders = 1; ThisFolderAndFiles = 2 }).Keys; ValueText = $null }
)
# The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (7 + 3 * 5 + 2) = 48 rows.
# The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (11 + 3 * 5 + 5) = 62 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 }
@ -1395,8 +1403,12 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'Typoo' }
@{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = 'Allow,Deny' }
@{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = @('Allow', 'Deny') }
@{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = @('Allow,Deny') }
@{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = [System.Collections.Generic.HashSet[string]]::new([string[]]@('Allow', 'Deny')) }
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'ThisFolderAndSubfolders,ThisFolderAndFiles' }
@{ 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')) }
}
# A named $null (a literal $null, a variable, an alias, splatting, or $PSDefaultParameterValues) must
@ -1415,6 +1427,8 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
@{ Description = 'the enum value'; Value = [System.Security.AccessControl.AccessControlType]::Deny }
@{ Description = 'an integer'; Value = 1 }
@{ Description = 'an array of one name'; Value = @('Deny') }
@{ Description = 'a list of one name'; Value = [System.Collections.Generic.List[string]]::new([string[]]@('Deny')) }
@{ Description = 'a hash set of one name'; Value = [System.Collections.Generic.HashSet[string]]::new([string[]]@('Deny')) }
)
$accessValidFlagsShapeCases = @(

12
Tests/ParameterSets.Audit.Tests.ps1

@ -1019,12 +1019,16 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF
@{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null }
)
# The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (5 + 3 * 5 + 2) = 42 rows.
# AppliesTo names one value, not a combination: PowerShell would add the numbers of a list, and an entry for a
# scope that the caller did not name would follow. AuditFlags and the two flags parameters take a list.
# The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (5 + 3 * 5 + 5) = 50 rows.
# AppliesTo names one value, not a combination: PowerShell would add the numbers of a list, 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.
$auditAppliesToListShapes = @(
@{ Shape = 'a comma list of two names'; Value = 'ThisFolderAndSubfolders,ThisFolderAndFiles'; ValueText = 'ThisFolderAndSubfolders,ThisFolderAndFiles' }
@{ Shape = 'an array of two names'; Value = @('ThisFolderAndSubfolders', 'ThisFolderAndFiles'); ValueText = 'ThisFolderAndSubfolders, ThisFolderAndFiles' }
@{ Shape = 'an array of one comma list'; Value = @('ThisFolderAndSubfolders,ThisFolderAndFiles'); ValueText = 'ThisFolderAndSubfolders,ThisFolderAndFiles' }
@{ Shape = 'a hash set of two names'; Value = [System.Collections.Generic.HashSet[string]]::new([string[]]@('ThisFolderAndSubfolders', 'ThisFolderAndFiles')); ValueText = $null }
@{ Shape = 'the keys of a hashtable'; Value = (@{ ThisFolderAndSubfolders = 1; ThisFolderAndFiles = 2 }).Keys; ValueText = $null }
)
$auditInvalidParameterCases = foreach ($cmdlet in 'Add-NTFSAudit', 'Remove-NTFSAudit') {
@ -1060,6 +1064,8 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'Typoo' }
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'ThisFolderAndSubfolders,ThisFolderAndFiles' }
@{ 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')) }
}
# A named $null (a literal $null, a variable, splatting, or $PSDefaultParameterValues) must stay a

Loading…
Cancel
Save