"Don't directly throw Exception" System.IO.FileSystem.AccessControl (#25696)

* Fix throw new Exception();

* Fix throw new Exception()

* Fix throw new Exception();

* Fix throw new Exception();

* Fix throw new Exception();

* Fix throw new Exception();

* Fix throw new Exception();

* Don't directly throw Exception

* test RemoveAccessRuleAll_AccessControlType_Deny_ThrowException and throw  InvalidOperationException

* change exception message

* add more descriptive exception message

Author: MarcoRossignoli

src/System.IO.FileSystem.AccessControl/src/Resources/Strings.resx

@@ -138,4 +138,10 @@
   <data name="PlatformNotSupported_AccessControl" xml:space="preserve">
     <value>Access Control List (ACL) APIs are part of resource management on Windows and are not supported on this platform.</value>
   </data>
-</root>
+  <data name="TypeUnrecognized_AccessControl" xml:space="preserve">
+    <value>AccessControlType '{0}' unrecognized</value>
+  </data>
+  <data name="InvalidOperation_RemoveFail" xml:space="preserve">
+    <value>Remove access rule failed</value>
+  </data>
+</root>

src/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/DirectoryObjectSecurity.cs

@@ -347,9 +347,8 @@ private bool ModifyAccess(AccessControlModification modification, ObjectAccessRu
                     case AccessControlModification.RemoveAll:
                         result = _securityDescriptor.DiscretionaryAcl.RemoveAccess(AccessControlType.Allow, sid, -1, InheritanceFlags.ContainerInherit, 0, ObjectAceFlags.None, Guid.Empty, Guid.Empty);
                         if (result == false)
-                        {
-                            Debug.Assert(false, "Invalid operation");
-                            throw new Exception();
+                        {                            
+                            throw new InvalidOperationException(SR.InvalidOperation_RemoveFail);
                         }
 
                         break;
@@ -393,9 +392,8 @@ private bool ModifyAccess(AccessControlModification modification, ObjectAccessRu
                     case AccessControlModification.RemoveAll:
                         result = _securityDescriptor.DiscretionaryAcl.RemoveAccess(AccessControlType.Deny, sid, -1, InheritanceFlags.ContainerInherit, 0, ObjectAceFlags.None, Guid.Empty, Guid.Empty);
                         if (result == false)
-                        {
-                            Debug.Assert(false, "Invalid operation");
-                            throw new Exception();
+                        {                            
+                            throw new InvalidOperationException(SR.InvalidOperation_RemoveFail);
                         }
 
                         break;
@@ -412,9 +410,8 @@ private bool ModifyAccess(AccessControlModification modification, ObjectAccessRu
                 }
             }
             else
-            {
-                Debug.Assert(false, "rule.AccessControlType unrecognized");
-                throw new Exception();
+            {                
+                throw new ArgumentException(SR.Format(SR.TypeUnrecognized_AccessControl, rule.AccessControlType));
             }
 
             modified = result;
@@ -488,9 +485,8 @@ private bool ModifyAudit(AccessControlModification modification, ObjectAuditRule
                 case AccessControlModification.RemoveAll:
                     result = _securityDescriptor.SystemAcl.RemoveAudit(AuditFlags.Failure | AuditFlags.Success, sid, -1, InheritanceFlags.ContainerInherit, 0, ObjectAceFlags.None, Guid.Empty, Guid.Empty);
                     if (result == false)
-                    {
-                        Debug.Assert(false, "Invalid operation");
-                        throw new Exception();
+                    {                        
+                        throw new InvalidOperationException(SR.InvalidOperation_RemoveFail);
                     }
 
                     break;

src/System.IO.FileSystem.AccessControl/tests/DirectoryObjectSecurityTests.cs

@@ -493,6 +493,23 @@ public void RemoveAccessRuleAll_AccessControlType_Allow_Succeeds()
             Assert.False(existingRules.Contains(customAccessRuleSynchronize));
         }
 
+        [Fact]        
+        public void RemoveAccessRuleAll_AccessControlType_Deny_ThrowException()
+        {
+            var descriptor = new CommonSecurityDescriptor(true, true, string.Empty);
+            var customObjectSecurity = new CustomDirectoryObjectSecurity(descriptor);
+
+            var objectTypeGuid = Guid.NewGuid();
+            var identityReference = new NTAccount(@"NT AUTHORITY\SYSTEM");
+            var customAccessRuleReadWrite = new CustomAccessRule(
+                identityReference, ReadWriteAccessMask, true, InheritanceFlags.ObjectInherit,
+                PropagationFlags.InheritOnly, objectTypeGuid, Guid.NewGuid(), AccessControlType.Deny
+                );
+
+            customObjectSecurity.AddAccessRule(customAccessRuleReadWrite);
+            AssertExtensions.Throws<InvalidOperationException, SystemException>(() => customObjectSecurity.RemoveAccessRuleAll(customAccessRuleReadWrite));
+        }
+
         [Fact]
         public void RemoveAccessRuleAll_AccessControlType_Deny_Succeeds()
         {