From 80fd36dc89abae74768c780f8f4c53002f998778 Mon Sep 17 00:00:00 2001 From: Cole Brown Date: Wed, 28 Jul 2021 11:23:40 -0700 Subject: [PATCH 1/9] Cleaned up whitespace in src/FileCache files. --- src/FileCache/BasicFileCacheManager.cs | 4 +-- src/FileCache/FileCache.cs | 44 ++++++++++++------------- src/FileCache/FileCacheManager.cs | 12 +++---- src/FileCache/HashedFileCacheManager.cs | 4 +-- src/FileCache/PriortyQueue.cs | 4 +-- 5 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/FileCache/BasicFileCacheManager.cs b/src/FileCache/BasicFileCacheManager.cs index c3e9c4f..a6fb4e6 100644 --- a/src/FileCache/BasicFileCacheManager.cs +++ b/src/FileCache/BasicFileCacheManager.cs @@ -11,7 +11,7 @@ namespace System.Runtime.Caching public class BasicFileCacheManager : FileCacheManager { /// - /// Returns a list of keys for a given region. + /// Returns a list of keys for a given region. /// /// /// @@ -33,7 +33,7 @@ public override IEnumerable GetKeys(string regionName = null) } /// - /// Builds a string that will place the specified file name within the appropriate + /// Builds a string that will place the specified file name within the appropriate /// cache and workspace folder. /// /// diff --git a/src/FileCache/FileCache.cs b/src/FileCache/FileCache.cs index 84464a9..89c832e 100644 --- a/src/FileCache/FileCache.cs +++ b/src/FileCache/FileCache.cs @@ -29,7 +29,7 @@ public class FileCache : ObjectCache private const string LastCleanedDateFile = "cache.lcd"; private const string CacheSizeFile = "cache.size"; // this is a file used to prevent multiple processes from trying to "clean" at the same time - private const string SemaphoreFile = "cache.sem"; + private const string SemaphoreFile = "cache.sem"; private long _currentCacheSize = 0; private PayloadMode _readMode = PayloadMode.Serializable; public string CacheDir { get; protected set; } @@ -52,7 +52,7 @@ public static FileCacheManagers DefaultCacheManager /// /// Used to abstract away the low-level details of file management. This allows - /// for multiple file formatting schemes based on use case. + /// for multiple file formatting schemes based on use case. /// public FileCacheManager CacheManager { get; protected set; } @@ -112,7 +112,7 @@ public PayloadMode PayloadReadMode { public TimeSpan FilenameAsPayloadSafetyMargin = TimeSpan.FromMinutes(10); /// - /// Used to determine how long the FileCache will wait for a file to become + /// Used to determine how long the FileCache will wait for a file to become /// available. Default (00:00:00) is indefinite. Should the timeout be /// reached, an exception will be thrown. /// @@ -136,7 +136,7 @@ public TimeSpan AccessTimeout /// /// Returns the approximate size of the file cache /// - public long CurrentCacheSize + public long CurrentCacheSize { get { @@ -163,7 +163,7 @@ private set CacheManager.WriteSysFile(CacheSizeFile, value); _currentCacheSize = value; } - } + } } /// @@ -171,7 +171,7 @@ private set /// public event EventHandler MaxCacheSizeReached = delegate { }; - public event EventHandler CacheResized = delegate { }; + public event EventHandler CacheResized = delegate { }; /// /// The default cache path used by FC. @@ -311,10 +311,10 @@ private void Init( bool setCacheDirToDefault = true, bool setBinderToDefault = true ) - { + { _name = "FileCache_" + _nameCounter; _nameCounter++; - + DefaultRegion = null; DefaultPolicy = new CacheItemPolicy(); MaxCacheSize = long.MaxValue; @@ -337,7 +337,7 @@ private void Init( // only set the clean interval if the user supplied it if (cleanInterval > new TimeSpan()) - { + { _cleanInterval = cleanInterval; } @@ -356,7 +356,7 @@ private void Init( } else if (calculateCacheSize || CurrentCacheSize == 0) { - // This is in an else if block, because CleanCacheAsync will + // This is in an else if block, because CleanCacheAsync will // update the cache size, so no need to do it twice. UpdateCacheSizeAsync(); } @@ -422,7 +422,7 @@ private bool ShouldClean() public long ShrinkCacheToSize(long newSize, string regionName = null) { long originalSize = 0, amount = 0, removed = 0; - + //lock down other treads from trying to shrink or clean using (FileStream cLock = GetCleaningLock()) { @@ -480,7 +480,7 @@ public void CleanCacheAsync() public long CleanCache(string regionName = null) { long removed = 0; - + //lock down other treads from trying to shrink or clean using (FileStream cLock = GetCleaningLock()) { @@ -529,10 +529,10 @@ public long CleanCache(string regionName = null) /// /// The amount of data that was actually removed private long DeleteOldestFiles(long amount, string regionName = null) - { + { // Verify that we actually need to shrink if (amount <= 0) - { + { return 0; } @@ -576,7 +576,7 @@ private long DeleteOldestFiles(long amount, string regionName = null) } /// - /// This method calls GetCacheSize on a separate thread to + /// This method calls GetCacheSize on a separate thread to /// calculate and then store the size of the cache. /// public void UpdateCacheSizeAsync() @@ -717,7 +717,7 @@ public void Flush(DateTime minDate, string regionName = null) } } /// - /// Returns the policy attached to a given cache item. + /// Returns the policy attached to a given cache item. /// /// The key of the item /// The region in which the key exists @@ -837,7 +837,7 @@ public override DefaultCacheCapabilities DefaultCacheCapabilities ; } } - + public override object Get(string key, string regionName = null) { FileCachePayload payload = CacheManager.ReadFile(PayloadReadMode, key, regionName) as FileCachePayload; @@ -861,7 +861,7 @@ public override object Get(string key, string regionName = null) //delete the file from the cache try { - // CT Note: I changed this to Remove from File.Delete so that the coresponding + // CT Note: I changed this to Remove from File.Delete so that the coresponding // policy file will be deleted as well, and CurrentCacheSize will be updated. Remove(key, regionName); } @@ -877,7 +877,7 @@ public override object Get(string key, string regionName = null) payload.Policy.AbsoluteExpiration = DateTime.Now.Add(payload.Policy.SlidingExpiration); WriteHelper(PayloadWriteMode, key, payload, regionName, true); } - + } } else @@ -967,7 +967,7 @@ public override object Remove(string key, string regionName = null) { object valueToDelete = null; - + if (Contains(key, regionName) == true) { @@ -993,7 +993,7 @@ public override object Remove(string key, string regionName = null) catch (IOException) { } - + } return valueToDelete; } @@ -1044,7 +1044,7 @@ public override Type BindToType(string assemblyName, string typeName) } } - // CT: This private class is used to help shrink the cache. + // CT: This private class is used to help shrink the cache. // It computes the total size of an entry including it's policy file. // It also implements IComparable functionality to allow for sorting based on access time private class CacheItemReference : IComparable diff --git a/src/FileCache/FileCacheManager.cs b/src/FileCache/FileCacheManager.cs index a7e600a..28121af 100644 --- a/src/FileCache/FileCacheManager.cs +++ b/src/FileCache/FileCacheManager.cs @@ -18,7 +18,7 @@ public abstract class FileCacheManager public SerializationBinder Binder { get; set; } /// - /// Used to determine how long the FileCache will wait for a file to become + /// Used to determine how long the FileCache will wait for a file to become /// available. Default (00:00:00) is indefinite. Should the timeout be /// reached, an exception will be thrown. /// @@ -219,16 +219,16 @@ public virtual long WriteFile(FileCache.PayloadMode mode, string key, FileCacheP } /// - /// Builds a string that will place the specified file name within the appropriate + /// Builds a string that will place the specified file name within the appropriate /// cache and workspace folder. /// /// /// /// public abstract string GetCachePath(string key, string regionName = null); - + /// - /// Returns a list of keys for a given region. + /// Returns a list of keys for a given region. /// /// /// @@ -262,7 +262,7 @@ public IEnumerable GetRegions() /// /// Reads data in from a system file. System files are not part of the - /// cache itself, but serve as a way for the cache to store data it + /// cache itself, but serve as a way for the cache to store data it /// needs to operate. /// /// The name of the sysfile (without directory) @@ -390,7 +390,7 @@ public virtual long DeleteFile(string key, string regionName = null) } catch (IOException ex) { - //Owning FC might be interested in this exception. + //Owning FC might be interested in this exception. throw ex; } return Math.Abs(bytesFreed); diff --git a/src/FileCache/HashedFileCacheManager.cs b/src/FileCache/HashedFileCacheManager.cs index aff7fc8..45abe92 100644 --- a/src/FileCache/HashedFileCacheManager.cs +++ b/src/FileCache/HashedFileCacheManager.cs @@ -85,7 +85,7 @@ private string GetFileName(string key, string regionName = null) } /// - /// Builds a string that will place the specified file name within the appropriate + /// Builds a string that will place the specified file name within the appropriate /// cache and workspace folder. /// /// @@ -106,7 +106,7 @@ public override string GetCachePath(string key, string regionName = null) } /// - /// Returns a list of keys for a given region. + /// Returns a list of keys for a given region. /// /// public override IEnumerable GetKeys(string regionName = null) diff --git a/src/FileCache/PriortyQueue.cs b/src/FileCache/PriortyQueue.cs index 8b5e7f5..7fe70d4 100644 --- a/src/FileCache/PriortyQueue.cs +++ b/src/FileCache/PriortyQueue.cs @@ -23,8 +23,8 @@ public class PriortyQueue where T : IComparable /// /// Default constructor. /// - /// The comparer to use. The default comparer will make the smallest item the root of the heap. - /// + /// The comparer to use. The default comparer will make the smallest item the root of the heap. + /// /// public PriortyQueue(IComparer comparer = null) { From d738c50847936b468a2c8d3cad310513117b65d0 Mon Sep 17 00:00:00 2001 From: Cole Brown Date: Tue, 27 Jul 2021 16:09:06 -0700 Subject: [PATCH 2/9] Updated FileCache to remove BinaryFormatter. FileCacheManager: - Added magic header number `CACHE_VERSION`. + `HeaderVersionValid()` and `HeaderVersionWrite()` use it in writing/reading to figure out if the files were created using the old (BinaryFormatter) or new (BinaryWrite) way. - Removed `ReadSysFile()`. + Replaced it with: - `ReadSysValue(string, out T)` + Just throws an Exception - below two functions are the actually used ones. - `ReadSysValue(string, out long)` - `ReadSysValue(string, out DateTime)` - Removed `WriteSysFile()`. + Replaced it with: - `WriteSysValue(string, T)` + Just throws an Exception - below two functions are the actually used ones. - `WriteSysValue(string, long)` - `WriteSysValue(string, DateTime)` FileCache: - Implemented `FileCache.PayloadMode.RawBytes` for read & write. + Removed code in `PayloadReadMode` property to prevent its use. + Implemented the `ReadFile()` function helper: `LoadRawPayloadData()`. + Updated `WriteFile()` (PayloadMode.RawBytes path) to use BinaryWriter instead of BinaryFormatter. --- src/FileCache/BasicFileCacheManager.cs | 2 - src/FileCache/FileCache.cs | 32 ++-- src/FileCache/FileCacheManager.cs | 224 ++++++++++++++++++++----- 3 files changed, 196 insertions(+), 62 deletions(-) diff --git a/src/FileCache/BasicFileCacheManager.cs b/src/FileCache/BasicFileCacheManager.cs index a6fb4e6..07bf7a1 100644 --- a/src/FileCache/BasicFileCacheManager.cs +++ b/src/FileCache/BasicFileCacheManager.cs @@ -74,7 +74,5 @@ public override string GetPolicyPath(string key, string regionName = null) } return filePath; } - - } } diff --git a/src/FileCache/FileCache.cs b/src/FileCache/FileCache.cs index 89c832e..fbe9bda 100644 --- a/src/FileCache/FileCache.cs +++ b/src/FileCache/FileCache.cs @@ -31,7 +31,6 @@ public class FileCache : ObjectCache // this is a file used to prevent multiple processes from trying to "clean" at the same time private const string SemaphoreFile = "cache.sem"; private long _currentCacheSize = 0; - private PayloadMode _readMode = PayloadMode.Serializable; public string CacheDir { get; protected set; } /// @@ -88,16 +87,7 @@ public enum PayloadMode /// /// Specified whether the payload is deserialized or just the file name. /// - public PayloadMode PayloadReadMode { - get => _readMode; - set { - if (value == PayloadMode.RawBytes) - { - throw new ArgumentException("The read mode cannot be set to RawBytes. Use the file name please."); - } - _readMode = value; - } - } + public PayloadMode PayloadReadMode { get; set; } = PayloadMode.Serializable; /// /// Specified how the payload is to be handled on add operations. @@ -144,12 +134,11 @@ public long CurrentCacheSize if (_currentCacheSize == 0) { // Read the system file for cache size - object cacheSizeObj = CacheManager.ReadSysFile(CacheSizeFile); - - // Did we successfully get data from the file? - if (cacheSizeObj != null) + long cacheSize; + if (CacheManager.ReadSysValue(CacheSizeFile, out cacheSize)) { - _currentCacheSize = (long) cacheSizeObj; + // Did we successfully get data from the file? Write it to our member var. + _currentCacheSize = cacheSize; } } @@ -160,7 +149,7 @@ private set // no need to do a pointless re-store of the same value if (_currentCacheSize != value || value == 0) { - CacheManager.WriteSysFile(CacheSizeFile, value); + CacheManager.WriteSysValue(CacheSizeFile, value); _currentCacheSize = value; } } @@ -395,11 +384,10 @@ private bool ShouldClean() try { // if the file can't be found, or is corrupt this will throw an exception - DateTime? lastClean = CacheManager.ReadSysFile(LastCleanedDateFile) as DateTime?; - - //AC: rewrote to be safer in null cases - if (lastClean == null) + DateTime lastClean; + if (!CacheManager.ReadSysValue(LastCleanedDateFile, out lastClean)) { + //AC: rewrote to be safer in cases where no value obtained. return true; } @@ -514,7 +502,7 @@ public long CleanCache(string regionName = null) } // mark that we've cleaned the cache - CacheManager.WriteSysFile(LastCleanedDateFile, DateTime.Now); + CacheManager.WriteSysValue(LastCleanedDateFile, DateTime.Now); // unlock cLock.Close(); diff --git a/src/FileCache/FileCacheManager.cs b/src/FileCache/FileCacheManager.cs index 28121af..e662b61 100644 --- a/src/FileCache/FileCacheManager.cs +++ b/src/FileCache/FileCacheManager.cs @@ -12,6 +12,11 @@ namespace System.Runtime.Caching { public abstract class FileCacheManager { + // Magic version for new sysfiles: 3.3.0 packed into a long. + protected const ulong CACHE_VERSION = ( 3 << 16 + + 3 << 8 + + 0 << 0); + public string CacheDir { get; set; } public string CacheSubFolder { get; set; } public string PolicySubFolder { get; set; } @@ -24,6 +29,40 @@ public abstract class FileCacheManager /// public TimeSpan AccessTimeout { get; set; } + + /// + /// Differentiate outdated cache formats from newer. + /// + /// Older caches use "BinaryFormatter", which is a security risk: + /// https://docs.microsoft.com/nl-nl/dotnet/standard/serialization/binaryformatter-security-guide#preferred-alternatives + /// + /// The newer caches have a 'magic' header we'll look for. + /// + /// BinaryReader opened to stream containing the file contents. + /// boolean indicating validity + protected bool HeaderVersionValid(BinaryReader reader) + { + // Don't much care about exceptions here - let them bubble up. + ulong version = reader.ReadUInt64(); + // Valid if magic header version matches. + return (version == CACHE_VERSION); + } + + /// + /// Differentiate outdated cache formats from newer. + /// + /// Older caches use "BinaryFormatter", which is a security risk: + /// https://docs.microsoft.com/nl-nl/dotnet/standard/serialization/binaryformatter-security-guide#preferred-alternatives + /// + /// The newer caches have a 'magic' header we'll look for. + /// + /// BinaryWriter opened to stream that will contain the file contents. + protected void HeaderVersionWrite(BinaryWriter writer) + { + // Don't much care about exceptions here - let them bubble up. + writer.Write(CACHE_VERSION); + } + protected virtual object DeserializePayloadData(string fileName, SerializationBinder objectBinder = null) { object data = null; @@ -98,9 +137,30 @@ public virtual FileCachePayload ReadFile(FileCache.PayloadMode mode, string key, return payload; } - private object LoadRawPayloadData(string cachePath) + private byte[] LoadRawPayloadData(string fileName) { - throw new NotSupportedException("Reading raw payload is not currently supported."); + byte[] data = null; + if (File.Exists(fileName)) + { + using (FileStream stream = GetStream(fileName, FileMode.Open, FileAccess.Read, FileShare.Read)) + { + using (BinaryReader reader = new BinaryReader(stream)) + { + // Check if it's valid version first. + if (!HeaderVersionValid(reader)) + { + // Failure - return invalid data. + return null; + + // `using` statements will clean up for us. + } + + // Valid - read entire file. + data = reader.ReadBytes(int.MaxValue); + } + } + } + return data; } protected virtual object Deserialize(string fileName, SerializationBinder objectBinder = null) @@ -165,7 +225,6 @@ public virtual long WriteFile(FileCache.PayloadMode mode, string key, FileCacheP case FileCache.PayloadMode.Serializable: using (FileStream stream = GetStream(cachedItemPath, FileMode.Create, FileAccess.Write, FileShare.None)) { - BinaryFormatter formatter = new BinaryFormatter(); formatter.Serialize(stream, data.Payload); } @@ -173,17 +232,20 @@ public virtual long WriteFile(FileCache.PayloadMode mode, string key, FileCacheP case FileCache.PayloadMode.RawBytes: using (FileStream stream = GetStream(cachedItemPath, FileMode.Create, FileAccess.Write, FileShare.None)) { - - if (data.Payload is byte[]) - { - byte[] dataPayload = (byte[])data.Payload; - stream.Write(dataPayload, 0, dataPayload.Length); - } - else if (data.Payload is Stream) + using (BinaryWriter writer = new BinaryWriter(stream)) { - Stream dataPayload = (Stream)data.Payload; - dataPayload.CopyTo(stream); - // no close or the like, we are not the owner + if (data.Payload is byte[]) + { + byte[] dataPayload = (byte[])data.Payload; + writer.Write(dataPayload); + } + else if (data.Payload is Stream) + { + Stream dataPayload = (Stream)data.Payload; + byte[] bytePayload = new byte[dataPayload.Length - dataPayload.Position]; + dataPayload.Read(bytePayload, (int)dataPayload.Position, bytePayload.Length); + // no close or the like for data.Payload - we are not the owner + } } } break; @@ -261,17 +323,56 @@ public IEnumerable GetRegions() public abstract string GetPolicyPath(string key, string regionName = null); /// - /// Reads data in from a system file. System files are not part of the - /// cache itself, but serve as a way for the cache to store data it - /// needs to operate. + /// Generic version of ReadSysValue just throws an ArgumentException to error on unknown new types. /// /// The name of the sysfile (without directory) - /// The data from the file - public object ReadSysFile(string filename) + /// The value read + /// success/failure boolean + public bool ReadSysValue(string filename, out T value) where T : struct { + throw new ArgumentException(string.Format("Type is currently unsupported: {0}", typeof(T).ToString()), "value"); + + // These types could be easily implemented following the `long` function as a template: + // - bool: + // + reader.ReadBoolean(); + // - byte: + // + reader.ReadByte(); + // - char: + // + reader.ReadChar(); + // - decimal: + // + reader.ReadDecimal(); + // - double: + // + reader.ReadDouble(); + // - short: + // + reader.ReadInt16(); + // - int: + // + reader.ReadInt32(); + // - long: + // + reader.ReadInt64(); + // - sbyte: + // + reader.ReadSbyte(); + // - ushort: + // + reader.ReadUInt16(); + // - uint: + // + reader.ReadUInt32(); + // - ulong: + // + reader.ReadUInt64(); + } + + /// + /// Read a `long` (64 bit signed int) from a sysfile. + /// + /// The name of the sysfile (without directory) + /// The value read or long.MinValue + /// success/failure boolean + public bool ReadSysValue(string filename, out long value) + { + // Return min value on fail. Success/fail will be either exception or bool return. + value = long.MinValue; + bool success = false; + // sys files go in the root directory string path = Path.Combine(CacheDir, filename); - object data = null; if (File.Exists(path)) { @@ -281,21 +382,33 @@ public object ReadSysFile(string filename) { using (FileStream stream = GetStream(path, FileMode.Open, FileAccess.Read, FileShare.Read)) { - BinaryFormatter formatter = new BinaryFormatter(); - try - { - data = formatter.Deserialize(stream); - } - catch (Exception) + using(BinaryReader reader = new BinaryReader(stream)) { - data = null; - } - finally - { - stream.Close(); + try + { + // The old "BinaryFormatter" sysfiles will fail this check. + if (HeaderVersionValid(reader)) + { + value = reader.ReadInt64(); + } + else + { + // Invalid version - return invalid value & failure. + value = long.MinValue; + success = false; + break; + } + } + catch (Exception) + { + value = long.MinValue; + // DriveCommerce: Need to rethrow to get the IOException caught. + throw; + } } + success = true; + break; } - break; } catch (IOException) { @@ -304,16 +417,47 @@ public object ReadSysFile(string filename) } } - return data; + // `value` already set correctly. + return success; } /// - /// Writes data to a system file that is not part of the cache itself, + /// Read a `DateTime` struct from a sysfile using `DateTime.FromBinary()`. + /// + /// The name of the sysfile (without directory) + /// The value read or DateTime.MinValue + /// success/failure boolean + public bool ReadSysValue(string filename, out DateTime value) + { + // DateTime is serialized as a long, so use that `ReadSysValue()` function. + long serialized; + if (ReadSysValue(filename, out serialized)) + { + value = DateTime.FromBinary(serialized); + return true; + } + // else failed: + value = DateTime.MinValue; + return false; + } + + /// + /// Generic version of `WriteSysValue` just throws an ArgumentException on unknown new types. + /// + /// The name of the sysfile (without directory) + /// The data to write to the sysfile + public void WriteSysValue(string filename, T data) where T : struct + { + throw new ArgumentException(string.Format("Type is currently unsupported: {0}", typeof(T).ToString()), "data"); + } + + /// + /// Writes a long to a system file that is not part of the cache itself, /// but is used to help it function. /// /// The name of the sysfile (without directory) - /// The data to write to the file - public void WriteSysFile(string filename, object data) + /// The long to write to the file + public void WriteSysValue(string filename, long data) { // sys files go in the root directory string path = Path.Combine(CacheDir, filename); @@ -321,9 +465,13 @@ public void WriteSysFile(string filename, object data) // write the data to the file using (FileStream stream = GetStream(path, FileMode.Create, FileAccess.Write, FileShare.Write)) { - BinaryFormatter formatter = new BinaryFormatter(); - formatter.Serialize(stream, data); - stream.Close(); + using (BinaryWriter writer = new BinaryWriter(stream)) + { + // Must write the magic version header first. + HeaderVersionWrite(writer); + + writer.Write(data); + } } } From 82ff59bdf0ddf274f450002ef824a6aac5766e2f Mon Sep 17 00:00:00 2001 From: Cole Brown Date: Wed, 28 Jul 2021 10:22:44 -0700 Subject: [PATCH 3/9] FileCache.SerializableCacheItemPolicy fix Removed BinaryFormatter usage from SerializableCacheItemPolicy. SerializableCacheItemPolicy now (de)serializes via functions: - Serialize() - Deserialize() These use BinaryWriter/BinaryReader instead of the security risk BinaryFormatter. SerializableCacheItemPolicy now use a magic header number like the cached files themselves do, in order to differentiate BinaryFormatter vs new policies. --- src/FileCache/FileCacheManager.cs | 35 ++++++--- src/FileCache/SerializableCacheItemPolicy.cs | 74 +++++++++++++++++++- 2 files changed, 97 insertions(+), 12 deletions(-) diff --git a/src/FileCache/FileCacheManager.cs b/src/FileCache/FileCacheManager.cs index e662b61..5361cc4 100644 --- a/src/FileCache/FileCacheManager.cs +++ b/src/FileCache/FileCacheManager.cs @@ -14,8 +14,8 @@ public abstract class FileCacheManager { // Magic version for new sysfiles: 3.3.0 packed into a long. protected const ulong CACHE_VERSION = ( 3 << 16 - + 3 << 8 - + 0 << 0); + + 3 << 8 + + 0 << 0); public string CacheDir { get; set; } public string CacheSubFolder { get; set; } @@ -127,8 +127,21 @@ public virtual FileCachePayload ReadFile(FileCache.PayloadMode mode, string key, } try { - // TODO: In part of the merge it looked like the policy was force serialized with LocalCacheBinder(), is this intended? - payload.Policy = Deserialize(policyPath) as SerializableCacheItemPolicy; + if (File.Exists(policyPath)) + { + using (FileStream stream = GetStream(policyPath, FileMode.Open, FileAccess.Read, FileShare.Read)) + { + using (BinaryReader reader = new BinaryReader(stream)) + { + // TODO: In part of the merge it looked like the policy was force serialized with LocalCacheBinder(), is this intended? + payload.Policy = SerializableCacheItemPolicy.Deserialize(reader, stream.Length); + } + } + } + else + { + payload.Policy = new SerializableCacheItemPolicy(); + } } catch { @@ -268,13 +281,13 @@ public virtual long WriteFile(FileCache.PayloadMode mode, string key, FileCacheP //write the cache policy using (FileStream stream = GetStream(cachedPolicy, FileMode.Create, FileAccess.Write, FileShare.None)) { - BinaryFormatter formatter = new BinaryFormatter(); - formatter.Serialize(stream, data.Policy); - - // adjust cache size - cacheSizeDelta += new FileInfo(cachedPolicy).Length; + using (BinaryWriter writer = new BinaryWriter(stream)) + { + data.Policy.Serialize(writer); - stream.Close(); + // adjust cache size + cacheSizeDelta += new FileInfo(cachedPolicy).Length; + } } return cacheSizeDelta; @@ -555,7 +568,7 @@ public override Type BindToType(string assemblyName, string typeName) // Get the type using the typeName and assemblyName typeToDeserialize = Type.GetType(String.Format("{0}, {1}", - typeName, assemblyName)); + typeName, assemblyName)); return typeToDeserialize; } diff --git a/src/FileCache/SerializableCacheItemPolicy.cs b/src/FileCache/SerializableCacheItemPolicy.cs index 1eebbeb..a93f864 100644 --- a/src/FileCache/SerializableCacheItemPolicy.cs +++ b/src/FileCache/SerializableCacheItemPolicy.cs @@ -7,11 +7,19 @@ FileCache is distributed under the Apache License 2.0. Consult "LICENSE.txt" included in this package for the Apache License 2.0. */ +using System.IO; + + namespace System.Runtime.Caching { [Serializable] public class SerializableCacheItemPolicy { + // Magic version for new policies: 3.3.0 packed into a long. + protected const ulong CACHE_VERSION = ( 3 << 16 + + 3 << 8 + + 0 << 0); + public DateTimeOffset AbsoluteExpiration { get; set; } private TimeSpan _slidingExpiration; @@ -24,7 +32,7 @@ public TimeSpan SlidingExpiration set { _slidingExpiration = value; - if (_slidingExpiration > new TimeSpan()) + if (_slidingExpiration > TimeSpan.Zero) { AbsoluteExpiration = DateTimeOffset.Now.Add(_slidingExpiration); } @@ -45,5 +53,69 @@ public SerializableCacheItemPolicy() /// The cache key that this particular policy refers to /// public string Key { get; set; } + + /// + /// Serialize this policy to the supplied BinaryWriter. + /// + /// Older policies use the "[Serializable]" attribute and BinaryFormatter, which is a security risk: + /// https://docs.microsoft.com/nl-nl/dotnet/standard/serialization/binaryformatter-security-guide#preferred-alternatives + /// + /// The newer caches have a 'magic' header we'll look for and serialize their fields manually. + /// + public void Serialize(BinaryWriter writer) + { + writer.Write(CACHE_VERSION); + + writer.Write(AbsoluteExpiration.Date.ToBinary()); + writer.Write(AbsoluteExpiration.Offset.TotalMilliseconds); + + writer.Write(SlidingExpiration.TotalMilliseconds); + + writer.Write(Key); + } + + /// + /// Deserialize a policy from the supplied BinaryReader. + /// + /// Older policies use the "[Serializable]" attribute and BinaryFormatter, which is a security risk: + /// https://docs.microsoft.com/nl-nl/dotnet/standard/serialization/binaryformatter-security-guide#preferred-alternatives + /// + /// The newer caches have a 'magic' header we'll look for and deserialize their fields manually. + /// If the 'magic' header isn't found, this returns an empty policy. + /// + public static SerializableCacheItemPolicy Deserialize(BinaryReader reader, long streamLength) + { + // Can't even check for the magic version number; return empty policy. + if (streamLength < sizeof(ulong)) + return new SerializableCacheItemPolicy(); + + try + { + var version = reader.ReadUInt64(); + if (version != CACHE_VERSION) + // Just return an empty policy if we read an invalid one. + // This is likely the older "BinaryFormatter"-serialized policy. + return new SerializableCacheItemPolicy(); + + return new SerializableCacheItemPolicy { + AbsoluteExpiration = new DateTimeOffset(DateTime.FromBinary(reader.ReadInt64()), + TimeSpan.FromMilliseconds(reader.ReadDouble())), + SlidingExpiration = TimeSpan.FromMilliseconds(reader.ReadDouble()), + Key = reader.ReadString(), + }; + } + catch (Exception error) + { + if (error is EndOfStreamException + || error is IOException + || error is ObjectDisposedException) + { + // Just return an empty policy if we failed to read. + return new SerializableCacheItemPolicy(); + } + // Didn't expect this error type; rethrow it. + throw; + } + } } } From dc7716077f3d98fb16cac949e6ce057cb5af9e4e Mon Sep 17 00:00:00 2001 From: Cole Brown Date: Wed, 11 Aug 2021 18:09:30 -0700 Subject: [PATCH 4/9] Unit test fixes. Bumped UnitTests to netcoreapp5.0 Nuked Microsoft.SourceLink.GitHub and GitVersionTask as they were causing build errors I didn't want to sort. Fixed 3 of 12 failing unit tests. - SerializableCacheItemPolicy was not serializing its time, only date. - SerializableCacheItemPolicy was messing up its expiration date when deserializing due to the SlidingExpiration setter. --- .../FileCache.UnitTests.csproj | 4 ++-- src/FileCache/FileCache.csproj | 16 ++++++++-------- src/FileCache/SerializableCacheItemPolicy.cs | 5 +++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/FileCache.UnitTests/FileCache.UnitTests.csproj b/src/FileCache.UnitTests/FileCache.UnitTests.csproj index 2743690..5ffe0f1 100644 --- a/src/FileCache.UnitTests/FileCache.UnitTests.csproj +++ b/src/FileCache.UnitTests/FileCache.UnitTests.csproj @@ -1,6 +1,6 @@  - net452;netcoreapp2.0 + net452;netcoreapp5.0 false true true @@ -31,4 +31,4 @@ - \ No newline at end of file + diff --git a/src/FileCache/FileCache.csproj b/src/FileCache/FileCache.csproj index d1fd45b..5a22820 100644 --- a/src/FileCache/FileCache.csproj +++ b/src/FileCache/FileCache.csproj @@ -1,7 +1,7 @@  net45;net48;netstandard2.0;netstandard2.1 - false + false cache objectcache System.Runtime.Caching.ObjectCache Apache-2.0 Adam Carter @@ -28,12 +28,12 @@ Auto - - - - All - - + + + + + + @@ -100,4 +100,4 @@ 2.0.0 - \ No newline at end of file + diff --git a/src/FileCache/SerializableCacheItemPolicy.cs b/src/FileCache/SerializableCacheItemPolicy.cs index a93f864..eb3979b 100644 --- a/src/FileCache/SerializableCacheItemPolicy.cs +++ b/src/FileCache/SerializableCacheItemPolicy.cs @@ -66,7 +66,7 @@ public void Serialize(BinaryWriter writer) { writer.Write(CACHE_VERSION); - writer.Write(AbsoluteExpiration.Date.ToBinary()); + writer.Write(AbsoluteExpiration.DateTime.ToBinary()); writer.Write(AbsoluteExpiration.Offset.TotalMilliseconds); writer.Write(SlidingExpiration.TotalMilliseconds); @@ -100,7 +100,8 @@ public static SerializableCacheItemPolicy Deserialize(BinaryReader reader, long return new SerializableCacheItemPolicy { AbsoluteExpiration = new DateTimeOffset(DateTime.FromBinary(reader.ReadInt64()), TimeSpan.FromMilliseconds(reader.ReadDouble())), - SlidingExpiration = TimeSpan.FromMilliseconds(reader.ReadDouble()), + // Don't clobber absolute by using sliding's setter; set the private value instead. + _slidingExpiration = TimeSpan.FromMilliseconds(reader.ReadDouble()), Key = reader.ReadString(), }; } From 4ba4a8d73329754f6865aa49d2f456045e396c9b Mon Sep 17 00:00:00 2001 From: Cole Brown Date: Mon, 23 Aug 2021 16:56:51 -0700 Subject: [PATCH 5/9] Fixed 3 unit tests. Fixed: - FileCacheTest.CacheSizeTest - FileCacheTest.ShrinkCacheTest - HashedFileCacheTest.CacheSizeTest Moved the calculation of the new policy file's size to be outside the 'using' blocks for writing it to disk so that it could read the actual file size instead of always getting 0. - It is moved entirely out of the using blocks to mirror the cache item's code. Removed a duplicated line of code in FileCache.WriteHelper(). - There was an `if` line that was there twice. - This has no effect on anything other than my sanity. --- src/FileCache/FileCache.cs | 10 +++++----- src/FileCache/FileCacheManager.cs | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/FileCache/FileCache.cs b/src/FileCache/FileCache.cs index fbe9bda..34ad22e 100644 --- a/src/FileCache/FileCache.cs +++ b/src/FileCache/FileCache.cs @@ -608,12 +608,14 @@ private long CacheSizeHelper(DirectoryInfo root) { size += fi.Length; } + // Add subdirectory sizes. var dis = root.EnumerateDirectories(); foreach (DirectoryInfo di in dis) { size += CacheSizeHelper(di); } + return size; } @@ -743,10 +745,9 @@ private void WriteHelper(PayloadMode mode, string key, FileCachePayload data, st //check to see if limit was reached if (CurrentCacheSize > MaxCacheSize) - if (CurrentCacheSize > MaxCacheSize) - { - MaxCacheSizeReached(this, new FileCacheEventArgs(CurrentCacheSize, MaxCacheSize)); - } + { + MaxCacheSizeReached(this, new FileCacheEventArgs(CurrentCacheSize, MaxCacheSize)); + } } #endregion @@ -955,7 +956,6 @@ public override object Remove(string key, string regionName = null) { object valueToDelete = null; - if (Contains(key, regionName) == true) { diff --git a/src/FileCache/FileCacheManager.cs b/src/FileCache/FileCacheManager.cs index 5361cc4..2d8bc09 100644 --- a/src/FileCache/FileCacheManager.cs +++ b/src/FileCache/FileCacheManager.cs @@ -284,12 +284,12 @@ public virtual long WriteFile(FileCache.PayloadMode mode, string key, FileCacheP using (BinaryWriter writer = new BinaryWriter(stream)) { data.Policy.Serialize(writer); - - // adjust cache size - cacheSizeDelta += new FileInfo(cachedPolicy).Length; } } + // Adjust cache size outside of the using blocks to ensure it's after the data is written. + cacheSizeDelta += new FileInfo(cachedPolicy).Length; + return cacheSizeDelta; } From 5a2e6c0200102236e5d4dd2a97684f9088f5563b Mon Sep 17 00:00:00 2001 From: Cole Brown Date: Tue, 24 Aug 2021 08:40:53 -0700 Subject: [PATCH 6/9] Fixed FileCacheTest.CleanCacheTest All FileCacheTest unit tests now passing. - 4 HashedFileCacheTest unit tests still failing. FileCacheManager was missing the DateTime version of `WriteSysValue()`. Added it to achieve function parity with `ReadSysValue()` functions. --- src/FileCache.UnitTests/FileCache.UnitTests.csproj | 2 +- src/FileCache/FileCache.cs | 4 +++- src/FileCache/FileCacheManager.cs | 12 ++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/FileCache.UnitTests/FileCache.UnitTests.csproj b/src/FileCache.UnitTests/FileCache.UnitTests.csproj index 5ffe0f1..ccc8f85 100644 --- a/src/FileCache.UnitTests/FileCache.UnitTests.csproj +++ b/src/FileCache.UnitTests/FileCache.UnitTests.csproj @@ -1,6 +1,6 @@  - net452;netcoreapp5.0 + netcoreapp5.0 false true true diff --git a/src/FileCache/FileCache.cs b/src/FileCache/FileCache.cs index 34ad22e..1212e09 100644 --- a/src/FileCache/FileCache.cs +++ b/src/FileCache/FileCache.cs @@ -692,8 +692,9 @@ public void Flush(DateTime minDate, string regionName = null) string policyPath = CacheManager.GetPolicyPath(key, region); string cachePath = CacheManager.GetCachePath(key, region); - // Update the Cache size + // Update the Cache size before flushing this item. CurrentCacheSize = GetCacheSize(); + //if either policy or cache are stale, delete both if (File.GetLastAccessTime(policyPath) < minDate || File.GetLastAccessTime(cachePath) < minDate) { @@ -706,6 +707,7 @@ public void Flush(DateTime minDate, string regionName = null) cLock.Close(); } } + /// /// Returns the policy attached to a given cache item. /// diff --git a/src/FileCache/FileCacheManager.cs b/src/FileCache/FileCacheManager.cs index 2d8bc09..3cf6b71 100644 --- a/src/FileCache/FileCacheManager.cs +++ b/src/FileCache/FileCacheManager.cs @@ -488,6 +488,18 @@ public void WriteSysValue(string filename, long data) } } + /// + /// Writes a long to a system file that is not part of the cache itself, + /// but is used to help it function. + /// + /// The name of the sysfile (without directory) + /// The DateTime to write to the file + public void WriteSysValue(string filename, DateTime data) + { + // Convert to long and use long's function. + WriteSysValue(filename, data.ToBinary()); + } + /// /// This function servies to centralize file stream access within this class. /// From fd9f41d923504b5cc5c8acfc28a6ab7c184d49aa Mon Sep 17 00:00:00 2001 From: Cole Brown Date: Tue, 24 Aug 2021 09:32:14 -0700 Subject: [PATCH 7/9] Simple README for the Unit Tests. - How to run. - Summary of suites. - WSL2 time bug. --- src/FileCache.UnitTests/README.md | 49 +++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 src/FileCache.UnitTests/README.md diff --git a/src/FileCache.UnitTests/README.md b/src/FileCache.UnitTests/README.md new file mode 100644 index 0000000..30de95a --- /dev/null +++ b/src/FileCache.UnitTests/README.md @@ -0,0 +1,49 @@ +Unit-Testing FileCache +------------------------ + +This will tell dotnet to build FileCache and run all of the tests. + +``` shell +cd /path/to/FileCache/src/FileCache.UnitTests +dotnet test FileCache.UnitTests.csproj +``` + +Unit-Test Suites +---------------- + +There are two test suites: + +1. FileCacheTest.cs + - These tests use the BasicFileCacheManager when running various unit-tests. + +1. HashedFileCache.cs + - These tests are similar, but use the HashedFileCacheManager. + + +WSL2 Warning! +---------------- + +Sometimes a WSL operating system's time can get out-of-sync with the actual time (see [this](https://github.com/microsoft/WSL/issues/4114)). If it's out of sync enough, some tests can fail if Windows is ultimately in charge of file stats like Last Accessed Time. This happens if your FileCache repo is on a Windows drive (i.e. something under /mnt/c, /mnt/d, etc). + +For example, FileCache.FlushRegionTest will fail with this error when your WSL'd Linux OS is 1 minute in the past: + +``` +Failed FlushRegionTest [1 s] +Error Message: + Expected result2 to be , but found "Value2". +Stack Trace: + at FluentAssertions.Execution.LateBoundTestFramework.Throw(String message) + at FluentAssertions.Execution.TestFrameworkProvider.Throw(String message) + at FluentAssertions.Execution.DefaultAssertionStrategy.HandleFailure(String message) + at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc) + at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc) + at FluentAssertions.Execution.AssertionScope.FailWith(String message, Object[] args) + at FluentAssertions.Primitives.ReferenceTypeAssertions`2.BeNull(String because, Object[] becauseArgs) + at FC.UnitTests.FileCacheTest.FlushRegionTest() in /mnt/path/to/FileCache/src/FileCache.UnitTests/FileCacheTest.cs:line 266 +``` + +To fix, you can try: + `sudo hwclock -s` + +If that doesn't work, you'll probably have to shutdown and restart the VMs: + `wsl --shutdown` From 51f2259facb784bcc7fa0ff64d85ed3088814689 Mon Sep 17 00:00:00 2001 From: Cole Brown Date: Tue, 24 Aug 2021 09:48:26 -0700 Subject: [PATCH 8/9] Upgraded package versions. Touched the unit testing project file. - Updated the nuget packages to their latest versions. - Added a System.Runtime.Caching 'ItemGroup Condition' for 'netcoreapp5.0'. - Made a comment about the magic Service GUID. --- src/FileCache.UnitTests/FileCache.UnitTests.csproj | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/FileCache.UnitTests/FileCache.UnitTests.csproj b/src/FileCache.UnitTests/FileCache.UnitTests.csproj index ccc8f85..ad01145 100644 --- a/src/FileCache.UnitTests/FileCache.UnitTests.csproj +++ b/src/FileCache.UnitTests/FileCache.UnitTests.csproj @@ -7,13 +7,15 @@ - - - - - + + + + + + + @@ -28,6 +30,8 @@ + + From 59e122254d1bda09e98e2bbb50205d96116c2197 Mon Sep 17 00:00:00 2001 From: Cole Brown Date: Thu, 2 Sep 2021 15:30:23 -0700 Subject: [PATCH 9/9] Fixed HashedFileCacheTest.FlushTest Extracted FileCacheManager's code for deserializing a policy into a new function. Made FileCacheManager and HashedFileCacheManager use that function. --- src/FileCache/FileCacheManager.cs | 54 +++++++++++++++---------- src/FileCache/HashedFileCacheManager.cs | 5 ++- 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/src/FileCache/FileCacheManager.cs b/src/FileCache/FileCacheManager.cs index 3cf6b71..b9515fd 100644 --- a/src/FileCache/FileCacheManager.cs +++ b/src/FileCache/FileCacheManager.cs @@ -97,6 +97,35 @@ protected virtual object DeserializePayloadData(string fileName, SerializationBi return data; } + protected SerializableCacheItemPolicy DeserializePolicyData(string policyPath) + { + SerializableCacheItemPolicy policy = null; + try + { + if (File.Exists(policyPath)) + { + using (FileStream stream = GetStream(policyPath, FileMode.Open, FileAccess.Read, FileShare.Read)) + { + using (BinaryReader reader = new BinaryReader(stream)) + { + // TODO: In part of the merge it looked like the policy was force serialized with LocalCacheBinder(), is this intended? + policy = SerializableCacheItemPolicy.Deserialize(reader, stream.Length); + } + } + } + else + { + policy = new SerializableCacheItemPolicy(); + } + } + catch + { + policy = new SerializableCacheItemPolicy(); + } + + return policy; + } + /// /// This function serves to centralize file reads within this class. /// @@ -125,28 +154,9 @@ public virtual FileCachePayload ReadFile(FileCache.PayloadMode mode, string key, default: throw new ArgumentOutOfRangeException(nameof(mode), mode, null); } - try - { - if (File.Exists(policyPath)) - { - using (FileStream stream = GetStream(policyPath, FileMode.Open, FileAccess.Read, FileShare.Read)) - { - using (BinaryReader reader = new BinaryReader(stream)) - { - // TODO: In part of the merge it looked like the policy was force serialized with LocalCacheBinder(), is this intended? - payload.Policy = SerializableCacheItemPolicy.Deserialize(reader, stream.Length); - } - } - } - else - { - payload.Policy = new SerializableCacheItemPolicy(); - } - } - catch - { - payload.Policy = new SerializableCacheItemPolicy(); - } + + payload.Policy = DeserializePolicyData(policyPath); + return payload; } diff --git a/src/FileCache/HashedFileCacheManager.cs b/src/FileCache/HashedFileCacheManager.cs index 45abe92..b0d62ea 100644 --- a/src/FileCache/HashedFileCacheManager.cs +++ b/src/FileCache/HashedFileCacheManager.cs @@ -12,6 +12,7 @@ namespace System.Runtime.Caching public class HashedFileCacheManager : FileCacheManager { private static IxxHash _hasher = xxHashFactory.Instance.Create(); + /// /// Returns a 64bit hash in hex of supplied key /// @@ -54,7 +55,7 @@ private string GetFileName(string key, string regionName = null) //check for correct key try { - SerializableCacheItemPolicy policy = Deserialize(fileName) as SerializableCacheItemPolicy; + SerializableCacheItemPolicy policy = DeserializePolicyData(fileName); if (key.CompareTo(policy.Key) == 0) { //correct key found! @@ -124,7 +125,7 @@ public override IEnumerable GetKeys(string regionName = null) { try { - SerializableCacheItemPolicy policy = Deserialize(file) as SerializableCacheItemPolicy; + SerializableCacheItemPolicy policy = DeserializePolicyData(file); keys.Add(policy.Key); } catch