From 071da4c41aa808c949a773b92dca6f88de9d11f3 Mon Sep 17 00:00:00 2001
From: Erhan Atesoglu <47518605+eanova@users.noreply.github.com>
Date: Wed, 11 Dec 2019 10:27:44 -0800
Subject: [PATCH] Verified the null checking in the extension methods. (#316)

* Verified the null checking in the extension methods.

* Apply suggestions from code review

Co-Authored-By: Carlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com>

* 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.
---
 .../src/System/IO/FileSystemAclExtensions.cs  | 38 ++++++++++++++++++-
 .../IO/FileSystemAclExtensions.net46.cs       | 15 ++++++++
 .../tests/FileSystemAclExtensionsTests.cs     |  4 +-
 3 files changed, 53 insertions(+), 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
index 1de6cd4ff7c63..a97d8cc9dae16 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
@@ -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/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.net46.cs b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.net46.cs
index 25ec5c5e668ab..f0475c694e431 100644
--- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.net46.cs
+++ b/src/libraries/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
index b477fa345c0b0..179e92c71ba81 100644
--- 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]