Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop checking multiple hashes #4135

Merged
merged 3 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 24 additions & 19 deletions Core/Net/NetModuleCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
}
}
}

Expand Down
12 changes: 10 additions & 2 deletions Netkan/Transformers/DownloadAttributeTransformer.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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));

Expand All @@ -63,5 +70,6 @@ public IEnumerable<Metadata> Transform(Metadata metadata, TransformOptions opts)
yield return metadata;
}
}
private static readonly ModuleVersion v1p34 = new ModuleVersion("v1.34");
}
}
6 changes: 3 additions & 3 deletions Spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
```
Expand Down