From 759898e02dcd578928a0b5fdbb9b5d403497773f Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Sat, 30 May 2020 18:34:58 -0500 Subject: [PATCH 1/2] Multi-match 'find', allow 'as' for Ships and GameData --- Core/ModuleInstaller.cs | 217 +---------- Core/Types/ModuleInstallDescriptor.cs | 350 ++++++++++++++---- GUI/Dialogs/CloneFakeKspDialog.cs | 2 +- Tests/Core/KSP.cs | 2 +- Tests/Core/ModuleInstaller.cs | 74 +--- .../Types/ModuleInstallDescriptorTests.cs | 77 ++-- 6 files changed, 333 insertions(+), 389 deletions(-) diff --git a/Core/ModuleInstaller.cs b/Core/ModuleInstaller.cs index b0554798f7..1ce1d5f78d 100644 --- a/Core/ModuleInstaller.cs +++ b/Core/ModuleInstaller.cs @@ -510,216 +510,6 @@ private bool IsReservedDirectory(string path) || path == ksp.ShipsThumbsSPH() || path == ksp.Missions(); } - /// - /// Given a stanza and an open zipfile, returns all files that would be installed - /// for this stanza. - /// - /// If a KSP instance is provided, it will be used to generate output paths, otherwise these will be null. - /// - /// Throws a BadInstallLocationKraken if the install stanza targets an - /// unknown install location (eg: not GameData, Ships, etc) - /// - /// Throws a BadMetadataKraken if the stanza resulted in no files being returned. - /// - /// Thrown when the installation path is not valid according to the spec. - internal static List FindInstallableFiles(ModuleInstallDescriptor stanza, ZipFile zipfile, KSP ksp) - { - string installDir; - bool makeDirs; - var files = new List(); - - // Normalize the path before doing everything else - string install_to = KSPPathUtils.NormalizePath(stanza.install_to); - - // Convert our stanza to a standard `file` type. This is a no-op if it's - // already the basic type. - stanza = stanza.ConvertFindToFile(zipfile); - - if (install_to == "GameData" || install_to.StartsWith("GameData/")) - { - // The installation path can be either "GameData" or a sub-directory of "GameData" - // but it cannot contain updirs - if (install_to.Contains("/../") || install_to.EndsWith("/..")) - throw new BadInstallLocationKraken("Invalid installation path: " + install_to); - - string subDir = install_to.Substring("GameData".Length); // remove "GameData" - subDir = subDir.StartsWith("/") ? subDir.Substring(1) : subDir; // remove a "/" at the beginning, if present - - // Add the extracted subdirectory to the path of KSP's GameData - installDir = ksp == null ? null : (KSPPathUtils.NormalizePath(ksp.GameData() + "/" + subDir)); - makeDirs = true; - } - else if (install_to.StartsWith("Ships")) - { - // Don't allow directory creation in ships directory - makeDirs = false; - - switch (install_to) - { - case "Ships": - installDir = ksp?.Ships(); - break; - case "Ships/VAB": - installDir = ksp?.ShipsVab(); - break; - case "Ships/SPH": - installDir = ksp?.ShipsSph(); - break; - case "Ships/@thumbs": - installDir = ksp?.ShipsThumbs(); - break; - case "Ships/@thumbs/VAB": - installDir = ksp?.ShipsThumbsVAB(); - break; - case "Ships/@thumbs/SPH": - installDir = ksp?.ShipsThumbsSPH(); - break; - default: - throw new BadInstallLocationKraken("Unknown install_to " + install_to); - } - } - else - { - switch (install_to) - { - case "Tutorial": - installDir = ksp?.Tutorial(); - makeDirs = true; - break; - - case "Scenarios": - installDir = ksp?.Scenarios(); - makeDirs = true; - break; - - case "Missions": - installDir = ksp?.Missions(); - makeDirs = true; - break; - - case "GameRoot": - installDir = ksp?.GameDir(); - makeDirs = false; - break; - - default: - throw new BadInstallLocationKraken("Unknown install_to " + install_to); - } - } - - // O(N^2) solution, as we're walking the zipfile for each stanza. - // Surely there's a better way, although this is fast enough we may not care. - - foreach (ZipEntry entry in zipfile) - { - // Skips things not prescribed by our install stanza. - if (!stanza.IsWanted(entry.Name)) - { - continue; - } - - // Prepare our file info. - InstallableFile file_info = new InstallableFile - { - source = entry, - makedir = makeDirs, - destination = null - }; - - // If we have a place to install it, fill that in... - if (installDir != null) - { - // Get the full name of the file. - string outputName = entry.Name; - - // Update our file info with the install location - file_info.destination = TransformOutputName(stanza.file, outputName, installDir, stanza.@as); - } - - files.Add(file_info); - } - - // If we have no files, then something is wrong! (KSP-CKAN/CKAN#93) - if (files.Count == 0) - { - // We have null as the first argument here, because we don't know which module we're installing - throw new BadMetadataKraken(null, String.Format("No files found in {0} to install!", stanza.file)); - } - - return files; - } - - /// - /// Transforms the name of the output. This will strip the leading directories from the stanza file from - /// output name and then combine it with the installDir. - /// EX: "kOS-1.1/GameData/kOS", "kOS-1.1/GameData/kOS/Plugins/kOS.dll", "GameData" will be transformed - /// to "GameData/kOS/Plugins/kOS.dll" - /// - /// The output name. - /// The file directive of the stanza. - /// The name of the file to transform. - /// The installation dir where the file should end up with. - internal static string TransformOutputName(string file, string outputName, string installDir, string @as) - { - string leadingPathToRemove = KSPPathUtils.GetLeadingPathElements(file); - - // Special-casing, if stanza.file is just "GameData" or "Ships", strip it. - // TODO: Do we need to do anything special for tutorials or GameRoot? - if ( - leadingPathToRemove == string.Empty && - (file == "GameData" || file == "Ships") - ) - { - leadingPathToRemove = file; - - // It's unclear what the behavior should be in this special case if `as` is specified, therefore - // disallow it. - if (!string.IsNullOrWhiteSpace(@as)) - { - throw new BadMetadataKraken(null, "Cannot specify `as` if `file` is GameData or Ships."); - } - } - - // If there's a leading path to remove, then we have some extra work that - // needs doing... - if (leadingPathToRemove != string.Empty) - { - string leadingRegEx = "^" + Regex.Escape(leadingPathToRemove) + "/"; - if (!Regex.IsMatch(outputName, leadingRegEx)) - { - throw new BadMetadataKraken(null, - String.Format("Output file name ({0}) not matching leading path of stanza.file ({1})", - outputName, leadingRegEx - ) - ); - } - // Strip off leading path name - outputName = Regex.Replace(outputName, leadingRegEx, ""); - } - - // If an `as` is specified, replace the first component in the file path with the value of `as` - // This works for both when `find` specifies a directory and when it specifies a file. - if (!string.IsNullOrWhiteSpace(@as)) - { - if (!@as.Contains("/") && !@as.Contains("\\")) - { - var components = outputName.Split(new[] { '/' }, StringSplitOptions.RemoveEmptyEntries); - components[0] = @as; - - outputName = string.Join("/", components); - } - else - { - throw new BadMetadataKraken(null, "`as` may not include path seperators."); - } - } - - // Return our snipped, normalised, and ready to go output filename! - return KSPPathUtils.NormalizePath( - Path.Combine(installDir, outputName) - ); - } - /// /// Given a module and an open zipfile, return all the files that would be installed /// for this module. @@ -739,13 +529,14 @@ public static List FindInstallableFiles(CkanModule module, ZipF { foreach (ModuleInstallDescriptor stanza in module.install) { - files.AddRange(FindInstallableFiles(stanza, zipfile, ksp)); + files.AddRange(stanza.FindInstallableFiles(zipfile, ksp)); } } else { - ModuleInstallDescriptor default_stanza = ModuleInstallDescriptor.DefaultInstallStanza(module.identifier, zipfile); - files.AddRange(FindInstallableFiles(default_stanza, zipfile, ksp)); + files.AddRange(ModuleInstallDescriptor + .DefaultInstallStanza(module.identifier) + .FindInstallableFiles(zipfile, ksp)); } } catch (BadMetadataKraken kraken) diff --git a/Core/Types/ModuleInstallDescriptor.cs b/Core/Types/ModuleInstallDescriptor.cs index d469b6bc56..c096237ef0 100644 --- a/Core/Types/ModuleInstallDescriptor.cs +++ b/Core/Types/ModuleInstallDescriptor.cs @@ -1,13 +1,17 @@ using System; +using System.IO; using System.ComponentModel; using System.Collections.Generic; using System.Linq; using System.Runtime.Serialization; using System.Text; using System.Text.RegularExpressions; +using System.Runtime.CompilerServices; + using ICSharpCode.SharpZipLib.Zip; using Newtonsoft.Json; -using System.IO; + +[assembly: InternalsVisibleTo("CKAN.Tests")] namespace CKAN { @@ -54,6 +58,20 @@ public class ModuleInstallDescriptor : ICloneable, IEquatable))] public List include_only_regexp; + [JsonIgnore] + private Regex inst_pattern = null; + + private static Regex ckanPattern = new Regex(".ckan$", + RegexOptions.IgnoreCase | RegexOptions.Compiled); + + private static Regex trailingSlashPattern = new Regex("/$", + RegexOptions.Compiled); + + private static string[] ReservedPaths = new string[] + { + "GameData", "Ships", "Missions" + }; + [OnDeserialized] internal void DeSerialisationFixes(StreamingContext like_i_could_care) { @@ -85,17 +103,9 @@ internal void DeSerialisationFixes(StreamingContext like_i_could_care) { throw new BadMetadataKraken(null, "Install stanzas can only contain filter or include_only directives, not both"); } - + // Normalize paths on load (note, doesn't cover assignment like in tests) install_to = KSPPathUtils.NormalizePath(install_to); - if (find != null) - { - find = KSPPathUtils.NormalizePath(find); - } - if (file != null) - { - file = KSPPathUtils.NormalizePath(file); - } } #endregion @@ -128,7 +138,7 @@ public override bool Equals(object other) { return Equals(other as ModuleInstallDescriptor); } - + /// /// Compare two install stanzas /// @@ -176,7 +186,7 @@ public bool Equals(ModuleInstallDescriptor otherStanza) return false; return true; } - + public override int GetHashCode() { // Tuple.Create only handles up to 8 params, we have 10+ @@ -189,7 +199,7 @@ public override int GetHashCode() install_to, @as ), - Tuple.Create( + Tuple.Create( filter, filter_regexp, include_only, @@ -201,46 +211,69 @@ public override int GetHashCode() /// /// Returns a default install stanza for the identifier provided. /// - public static ModuleInstallDescriptor DefaultInstallStanza(string ident, ZipFile zipfile) + /// + /// { "find": "ident", "install_to": "GameData" } + /// + public static ModuleInstallDescriptor DefaultInstallStanza(string ident) { - // Really this is just making a dummy `find` stanza and returning the processed - // result. - var stanza = new ModuleInstallDescriptor(); - stanza.install_to = "GameData"; - stanza.find = ident; - return stanza.ConvertFindToFile(zipfile); + return new ModuleInstallDescriptor() + { + find = ident, + install_to = "GameData", + }; } #endregion + private void EnsurePattern() + { + if (inst_pattern == null) + { + if (file != null) + { + file = KSPPathUtils.NormalizePath(file); + inst_pattern = new Regex(@"^" + Regex.Escape(file) + @"(/|$)", + RegexOptions.IgnoreCase | RegexOptions.Compiled); + } + else if (find != null) + { + find = KSPPathUtils.NormalizePath(find); + inst_pattern = new Regex(@"(?:^|/)" + Regex.Escape(find) + @"(/|$)", + RegexOptions.IgnoreCase | RegexOptions.Compiled); + } + else if (find_regexp != null) + { + inst_pattern = new Regex(find_regexp, + RegexOptions.IgnoreCase | RegexOptions.Compiled); + } + else + { + throw new UnsupportedKraken("Install stanzas requires `file` or `find` or `find_regexp`."); + } + } + } + /// /// Returns true if the path provided should be installed by this stanza. /// Can *only* be used on `file` stanzas, throws an UnsupportedKraken if called /// on a `find` stanza. /// Use `ConvertFindToFile` to convert `find` to `file` stanzas. /// - public bool IsWanted(string path) + private bool IsWanted(string path) { - if (file == null) - { - throw new UnsupportedKraken(".IsWanted only works with `file` stanzas."); - } + EnsurePattern(); // Make sure our path always uses slashes we expect. string normalised_path = path.Replace('\\', '/'); - // We want everthing that matches our 'file', either as an exact match, - // or as a path leading up to it. - string wanted_filter = "^" + Regex.Escape(file) + "(/|$)"; - // If it doesn't match our install path, ignore it. - if (!Regex.IsMatch(normalised_path, wanted_filter)) + if (!inst_pattern.IsMatch(normalised_path)) { return false; } // Skip the file if it's a ckan file, these should never be copied to GameData. - if (Regex.IsMatch(normalised_path, ".ckan$", RegexOptions.IgnoreCase)) + if (ckanPattern.IsMatch(normalised_path)) { return false; } @@ -280,61 +313,230 @@ public bool IsWanted(string path) } /// - /// Given a zipfile, returns a `file` ModuleInstallDescriptor that can be used for - /// installation. - /// Returns `this` if already of a `file` type. + /// Given an open zipfile, returns all files that would be installed + /// for this stanza. + /// + /// If a KSP instance is provided, it will be used to generate output paths, otherwise these will be null. + /// + /// Throws a BadInstallLocationKraken if the install stanza targets an + /// unknown install location (eg: not GameData, Ships, etc) + /// + /// Throws a BadMetadataKraken if the stanza resulted in no files being returned. /// - /// Downloaded ZIP file containing the mod - public ModuleInstallDescriptor ConvertFindToFile(ZipFile zipfile) + /// Thrown when the installation path is not valid according to the spec. + public List FindInstallableFiles(ZipFile zipfile, KSP ksp) { - // If we're already a file type stanza, then we have nothing to do. - if (this.file != null) - return this; - - // Match *only* things with our find string as a directory. - // We can't just look for directories, because some zipfiles - // don't include entries for directories, but still include entries - // for the files they contain. - Regex inst_filt = this.find != null - ? new Regex(@"(?:^|/)" + Regex.Escape(this.find) + @"$", RegexOptions.IgnoreCase) - : new Regex(this.find_regexp, RegexOptions.IgnoreCase); - - // Find the shortest directory path that matches our filter, - // including all parent directories of all entries. - string shortest = null; + string installDir; + bool makeDirs; + var files = new List(); + + // Normalize the path before doing everything else + string install_to = KSPPathUtils.NormalizePath(this.install_to); + + if (install_to == "GameData" || install_to.StartsWith("GameData/")) + { + // The installation path can be either "GameData" or a sub-directory of "GameData" + // but it cannot contain updirs + if (install_to.Contains("/../") || install_to.EndsWith("/..")) + throw new BadInstallLocationKraken("Invalid installation path: " + install_to); + + string subDir = install_to.Substring("GameData".Length); // remove "GameData" + subDir = subDir.StartsWith("/") ? subDir.Substring(1) : subDir; // remove a "/" at the beginning, if present + + // Add the extracted subdirectory to the path of KSP's GameData + installDir = ksp == null ? null : (KSPPathUtils.NormalizePath(ksp.GameData() + "/" + subDir)); + makeDirs = true; + } + else if (install_to.StartsWith("Ships")) + { + // Don't allow directory creation in ships directory + makeDirs = false; + + switch (install_to) + { + case "Ships": + installDir = ksp?.Ships(); + break; + case "Ships/VAB": + installDir = ksp?.ShipsVab(); + break; + case "Ships/SPH": + installDir = ksp?.ShipsSph(); + break; + case "Ships/@thumbs": + installDir = ksp?.ShipsThumbs(); + break; + case "Ships/@thumbs/VAB": + installDir = ksp?.ShipsThumbsVAB(); + break; + case "Ships/@thumbs/SPH": + installDir = ksp?.ShipsThumbsSPH(); + break; + default: + throw new BadInstallLocationKraken("Unknown install_to " + install_to); + } + } + else + { + switch (install_to) + { + case "Tutorial": + installDir = ksp?.Tutorial(); + makeDirs = true; + break; + + case "Scenarios": + installDir = ksp?.Scenarios(); + makeDirs = true; + break; + + case "Missions": + installDir = ksp?.Missions(); + makeDirs = true; + break; + + case "GameRoot": + installDir = ksp?.GameDir(); + makeDirs = false; + break; + + default: + throw new BadInstallLocationKraken("Unknown install_to " + install_to); + } + } + + // O(N^2) solution, as we're walking the zipfile for each stanza. + // Surely there's a better way, although this is fast enough we may not care. + foreach (ZipEntry entry in zipfile) { - bool is_file = !entry.IsDirectory; - // Normalize path before searching (path separator as '/', no trailing separator) - for (string path = Regex.Replace(entry.Name.Replace('\\', '/'), "/$", ""); - !string.IsNullOrEmpty(path); - path = Path.GetDirectoryName(path).Replace('\\', '/'), is_file = false) + // Skips dirs and things not prescribed by our install stanza. + if (!IsWanted(entry.Name)) + { + continue; + } + + // Prepare our file info. + InstallableFile file_info = new InstallableFile + { + source = entry, + makedir = makeDirs, + destination = null + }; + + // If we have a place to install it, fill that in... + if (installDir != null) + { + // Get the full name of the file. + // Update our file info with the install location + file_info.destination = TransformOutputName( + entry.Name, installDir, @as); + } + + files.Add(file_info); + } + + // If we have no files, then something is wrong! (KSP-CKAN/CKAN#93) + if (files.Count == 0) + { + // We have null as the first argument here, because we don't know which module we're installing + throw new BadMetadataKraken(null, String.Format("No files found matching {0} to install!", DescribeMatch())); + } + + return files; + } + + /// + /// Transforms the name of the output. This will strip the leading directories from the stanza file from + /// output name and then combine it with the installDir. + /// EX: "kOS-1.1/GameData/kOS", "kOS-1.1/GameData/kOS/Plugins/kOS.dll", "GameData" will be transformed + /// to "GameData/kOS/Plugins/kOS.dll" + /// + /// The name of the file to transform + /// The installation dir where the file should end up with + /// The output name + internal string TransformOutputName(string outputName, string installDir, string @as) + { + string leadingPathToRemove = Path + .GetDirectoryName(ShortestMatchingPrefix(outputName)) + .Replace('\\', '/'); + + if (!string.IsNullOrEmpty(leadingPathToRemove)) + { + Regex leadingRE = new Regex( + "^" + Regex.Escape(leadingPathToRemove) + "/", + RegexOptions.Compiled); + if (!leadingRE.IsMatch(outputName)) { + throw new BadMetadataKraken(null, String.Format( + "Output file name ({0}) not matching leading path of stanza ({1})", + outputName, leadingPathToRemove)); + } + // Strip off leading path name + outputName = leadingRE.Replace(outputName, ""); + } - // Skip file paths if not allowed - if (!find_matches_files && is_file) - continue; + // Now outputname looks like PATH/what/ever/file.ext, where + // PATH is the part that matched `file` or `find` or `find_regexp` - // Is this a shorter matching path? - if ((string.IsNullOrEmpty(shortest) || path.Length < shortest.Length) - && inst_filt.IsMatch(path)) - shortest = path; + if (!string.IsNullOrWhiteSpace(@as)) + { + if (@as.Contains("/") || @as.Contains("\\")) + { + throw new BadMetadataKraken(null, "`as` may not include path separators."); } + // Replace first path component with @as + outputName = ReplaceFirstPiece(outputName, "/", @as); } - if (string.IsNullOrEmpty(shortest)) + else + { + var reservedPrefix = ReservedPaths.FirstOrDefault(prefix => + outputName.StartsWith(prefix, StringComparison.InvariantCultureIgnoreCase)); + if (reservedPrefix != null) + { + // If we try to install a folder with the same name as + // one of the reserved directories, strip it off. + // Delete reservedPrefix and one forward slash + outputName = outputName.Substring(reservedPrefix.Length + 1); + } + } + + // Return our snipped, normalised, and ready to go output filename! + return KSPPathUtils.NormalizePath( + Path.Combine(installDir, outputName) + ); + } + + private string ShortestMatchingPrefix(string fullPath) + { + EnsurePattern(); + + string shortest = fullPath; + for (string path = trailingSlashPattern.Replace(fullPath.Replace('\\', '/'), ""); + !string.IsNullOrEmpty(path); + path = Path.GetDirectoryName(path).Replace('\\', '/')) { - throw new FileNotFoundKraken( - this.find ?? this.find_regexp, - String.Format("Could not find {0} entry in zipfile to install", this.find ?? this.find_regexp) - ); + if (inst_pattern.IsMatch(path)) + { + shortest = path; + } + else + { + break; + } } + return shortest; + } - // Fill in our stanza, and remove our old `find` and `find_regexp` info. - ModuleInstallDescriptor stanza = (ModuleInstallDescriptor)this.Clone(); - stanza.file = shortest; - stanza.find = null; - stanza.find_regexp = null; - return stanza; + private static string ReplaceFirstPiece(string text, string delimiter, string replacement) + { + int pos = text.IndexOf(delimiter); + if (pos < 0) + { + // No delimiter, replace whole string + return replacement; + } + return replacement + text.Substring(pos); } public string DescribeMatch() diff --git a/GUI/Dialogs/CloneFakeKspDialog.cs b/GUI/Dialogs/CloneFakeKspDialog.cs index 5ace57c4c0..cbdb9efce7 100644 --- a/GUI/Dialogs/CloneFakeKspDialog.cs +++ b/GUI/Dialogs/CloneFakeKspDialog.cs @@ -11,7 +11,7 @@ namespace CKAN { /// /// The GUI implementation of clone and fake. - /// It's a seperate window, handling the whole process. + /// It's a separate window, handling the whole process. /// public partial class CloneFakeKspDialog : Form { diff --git a/Tests/Core/KSP.cs b/Tests/Core/KSP.cs index 311e91ae4d..53cb563189 100644 --- a/Tests/Core/KSP.cs +++ b/Tests/Core/KSP.cs @@ -59,7 +59,7 @@ public void IsGameDir() [Test] public void Training() { - //Use Uri to avoid issues with windows vs linux line seperators. + //Use Uri to avoid issues with windows vs linux line separators. var canonicalPath = new Uri(Path.Combine(ksp_dir, "saves", "training")).LocalPath; var training = new Uri(ksp.Tutorial()).LocalPath; Assert.AreEqual(canonicalPath, training); diff --git a/Tests/Core/ModuleInstaller.cs b/Tests/Core/ModuleInstaller.cs index 547fc423bd..41ec6146a5 100644 --- a/Tests/Core/ModuleInstaller.cs +++ b/Tests/Core/ModuleInstaller.cs @@ -5,9 +5,10 @@ using System.Linq; using System.Text.RegularExpressions; using System.Transactions; -using CKAN; using ICSharpCode.SharpZipLib.Zip; using NUnit.Framework; + +using CKAN; using Tests.Core.Configuration; using Tests.Data; @@ -64,31 +65,16 @@ public void GenerateDefaultInstall() string filename = TestData.DogeCoinFlagZip(); using (var zipfile = new ZipFile(filename)) { - ModuleInstallDescriptor stanza = ModuleInstallDescriptor.DefaultInstallStanza("DogeCoinFlag", zipfile); + ModuleInstallDescriptor stanza = ModuleInstallDescriptor.DefaultInstallStanza("DogeCoinFlag"); - TestDogeCoinStanza(stanza); + Assert.AreEqual("GameData", stanza.install_to); + Assert.AreEqual("DogeCoinFlag", stanza.find); // Same again, but screwing up the case (we see this *all the time*) - ModuleInstallDescriptor stanza2 = ModuleInstallDescriptor.DefaultInstallStanza("DogecoinFlag", zipfile); - - TestDogeCoinStanza(stanza2); - - // Now what happens if we can't find what to install? - - Assert.Throws(delegate - { - ModuleInstallDescriptor.DefaultInstallStanza("Xyzzy", zipfile); - }); + ModuleInstallDescriptor stanza2 = ModuleInstallDescriptor.DefaultInstallStanza("DogecoinFlag"); - // Make sure the FNFKraken looks like what we expect. - try - { - ModuleInstallDescriptor.DefaultInstallStanza("Xyzzy", zipfile); - } - catch (FileNotFoundKraken kraken) - { - Assert.AreEqual("Xyzzy", kraken.file); - } + Assert.AreEqual("GameData", stanza2.install_to); + Assert.AreEqual("DogecoinFlag", stanza2.find); } } @@ -371,7 +357,7 @@ public void CorruptZip_242() using (var zipfile = new ZipFile(corrupt_dogezip)) { // GenerateDefault Install - ModuleInstallDescriptor.DefaultInstallStanza("DogeCoinFlag", zipfile); + ModuleInstallDescriptor.DefaultInstallStanza("DogeCoinFlag"); // FindInstallableFiles CkanModule dogemod = TestData.DogeCoinFlag_101_module(); @@ -379,36 +365,6 @@ public void CorruptZip_242() } } - [TestCase("GameData/kOS", "GameData/kOS/Plugins/kOS.dll", "GameData", null, "GameData/kOS/Plugins/kOS.dll")] - [TestCase("kOS-1.1/GameData/kOS", "kOS-1.1/GameData/kOS/Plugins/kOS.dll", "GameData", null, "GameData/kOS/Plugins/kOS.dll")] - [TestCase("ModuleManager.2.5.1.dll", "ModuleManager.2.5.1.dll", "GameData", null, "GameData/ModuleManager.2.5.1.dll")] - [TestCase("Ships", "Ships/SPH/FAR Firehound.craft", "SomeDir/Ships", null, "SomeDir/Ships/SPH/FAR Firehound.craft")] - [TestCase("GameData/kOS", "GameData/kOS/Plugins/kOS.dll", "GameData", "kOS-Renamed", "GameData/kOS-Renamed/Plugins/kOS.dll")] - [TestCase("kOS-1.1/GameData/kOS", "kOS-1.1/GameData/kOS/Plugins/kOS.dll", "GameData", "kOS-Renamed", "GameData/kOS-Renamed/Plugins/kOS.dll")] - [TestCase("ModuleManager.2.5.1.dll", "ModuleManager.2.5.1.dll", "GameData", "ModuleManager-Renamed.dll", "GameData/ModuleManager-Renamed.dll")] - public void TransformOutputName(string file, string outputName, string installDir, string @as, string expected) - { - // Act - var result = CKAN.ModuleInstaller.TransformOutputName(file, outputName, installDir, @as); - - // Assert - Assert.That(result, Is.EqualTo(expected)); - } - - [TestCase("GameData", "GameData/kOS/Plugins/kOS.dll", "GameData", "GameData-Renamed")] - [TestCase("Ships", "Ships/SPH/FAR Firehound.craft", "SomeDir/Ships", "Ships-Renamed")] - [TestCase("GameData/kOS", "GameData/kOS/Plugins/kOS.dll", "GameData", "kOS/Renamed")] - [TestCase("kOS-1.1/GameData/kOS", "kOS-1.1/GameData/kOS/Plugins/kOS.dll", "GameData", "kOS/Renamed")] - [TestCase("ModuleManager.2.5.1.dll", "ModuleManager.2.5.1.dll", "GameData", "Renamed/ModuleManager.dll")] - public void TransformOutputNameThrowsOnInvalidParameters(string file, string outputName, string installDir, string @as) - { - // Act - TestDelegate act = () => CKAN.ModuleInstaller.TransformOutputName(file, outputName, installDir, @as); - - // Assert - Assert.That(act, Throws.Exception); - } - private string CopyDogeFromZip() { string dogezip = TestData.DogeCoinFlagZip(); @@ -704,7 +660,7 @@ public void AllowsInstallsToShipsDirectories(string directory) List results; using (var ksp = new DisposableKSP()) { - results = CKAN.ModuleInstaller.FindInstallableFiles(mod.install.First(), zip, ksp.KSP); + results = mod.install.First().FindInstallableFiles(zip, ksp.KSP); } // Assert @@ -745,7 +701,7 @@ public void AllowInstallsToScenarios() List results; using (var ksp = new DisposableKSP()) { - results = CKAN.ModuleInstaller.FindInstallableFiles(mod.install.First(), zip, ksp.KSP); + results = mod.install.First().FindInstallableFiles(zip, ksp.KSP); Assert.AreEqual( CKAN.KSPPathUtils.NormalizePath( @@ -773,13 +729,5 @@ public void UnsuccessfulReplacement() // Assert that DogeCoinFlag has not been removed and DogeTokenFlag has not been installed Assert.IsTrue(true); } - - - private static void TestDogeCoinStanza(ModuleInstallDescriptor stanza) - { - Assert.AreEqual("GameData", stanza.install_to); - Assert.AreEqual("DogeCoinFlag-1.01/GameData/DogeCoinFlag", stanza.file); - } - } } diff --git a/Tests/Core/Types/ModuleInstallDescriptorTests.cs b/Tests/Core/Types/ModuleInstallDescriptorTests.cs index ecef167a43..aeafc1cd65 100644 --- a/Tests/Core/Types/ModuleInstallDescriptorTests.cs +++ b/Tests/Core/Types/ModuleInstallDescriptorTests.cs @@ -37,52 +37,55 @@ public void Null_Filters() } } - [Test] - [TestCase("/DogeCoinFlag", "DogeCoinFlag-1.01/GameData/DogeCoinFlag","Anchored")] - [TestCase("DogeCoinFlag", "DogeCoinFlag-1.01","Missing anchors")] - public void regexp_filter_1089(string regexp, string expected, string comment) + private static void test_filter(List filter, string message) { - string json = @"{ - ""install_to"" : ""GameData"", - ""find_regexp"" : """ + regexp + @""" - }"; + if (filter != null) + { + Assert.IsFalse(filter.Contains(null), message); + } + } - var mid = JsonConvert.DeserializeObject(json); - var zip = new ZipFile(TestData.DogeCoinFlagZipWithExtras()); + [TestCase("GameData/kOS", "GameData/kOS/Plugins/kOS.dll", "GameData", null, "GameData/kOS/Plugins/kOS.dll")] + [TestCase("kOS-1.1/GameData/kOS", "kOS-1.1/GameData/kOS/Plugins/kOS.dll", "GameData", null, "GameData/kOS/Plugins/kOS.dll")] + [TestCase("ModuleManager.2.5.1.dll", "ModuleManager.2.5.1.dll", "GameData", null, "GameData/ModuleManager.2.5.1.dll")] - mid = mid.ConvertFindToFile(zip); - Assert.AreEqual(expected, mid.file, comment); - } + [TestCase("Ships", "Ships/SPH/FAR Firehound.craft", "SomeDir/Ships", null, "SomeDir/Ships/SPH/FAR Firehound.craft")] - /// - /// Test that an install clause with a find property will find a directory in a zip - /// even if that directory doesn't have its own entry or a file directly within it. - /// - /// Covers: https://github.com/KSP-CKAN/CKAN/pull/2120 - /// - /// Directory to find in zip - /// Path to DogeCoinFlag zip file with no directory entries - [Test] - [TestCase("DogeCoinFlag", "DogeCoinFlag-1.01-no-dir-entries.zip")] - public void find_without_directory_entry(string find_str, string zip_file_path) + + [TestCase("GameData/kOS", "GameData/kOS/Plugins/kOS.dll", "GameData", "kOS-Renamed", "GameData/kOS-Renamed/Plugins/kOS.dll")] + [TestCase("kOS-1.1/GameData/kOS", "kOS-1.1/GameData/kOS/Plugins/kOS.dll", "GameData", "kOS-Renamed", "GameData/kOS-Renamed/Plugins/kOS.dll")] + [TestCase("ModuleManager.2.5.1.dll", "ModuleManager.2.5.1.dll", "GameData", "ModuleManager-Renamed.dll", "GameData/ModuleManager-Renamed.dll")] + [TestCase("GameData", "GameData/kOS/Plugins/kOS.dll", "GameData", "GameData-Renamed", "GameData/GameData-Renamed/kOS/Plugins/kOS.dll")] + [TestCase("Ships", "Ships/SPH/FAR Firehound.craft", "SomeDir/Ships", "Ships-Renamed", "SomeDir/Ships/Ships-Renamed/SPH/FAR Firehound.craft")] + public void TransformOutputName(string file, string outputName, string installDir, string @as, string expected) { - ModuleInstallDescriptor find_descrip = JsonConvert.DeserializeObject( - @"{ ""install_to"": ""GameData"", ""find"": """ + find_str + @""" }" - ); - ZipFile zf = new ZipFile(TestData.DataDir(zip_file_path)); - - ModuleInstallDescriptor file_descrip = find_descrip.ConvertFindToFile(zf); - Assert.IsNull( file_descrip.find); - Assert.IsNotNull(file_descrip.file); + // Arrange + var stanza = JsonConvert.DeserializeObject( + $"{{\"file\": \"{file}\"}}"); + + // Act + var result = stanza.TransformOutputName(outputName, installDir, @as); + + // Assert + Assert.That(result, Is.EqualTo(expected)); } - private static void test_filter(List filter, string message) + [TestCase("GameData/kOS", "GameData/kOS/Plugins/kOS.dll", "GameData", "kOS/Renamed")] + [TestCase("kOS-1.1/GameData/kOS", "kOS-1.1/GameData/kOS/Plugins/kOS.dll", "GameData", "kOS/Renamed")] + [TestCase("ModuleManager.2.5.1.dll", "ModuleManager.2.5.1.dll", "GameData", "Renamed/ModuleManager.dll")] + public void TransformOutputNameThrowsOnInvalidParameters(string file, string outputName, string installDir, string @as) { - if (filter != null) - { - Assert.IsFalse(filter.Contains(null), message); - } + // Arrange + var stanza = JsonConvert.DeserializeObject( + $"{{\"file\": \"{file}\"}}"); + + // Act + TestDelegate act = () => stanza.TransformOutputName(outputName, installDir, @as); + + // Assert + Assert.That(act, Throws.Exception); } + } } From fb0ba99c624e3d03d43afa704f1941bde3023d44 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Sun, 7 Jun 2020 14:04:08 -0500 Subject: [PATCH 2/2] Throw an error on duplicate file install in Netkan --- Netkan/Validators/InstallsFilesValidator.cs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/Netkan/Validators/InstallsFilesValidator.cs b/Netkan/Validators/InstallsFilesValidator.cs index e546bd353f..4f3f02c749 100644 --- a/Netkan/Validators/InstallsFilesValidator.cs +++ b/Netkan/Validators/InstallsFilesValidator.cs @@ -1,6 +1,8 @@ using System.Linq; + using CKAN.NetKAN.Model; using CKAN.NetKAN.Services; +using CKAN.Extensions; namespace CKAN.NetKAN.Validators { @@ -30,9 +32,12 @@ public void Validate(Metadata metadata) mod.DescribeInstallStanzas() )); } - + + // Get the files the module will install + var allFiles = _moduleService.FileDestinations(mod, file).Memoize(); + // Make sure no paths include GameData other than at the start - var gamedatas = _moduleService.FileDestinations(mod, file) + var gamedatas = allFiles .Where(p => p.StartsWith("GameData") && p.LastIndexOf("/GameData/") > 0) .ToList(); if (gamedatas.Any()) @@ -40,6 +45,17 @@ public void Validate(Metadata metadata) var badPaths = string.Join("\r\n", gamedatas); throw new Kraken($"GameData directory found within GameData:\r\n{badPaths}"); } + + // Make sure we won't try to overwrite our own files + var duplicates = allFiles + .GroupBy(f => f) + .SelectMany(grp => grp.Skip(1)) + .ToList(); + if (duplicates.Any()) + { + var badPaths = string.Join("\r\n", duplicates); + throw new Kraken($"Multiple files attempted to install to:\r\n{badPaths}"); + } } } }