Browse Source

fix(access): count a sequence as a list when its enumerator fails after the second element

The check of a list for -AccessType and -AppliesTo stops reading a collection
after the second element. The loop then releases the enumerator, and when that
failed the catch handed the collection to the conversion of PowerShell with
isList still false. PowerShell combines the numbers of a list bit by bit, so a
sequence of Allow and Deny whose enumerator throws in Dispose created a Deny
entry, and one of SubfoldersOnly and FilesOnly a scope that nobody named. The
catch keeps what the loop knows: two elements read make a list, which is
rejected like every other list. Only a custom enumerator can do this; the last
review of round 2 found it.

Tests, written first: 4 rows failed on the previous commit in all four
configurations and none fails now (a compiled stand-in whose enumerator throws
in Dispose, for AccessType and AppliesTo, named and piped).

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
b436423ab8
  1. 4
      NTFSSecurity/ParameterConversion.cs
  2. 92
      Tests/ParameterSets.Access.Tests.ps1

4
NTFSSecurity/ParameterConversion.cs

@ -94,7 +94,7 @@ namespace NTFSSecurity
// a long or endless sequence is not read to its end), a collection of one element that holds such a list, or
// collections nested deeper than MaxNesting; 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.
// convert with its own error; one whose enumeration fails after its second element is a list all the same.
private static object TakeOneValue(object value, out bool isList)
{
isList = false;
@ -134,6 +134,8 @@ namespace NTFSSecurity
}
catch (Exception)
{
// The loop releases the enumerator when it stops after the second element, and that can fail.
isList = count > 1;
return value;
}

92
Tests/ParameterSets.Access.Tests.ps1

@ -1775,6 +1775,98 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
}
}
# A list is rejected when the conversion has read its second element. The loop that reads it then releases the
# enumerator, and an enumerator that fails there must not turn the list into a value that PowerShell converts.
Context 'A sequence whose enumerator fails when it is released' {
BeforeAll {
if (-not ('NtfsSecurityTests.SequenceWhoseEnumeratorFailsOnDispose' -as [type])) {
Add-Type -TypeDefinition @'
namespace NtfsSecurityTests
{
public sealed class SequenceWhoseEnumeratorFailsOnDispose : System.Collections.IEnumerable
{
private readonly object[] items;
public SequenceWhoseEnumeratorFailsOnDispose(params object[] items)
{
this.items = items;
}
public System.Collections.IEnumerator GetEnumerator()
{
return new Enumerator(this.items);
}
private sealed class Enumerator : System.Collections.IEnumerator, System.IDisposable
{
private readonly object[] items;
private int index = -1;
public Enumerator(object[] items)
{
this.items = items;
}
public object Current
{
get { return this.items[this.index]; }
}
public bool MoveNext()
{
this.index++;
return this.index < this.items.Length;
}
public void Reset()
{
this.index = -1;
}
public void Dispose()
{
throw new System.InvalidOperationException("The enumerator fails when it is released.");
}
}
}
}
'@
}
}
It 'Should reject a list of two values for <Parameter> given as <Mode>, and change nothing' -ForEach @(
@{ Parameter = 'AccessType'; Mode = 'named'; Values = @('Allow', 'Deny') }
@{ Parameter = 'AccessType'; Mode = 'piped'; Values = @('Allow', 'Deny') }
@{ Parameter = 'AppliesTo'; Mode = 'named'; Values = @('SubfoldersOnly', 'FilesOnly') }
@{ Parameter = 'AppliesTo'; Mode = 'piped'; Values = @('SubfoldersOnly', 'FilesOnly') }
) {
$folder = New-TestSandboxItem -Sandbox $sandbox -Name 'ReleaseFails' -Directory
$before = (Get-Acl -LiteralPath $folder).Sddl
$sequence = [NtfsSecurityTests.SequenceWhoseEnumeratorFailsOnDispose]::new([object[]] $Values)
$caughtId = $null
if ($Mode -eq 'named') {
$namedArguments = @{ Path = $folder; Account = 'S-1-1-0'; AccessRights = 'ReadData' }
$namedArguments[$Parameter] = $sequence
try {
Add-NTFSAccess @namedArguments -ErrorAction Stop
}
catch {
$caughtId = $_.FullyQualifiedErrorId
}
}
else {
$properties = @{ Path = $folder; Account = 'S-1-1-0'; AccessRights = 'ReadData' }
$properties[$Parameter] = $sequence
[pscustomobject] $properties | Add-NTFSAccess -ErrorVariable pipelineErrors -ErrorAction SilentlyContinue
$caughtId = @($pipelineErrors)[0].FullyQualifiedErrorId
}
$caughtId | Should -BeLike 'InvalidParameterValue,*'
(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.

Loading…
Cancel
Save