From 299d25af273ea10f4066520ebc5a37f86acf7c7e Mon Sep 17 00:00:00 2001 From: Matt Nadareski Date: Wed, 20 Apr 2022 13:47:55 -0700 Subject: [PATCH] Implement Drive.Create for safety --- CHANGELIST.md | 1 + MPF.Check/Program.cs | 2 +- MPF.Core/Data/Drive.cs | 136 ++++++++++++++++------- MPF.Test/Library/DumpEnvironmentTests.cs | 6 +- 4 files changed, 100 insertions(+), 45 deletions(-) diff --git a/CHANGELIST.md b/CHANGELIST.md index eabf0223..c108f4dd 100644 --- a/CHANGELIST.md +++ b/CHANGELIST.md @@ -89,6 +89,7 @@ - Possibly fix tab regex replacement - Add PIC.bin to log zip - Organize projects in solution +- Implement Drive.Create for safety ### 2.3 (2022-02-05) - Start overhauling Redump information pulling, again diff --git a/MPF.Check/Program.cs b/MPF.Check/Program.cs index d1090198..8f934c34 100644 --- a/MPF.Check/Program.cs +++ b/MPF.Check/Program.cs @@ -53,7 +53,7 @@ namespace MPF.Check // Now populate an environment Drive drive = null; if (!string.IsNullOrWhiteSpace(path)) - drive = new Drive(null, new DriveInfo(path)); + drive = Drive.Create(null, path); var env = new DumpEnvironment(options, "", filepath, drive, knownSystem, mediaType, null); diff --git a/MPF.Core/Data/Drive.cs b/MPF.Core/Data/Drive.cs index feb0e229..9d99aadd 100644 --- a/MPF.Core/Data/Drive.cs +++ b/MPF.Core/Data/Drive.cs @@ -23,10 +23,11 @@ namespace MPF.Core.Data /// /// TODO: This needs to be less Windows-centric. Devices do not always have a single letter that can be used. /// TODO: Can the Aaru models be used instead of the ones I've created here? - /// TODO: Reduce reliance on the DriveInfo object, if possible /// public class Drive { + #region Fields + /// /// Represents drive type /// @@ -35,46 +36,32 @@ namespace MPF.Core.Data /// /// Drive partition format /// - public string DriveFormat => driveInfo?.DriveFormat; - - /// - /// Windows drive letter - /// - public char Letter => driveInfo?.Name[0] ?? '\0'; + public string DriveFormat { get; private set; } = null; /// /// Windows drive path /// - public string Name => driveInfo?.Name; + public string Name { get; private set; } = null; /// /// Represents if Windows has marked the drive as active /// - public bool MarkedActive => driveInfo?.IsReady ?? false; + public bool MarkedActive { get; private set; } = false; /// /// Represents the total size of the drive /// - public long TotalSize => driveInfo?.TotalSize ?? default; + public long TotalSize { get; private set; } = default; /// /// Media label as read by Windows /// /// The try/catch is needed because Windows will throw an exception if the drive is not marked as active - public string VolumeLabel - { - get - { - try - { - return driveInfo?.VolumeLabel; - } - catch - { - return null; - } - } - } + public string VolumeLabel { get; private set; } = null; + + #endregion + + #region Derived Fields /// /// Media label as read by Windows, formatted to avoid odd outputs @@ -84,12 +71,12 @@ namespace MPF.Core.Data get { string volumeLabel = Template.DiscNotDetected; - if (driveInfo.IsReady) + if (this.MarkedActive) { - if (string.IsNullOrWhiteSpace(driveInfo.VolumeLabel)) + if (string.IsNullOrWhiteSpace(this.VolumeLabel)) volumeLabel = "track"; else - volumeLabel = driveInfo.VolumeLabel; + volumeLabel = this.VolumeLabel; } foreach (char c in Path.GetInvalidFileNameChars()) @@ -100,16 +87,72 @@ namespace MPF.Core.Data } /// - /// DriveInfo object representing the drive, if possible + /// Windows drive letter /// - private DriveInfo driveInfo; + public char Letter => this.Name == null || this.Name.Length == 0 ? '\0' : this.Name[0]; - public Drive(InternalDriveType? driveType, DriveInfo driveInfo) + #endregion + + /// + /// Protected constructor + /// + protected Drive() { } + + /// + /// Create a new Drive object from a drive type and device path + /// + /// InternalDriveType value representing the drive type + /// Path to the device according to the local machine + public static Drive Create(InternalDriveType? driveType, string devicePath) { - this.InternalDriveType = driveType; - this.driveInfo = driveInfo; + // Create a new, empty drive object + var drive = new Drive() + { + InternalDriveType = driveType, + }; + + // If we have an invalid device path, return null + if (string.IsNullOrWhiteSpace(devicePath)) + return null; + + // Sanitize a Windows-formatted long device path + if (devicePath.StartsWith("\\\\.\\")) + devicePath = devicePath.Substring("\\\\.\\".Length); + + // Create and validate the drive info object + var driveInfo = new DriveInfo(devicePath); + if (driveInfo == null || driveInfo == default) + return null; + + // Fill in the rest of the data + drive.PopulateFromDriveInfo(driveInfo); + + return drive; } + /// + /// Populate all fields from a DriveInfo object + /// + /// DriveInfo object to populate from + private void PopulateFromDriveInfo(DriveInfo driveInfo) + { + // If we have an invalid DriveInfo, just return + if (driveInfo == null || driveInfo == default) + return; + + // Populate the data fields + this.Name = driveInfo.Name; + this.MarkedActive = driveInfo.IsReady; + if (this.MarkedActive) + { + this.DriveFormat = driveInfo.DriveFormat; + this.TotalSize = driveInfo.TotalSize; + this.VolumeLabel = driveInfo.VolumeLabel; + } + } + + #region Public Functionality + /// /// Create a list of active drives matched to their volume labels /// @@ -343,7 +386,7 @@ namespace MPF.Core.Data public byte[] ReadSector(long num, int size = 2048) { // Missing drive leter is not supported - if (string.IsNullOrEmpty(this.driveInfo?.Name)) + if (string.IsNullOrEmpty(this.Name)) return null; // We don't support negative sectors @@ -380,7 +423,14 @@ namespace MPF.Core.Data /// Refresh the current drive information based on path /// public void RefreshDrive() - => this.driveInfo = DriveInfo.GetDrives().FirstOrDefault(d => d?.Name == this.Name); + { + var driveInfo = DriveInfo.GetDrives().FirstOrDefault(d => d?.Name == this.Name); + this.PopulateFromDriveInfo(driveInfo); + } + + #endregion + + #region Helpers #if NETFRAMEWORK @@ -405,7 +455,7 @@ namespace MPF.Core.Data // Get all supported drive types var drives = DriveInfo.GetDrives() .Where(d => desiredDriveTypes.Contains(d.DriveType)) - .Select(d => new Drive(EnumConverter.ToInternalDriveType(d.DriveType), d)) + .Select(d => Create(EnumConverter.ToInternalDriveType(d.DriveType), d.Name)) .ToList(); // Get the floppy drives and set the flag from removable @@ -572,9 +622,9 @@ namespace MPF.Core.Data var desc = ftr.Descriptors.First(d => d.Code == 0x0000); bool isOptical = IsOptical(desc.Data); if (isOptical) - return new Drive(Data.InternalDriveType.Optical, new DriveInfo(windowsLocalDevicePath)); + return Create(Data.InternalDriveType.Optical, windowsLocalDevicePath); else if (!ignoreFixedDrives) - return new Drive(Data.InternalDriveType.Removable, new DriveInfo(windowsLocalDevicePath)); + return Create(Data.InternalDriveType.Removable, windowsLocalDevicePath); } } @@ -583,20 +633,20 @@ namespace MPF.Core.Data switch (dev.Type) { case DeviceType.MMC: - return new Drive(Data.InternalDriveType.Removable, new DriveInfo(windowsLocalDevicePath)); + return Create(Data.InternalDriveType.Removable, windowsLocalDevicePath); case DeviceType.SecureDigital: - return new Drive(Data.InternalDriveType.Removable, new DriveInfo(windowsLocalDevicePath)); + return Create(Data.InternalDriveType.Removable, windowsLocalDevicePath); } if (dev.IsUsb) - return new Drive(Data.InternalDriveType.Removable, new DriveInfo(windowsLocalDevicePath)); + return Create(Data.InternalDriveType.Removable, windowsLocalDevicePath); if (dev.IsFireWire) - return new Drive(Data.InternalDriveType.Removable, new DriveInfo(windowsLocalDevicePath)); + return Create(Data.InternalDriveType.Removable, windowsLocalDevicePath); if (dev.IsPcmcia) - return new Drive(Data.InternalDriveType.Removable, new DriveInfo(windowsLocalDevicePath)); + return Create(Data.InternalDriveType.Removable, windowsLocalDevicePath); } dev.Close(); @@ -744,5 +794,7 @@ namespace MPF.Core.Data } #endif + + #endregion } } diff --git a/MPF.Test/Library/DumpEnvironmentTests.cs b/MPF.Test/Library/DumpEnvironmentTests.cs index 4349ab83..6f58071a 100644 --- a/MPF.Test/Library/DumpEnvironmentTests.cs +++ b/MPF.Test/Library/DumpEnvironmentTests.cs @@ -18,9 +18,11 @@ namespace MPF.Test.Library public void ParametersValidTest(string parameters, char letter, bool isFloppy, MediaType? mediaType, bool expected) { var options = new Options() { InternalProgram = InternalProgram.DiscImageCreator }; + + // TODO: This relies on creating real objects for the drive. Can we mock this out instead? var drive = isFloppy - ? new Drive(InternalDriveType.Floppy, new DriveInfo(letter.ToString())) - : new Drive(InternalDriveType.Optical, new DriveInfo(letter.ToString())); + ? Drive.Create(InternalDriveType.Floppy, letter.ToString()) + : Drive.Create(InternalDriveType.Optical, letter.ToString()); var env = new DumpEnvironment(options, string.Empty, string.Empty, drive, RedumpSystem.IBMPCcompatible, mediaType, parameters);