diff --git a/NTFSSecurity/ParameterConversion.cs b/NTFSSecurity/ParameterConversion.cs index f7d1e49..56a2457 100644 --- a/NTFSSecurity/ParameterConversion.cs +++ b/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 diff --git a/Tests/ParameterSets.Access.Tests.ps1 b/Tests/ParameterSets.Access.Tests.ps1 index 4950ccc..d4e9a20 100644 --- a/Tests/ParameterSets.Access.Tests.ps1 +++ b/Tests/ParameterSets.Access.Tests.ps1 @@ -1834,11 +1834,15 @@ namespace NtfsSecurityTests } } - It 'Should reject a list of two values for given as , 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') } + It 'Should reject for given as , and change nothing' -ForEach @( + foreach ($mode in 'named', 'piped') { + @{ Parameter = 'AccessType'; Mode = $mode; Shape = 'a sequence of two values'; Values = @('Allow', 'Deny') } + @{ Parameter = 'AccessType'; Mode = $mode; Shape = 'a sequence of one comma list'; Values = @('Allow,Deny') } + @{ 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 ' -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