Merge #4135 Stop checking multiple hashes

KSP-CKAN · Jul 31, 2024 · 393544d · 393544d
2 parents 5c1b90a + 68fa8eb
commit 393544d
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -49,6 +49,7 @@ All notable changes to this project will be documented in this file.
 - [Core] Use DriveInfo constructor to get drive from path (#4125 by: HebaruSan)
 - [Core] Detect changes to `replaced_by` (#4127 by: HebaruSan)
 - [Multiple] Update changeset on Replace checkbox change, other `replaced_by` fixes (#4128 by: HebaruSan)
+- [Multiple] Stop checking multiple hashes (#4135 by: HebaruSan)
 
 ### Internal
 

diff --git a/Core/Net/NetModuleCache.cs b/Core/Net/NetModuleCache.cs
@@ -155,10 +155,8 @@ public string GetFileHashSha256(string filePath, IProgress<long> progress, Cance
         /// </returns>
         public string Store(CkanModule module, string path, IProgress<long> progress, string description = null, bool move = false, CancellationToken cancelToken = default)
         {
-            // ZipValid takes a lot longer than the hash steps, so scale them 60:20:20
-            const int zipValidPercent   = 60;
-            const int hashSha1Percent   = 20;
-            const int hashSha256Percent = 20;
+            // ZipValid takes a lot longer than the hash check, so scale them 70:30 if hashes are present
+            int zipValidPercent = module.download_hash == null ? 100 : 70;
 
             progress?.Report(0);
             // Check file exists
@@ -192,24 +190,31 @@ public string Store(CkanModule module, string path, IProgress<long> progress, st
             // Some older metadata doesn't have hashes
             if (module.download_hash != null)
             {
-                // Check SHA1 match
-                string sha1 = GetFileHashSha1(path, new Progress<long>(percent =>
-                    progress?.Report(zipValidPercent + (percent * hashSha1Percent / 100))), cancelToken);
-                if (sha1 != module.download_hash.sha1)
+                int hashPercent = 100 - zipValidPercent;
+                // Only check one hash, sha256 if it's set, sha1 otherwise
+                if (!string.IsNullOrEmpty(module.download_hash.sha256))
                 {
-                    throw new InvalidModuleFileKraken(module, path, string.Format(
-                        Properties.Resources.NetModuleCacheMismatchSHA1,
-                        module, path, sha1, module.download_hash.sha1));
+                    // Check SHA256 match
+                    string sha256 = GetFileHashSha256(path, new Progress<long>(percent =>
+                        progress?.Report(zipValidPercent + (percent * hashPercent / 100))), cancelToken);
+                    if (sha256 != module.download_hash.sha256)
+                    {
+                        throw new InvalidModuleFileKraken(module, path, string.Format(
+                            Properties.Resources.NetModuleCacheMismatchSHA256,
+                            module, path, sha256, module.download_hash.sha256));
+                    }
                 }
-
-                // Check SHA256 match
-                string sha256 = GetFileHashSha256(path, new Progress<long>(percent =>
-                    progress?.Report(zipValidPercent + hashSha1Percent + (percent * hashSha256Percent / 100))), cancelToken);
-                if (sha256 != module.download_hash.sha256)
+                else if (!string.IsNullOrEmpty(module.download_hash.sha1))
                 {
-                    throw new InvalidModuleFileKraken(module, path, string.Format(
-                        Properties.Resources.NetModuleCacheMismatchSHA256,
-                        module, path, sha256, module.download_hash.sha256));
+                    // Check SHA1 match
+                    string sha1 = GetFileHashSha1(path, new Progress<long>(percent =>
+                        progress?.Report(zipValidPercent + (percent * hashPercent / 100))), cancelToken);
+                    if (sha1 != module.download_hash.sha1)
+                    {
+                        throw new InvalidModuleFileKraken(module, path, string.Format(
+                            Properties.Resources.NetModuleCacheMismatchSHA1,
+                            module, path, sha1, module.download_hash.sha1));
+                    }
                 }
             }
 

diff --git a/Netkan/Transformers/DownloadAttributeTransformer.cs b/Netkan/Transformers/DownloadAttributeTransformer.cs
@@ -1,7 +1,10 @@
 using System;
 using System.Collections.Generic;
+
 using log4net;
 using Newtonsoft.Json.Linq;
+
+using CKAN.Versioning;
 using CKAN.NetKAN.Extensions;
 using CKAN.NetKAN.Model;
 using CKAN.NetKAN.Services;
@@ -45,8 +48,12 @@ public IEnumerable<Metadata> Transform(Metadata metadata, TransformOptions opts)
                     json["download_hash"] = new JObject();
 
                     var download_hashJson = (JObject)json["download_hash"];
-                    Log.Debug("Calculating download SHA1...");
-                    download_hashJson.SafeAdd("sha1", _fileService.GetFileHashSha1(file));
+                    // Older clients will complain if download_hash is set without sha1
+                    if (metadata.SpecVersion <= v1p34)
+                    {
+                        Log.Debug("Calculating download SHA1...");
+                        download_hashJson.SafeAdd("sha1", _fileService.GetFileHashSha1(file));
+                    }
                     Log.Debug("Calculating download SHA256...");
                     download_hashJson.SafeAdd("sha256", _fileService.GetFileHashSha256(file));
 
@@ -63,5 +70,6 @@ public IEnumerable<Metadata> Transform(Metadata metadata, TransformOptions opts)
                 yield return metadata;
             }
         }
+        private static readonly ModuleVersion v1p34 = new ModuleVersion("v1.34");
     }
 }
diff --git a/Spec.md b/Spec.md
@@ -451,14 +451,14 @@ and not filled in by hand.
 
 ##### download_hash
 
-If supplied, `download_hash` is an object of hash digests. Currently
-SHA1 and SHA256 calculated hashes of the resulting file downloaded.
+If supplied, `download_hash` is an object of hash digests of the downloaded file.
+Clients up to **v1.34** require both `sha1` and `sha256` to be populated, but
+later clients allow either or both to be omitted.
 It is recommended that this field is only generated by automated
 tools (where it is encouraged), and not filled in by hand.
 
 ```json
     "download_hash": {
-        "sha1": "1F4B3F21A77D4A302E3417A7C7A24A0B63740FC5",
         "sha256": "E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855"
     }
 ```