From 0d23bdd2133d32867e7c83cc616d04243a55cf33 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Tue, 4 Feb 2020 17:36:54 -0800 Subject: [PATCH 1/4] Annotate System.IO.FileSystem.AccessControl for nullable --- .../System.IO.FileSystem.AccessControl.csproj | 1 + .../src/System/IO/FileSystemAclExtensions.cs | 2 +- .../AccessControl/DirectoryObjectSecurity.cs | 33 +++++++++---------- .../Security/AccessControl/FileSecurity.cs | 2 +- .../AccessControl/FileSystemSecurity.cs | 20 +++++------ 5 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj b/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj index 2a7b759bed04b..41d5dbe0fb7d0 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj @@ -5,6 +5,7 @@ SR.PlatformNotSupported_AccessControl net461-Windows_NT-Debug;net461-Windows_NT-Release;$(NetCoreAppCurrent)-Windows_NT-Debug;$(NetCoreAppCurrent)-Windows_NT-Release;$(NetFrameworkCurrent)-Windows_NT-Debug;$(NetFrameworkCurrent)-Windows_NT-Release;netstandard2.0-Debug;netstandard2.0-Release;netstandard2.0-Windows_NT-Debug;netstandard2.0-Windows_NT-Release annotations + enable diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs index e660091c41712..80d3dd72a4ac6 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs @@ -322,7 +322,7 @@ private static void ValidateFileHandle(SafeFileHandle handle, string fullPath) // probably be consistent w/ every other directory. int errorCode = Marshal.GetLastWin32Error(); - if (errorCode == Interop.Errors.ERROR_PATH_NOT_FOUND && fullPath.Length == Path.GetPathRoot(fullPath).Length) + if (errorCode == Interop.Errors.ERROR_PATH_NOT_FOUND && fullPath.Length == Path.GetPathRoot(fullPath)!.Length) { errorCode = Interop.Errors.ERROR_ACCESS_DENIED; } diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/DirectoryObjectSecurity.cs b/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/DirectoryObjectSecurity.cs index 7bfd44f1b85f8..6ec2ff89ffa2d 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/DirectoryObjectSecurity.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/DirectoryObjectSecurity.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Diagnostics; using System.Security.Principal; namespace System.Security.AccessControl @@ -58,7 +59,7 @@ private AuthorizationRuleCollection GetRules(bool access, bool includeExplicit, throw new ArgumentException(SR.Arg_MustBeIdentityReferenceType, nameof(targetType)); } - CommonAcl acl = null; + CommonAcl? acl = null; if (access) { @@ -83,7 +84,7 @@ private AuthorizationRuleCollection GetRules(bool access, bool includeExplicit, return result; } - IdentityReferenceCollection irTarget = null; + IdentityReferenceCollection? irTarget = null; if (targetType != typeof(SecurityIdentifier)) { @@ -100,7 +101,7 @@ private AuthorizationRuleCollection GetRules(bool access, bool includeExplicit, // A better way would be to have an internal method that would canonicalize the ACL // and call it once, then use the RawAcl. // - QualifiedAce ace = acl[i] as QualifiedAce; + QualifiedAce? ace = acl[i] as QualifiedAce; if (ace == null) { @@ -150,7 +151,7 @@ private AuthorizationRuleCollection GetRules(bool access, bool includeExplicit, // A better way would be to have an internal method that would canonicalize the ACL // and call it once, then use the RawAcl. // - QualifiedAce ace = acl[i] as CommonAce; + QualifiedAce? ace = acl[i] as CommonAce; if (ace == null) { @@ -189,7 +190,7 @@ private AuthorizationRuleCollection GetRules(bool access, bool includeExplicit, if ((includeExplicit && ((ace.AceFlags & AceFlags.Inherited) == 0)) || (includeInherited && ((ace.AceFlags & AceFlags.Inherited) != 0))) { - IdentityReference iref = (targetType == typeof(SecurityIdentifier)) ? ace.SecurityIdentifier : irTarget[i]; + IdentityReference iref = (targetType == typeof(SecurityIdentifier)) ? ace.SecurityIdentifier : irTarget![i]; if (access) { @@ -204,15 +205,13 @@ private AuthorizationRuleCollection GetRules(bool access, bool includeExplicit, type = AccessControlType.Deny; } - if (ace is ObjectAce) + if (ace is ObjectAce objectAce) { - ObjectAce objectAce = ace as ObjectAce; - result.AddRule(AccessRuleFactory(iref, objectAce.AccessMask, objectAce.IsInherited, objectAce.InheritanceFlags, objectAce.PropagationFlags, type, objectAce.ObjectAceType, objectAce.InheritedObjectAceType)); } else { - CommonAce commonAce = ace as CommonAce; + CommonAce? commonAce = ace as CommonAce; if (commonAce == null) { @@ -224,15 +223,13 @@ private AuthorizationRuleCollection GetRules(bool access, bool includeExplicit, } else { - if (ace is ObjectAce) + if (ace is ObjectAce objectAce) { - ObjectAce objectAce = ace as ObjectAce; - result.AddRule(AuditRuleFactory(iref, objectAce.AccessMask, objectAce.IsInherited, objectAce.InheritanceFlags, objectAce.PropagationFlags, objectAce.AuditFlags, objectAce.ObjectAceType, objectAce.InheritedObjectAceType)); } else { - CommonAce commonAce = ace as CommonAce; + CommonAce? commonAce = ace as CommonAce; if (commonAce == null) { @@ -291,8 +288,9 @@ private bool ModifyAccess(AccessControlModification modification, ObjectAccessRu } } - SecurityIdentifier sid = rule.IdentityReference.Translate(typeof(SecurityIdentifier)) as SecurityIdentifier; + SecurityIdentifier sid = (SecurityIdentifier)rule.IdentityReference.Translate(typeof(SecurityIdentifier)); + Debug.Assert(SecurityDescriptor.DiscretionaryAcl != null); if (rule.AccessControlType == AccessControlType.Allow) { switch (modification) @@ -431,8 +429,9 @@ private bool ModifyAudit(AccessControlModification modification, ObjectAuditRule } } - SecurityIdentifier sid = rule.IdentityReference.Translate(typeof(SecurityIdentifier)) as SecurityIdentifier; + SecurityIdentifier sid = (SecurityIdentifier)rule.IdentityReference.Translate(typeof(SecurityIdentifier)); + Debug.Assert(SecurityDescriptor.SystemAcl != null); switch (modification) { case AccessControlModification.Add: @@ -504,7 +503,7 @@ protected override bool ModifyAccess(AccessControlModification modification, Acc // SR.AccessControl_InvalidAccessRuleType, // "rule"); //} - return ModifyAccess(modification, rule as ObjectAccessRule, out modified); + return ModifyAccess(modification, (ObjectAccessRule)rule, out modified); } protected override bool ModifyAudit(AccessControlModification modification, AuditRule rule, out bool modified) @@ -516,7 +515,7 @@ protected override bool ModifyAudit(AccessControlModification modification, Audi // SR.AccessControl_InvalidAuditRuleType, // "rule"); //} - return ModifyAudit(modification, rule as ObjectAuditRule, out modified); + return ModifyAudit(modification, (ObjectAuditRule)rule, out modified); } #endregion diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSecurity.cs b/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSecurity.cs index ac531a937d5e1..8a4a089f1fd4b 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSecurity.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSecurity.cs @@ -19,7 +19,7 @@ public FileSecurity(string fileName, AccessControlSections includeSections) { } - internal FileSecurity(SafeFileHandle handle, AccessControlSections includeSections) + internal FileSecurity(SafeFileHandle? handle, AccessControlSections includeSections) : base(false, handle, includeSections, false) { } diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSystemSecurity.cs b/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSystemSecurity.cs index e17cbf542322f..5d20525cad592 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSystemSecurity.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSystemSecurity.cs @@ -14,23 +14,23 @@ public abstract class FileSystemSecurity : NativeObjectSecurity private const ResourceType s_ResourceType = ResourceType.FileObject; internal FileSystemSecurity(bool isContainer) - : base(isContainer, s_ResourceType, _HandleErrorCode, isContainer) + : base(isContainer, s_ResourceType, _HandleErrorCode!, isContainer) { } internal FileSystemSecurity(bool isContainer, string name, AccessControlSections includeSections, bool isDirectory) - : base(isContainer, s_ResourceType, PathInternal.EnsureExtendedPrefixIfNeeded(Path.GetFullPath(name)), includeSections, _HandleErrorCode, isDirectory) + : base(isContainer, s_ResourceType, PathInternal.EnsureExtendedPrefixIfNeeded(Path.GetFullPath(name)), includeSections, _HandleErrorCode!, isDirectory) { } - internal FileSystemSecurity(bool isContainer, SafeFileHandle handle, AccessControlSections includeSections, bool isDirectory) - : base(isContainer, s_ResourceType, handle, includeSections, _HandleErrorCode, isDirectory) + internal FileSystemSecurity(bool isContainer, SafeFileHandle? handle, AccessControlSections includeSections, bool isDirectory) + : base(isContainer, s_ResourceType, handle, includeSections, _HandleErrorCode!, isDirectory) { } - private static Exception _HandleErrorCode(int errorCode, string name, SafeHandle handle, object context) + private static Exception? _HandleErrorCode(int errorCode, string? name, SafeHandle? handle, object? context) { - Exception exception = null; + Exception? exception = null; switch (errorCode) { @@ -176,9 +176,7 @@ public bool RemoveAccessRule(FileSystemAccessRule rule) for (int i = 0; i < rules.Count; i++) { - FileSystemAccessRule fsrule = rules[i] as FileSystemAccessRule; - - if ((fsrule != null) && (fsrule.FileSystemRights == rule.FileSystemRights) + if ((rules[i] is FileSystemAccessRule fsrule) && (fsrule.FileSystemRights == rule.FileSystemRights) && (fsrule.IdentityReference == rule.IdentityReference) && (fsrule.AccessControlType == rule.AccessControlType)) { @@ -222,9 +220,7 @@ public void RemoveAccessRuleSpecific(FileSystemAccessRule rule) for (int i = 0; i < rules.Count; i++) { - FileSystemAccessRule fsrule = rules[i] as FileSystemAccessRule; - - if ((fsrule != null) && (fsrule.FileSystemRights == rule.FileSystemRights) + if ((rules[i] is FileSystemAccessRule fsrule) && (fsrule.FileSystemRights == rule.FileSystemRights) && (fsrule.IdentityReference == rule.IdentityReference) && (fsrule.AccessControlType == rule.AccessControlType)) { From 2b387f7c70e37766992263e7ae6cff62e841eba7 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Wed, 5 Feb 2020 16:12:12 -0800 Subject: [PATCH 2/4] Enabling nullable in ref, removign unneeded bangs --- .../ref/System.IO.FileSystem.AccessControl.csproj | 1 + .../src/System/Security/AccessControl/FileSystemSecurity.cs | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.csproj b/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.csproj index dbc2ca8c72681..c4c12d684a59b 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.csproj +++ b/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.csproj @@ -2,6 +2,7 @@ true net461-Debug;net461-Debug;net461-Release;net461-Release;$(NetFrameworkCurrent)-Debug;$(NetFrameworkCurrent)-Release;netstandard2.0-Debug;netstandard2.0-Release + enable diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSystemSecurity.cs b/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSystemSecurity.cs index 5d20525cad592..311ded95de0a7 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSystemSecurity.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System/Security/AccessControl/FileSystemSecurity.cs @@ -14,17 +14,17 @@ public abstract class FileSystemSecurity : NativeObjectSecurity private const ResourceType s_ResourceType = ResourceType.FileObject; internal FileSystemSecurity(bool isContainer) - : base(isContainer, s_ResourceType, _HandleErrorCode!, isContainer) + : base(isContainer, s_ResourceType, _HandleErrorCode, isContainer) { } internal FileSystemSecurity(bool isContainer, string name, AccessControlSections includeSections, bool isDirectory) - : base(isContainer, s_ResourceType, PathInternal.EnsureExtendedPrefixIfNeeded(Path.GetFullPath(name)), includeSections, _HandleErrorCode!, isDirectory) + : base(isContainer, s_ResourceType, PathInternal.EnsureExtendedPrefixIfNeeded(Path.GetFullPath(name)), includeSections, _HandleErrorCode, isDirectory) { } internal FileSystemSecurity(bool isContainer, SafeFileHandle? handle, AccessControlSections includeSections, bool isDirectory) - : base(isContainer, s_ResourceType, handle, includeSections, _HandleErrorCode!, isDirectory) + : base(isContainer, s_ResourceType, handle, includeSections, _HandleErrorCode, isDirectory) { } From 22bdb7adea76c5b12d630dfd7661a15eeaec415e Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 6 Feb 2020 12:37:46 -0800 Subject: [PATCH 3/4] Remove annotations --- .../src/System.IO.FileSystem.AccessControl.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj b/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj index 41d5dbe0fb7d0..f5ca42fcf40a3 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj @@ -4,7 +4,6 @@ true SR.PlatformNotSupported_AccessControl net461-Windows_NT-Debug;net461-Windows_NT-Release;$(NetCoreAppCurrent)-Windows_NT-Debug;$(NetCoreAppCurrent)-Windows_NT-Release;$(NetFrameworkCurrent)-Windows_NT-Debug;$(NetFrameworkCurrent)-Windows_NT-Release;netstandard2.0-Debug;netstandard2.0-Release;netstandard2.0-Windows_NT-Debug;netstandard2.0-Windows_NT-Release - annotations enable From 5381a49f450acccab26cf2c0f6b2f70de6a1c97a Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 6 Feb 2020 13:34:44 -0800 Subject: [PATCH 4/4] Update formatting --- .../ref/System.IO.FileSystem.AccessControl.csproj | 2 +- .../src/System.IO.FileSystem.AccessControl.csproj | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.csproj b/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.csproj index f1d3c14872e86..ef697c4d2cb55 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.csproj +++ b/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.csproj @@ -3,7 +3,7 @@ true netstandard2.0;$(NetFrameworkCurrent);net461 true - enable + enable diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj b/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj index b2bf966c38cb6..3f9c4d8fe6bff 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj @@ -1,4 +1,4 @@ - + true true @@ -6,7 +6,7 @@ netstandard2.0;netstandard2.0-Windows_NT;net461-Windows_NT;$(NetCoreAppCurrent)-Windows_NT;$(NetFrameworkCurrent)-Windows_NT true true - enable + enable