fix: throw correct exception when creating a FileStream with invali…

…d mode/access combination (#973) Fixes #884. In addition to the thrown exceptions some implementations and tests had to be adapted: - [`File.Open`](https://github.com/dotnet/runtime/blob/v6.0.16/src/libraries/System.Private.CoreLib/src/System/IO/File.cs#L152) with Append mode should use Write access - [`FileInfo.AppendText`](https://github.com/dotnet/runtime/blob/v6.0.16/src/libraries/System.Private.CoreLib/src/System/IO/FileInfo.cs#L85) should open the stream with Write access The corresponding tests had to be adapted!
TestableIO · Apr 19, 2023 · e84ace2 · e84ace2
1 parent 2e515ed
commit e84ace2
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 15 deletions.
diff --git a/src/TestableIO.System.IO.Abstractions.TestingHelpers/CommonExceptions.cs b/src/TestableIO.System.IO.Abstractions.TestingHelpers/CommonExceptions.cs
@@ -65,6 +65,12 @@ public static IOException ProcessCannotAccessFileInUse(string paramName = null)
         public static IOException FileAlreadyExists(string paramName) =>
             new IOException(string.Format(StringResources.Manager.GetString("FILE_ALREADY_EXISTS"), paramName));
 
+        public static ArgumentException InvalidAccessCombination(FileMode mode, FileAccess access)
+            => new ArgumentException(string.Format(StringResources.Manager.GetString("INVALID_ACCESS_COMBINATION"), mode, access), nameof(access));
+
+        public static ArgumentException AppendAccessOnlyInWriteOnlyMode()
+            => new ArgumentException(string.Format(StringResources.Manager.GetString("APPEND_ACCESS_ONLY_IN_WRITE_ONLY_MODE")), "access");
+
         public static NotImplementedException NotImplemented() =>
             new NotImplementedException(StringResources.Manager.GetString("NOT_IMPLEMENTED_EXCEPTION"));
     }

diff --git a/src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFile.cs b/src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFile.cs
@@ -544,7 +544,7 @@ public override FileSystemStream Open(string path, FileMode mode)
         {
             mockFileDataAccessor.PathVerifier.IsLegalAbsoluteOrRelative(path, "path");
 
-            return Open(path, mode, FileAccess.ReadWrite, FileShare.None);
+            return Open(path, mode, mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite, FileShare.None);
         }
 
         /// <inheritdoc />

diff --git a/src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileInfo.cs b/src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileInfo.cs
@@ -208,7 +208,7 @@ public override string Name
         /// <inheritdoc />
         public override StreamWriter AppendText()
         {
-            return new StreamWriter(new MockFileStream(mockFileSystem, FullName, FileMode.Append));
+            return new StreamWriter(new MockFileStream(mockFileSystem, FullName, FileMode.Append, FileAccess.Write));
         }
 
         /// <inheritdoc />

diff --git a/src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileStream.cs b/src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileStream.cs
@@ -28,6 +28,8 @@ public MockFileStream(
                (options & FileOptions.Asynchronous) != 0)
 
         {
+            ThrowIfInvalidModeAccess(mode, access);
+
             this.mockFileDataAccessor = mockFileDataAccessor ?? throw new ArgumentNullException(nameof(mockFileDataAccessor));
             this.path = path;
             this.options = options;
@@ -78,6 +80,29 @@ public MockFileStream(
             this.access = access;
         }
 
+        private static void ThrowIfInvalidModeAccess(FileMode mode, FileAccess access)
+        {
+            if (mode == FileMode.Append)
+            {
+                if (access == FileAccess.Read)
+                {
+                    throw CommonExceptions.InvalidAccessCombination(mode, access);
+                }
+
+                if (access != FileAccess.Write)
+                {
+                    throw CommonExceptions.AppendAccessOnlyInWriteOnlyMode();
+                }
+            }
+
+            if (!access.HasFlag(FileAccess.Write) &&
+                (mode == FileMode.Truncate || mode == FileMode.CreateNew ||
+                 mode == FileMode.Create || mode == FileMode.Append))
+            {
+                throw CommonExceptions.InvalidAccessCombination(mode, access);
+            }
+        }
+
         /// <inheritdoc />
         public override bool CanRead => access.HasFlag(FileAccess.Read);
 

diff --git a/src/TestableIO.System.IO.Abstractions.TestingHelpers/Properties/Resources.resx b/src/TestableIO.System.IO.Abstractions.TestingHelpers/Properties/Resources.resx
@@ -156,4 +156,10 @@
   <data name="FILE_ALREADY_EXISTS" xml:space="preserve">
     <value>The file '{0}' already exists.</value>
   </data>
+  <data name="APPEND_ACCESS_ONLY_IN_WRITE_ONLY_MODE" xml:space="preserve">
+    <value>Append access can be requested only in write-only mode.</value>
+  </data>
+  <data name="INVALID_ACCESS_COMBINATION" xml:space="preserve">
+    <value>Combining FileMode: {0} with FileAccess: {1} is invalid.</value>
+  </data>
 </root>
diff --git a/tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileAdjustTimesTest.cs b/tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileAdjustTimesTest.cs
@@ -76,7 +76,7 @@ public void MockFile_AfterMove_ShouldUpdateLastAccessTime()
 
         [TestCase(FileMode.Open, FileAccess.ReadWrite)]
         [TestCase(FileMode.OpenOrCreate, FileAccess.Write)]
-        [TestCase(FileMode.Append, FileAccess.ReadWrite)]
+        [TestCase(FileMode.Append, FileAccess.Write)]
         public void MockFile_AfterOpen_WithWriteAccess_ShouldUpdateLastAccessAndLastWriteTime(FileMode fileMode, FileAccess fileAccess)
         {
             var creationTime = DateTime.UtcNow.AddDays(10);
@@ -98,7 +98,6 @@ public void MockFile_AfterOpen_WithWriteAccess_ShouldUpdateLastAccessAndLastWrit
 
         [TestCase(FileMode.Open, FileAccess.Read)]
         [TestCase(FileMode.OpenOrCreate, FileAccess.Read)]
-        [TestCase(FileMode.Append, FileAccess.Read)]
         public void MockFile_AfterOpen_WithReadOnlyAccess_ShouldUpdateLastAccessTime(FileMode fileMode, FileAccess fileAccess)
         {
             var creationTime = DateTime.UtcNow.AddDays(10);

diff --git a/tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileOpenTests.cs b/tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileOpenTests.cs
@@ -95,14 +95,6 @@ public void MockFile_Open_OpensExistingFileOnAppend()
 
             Assert.That(stream.Position, Is.EqualTo(file.Contents.Length));
             Assert.That(stream.Length, Is.EqualTo(file.Contents.Length));
-
-            stream.Seek(0, SeekOrigin.Begin);
-
-            byte[] data;
-            using (var br = new BinaryReader(stream))
-                data = br.ReadBytes((int)stream.Length);
-
-            CollectionAssert.AreEqual(file.Contents, data);
         }
 
         [Test]

diff --git a/tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileStreamFactoryTests.cs b/tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileStreamFactoryTests.cs
@@ -23,7 +23,7 @@ public void MockFileStreamFactory_CreateForExistingFile_ShouldReturnStream(FileM
             var fileStreamFactory = new MockFileStreamFactory(fileSystem);
 
             // Act
-            var result = fileStreamFactory.Create(@"c:\existing.txt", fileMode);
+            var result = fileStreamFactory.Create(@"c:\existing.txt", fileMode, FileAccess.Write);
 
             // Assert
             Assert.IsNotNull(result);
@@ -39,7 +39,7 @@ public void MockFileStreamFactory_CreateForNonExistingFile_ShouldReturnStream(Fi
             var fileStreamFactory = new MockFileStreamFactory(fileSystem);
 
             // Act
-            var result = fileStreamFactory.Create(XFS.Path(@"c:\not_existing.txt"), fileMode);
+            var result = fileStreamFactory.Create(XFS.Path(@"c:\not_existing.txt"), fileMode, FileAccess.Write);
 
             // Assert
             Assert.IsNotNull(result);
@@ -72,7 +72,6 @@ public void MockFileStreamFactory_CreateForAnExistingFile_ShouldReplaceFileConte
         [TestCase(FileMode.Create)]
         [TestCase(FileMode.Open)]
         [TestCase(FileMode.CreateNew)]
-        [TestCase(FileMode.Append)]
         public void MockFileStreamFactory_CreateInNonExistingDirectory_ShouldThrowDirectoryNotFoundException(FileMode fileMode)
         {
             // Arrange
@@ -86,6 +85,33 @@ public void MockFileStreamFactory_CreateInNonExistingDirectory_ShouldThrowDirect
             Assert.Throws<DirectoryNotFoundException>(() => fileStreamFactory.Create(XFS.Path(@"C:\Test\NonExistingDirectory\some_random_file.txt"), fileMode));
         }
 
+        [Test]
+        public void MockFileStreamFactory_AppendAccessWithReadWriteMode_ShouldThrowArgumentException()
+        {
+            var fileSystem = new MockFileSystem();
+
+            Assert.Throws<ArgumentException>(() =>
+            {
+                fileSystem.FileStream.New(XFS.Path(@"c:\path.txt"), FileMode.Append, FileAccess.ReadWrite);
+            });
+        }
+
+        [Test]
+        [TestCase(FileMode.Append)]
+        [TestCase(FileMode.Truncate)]
+        [TestCase(FileMode.Create)]
+        [TestCase(FileMode.CreateNew)]
+        [TestCase(FileMode.Append)]
+        public void MockFileStreamFactory_InvalidModeForReadAccess_ShouldThrowArgumentException(FileMode fileMode)
+        {
+            var fileSystem = new MockFileSystem();
+
+            Assert.Throws<ArgumentException>(() =>
+            {
+                fileSystem.FileStream.New(XFS.Path(@"c:\path.txt"), fileMode, FileAccess.Read);
+            });
+        }
+
         [Test]
         [TestCase(FileMode.Open)]
         [TestCase(FileMode.Truncate)]