Verified the null checking in the extension methods. (dotnet#316)

* Verified the null checking in the extension methods. * Apply suggestions from code review Co-Authored-By: Carlos Sanchez Lopez <[email protected]> * Updated Test to check for named parameters. Removed additional braces * This fixes null checks in NetFX. Adds equivalent null checks to NetFX. Adds back Allman style braces.
alexchro93 · Dec 11, 2019 · 071da4c · 071da4c
1 parent 91a7174
commit 071da4c
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 4 deletions.
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
@@ -24,7 +24,9 @@ public static DirectorySecurity GetAccessControl(this DirectoryInfo directoryInf
         public static void SetAccessControl(this DirectoryInfo directoryInfo, DirectorySecurity directorySecurity)
         {
             if (directorySecurity == null)
+            {
                 throw new ArgumentNullException(nameof(directorySecurity));
+            }
 
             string fullPath = Path.GetFullPath(directoryInfo.FullName);
             directorySecurity.Persist(fullPath);
@@ -43,30 +45,56 @@ public static FileSecurity GetAccessControl(this FileInfo fileInfo, AccessContro
         public static void SetAccessControl(this FileInfo fileInfo, FileSecurity fileSecurity)
         {
             if (fileSecurity == null)
+            {
                 throw new ArgumentNullException(nameof(fileSecurity));
-
+            }
             string fullPath = Path.GetFullPath(fileInfo.FullName);
             // Appropriate security check should be done for us by FileSecurity.
             fileSecurity.Persist(fullPath);
         }
 
+        /// <summary>
+        /// This extension method for FileStream returns a FileSecurity object containing security descriptors from the Access, Owner, and Group AccessControlSections.
+        /// </summary>
+        /// <param name="fileStream">An object that represents the file for retrieving security descriptors from</param>
+        /// <exception cref="ArgumentNullException"><paramref name="fileStream" /> is <see langword="null" />.</exception>
+        /// <exception cref="ObjectDisposedException">The file stream is closed.</exception>
         public static FileSecurity GetAccessControl(this FileStream fileStream)
         {
+            if (fileStream == null)
+            {
+                throw new ArgumentNullException(nameof(fileStream));
+            }
+
             SafeFileHandle handle = fileStream.SafeFileHandle;
             if (handle.IsClosed)
             {
                 throw new ObjectDisposedException(null, SR.ObjectDisposed_FileClosed);
             }
+
             return new FileSecurity(handle, AccessControlSections.Access | AccessControlSections.Owner | AccessControlSections.Group);
         }
 
+        /// <summary>
+        /// This extension method for FileStream sets the security descriptors for the file using a FileSecurity instance.
+        /// </summary>
+        /// <param name="fileStream">An object that represents the file to apply security changes to.</param>
+        /// <param name="fileSecurity">An object that determines the access control and audit security for the file.</param>
+        /// <exception cref="ArgumentNullException"><paramref name="fileStream" /> or <paramref name="fileSecurity" /> is <see langword="null" />.</exception>
+        /// <exception cref="ObjectDisposedException">The file stream is closed.</exception>
         public static void SetAccessControl(this FileStream fileStream, FileSecurity fileSecurity)
         {
-            SafeFileHandle handle = fileStream.SafeFileHandle;
+            if (fileStream == null)
+            {
+                throw new ArgumentNullException(nameof(fileStream));
+            }
 
             if (fileSecurity == null)
+            {
                 throw new ArgumentNullException(nameof(fileSecurity));
+            }
 
+            SafeFileHandle handle = fileStream.SafeFileHandle;
             if (handle.IsClosed)
             {
                 throw new ObjectDisposedException(null, SR.ObjectDisposed_FileClosed);
@@ -85,10 +113,14 @@ public static void SetAccessControl(this FileStream fileStream, FileSecurity fil
         public static void Create(this DirectoryInfo directoryInfo, DirectorySecurity directorySecurity)
         {
             if (directoryInfo == null)
+            {
                 throw new ArgumentNullException(nameof(directoryInfo));
+            }
 
             if (directorySecurity == null)
+            {
                 throw new ArgumentNullException(nameof(directorySecurity));
+            }
 
             FileSystem.CreateDirectory(directoryInfo.FullName, directorySecurity.GetSecurityDescriptorBinaryForm());
         }
@@ -173,10 +205,12 @@ private static FileAccess GetFileStreamFileAccess(FileSystemRights rights)
             {
                 access = FileAccess.Read;
             }
+
             if ((rights & FileSystemRights.WriteData) != 0 || ((int)rights & Interop.Kernel32.GenericOperations.GENERIC_WRITE) != 0)
             {
                 access = access == FileAccess.Read ? FileAccess.ReadWrite : FileAccess.Write;
             }
+
             return access;
         }
 

diff --git a/...braries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.net46.cs b/...braries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.net46.cs
@@ -62,11 +62,26 @@ public static void SetAccessControl(this FileInfo fileInfo, FileSecurity fileSec
 
         public static FileSecurity GetAccessControl(this FileStream fileStream)
         {
+            if (fileStream == null)
+            {
+                throw new ArgumentNullException(nameof(fileStream));
+            }
+
             return fileStream.GetAccessControl();
         }
 
         public static void SetAccessControl(this FileStream fileStream, FileSecurity fileSecurity)
         {
+            if (fileStream == null)
+            {
+                throw new ArgumentNullException(nameof(fileStream));
+            }
+
+            if (fileSecurity == null)
+            {
+                throw new ArgumentNullException(nameof(fileSecurity));
+            }
+
             fileStream.SetAccessControl(fileSecurity);
         }
     }

diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs
@@ -106,7 +106,7 @@ public void GetAccessControl_FileInfo_AccessControlSections_ReturnsValidObject()
         [Fact]
         public void GetAccessControl_Filestream_InvalidArguments()
         {
-            Assert.Throws<NullReferenceException>(() => FileSystemAclExtensions.GetAccessControl((FileStream)null));
+            Assert.Throws<ArgumentNullException>("fileStream", () => FileSystemAclExtensions.GetAccessControl((FileStream)null));
         }
 
         [Fact]
@@ -175,7 +175,7 @@ public void SetAccessControl_FileInfo_FileSecurity_Success()
         [Fact]
         public void SetAccessControl_FileStream_FileSecurity_InvalidArguments()
         {
-            Assert.Throws<NullReferenceException>(() => FileSystemAclExtensions.SetAccessControl((FileStream)null, (FileSecurity)null));
+            Assert.Throws<ArgumentNullException>("fileStream", () => FileSystemAclExtensions.SetAccessControl((FileStream)null, (FileSecurity)null));
         }
 
         [Fact]