Fix NRE on passing empty string to LoadRom, simplify HawkFile creation

HawkFile now correctly sets FullPathWithoutMember to "" and Exists to false,
meaning RomLoader will return false instead of crashing.
This commit is contained in:
YoshiRulz 2020-05-21 15:45:08 +10:00
parent 9ae272e778
commit ab1dca3e41
No known key found for this signature in database
GPG Key ID: C4DE31C245353FB7
2 changed files with 131 additions and 166 deletions

View File

@ -214,38 +214,21 @@ namespace BizHawk.Client.Common
public bool LoadRom(string path, CoreComm nextComm, string launchLibretroCore, bool forceAccurateCore = false, int recursiveCount = 0)
{
if (path == null) return false;
if (recursiveCount > 1) // hack to stop recursive calls from endlessly rerunning if we can't load it
{
DoLoadErrorCallback("Failed multiple attempts to load ROM.", "");
return false;
}
bool cancel = false;
if (path == null)
{
return false;
}
using var file = new HawkFile(); // I'm almost certain that we'll see NREs unless Open or Parse is called, so I deprecated this ctor as a nag --yoshi
// only try mounting a file if a filename was given
if (!string.IsNullOrEmpty(path))
{
// MAME uses these extensions for arcade ROMs, but also accepts all sorts of variations of archives, folders, and files. if we let archive loader handle this, it won't know where to stop, since it'd require MAME's ROM database (which contains ROM names and blob hashes) to look things up, and even then it might be confused by archive/folder structure
// so assume the user provides the proper ROM directly, and handle possible errors later
if (OpenAdvanced is OpenAdvanced_MAME)
{
file.NonArchiveExtensions = new[] { ".zip", ".7z" };
}
file.Open(path);
// if the provided file doesn't even exist, give up!
if (!file.Exists)
{
return false;
}
}
using var file = new HawkFile(
path,
nonArchiveExtensions: OpenAdvanced is OpenAdvanced_MAME
? new[] { ".zip", ".7z" } // MAME uses these extensions for arcade ROMs, but also accepts all sorts of variations of archives, folders, and files. if we let archive loader handle this, it won't know where to stop, since it'd require MAME's ROM database (which contains ROM names and blob hashes) to look things up, and even then it might be confused by archive/folder structure. so assume the user provides the proper ROM directly, and handle possible errors later
: null
);
if (!file.Exists) return false; // if the provided file doesn't even exist, give up!
CanonicalFullPath = file.CanonicalFullPath;
@ -256,6 +239,7 @@ namespace BizHawk.Client.Common
try
{
string ext = null;
var cancel = false;
if (OpenAdvanced is OpenAdvanced_Libretro)
{

View File

@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
@ -21,19 +22,16 @@ namespace BizHawk.Common
/// </remarks>
public sealed class HawkFile : IDisposable
{
private List<HawkArchiveFileItem>? _archiveItems;
private readonly List<HawkArchiveFileItem>? _archiveItems;
private Stream? _boundStream;
private IHawkArchiveFile? _extractor;
private bool _rootExists;
private readonly bool _rootExists;
private Stream? _rootStream;
/// <summary>These file extensions are assumed to not be archives (with default value, mitigates high false positive rate caused by weak archive detection signatures).</summary>
public IReadOnlyCollection<string> NonArchiveExtensions = CommonNonArchiveExtensions;
/// <exception cref="InvalidOperationException"><see cref="IsArchive"/> is <see langword="false"/></exception>
public IList<HawkArchiveFileItem> ArchiveItems => (IsArchive ? _archiveItems : null) ?? throw new InvalidOperationException("Can't get archive items from non-archive");
@ -44,27 +42,25 @@ namespace BizHawk.Common
/// <summary>returns the complete canonical full path ("c:\path\to\archive|member") of the bound file</summary>
[HawkFilePath]
public string? CanonicalFullPath => MakeCanonicalName(SafeFullPathWithoutMember, ArchiveMemberPath);
public string CanonicalFullPath => MakeCanonicalName(FullPathWithoutMember, ArchiveMemberPath);
/// <summary>returns the complete canonical name ("archive|member") of the bound file</summary>
[HawkFilePath]
public string? CanonicalName => MakeCanonicalName(Path.GetFileName(SafeFullPathWithoutMember), ArchiveMemberPath);
public string CanonicalName => MakeCanonicalName(Path.GetFileName(FullPathWithoutMember), ArchiveMemberPath);
/// <summary>Gets the directory containing the root</summary>
public string? Directory => Path.GetDirectoryName(SafeFullPathWithoutMember);
public string? Directory => Path.GetDirectoryName(FullPathWithoutMember);
/// <value><see cref="true"/> iff a file is bound and the bound file exists</value>
/// <remarks>NOTE: this isn't set until the file is <see cref="Open">Opened</see>. Not too great...</remarks>
public bool Exists { get; private set; }
public readonly bool Exists;
/// <summary>returns the extension of Name</summary>
public string? Extension => Path.GetExtension(Name).ToUpperInvariant();
/// <summary>returns the extension of Name in uppercase</summary>
public string Extension => Path.GetExtension(Name).ToUpperInvariant();
/// <value>returns the complete full path of the bound file, excluding the archive member portion</value>
/// <remarks>assigned in <see cref="Open"/> and <see cref="Parse"/>, but if neither is called may be <see langword="null"/> and cause NREs</remarks>
public string? FullPathWithoutMember { get; private set; }
public readonly string FullPathWithoutMember;
public bool IsArchive { get; private set; }
public readonly bool IsArchive;
/// <summary>Indicates whether the file is an archive member (IsArchive && IsBound[to member])</summary>
public bool IsArchiveMember => IsArchive && IsBound;
@ -73,133 +69,38 @@ namespace BizHawk.Common
public bool IsBound => _boundStream != null;
/// <summary>returns the virtual name of the bound file (disregarding the archive). Useful as a basic content identifier.</summary>
public string Name => ArchiveMemberPath ?? SafeFullPathWithoutMember;
private string SafeFullPathWithoutMember => FullPathWithoutMember ?? throw new NullReferenceException($"this is related to the deprecated no-arg ctor, {nameof(FullPathWithoutMember)} is only assigned in {nameof(Open)}/{nameof(Parse)}");
[Obsolete]
public HawkFile() {}
public string Name => ArchiveMemberPath ?? FullPathWithoutMember;
/// <summary>Makes a new HawkFile based on the provided path.</summary>
/// <remarks>If <paramref name="delayIOAndDearchive"/> is <see langword="true"/>, <see cref="Parse"/> will be called instead of <see cref="Open"/>.</remarks>
public HawkFile([HawkFilePath] string path, bool delayIOAndDearchive = false)
/// <param name="delayIOAndDearchive">Pass <see langword="true"/> to only populate a few fields (those that can be computed from the string <paramref name="path"/>), which is less computationally expensive.</param>
/// <param name="nonArchiveExtensions">
/// These file extensions are assumed to not be archives. Include the leading period in each, and use lowercase.<br/>
/// Does not apply when <paramref name="delayIOAndDearchive"/> is <see langword="true"/>.<br/>
/// If <see langword="null"/> is passed (the default), uses <see cref="CommonNonArchiveExtensions"/> which should mitigate false positives caused by weak archive detection signatures.
/// </param>
public HawkFile([HawkFilePath] string path, bool delayIOAndDearchive = false, IReadOnlyCollection<string>? nonArchiveExtensions = null)
{
if (delayIOAndDearchive) Parse(path);
else Open(path);
}
/// <summary>binds the specified ArchiveItem which you should have gotten by interrogating an archive hawkfile</summary>
public HawkFile? BindArchiveMember(HawkArchiveFileItem item) => BindArchiveMember(item.Index);
/// <summary>binds the selected archive index</summary>
/// <exception cref="InvalidOperationException">stream already bound</exception>
public HawkFile? BindArchiveMember(int index)
{
if (!_rootExists) return this;
if (_boundStream != null) throw new InvalidOperationException("stream already bound!");
if (_archiveItems == null || _extractor == null) throw new InvalidOperationException("not an archive");
var archiveIndex = _archiveItems[index].ArchiveIndex;
_boundStream = new MemoryStream();
_extractor.ExtractFile(archiveIndex, _boundStream);
_boundStream.Position = 0;
ArchiveMemberPath = _archiveItems[index].Name; // TODO - maybe go through our own list of names? maybe not, its indices don't match...
#if DEBUG
Console.WriteLine($"{nameof(HawkFile)} bound {CanonicalFullPath}");
#endif
BoundIndex = archiveIndex;
return this;
}
/// <summary>binds a path within the archive; returns null if that path didnt exist.</summary>
public HawkFile? BindArchiveMember(string? name)
{
var ai = FindArchiveMember(name);
return ai == null ? null : BindArchiveMember(ai.Value);
}
/// <exception cref="InvalidOperationException">stream already bound</exception>
private HawkFile? BindByExtensionCore(bool first, params string[] extensions)
{
if (!_rootExists) return this;
if (_boundStream != null) throw new InvalidOperationException("stream already bound!");
if (_archiveItems == null || _extractor == null)
if (delayIOAndDearchive)
{
// open uncompressed file
if (extensions.Length == 0
|| Path.GetExtension(SafeFullPathWithoutMember).Substring(1).In(extensions))
var split = SplitArchiveMemberPath(path);
if (split != null)
{
BindRoot();
(path, ArchiveMemberPath) = split.Value;
IsArchive = true; // we'll assume that the '|' is only used for archives
}
FullPathWithoutMember = path;
return;
}
else
{
if (extensions.Length != 0)
{
var candidates = _archiveItems.Where(item => Path.GetExtension(item.Name).Substring(1).In(extensions)).ToList();
if (candidates.Count != 0 && first || candidates.Count == 1) BindArchiveMember(candidates[0].Index);
}
else if (first || _archiveItems.Count == 1)
{
BindArchiveMember(0);
}
}
return this;
}
/// <summary>Binds the first item in the archive (or the file itself), assuming that there is anything in the archive.</summary>
public HawkFile? BindFirst() => BindFirstOf();
/// <summary>Binds the first item in the archive (or the file itself) if the extension matches one of the supplied templates.</summary>
/// <remarks>You probably should use <see cref="BindSoleItemOf"/> or the archive chooser instead.</remarks>
public HawkFile? BindFirstOf(params string[] extensions) => BindByExtensionCore(true, extensions);
/// <summary>causes the root to be bound (in the case of non-archive files)</summary>
private void BindRoot()
{
_boundStream = _rootStream;
#if DEBUG
Console.WriteLine($"{nameof(HawkFile)} bound {CanonicalFullPath}");
#endif
}
/// <summary>binds one of the supplied extensions if there is only one match in the archive</summary>
public HawkFile? BindSoleItemOf(params string[] extensions) => BindByExtensionCore(false, extensions);
public void Dispose()
{
Unbind();
_extractor?.Dispose();
_extractor = null;
_rootStream?.Dispose();
_rootStream = null;
}
/// <summary>finds an ArchiveItem with the specified name (path) within the archive; returns null if it doesnt exist</summary>
public HawkArchiveFileItem? FindArchiveMember(string? name) => ArchiveItems.FirstOrDefault(ai => ai.Name == name);
/// <returns>a stream for the currently bound file</returns>
/// <exception cref="InvalidOperationException">no stream bound (haven't called <see cref="BindArchiveMember(int)"/> or overload)</exception>
public Stream GetStream() => _boundStream ?? throw new InvalidOperationException($"{nameof(HawkFile)}: Can't call {nameof(GetStream)}() before you've successfully bound something!");
/// <summary>Opens the file at <paramref name="path"/>. This may take a while if the file is an archive, as it may be accessed and scanned.</summary>
/// <exception cref="InvalidOperationException">already opened via <see cref="HawkFile(string)"/>, this method, or <see cref="Parse"/></exception>
public void Open([HawkFilePath] string path)
{
if (FullPathWithoutMember != null) throw new InvalidOperationException($"Don't reopen a {nameof(HawkFile)}.");
string? autobind = null;
var split = SplitArchiveMemberPath(path);
if (split != null) (path, autobind) = split.Value;
_rootExists = new FileInfo(path).Exists;
if (!_rootExists) return;
var split1 = SplitArchiveMemberPath(path);
if (split1 != null) (path, autobind) = split1.Value;
FullPathWithoutMember = path;
Exists = true;
Exists = _rootExists = !string.IsNullOrEmpty(path) && new FileInfo(path).Exists;
if (!_rootExists) return;
if (DearchivalMethod != null
&& !NonArchiveExtensions.Contains(Path.GetExtension(path).ToLowerInvariant())
&& !(nonArchiveExtensions ?? CommonNonArchiveExtensions).Contains(Path.GetExtension(path).ToLowerInvariant())
&& DearchivalMethod.CheckSignature(path, out _, out _))
{
_extractor = DearchivalMethod.Construct(path);
@ -246,18 +147,98 @@ namespace BizHawk.Common
}
}
/// <returns>an unopened <see cref="HawkFile"/> with only some fields populated, specifically those where the value is in <paramref name="path"/></returns>
public void Parse([HawkFilePath] string path)
/// <summary>binds the specified ArchiveItem which you should have gotten by interrogating an archive hawkfile</summary>
public HawkFile BindArchiveMember(HawkArchiveFileItem item) => BindArchiveMember(item.Index);
/// <summary>binds the selected archive index</summary>
/// <exception cref="InvalidOperationException">stream already bound</exception>
public HawkFile BindArchiveMember(int index)
{
var split = SplitArchiveMemberPath(path);
if (split != null)
{
(path, ArchiveMemberPath) = split.Value;
IsArchive = true; // we'll assume that the '|' is only used for archives
}
FullPathWithoutMember = path;
if (!_rootExists) return this;
if (_boundStream != null) throw new InvalidOperationException("stream already bound!");
if (_archiveItems == null || _extractor == null) throw new InvalidOperationException("not an archive");
var archiveIndex = _archiveItems[index].ArchiveIndex;
_boundStream = new MemoryStream();
_extractor.ExtractFile(archiveIndex, _boundStream);
_boundStream.Position = 0;
ArchiveMemberPath = _archiveItems[index].Name; // TODO - maybe go through our own list of names? maybe not, its indices don't match...
Debug.WriteLine($"{nameof(HawkFile)} bound {CanonicalFullPath}");
BoundIndex = archiveIndex;
return this;
}
/// <summary>binds a path within the archive; returns null if that path didnt exist.</summary>
public HawkFile? BindArchiveMember(string? name)
{
var ai = FindArchiveMember(name);
return ai == null ? null : BindArchiveMember(ai.Value);
}
/// <exception cref="InvalidOperationException">stream already bound</exception>
private HawkFile BindByExtensionCore(bool first, params string[] extensions)
{
if (!_rootExists) return this;
if (_boundStream != null) throw new InvalidOperationException("stream already bound!");
if (_archiveItems == null || _extractor == null)
{
// open uncompressed file
if (extensions.Length == 0
|| Path.GetExtension(FullPathWithoutMember).Substring(1).In(extensions))
{
BindRoot();
}
}
else
{
if (extensions.Length != 0)
{
var candidates = _archiveItems.Where(item => Path.GetExtension(item.Name).Substring(1).In(extensions)).ToList();
if (candidates.Count != 0 && first || candidates.Count == 1) BindArchiveMember(candidates[0].Index);
}
else if (first || _archiveItems.Count == 1)
{
BindArchiveMember(0);
}
}
return this;
}
/// <summary>Binds the first item in the archive (or the file itself), assuming that there is anything in the archive.</summary>
public HawkFile BindFirst() => BindFirstOf();
/// <summary>Binds the first item in the archive (or the file itself) if the extension matches one of the supplied templates.</summary>
/// <remarks>You probably should use <see cref="BindSoleItemOf"/> or the archive chooser instead.</remarks>
public HawkFile BindFirstOf(params string[] extensions) => BindByExtensionCore(true, extensions);
/// <summary>causes the root to be bound (in the case of non-archive files)</summary>
private void BindRoot()
{
_boundStream = _rootStream;
Debug.WriteLine($"{nameof(HawkFile)} bound {CanonicalFullPath}");
}
/// <summary>binds one of the supplied extensions if there is only one match in the archive</summary>
public HawkFile BindSoleItemOf(params string[] extensions) => BindByExtensionCore(false, extensions);
public void Dispose()
{
Unbind();
_extractor?.Dispose();
_extractor = null;
_rootStream?.Dispose();
_rootStream = null;
}
/// <summary>finds an ArchiveItem with the specified name (path) within the archive; returns null if it doesnt exist</summary>
public HawkArchiveFileItem? FindArchiveMember(string? name) => ArchiveItems.FirstOrDefault(ai => ai.Name == name);
/// <returns>a stream for the currently bound file</returns>
/// <exception cref="InvalidOperationException">no stream bound (haven't called <see cref="BindArchiveMember(int)"/> or overload)</exception>
public Stream GetStream() => _boundStream ?? throw new InvalidOperationException($"{nameof(HawkFile)}: Can't call {nameof(GetStream)}() before you've successfully bound something!");
/// <summary>attempts to read all the content from the file</summary>
public byte[] ReadAllBytes()
{
@ -279,7 +260,7 @@ namespace BizHawk.Common
/// <summary>Set this with an instance which can construct archive handlers as necessary for archive handling.</summary>
public static IFileDearchivalMethod<IHawkArchiveFile>? DearchivalMethod;
private static readonly IReadOnlyCollection<string> CommonNonArchiveExtensions = new[] { ".smc", ".sfc", ".dll" };
public static readonly IReadOnlyCollection<string> CommonNonArchiveExtensions = new[] { ".smc", ".sfc", ".dll" };
/// <summary>Utility: Uses full HawkFile processing to determine whether a file exists at the provided path</summary>
public static bool ExistsAt(string path)