Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: throw correct exception when creating a FileStream with invalid mode/access combination #973

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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)]
Expand Down