Skip to content

Commit

Permalink
move the validaiton logic to FileStreamOptions (#53869)
Browse files Browse the repository at this point in the history
Co-authored-by: Stephen Toub <stoub@microsoft.com>
  • Loading branch information
adamsitnik and stephentoub authored Jun 9, 2021
1 parent 9a64a29 commit 78579ef
Show file tree
Hide file tree
Showing 12 changed files with 281 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Linq;
using Xunit;

namespace System.IO.Tests
{
public class FileStream_FileStreamOptions : FileSystemTest
{
[Fact]
public void NullOptionsThrows()
{
AssertExtensions.Throws<ArgumentNullException>("options", () => new FileStream(GetTestFilePath(), options: null));
}

[Theory]
[InlineData(FileMode.Create)]
[InlineData(FileMode.CreateNew)]
[InlineData(FileMode.Append)]
[InlineData(FileMode.Truncate)]
public void ModesThatRequireWriteAccessThrowWhenReadAccessIsProvided(FileMode fileMode)
{
Assert.Throws<ArgumentException>(() => new FileStream(GetTestFilePath(), new FileStreamOptions
{
Mode = fileMode,
Access = FileAccess.Read
}));
}

[Theory]
[InlineData(FileAccess.Read)]
[InlineData(FileAccess.ReadWrite)]
public void AppendWorksOnlyForWriteAccess(FileAccess fileAccess)
{
Assert.Throws<ArgumentException>(() => new FileStream(GetTestFilePath(), new FileStreamOptions
{
Mode = FileMode.Append,
Access = fileAccess
}));
}

[Fact]
public void Mode()
{
Assert.Equal(FileMode.Open, new FileStreamOptions().Mode);

FileMode[] validValues = Enum.GetValues<FileMode>();

foreach (var vaidValue in validValues)
{
Assert.Equal(vaidValue, (new FileStreamOptions { Mode = vaidValue }).Mode);
}

Assert.Throws<ArgumentOutOfRangeException>(() => new FileStreamOptions { Mode = validValues.Min() - 1 });
Assert.Throws<ArgumentOutOfRangeException>(() => new FileStreamOptions { Mode = validValues.Max() + 1 });
}

[Fact]
public void Access()
{
Assert.Equal(FileAccess.Read, new FileStreamOptions().Access);

FileAccess[] validValues = Enum.GetValues<FileAccess>();

foreach (var vaidValue in validValues)
{
Assert.Equal(vaidValue, (new FileStreamOptions { Access = vaidValue }).Access);
}

Assert.Throws<ArgumentOutOfRangeException>(() => new FileStreamOptions { Access = validValues.Min() - 1 });
Assert.Throws<ArgumentOutOfRangeException>(() => new FileStreamOptions { Access = validValues.Max() + 1 });
}

[Fact]
public void Share()
{
Assert.Equal(FileShare.Read, new FileStreamOptions().Share);

FileShare[] validValues = Enum.GetValues<FileShare>();

foreach (var vaidValue in validValues)
{
Assert.Equal(vaidValue, (new FileStreamOptions { Share = vaidValue }).Share);
}

FileShare all = validValues.Aggregate((x, y) => x | y);
Assert.Equal(all, (new FileStreamOptions { Share = all }).Share);

Assert.Throws<ArgumentOutOfRangeException>(() => new FileStreamOptions { Share = validValues.Min() - 1 });
Assert.Throws<ArgumentOutOfRangeException>(() => new FileStreamOptions { Share = all + 1 });
}

[Fact]
public void Options()
{
Assert.Equal(FileOptions.None, new FileStreamOptions().Options);

FileOptions[] validValues = Enum.GetValues<FileOptions>();

foreach (var option in validValues)
{
Assert.Equal(option, (new FileStreamOptions { Options = option }).Options);
}

FileOptions all = validValues.Aggregate((x, y) => x | y);
Assert.Equal(all, (new FileStreamOptions { Options = all }).Options);

Assert.Throws<ArgumentOutOfRangeException>(() => new FileStreamOptions { Options = validValues.Min() - 1 });
Assert.Throws<ArgumentOutOfRangeException>(() => new FileStreamOptions { Options = all + 1 });
}

[Fact]
public void PreallocationSize()
{
Assert.Equal(0, new FileStreamOptions().PreallocationSize);

Assert.Equal(0, new FileStreamOptions { PreallocationSize = 0 }.PreallocationSize);
Assert.Equal(1, new FileStreamOptions { PreallocationSize = 1 }.PreallocationSize);
Assert.Equal(123, new FileStreamOptions { PreallocationSize = 123 }.PreallocationSize);

Assert.Throws<ArgumentOutOfRangeException>(() => new FileStreamOptions { PreallocationSize = -1 });
}

[Fact]
public void BufferSize()
{
Assert.Equal(4096, new FileStreamOptions().BufferSize);

Assert.Equal(0, new FileStreamOptions { BufferSize = 0 }.BufferSize);
Assert.Equal(1, new FileStreamOptions { BufferSize = 1 }.BufferSize);
Assert.Equal(123, new FileStreamOptions { BufferSize = 123 }.BufferSize);

Assert.Throws<ArgumentOutOfRangeException>(() => new FileStreamOptions { BufferSize = -1 });
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ public abstract class FileStream_ctor_options_as_base : FileStream_ctor_str_fm_f
{
protected abstract long PreallocationSize { get; }

protected override string GetExpectedParamName(string paramName) => "value";

protected override FileStream CreateFileStream(string path, FileMode mode)
=> new FileStream(path,
new FileStreamOptions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ protected virtual FileStream CreateFileStream(string path, FileMode mode)

protected virtual long InitialLength => 0;

protected virtual string GetExpectedParamName(string paramName) => paramName;

[Fact]
public void NullPathThrows()
{
Expand All @@ -35,7 +37,9 @@ public void DirectoryThrows()
[Fact]
public void InvalidModeThrows()
{
AssertExtensions.Throws<ArgumentOutOfRangeException>("mode", () => CreateFileStream(GetTestFilePath(), ~FileMode.Open));
AssertExtensions.Throws<ArgumentOutOfRangeException>(
GetExpectedParamName("mode"),
() => CreateFileStream(GetTestFilePath(), ~FileMode.Open));
}

[Theory, MemberData(nameof(TrailingCharacters))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ protected virtual FileStream CreateFileStream(string path, FileMode mode, FileAc
[Fact]
public void InvalidAccessThrows()
{
AssertExtensions.Throws<ArgumentOutOfRangeException>("access", () => CreateFileStream(GetTestFilePath(), FileMode.Create, ~FileAccess.Read));
AssertExtensions.Throws<ArgumentOutOfRangeException>(
GetExpectedParamName("access"),
() => CreateFileStream(GetTestFilePath(), FileMode.Create, ~FileAccess.Read));
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ protected virtual FileStream CreateFileStream(string path, FileMode mode, FileAc
[Fact]
public void InvalidShareThrows()
{
AssertExtensions.Throws<ArgumentOutOfRangeException>("share", () => CreateFileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, ~FileShare.None));
AssertExtensions.Throws<ArgumentOutOfRangeException>(
GetExpectedParamName("share"),
() => CreateFileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, ~FileShare.None));
}

private static readonly FileShare[] s_shares =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ protected virtual FileStream CreateFileStream(string path, FileMode mode, FileAc
[Fact]
public void NegativeBufferSizeThrows()
{
AssertExtensions.Throws<ArgumentOutOfRangeException>("bufferSize", () => CreateFileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.Read, -1));
AssertExtensions.Throws<ArgumentOutOfRangeException>(
GetExpectedParamName("bufferSize"),
() => CreateFileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.Read, -1));
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ protected virtual FileStream CreateFileStream(string path, FileMode mode, FileAc
[Fact]
public void InvalidFileOptionsThrow()
{
AssertExtensions.Throws<ArgumentOutOfRangeException>("options", () => CreateFileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.Read, c_DefaultBufferSize, ~FileOptions.None));
AssertExtensions.Throws<ArgumentOutOfRangeException>(
GetExpectedParamName("options"),
() => CreateFileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.Read, c_DefaultBufferSize, ~FileOptions.None));
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
<Compile Include="FileStream\IsAsync.cs" />
<Compile Include="FileStream\Name.cs" />
<Compile Include="FileStream\CopyToAsync.cs" />
<Compile Include="FileStream\FileStreamOptions.cs" />
<Compile Include="FileStream\Flush.cs" />
<Compile Include="FileStream\Dispose.cs" />
<Compile Include="FileStream\WriteAsync.cs" />
Expand Down
38 changes: 31 additions & 7 deletions src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,16 +144,11 @@ public FileStream(string path, FileMode mode, FileAccess access, FileShare share
/// </summary>
/// <param name="path">A relative or absolute path for the file that the current <see cref="System.IO.FileStream" /> instance will encapsulate.</param>
/// <param name="options">An object that describes optional <see cref="System.IO.FileStream" /> parameters to use.</param>
/// <exception cref="T:System.ArgumentNullException"><paramref name="path" /> is <see langword="null" />.</exception>
/// <exception cref="T:System.ArgumentNullException"><paramref name="path" /> or <paramref name="options" /> is <see langword="null" />.</exception>
/// <exception cref="T:System.ArgumentException"><paramref name="path" /> is an empty string (""), contains only white space, or contains one or more invalid characters.
/// -or-
/// <paramref name="path" /> refers to a non-file device, such as <c>CON:</c>, <c>COM1:</c>, <c>LPT1:</c>, etc. in an NTFS environment.</exception>
/// <exception cref="T:System.NotSupportedException"><paramref name="path" /> refers to a non-file device, such as <c>CON:</c>, <c>COM1:</c>, <c>LPT1:</c>, etc. in a non-NTFS environment.</exception>
/// <exception cref="T:System.ArgumentOutOfRangeException"><see cref="System.IO.FileStreamOptions.BufferSize" /> is negative.
/// -or-
/// <see cref="System.IO.FileStreamOptions.PreallocationSize" /> is negative.
/// -or-
/// <see cref="System.IO.FileStreamOptions.Mode" />, <see cref="System.IO.FileStreamOptions.Access" />, or <see cref="System.IO.FileStreamOptions.Share" /> contain an invalid value.</exception>
/// <exception cref="T:System.IO.FileNotFoundException">The file cannot be found, such as when <see cref="System.IO.FileStreamOptions.Mode" /> is <see langword="FileMode.Truncate" /> or <see langword="FileMode.Open" />, and the file specified by <paramref name="path" /> does not exist. The file must already exist in these modes.</exception>
/// <exception cref="T:System.IO.IOException">An I/O error, such as specifying <see langword="FileMode.CreateNew" /> when the file specified by <paramref name="path" /> already exists, occurred.
/// -or-
Expand All @@ -169,8 +164,37 @@ public FileStream(string path, FileMode mode, FileAccess access, FileShare share
/// <see cref="F:System.IO.FileOptions.Encrypted" /> is specified for <see cref="System.IO.FileStreamOptions.Options" /> , but file encryption is not supported on the current platform.</exception>
/// <exception cref="T:System.IO.PathTooLongException">The specified path, file name, or both exceed the system-defined maximum length. </exception>
public FileStream(string path, FileStreamOptions options)
: this(path, options.Mode, options.Access, options.Share, options.BufferSize, options.Options, options.PreallocationSize)
{
if (path is null)
{
throw new ArgumentNullException(nameof(path), SR.ArgumentNull_Path);
}
else if (path.Length == 0)
{
throw new ArgumentException(SR.Argument_EmptyPath, nameof(path));
}
else if (options is null)
{
throw new ArgumentNullException(nameof(options));
}
else if ((options.Access & FileAccess.Read) != 0 && options.Mode == FileMode.Append)
{
throw new ArgumentException(SR.Argument_InvalidAppendMode, nameof(options));
}
else if ((options.Access & FileAccess.Write) == 0)
{
if (options.Mode == FileMode.Truncate || options.Mode == FileMode.CreateNew || options.Mode == FileMode.Create || options.Mode == FileMode.Append)
{
throw new ArgumentException(SR.Format(SR.Argument_InvalidFileModeAndAccessCombo, options.Mode, options.Access), nameof(options));
}
}
else if ((options.Access & FileAccess.Write) == FileAccess.Write)
{
SerializationInfo.ThrowIfDeserializationInProgress("AllowFileWrites", ref s_cachedSerializationSwitch);
}

_strategy = FileStreamHelpers.ChooseStrategy(
this, path, options.Mode, options.Access, options.Share, options.BufferSize, options.Options, options.PreallocationSize);
}

private FileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options, long preallocationSize)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,109 @@ namespace System.IO
{
public sealed class FileStreamOptions
{
private FileMode _mode = FileMode.Open;
private FileAccess _access = FileAccess.Read;
private FileShare _share = FileStream.DefaultShare;
private FileOptions _options;
private long _preallocationSize;
private int _bufferSize = FileStream.DefaultBufferSize;

/// <summary>
/// One of the enumeration values that determines how to open or create the file.
/// </summary>
public FileMode Mode { get; set; }
/// <exception cref="T:System.ArgumentOutOfRangeException">When <paramref name="value" /> contains an invalid value.</exception>
public FileMode Mode
{
get => _mode;
set
{
if (value < FileMode.CreateNew || value > FileMode.Append)
{
ThrowHelper.ArgumentOutOfRangeException_Enum_Value();
}

_mode = value;
}
}

/// <summary>
/// A bitwise combination of the enumeration values that determines how the file can be accessed by the <see cref="FileStream" /> object. This also determines the values returned by the <see cref="System.IO.FileStream.CanRead" /> and <see cref="System.IO.FileStream.CanWrite" /> properties of the <see cref="FileStream" /> object.
/// </summary>
public FileAccess Access { get; set; } = FileAccess.Read;
/// <exception cref="T:System.ArgumentOutOfRangeException">When <paramref name="value" /> contains an invalid value.</exception>
public FileAccess Access
{
get => _access;
set
{
if (value < FileAccess.Read || value > FileAccess.ReadWrite)
{
ThrowHelper.ArgumentOutOfRangeException_Enum_Value();
}

_access = value;
}
}

/// <summary>
/// A bitwise combination of the enumeration values that determines how the file will be shared by processes. The default value is <see cref="System.IO.FileShare.Read" />.
/// </summary>
public FileShare Share { get; set; } = FileStream.DefaultShare;
/// <exception cref="T:System.ArgumentOutOfRangeException">When <paramref name="value" /> contains an invalid value.</exception>
public FileShare Share
{
get => _share;
set
{
// don't include inheritable in our bounds check for share
FileShare tempshare = value & ~FileShare.Inheritable;
if (tempshare < FileShare.None || tempshare > (FileShare.ReadWrite | FileShare.Delete))
{
ThrowHelper.ArgumentOutOfRangeException_Enum_Value();
}

_share = value;
}
}

/// <summary>
/// A bitwise combination of the enumeration values that specifies additional file options. The default value is <see cref="System.IO.FileOptions.None" />, which indicates synchronous IO.
/// </summary>
public FileOptions Options { get; set; }
/// <exception cref="T:System.ArgumentOutOfRangeException">When <paramref name="value" /> contains an invalid value.</exception>
public FileOptions Options
{
get => _options;
set
{
// NOTE: any change to FileOptions enum needs to be matched here in the error validation
if (value != FileOptions.None && (value & ~(FileOptions.WriteThrough | FileOptions.Asynchronous | FileOptions.RandomAccess | FileOptions.DeleteOnClose | FileOptions.SequentialScan | FileOptions.Encrypted | (FileOptions)0x20000000 /* NoBuffering */)) != 0)
{
ThrowHelper.ArgumentOutOfRangeException_Enum_Value();
}

_options = value;
}
}

/// <summary>
/// The initial allocation size in bytes for the file. A positive value is effective only when a regular file is being created, overwritten, or replaced.
/// When the value is negative, the <see cref="FileStream" /> constructor throws an <see cref="ArgumentOutOfRangeException" />.
/// Negative values are not allowed.
/// In other cases (including the default 0 value), it's ignored.
/// </summary>
public long PreallocationSize { get; set; }
/// <exception cref="T:System.ArgumentOutOfRangeException">When <paramref name="value" /> is negative.</exception>
public long PreallocationSize
{
get => _preallocationSize;
set => _preallocationSize = value >= 0 ? value : throw new ArgumentOutOfRangeException(nameof(value), SR.ArgumentOutOfRange_NeedNonNegNum);
}

/// <summary>
/// The size of the buffer used by <see cref="FileStream" /> for buffering. The default buffer size is 4096.
/// 0 or 1 means that buffering should be disabled. Negative values are not allowed.
/// </summary>
public int BufferSize { get; set; } = FileStream.DefaultBufferSize;
/// <exception cref="T:System.ArgumentOutOfRangeException">When <paramref name="value" /> is negative.</exception>
public int BufferSize
{
get => _bufferSize;
set => _bufferSize = value >= 0 ? value : throw new ArgumentOutOfRangeException(nameof(value), SR.ArgumentOutOfRange_NeedNonNegNum);
}
}
}
Loading

0 comments on commit 78579ef

Please sign in to comment.