Skip to content

Commit

Permalink
Remove redundant pre-install ZIP validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@HebaruSan
HebaruSan committed Oct 10, 2023
1 parent ae2a786 commit 727fb65
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 157 deletions.
6 changes: 3 additions & 3 deletions Core/ModuleInstaller.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public static string CachedOrDownload(CkanModule module, NetModuleCache cache, s
filename = CkanModule.StandardName(module.identifier, module.version);
}

string full_path = cache.GetCachedZip(module);
string full_path = cache.GetCachedFilename(module);
if (full_path == null)
{
return Download(module, filename, cache);
Expand Down Expand Up @@ -264,7 +264,7 @@ private void Install(CkanModule module, bool autoInstalled, Registry registry, r
}

// Find ZIP in the cache if we don't already have it.
filename = filename ?? Cache.GetCachedZip(module);
filename = filename ?? Cache.GetCachedFilename(module);

// If we *still* don't have a file, then kraken bitterly.
if (filename == null)
Expand Down Expand Up @@ -1284,7 +1284,7 @@ public static IEnumerable<string> PrioritizedHosts(IEnumerable<Uri> urls)
/// </summary>
private void DownloadModules(IEnumerable<CkanModule> mods, IDownloader downloader)
{
List<CkanModule> downloads = mods.Where(module => !Cache.IsCachedZip(module)).ToList();
List<CkanModule> downloads = mods.Where(module => !Cache.IsCached(module)).ToList();

if (downloads.Count > 0)
{
Expand Down
128 changes: 0 additions & 128 deletions Core/Net/NetFileCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

using log4net;
using ChinhDo.Transactions.FileManager;
using ICSharpCode.SharpZipLib.Zip;

using CKAN.Extensions;
using CKAN.Versioning;
Expand All @@ -36,13 +35,6 @@ public class NetFileCache : IDisposable
private static readonly Regex cacheFileRegex = new Regex("^[0-9A-F]{8}-", RegexOptions.Compiled);
private static readonly ILog log = LogManager.GetLogger(typeof (NetFileCache));

static NetFileCache()
{
// SharpZibLib 1.1.0 changed this to default to false, but we depend on it for international mods.
// https://github.com/icsharpcode/SharpZipLib/issues/591
ZipStrings.UseUnicode = true;
}

/// <summary>
/// Initialize a cache given a GameInstanceManager
/// </summary>
Expand Down Expand Up @@ -163,13 +155,6 @@ public bool IsCached(Uri url, out string outFilename)
return outFilename != null;
}

/// <summary>
/// Returns true if our given URL is cached, *and* it passes zip
/// validation tests. Prefer this over IsCached when working with
/// zip files.
/// </summary>
public bool IsCachedZip(Uri url) => GetCachedZip(url) != null;

/// <summary>
/// Returns true if a file matching the given URL is cached, but makes no
/// attempts to check if it's even valid. This is very fast.
Expand Down Expand Up @@ -260,38 +245,6 @@ private string scanDirectory(Dictionary<string, string> files, string findHash,
return null;
}

/// <summary>
/// Returns the filename for a cached URL, if and only if it
/// passes zipfile validation tests. Prefer this to GetCachedFilename
/// when working with zip files. Returns null if not available, or
/// validation failed.
///
/// Low level CRC (cyclic redundancy check) checks will be done.
/// This can take time on order of seconds for larger zip files.
/// </summary>
public string GetCachedZip(Uri url)
{
string filename = GetCachedFilename(url);
if (string.IsNullOrEmpty(filename))
{
return null;
}
else
{
string invalidReason;
if (ZipValid(filename, out invalidReason, null))
{
return filename;
}
else
{
// Purge invalid cache entries
File.Delete(filename);
return null;
}
}
}

/// <summary>
/// Count the files and bytes in the cache
/// </summary>
Expand Down Expand Up @@ -442,87 +395,6 @@ private List<FileInfo> allFiles(bool includeInProgress = false)
).ToList();
}

/// <summary>
/// Check whether a ZIP file is valid
/// </summary>
/// <param name="filename">Path to zip file to check</param>
/// <param name="invalidReason">Description of problem with the file</param>
/// <param name="progress">Callback to notify as we traverse the input, called with percentages from 0 to 100</param>
/// <returns>
/// True if valid, false otherwise. See invalidReason param for explanation.
/// </returns>
public static bool ZipValid(string filename, out string invalidReason, IProgress<long> progress)
{
try
{
if (filename != null)
{
using (ZipFile zip = new ZipFile(filename))
{
string zipErr = null;
// Limit progress updates to 100 per ZIP file
long highestPercent = -1;
// Perform CRC and other checks
if (zip.TestArchive(true, TestStrategy.FindFirstError,
(TestStatus st, string msg) =>
{
// This delegate is called as TestArchive proceeds through its
// steps, both routine and abnormal.
// The second parameter is non-null if an error occurred.
if (st != null && !st.EntryValid && !string.IsNullOrEmpty(msg))
{
// Capture the error string so we can return it
zipErr = string.Format(
Properties.Resources.NetFileCacheZipError,
st.Operation, st.Entry?.Name, msg);
}
else if (st.Entry != null && progress != null)
{
// Report progress
var percent = 100 * st.Entry.ZipFileIndex / zip.Count;
if (percent > highestPercent)
{
progress.Report(percent);
highestPercent = percent;
}
}
}))
{
invalidReason = "";
return true;
}
else
{
invalidReason = zipErr ?? Properties.Resources.NetFileCacheZipTestArchiveFalse;
return false;
}
}
}
else
{
invalidReason = Properties.Resources.NetFileCacheNullFileName;
return false;
}
}
catch (ZipException ze)
{
// Save the errors someplace useful
invalidReason = ze.Message;
return false;
}
catch (ArgumentException ex)
{
invalidReason = ex.Message;
return false;
}
catch (NotSupportedException nse) when (Platform.IsMono)
{
// SharpZipLib throws this if your locale isn't installed on Mono
invalidReason = string.Format(Properties.Resources.NetFileCacheMonoNotSupported, nse.Message);
return false;
}
}

/// <summary>
/// Stores the results of a given URL in the cache.
/// Description is adjusted to be filesystem-safe and then appended to the file hash when saving.
Expand Down
99 changes: 90 additions & 9 deletions Core/Net/NetModuleCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System.Threading;
using System.Security.Cryptography;

using ICSharpCode.SharpZipLib.Zip;

namespace CKAN
{
/// <summary>
Expand All @@ -16,6 +18,12 @@ namespace CKAN
/// </summary>
public class NetModuleCache : IDisposable
{
static NetModuleCache()
{
// SharpZibLib 1.1.0 changed this to default to false, but we depend on it for international mods.
// https://github.com/icsharpcode/SharpZipLib/issues/591
ZipStrings.UseUnicode = true;
}

/// <summary>
/// Initialize the cache
Expand Down Expand Up @@ -66,20 +74,13 @@ public bool IsCached(CkanModule m, out string outFilename)
outFilename = null;
return false;
}
public bool IsCachedZip(CkanModule m)
=> m.download?.Any(dlUri => cache.IsCachedZip(dlUri))
?? false;
public bool IsMaybeCachedZip(CkanModule m)
=> m.download?.Any(dlUri => cache.IsMaybeCachedZip(dlUri, m.release_date))
?? false;
public string GetCachedFilename(CkanModule m)
=> m.download?.Select(dlUri => cache.GetCachedFilename(dlUri, m.release_date))
.Where(filename => filename != null)
.FirstOrDefault();
public string GetCachedZip(CkanModule m)
=> m.download?.Select(dlUri => cache.GetCachedZip(dlUri))
.Where(filename => filename != null)
.FirstOrDefault();
public void GetSizeInfo(out int numFiles, out long numBytes, out long bytesFree)
{
cache.GetSizeInfo(out numFiles, out numBytes, out bytesFree);
Expand Down Expand Up @@ -172,8 +173,7 @@ public string DescribeAvailability(CkanModule m)
cancelToken.ThrowIfCancellationRequested();

// Check valid CRC
string invalidReason;
if (!NetFileCache.ZipValid(path, out invalidReason, new Progress<long>(percent =>
if (!ZipValid(path, out string invalidReason, new Progress<long>(percent =>
progress?.Report(percent * zipValidPercent / 100))))
{
throw new InvalidModuleFileKraken(module, path, string.Format(
Expand Down Expand Up @@ -216,6 +216,87 @@ public string DescribeAvailability(CkanModule m)
return success;
}

/// <summary>
/// Check whether a ZIP file is valid
/// </summary>
/// <param name="filename">Path to zip file to check</param>
/// <param name="invalidReason">Description of problem with the file</param>
/// <param name="progress">Callback to notify as we traverse the input, called with percentages from 0 to 100</param>
/// <returns>
/// True if valid, false otherwise. See invalidReason param for explanation.
/// </returns>
public static bool ZipValid(string filename, out string invalidReason, IProgress<long> progress)
{
try
{
if (filename != null)
{
using (ZipFile zip = new ZipFile(filename))
{
string zipErr = null;
// Limit progress updates to 100 per ZIP file
long highestPercent = -1;
// Perform CRC and other checks
if (zip.TestArchive(true, TestStrategy.FindFirstError,
(TestStatus st, string msg) =>
{
// This delegate is called as TestArchive proceeds through its
// steps, both routine and abnormal.
// The second parameter is non-null if an error occurred.
if (st != null && !st.EntryValid && !string.IsNullOrEmpty(msg))
{
// Capture the error string so we can return it
zipErr = string.Format(
Properties.Resources.NetFileCacheZipError,
st.Operation, st.Entry?.Name, msg);
}
else if (st.Entry != null && progress != null)
{
// Report progress
var percent = 100 * st.Entry.ZipFileIndex / zip.Count;
if (percent > highestPercent)
{
progress.Report(percent);
highestPercent = percent;
}
}
}))
{
invalidReason = "";
return true;
}
else
{
invalidReason = zipErr ?? Properties.Resources.NetFileCacheZipTestArchiveFalse;
return false;
}
}
}
else
{
invalidReason = Properties.Resources.NetFileCacheNullFileName;
return false;
}
}
catch (ZipException ze)
{
// Save the errors someplace useful
invalidReason = ze.Message;
return false;
}
catch (ArgumentException ex)
{
invalidReason = ex.Message;
return false;
}
catch (NotSupportedException nse) when (Platform.IsMono)
{
// SharpZipLib throws this if your locale isn't installed on Mono
invalidReason = string.Format(Properties.Resources.NetFileCacheMonoNotSupported, nse.Message);
return false;
}
}

/// <summary>
/// Remove a module's download files from the cache
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion Netkan/Services/CachingHttpService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ private string DownloadPackage(Uri url, string identifier, DateTime? updated, Ur
case FileType.Zip:
extension = "zip";
string invalidReason;
if (!NetFileCache.ZipValid(downloadedFile, out invalidReason, null))
if (!NetModuleCache.ZipValid(downloadedFile, out invalidReason, null))
{
log.Debug($"{url} is not a valid ZIP file: {invalidReason}");
File.Delete(downloadedFile);
Expand Down
12 changes: 6 additions & 6 deletions Tests/Core/Cache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,21 +161,21 @@ public void ZipValidation()
// We could use any URL, but this one is awesome. <3
Uri url = new Uri("http://kitte.nz/");

Assert.IsFalse(cache.IsCachedZip(url));
Assert.IsFalse(cache.IsCached(url));

// Store a bad zip.
cache.Store(url, TestData.DogeCoinFlagZipCorrupt());

// Make sure it's stored, but not valid as a zip
// Make sure it's stored
Assert.IsTrue(cache.IsCached(url));
Assert.IsFalse(cache.IsCachedZip(url));
// Make sure it's not valid as a zip
Assert.IsFalse(NetModuleCache.ZipValid(cache.GetCachedFilename(url), out string invalidReason, null));

// Store a good zip.
cache.Store(url, TestData.DogeCoinFlagZip());

// Make sure it's stored, and valid.
Assert.IsTrue(cache.IsCached(url));
Assert.IsTrue(cache.IsCachedZip(url));
}

[Test]
Expand All @@ -189,7 +189,7 @@ public void ZipValid_ContainsFilenameWithBadChars_NoException()
bool valid = false;
string reason = "";
Assert.DoesNotThrow(() =>
valid = NetFileCache.ZipValid(TestData.ZipWithBadChars, out reason, null));
valid = NetModuleCache.ZipValid(TestData.ZipWithBadChars, out reason, null));

// The file is considered valid on Linux;
// only check the reason if found invalid
Expand All @@ -212,7 +212,7 @@ public void ZipValid_ContainsFilenameWithUnicodeChars_Valid()
string reason = null;

Assert.DoesNotThrow(() =>
valid = NetFileCache.ZipValid(TestData.ZipWithUnicodeChars, out reason, null));
valid = NetModuleCache.ZipValid(TestData.ZipWithUnicodeChars, out reason, null));
Assert.IsTrue(valid, reason);
}

Expand Down
4 changes: 2 additions & 2 deletions Tests/Core/ModuleInstallerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,13 +407,13 @@ public void CanInstallMod()
Assert.IsFalse(File.Exists(mod_file_path));

// Copy the zip file to the cache directory.
Assert.IsFalse(manager.Cache.IsCachedZip(TestData.DogeCoinFlag_101_module()));
Assert.IsFalse(manager.Cache.IsCached(TestData.DogeCoinFlag_101_module()));

string cache_path = manager.Cache.Store(TestData.DogeCoinFlag_101_module(),
TestData.DogeCoinFlagZip(),
new Progress<long>(bytes => {}));

Assert.IsTrue(manager.Cache.IsCachedZip(TestData.DogeCoinFlag_101_module()));
Assert.IsTrue(manager.Cache.IsCached(TestData.DogeCoinFlag_101_module()));
Assert.IsTrue(File.Exists(cache_path));

var registry = CKAN.RegistryManager.Instance(manager.CurrentInstance, repoData.Manager).registry;
Expand Down
Loading

0 comments on commit 727fb65

Please sign in to comment.