fix(access): ignore a failure to release the enumerator when a value is read

The previous commit made a sequence whose enumerator throws in Dispose after
the second element count as a list, and left every other count to the catch,
which hands the collection to the conversion of PowerShell. A sequence of one
element whose enumerator fails in Dispose therefore got through when that
element was a comma list or an array of two values: PowerShell combined the
numbers bit by bit and the entry was created with a value that nobody named.
The reading is done with an explicit enumerator that is released in its own
guard now, so a failing release changes nothing of what was read, whatever the
number of elements. The catch is back to what it was: a reading that fails
leaves the collection to the conversion, which rejects it with its own error.

Tests, written first: 8 rows failed on the previous commit in all four
configurations and none fails now (a compiled stand-in whose enumerator throws
in Dispose; a sequence of one comma list and of one array of two values, for
AccessType and AppliesTo, named and piped). Two rows pin that a sequence of
one valid value stays valid.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>

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; one whose enumeration fails after its second element is a list all the same.
+        // convert with its own error. A failure to release the enumerator after the elements were read changes nothing.
         private static object TakeOneValue(object value, out bool isList)
         {
             isList = false;
@@ -119,9 +119,11 @@ namespace NTFSSecurity
 
                 var count = 0;
                 object only = null;
+                IEnumerator enumerator = null;
                 try
                 {
-                    foreach (var item in collection)
+                    enumerator = collection.GetEnumerator();
+                    while (enumerator.MoveNext())
                     {
                         count++;
                         if (count > 1)
@@ -129,15 +131,17 @@ namespace NTFSSecurity
                             break;
                         }
 
-                        only = item;
+                        only = enumerator.Current;
                     }
                 }
                 catch (Exception)
                 {
-                    // The loop releases the enumerator when it stops after the second element, and that can fail.
-                    isList = count > 1;
                     return value;
                 }
+                finally
+                {
+                    Release(enumerator);
+                }
 
                 if (count == 1)
                 {
@@ -153,6 +157,25 @@ namespace NTFSSecurity
             return value;
         }
 
+        // Releasing the enumerator of a collection that was read can fail (only a custom enumerator does), and that does
+        // not change what was read.
+        private static void Release(IEnumerator enumerator)
+        {
+            var disposable = enumerator as IDisposable;
+            if (disposable == null)
+            {
+                return;
+            }
+
+            try
+            {
+                disposable.Dispose();
+            }
+            catch (Exception)
+            {
+            }
+        }
+
         // 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. 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

Tests/ParameterSets.Access.Tests.ps1

@@ -1834,11 +1834,15 @@ namespace NtfsSecurityTests
             }
         }
 
-        It 'Should reject a list of two values for <Parameter> given as <Mode>, and change nothing' -ForEach @(
+        It 'Should reject <Shape> for <Parameter> given as <Mode>, and change nothing' -ForEach @(
-            @{ Parameter = 'AccessType'; Mode = 'named'; Values = @('Allow', 'Deny') }
+            foreach ($mode in 'named', 'piped') {
-            @{ Parameter = 'AccessType'; Mode = 'piped'; Values = @('Allow', 'Deny') }
+                @{ Parameter = 'AccessType'; Mode = $mode; Shape = 'a sequence of two values'; Values = @('Allow', 'Deny') }
-            @{ Parameter = 'AppliesTo'; Mode = 'named'; Values = @('SubfoldersOnly', 'FilesOnly') }
+                @{ Parameter = 'AccessType'; Mode = $mode; Shape = 'a sequence of one comma list'; Values = @('Allow,Deny') }
-            @{ Parameter = 'AppliesTo'; Mode = 'piped'; Values = @('SubfoldersOnly', 'FilesOnly') }
+                @{ Parameter = 'AccessType'; Mode = $mode; Shape = 'a sequence of one array of two values'; Values = , @('Allow', 'Deny') }
+                @{ Parameter = 'AppliesTo'; Mode = $mode; Shape = 'a sequence of two values'; Values = @('SubfoldersOnly', 'FilesOnly') }
+                @{ Parameter = 'AppliesTo'; Mode = $mode; Shape = 'a sequence of one comma list'; Values = @('ThisFolderOnly,FilesOnly') }
+                @{ Parameter = 'AppliesTo'; Mode = $mode; Shape = 'a sequence of one array of two values'; Values = , @('SubfoldersOnly', 'FilesOnly') }
+            }
         ) {
             $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'ReleaseFails' -Directory
             $before = (Get-Acl -LiteralPath $folder).Sddl
@@ -1865,6 +1869,26 @@ namespace NtfsSecurityTests
             $caughtId | Should -BeLike 'InvalidParameterValue,*'
             (Get-Acl -LiteralPath $folder).Sddl | Should -BeExactly $before
         }
+
+        # What was read is what counts: a failing release does not turn one valid value into an error.
+        It 'Should still accept a sequence of one valid value for -AccessType given as <Mode>' -ForEach @(
+            @{ Mode = 'named' }
+            @{ Mode = 'piped' }
+        ) {
+            $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'ReleaseFailsValid' -Directory
+            $sequence = [NtfsSecurityTests.SequenceWhoseEnumeratorFailsOnDispose]::new([object[]] @('Deny'))
+
+            if ($Mode -eq 'named') {
+                Add-NTFSAccess -Path $folder -Account 'S-1-1-0' -AccessRights ReadData -AccessType $sequence -ErrorAction Stop
+            }
+            else {
+                [pscustomobject]@{ Path = $folder; Account = 'S-1-1-0'; AccessRights = 'ReadData'; AccessType = $sequence } | Add-NTFSAccess -ErrorAction Stop
+            }
+
+            $rule = @((Get-Acl -LiteralPath $folder).GetAccessRules($true, $false, $sidType) | Where-Object -FilterScript { $_.IdentityReference.Value -eq 'S-1-1-0' })
+            $rule | Should -HaveCount 1
+            $rule[0].AccessControlType | Should -Be ([System.Security.AccessControl.AccessControlType]::Deny)
+        }
     }
 
     # A typed parameter ends a named $null (for example a variable that misspells a property, such as -AccessType