From 5c3581046d0a8017c51a6ab2a803bfbfe4cd8cf6 Mon Sep 17 00:00:00 2001 From: Jeremy Powell Date: Thu, 21 Nov 2024 14:26:15 +1300 Subject: [PATCH] Improve argument/state validation --- OpenMcdf.Tests/RootStorageTests.cs | 16 ++++++++++++++-- OpenMcdf.Tests/StreamTests.cs | 2 +- OpenMcdf/RootContext.cs | 25 ++++++++++++++++++++----- OpenMcdf/RootStorage.cs | 23 +++++++++++++++-------- OpenMcdf/Storage.cs | 11 +++++++++++ OpenMcdf/StreamExtensions.cs | 4 +++- StructuredStorageExplorer/MainForm.cs | 2 +- 7 files changed, 65 insertions(+), 18 deletions(-) diff --git a/OpenMcdf.Tests/RootStorageTests.cs b/OpenMcdf.Tests/RootStorageTests.cs index 54600b4a..45995426 100644 --- a/OpenMcdf.Tests/RootStorageTests.cs +++ b/OpenMcdf.Tests/RootStorageTests.cs @@ -5,7 +5,7 @@ public sealed class RootStorageTests { [TestMethod] [DoNotParallelize] // Test sharing - [DataRow("MultipleStorage.cfs")] + [DataRow("TestStream_v3_0.cfs")] public void Open(string fileName) { using var rootStorage = RootStorage.OpenRead(fileName); @@ -13,7 +13,19 @@ public void Open(string fileName) Assert.ThrowsException(() => RootStorage.Open(fileName, FileMode.Open)); Assert.ThrowsException(() => RootStorage.Open(fileName, FileMode.Open, FileAccess.ReadWrite)); - Assert.ThrowsException(() => RootStorage.OpenWrite(fileName)); + + using CfbStream stream = rootStorage.OpenStream("TestStream"); + Assert.ThrowsException(() => stream.WriteByte(0)); + + Assert.ThrowsException(() => rootStorage.CreateStream("TestStream2")); + Assert.ThrowsException(() => rootStorage.CreateStorage("TestStream2")); + Assert.ThrowsException(() => rootStorage.Delete("TestStream")); + Assert.ThrowsException(() => rootStorage.Commit()); + Assert.ThrowsException(() => rootStorage.Revert()); + Assert.ThrowsException(() => rootStorage.CreationTime = DateTime.MinValue); + Assert.ThrowsException(() => rootStorage.ModifiedTime = DateTime.MinValue); + Assert.ThrowsException(() => rootStorage.CLISD = Guid.Empty); + Assert.ThrowsException(() => rootStorage.StateBits = 0); } [TestMethod] diff --git a/OpenMcdf.Tests/StreamTests.cs b/OpenMcdf.Tests/StreamTests.cs index 6adc1760..3a52694e 100644 --- a/OpenMcdf.Tests/StreamTests.cs +++ b/OpenMcdf.Tests/StreamTests.cs @@ -20,7 +20,7 @@ public void OpenStream(string fileName) Assert.ThrowsException(() => rootStorage.OpenStream("")); - CfbStream stream = rootStorage.OpenStream("TestStream"); + using CfbStream stream = rootStorage.OpenStream("TestStream"); Assert.AreEqual("TestStream", stream.EntryInfo.Name); } diff --git a/OpenMcdf/RootContext.cs b/OpenMcdf/RootContext.cs index f95addd8..d263e65c 100644 --- a/OpenMcdf/RootContext.cs +++ b/OpenMcdf/RootContext.cs @@ -1,4 +1,6 @@ -namespace OpenMcdf; +using System.Diagnostics.CodeAnalysis; + +namespace OpenMcdf; enum IOContextFlags { @@ -152,6 +154,20 @@ public void Dispose() } } + [MemberNotNull(nameof(writer))] + public void ThrowIfNotWritable() + { + if (writer is null) + throw new NotSupportedException("Root storage is not writable."); + } + + [MemberNotNull(nameof(transactedStream))] + public void ThrowIfNotTransacted() + { + if (transactedStream is null) + throw new NotSupportedException("Cannot commit non-transacted storage."); + } + public void Flush() { Fat.Flush(); @@ -181,8 +197,8 @@ public void WriteHeader() public void Commit() { - if (writer is null || transactedStream is null) - throw new InvalidOperationException("Cannot commit non-transacted storage."); + ThrowIfNotWritable(); + ThrowIfNotTransacted(); miniStream?.Flush(); miniFat?.Flush(); @@ -194,8 +210,7 @@ public void Commit() public void Revert() { - if (writer is null || transactedStream is null) - throw new InvalidOperationException("Cannot revert non-transacted storage."); + ThrowIfNotTransacted(); transactedStream.Revert(); } diff --git a/OpenMcdf/RootStorage.cs b/OpenMcdf/RootStorage.cs index 651a088f..0887fdc5 100644 --- a/OpenMcdf/RootStorage.cs +++ b/OpenMcdf/RootStorage.cs @@ -24,6 +24,18 @@ public sealed class RootStorage : Storage, IDisposable { readonly StorageModeFlags storageModeFlags; + private static void ThrowIfInvalid(FileMode mode) + { + if (mode is FileMode.Append) + throw new ArgumentException("Append mode is not valid for compound files.", nameof(mode)); + } + + private static void ThrowIfInvalid(FileAccess access) + { + if (access is FileAccess.Write) + throw new ArgumentException("Write-only access is not valid for compound files.", nameof(access)); + } + private static void ThrowIfLeaveOpen(StorageModeFlags flags) { if (flags.HasFlag(StorageModeFlags.LeaveOpen)) @@ -64,6 +76,7 @@ public static RootStorage Create(Stream stream, Version version = Version.V3, St public static RootStorage Open(string fileName, FileMode mode, StorageModeFlags flags = StorageModeFlags.None) { + ThrowIfInvalid(mode); ThrowIfLeaveOpen(flags); FileStream stream = File.Open(fileName, mode); @@ -72,6 +85,8 @@ public static RootStorage Open(string fileName, FileMode mode, StorageModeFlags public static RootStorage Open(string fileName, FileMode mode, FileAccess access, StorageModeFlags flags = StorageModeFlags.None) { + ThrowIfInvalid(mode); + ThrowIfInvalid(access); ThrowIfLeaveOpen(flags); FileStream stream = File.Open(fileName, mode, access); @@ -97,14 +112,6 @@ public static RootStorage OpenRead(string fileName, StorageModeFlags flags = Sto return Open(stream, flags); } - public static RootStorage OpenWrite(string fileName, StorageModeFlags flags = StorageModeFlags.None) - { - ThrowIfLeaveOpen(flags); - - FileStream stream = File.OpenWrite(fileName); - return Open(stream, flags); - } - RootStorage(RootContextSite rootContextSite, StorageModeFlags storageModeFlags) : base(rootContextSite, rootContextSite.Context.RootEntry, null) { diff --git a/OpenMcdf/Storage.cs b/OpenMcdf/Storage.cs index 7ba2b507..16337bc5 100644 --- a/OpenMcdf/Storage.cs +++ b/OpenMcdf/Storage.cs @@ -32,6 +32,8 @@ public Guid CLISD get => directoryEntry.CLSID; set { + Context.ThrowIfNotWritable(); + directoryEntry.CLSID = value; Context.DirectoryEntries.Write(directoryEntry); } @@ -42,6 +44,8 @@ public DateTime CreationTime get => directoryEntry.CreationTime; set { + Context.ThrowIfNotWritable(); + directoryEntry.CreationTime = value; Context.DirectoryEntries.Write(directoryEntry); } @@ -52,6 +56,8 @@ public DateTime ModifiedTime get => directoryEntry.ModifiedTime; set { + Context.ThrowIfNotWritable(); + directoryEntry.ModifiedTime = value; Context.DirectoryEntries.Write(directoryEntry); } @@ -62,6 +68,8 @@ public uint StateBits get => directoryEntry.StateBits; set { + Context.ThrowIfNotWritable(); + directoryEntry.StateBits = value; Context.DirectoryEntries.Write(directoryEntry); } @@ -125,6 +133,7 @@ public Storage CreateStorage(string name) ThrowHelper.ThrowIfNameIsInvalid(name); this.ThrowIfDisposed(Context.IsDisposed); + Context.ThrowIfNotWritable(); DirectoryEntry entry = AddDirectoryEntry(StorageType.Storage, name); return new Storage(ContextSite, entry, this); @@ -135,6 +144,7 @@ public CfbStream CreateStream(string name) ThrowHelper.ThrowIfNameIsInvalid(name); this.ThrowIfDisposed(Context.IsDisposed); + Context.ThrowIfNotWritable(); DirectoryEntry entry = AddDirectoryEntry(StorageType.Stream, name); return new CfbStream(ContextSite, entry, this); @@ -207,6 +217,7 @@ public void Delete(string name) ThrowHelper.ThrowIfNameIsInvalid(name); this.ThrowIfDisposed(Context.IsDisposed); + Context.ThrowIfNotWritable(); directoryTree.TryGetDirectoryEntry(name, out DirectoryEntry? entry); if (entry is null) diff --git a/OpenMcdf/StreamExtensions.cs b/OpenMcdf/StreamExtensions.cs index 75b58f58..9cfc928a 100644 --- a/OpenMcdf/StreamExtensions.cs +++ b/OpenMcdf/StreamExtensions.cs @@ -36,7 +36,9 @@ public static int ReadByteCore(this Stream stream) public static void WriteByteCore(this Stream stream, byte value) { - ReadOnlySpan bytes = stackalloc byte[] { value }; + stream.ThrowIfNotWritable(); + + ReadOnlySpan bytes = [value]; stream.Write(bytes); } diff --git a/StructuredStorageExplorer/MainForm.cs b/StructuredStorageExplorer/MainForm.cs index 279d02b5..9a89515e 100644 --- a/StructuredStorageExplorer/MainForm.cs +++ b/StructuredStorageExplorer/MainForm.cs @@ -142,7 +142,7 @@ private void LoadFile(string fileName) cf = null; // Load file - cf = RootStorage.OpenWrite(fileName); + cf = RootStorage.Open(fileName, FileMode.Open); fileNameLabel.Text = fileName; saveAsToolStripMenuItem.Enabled = true;