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

Present new binary for checking hashes #2766

Merged
merged 18 commits into from
Mar 11, 2019

Conversation

Ash258
Copy link
Contributor

@Ash258 Ash258 commented Nov 13, 2018

Should be working, but maybe I missed some use-case or something else.

For external buckets use: something like this

bin/checkhashes.ps1 Outdated Show resolved Hide resolved
@r15ch13
Copy link
Member

r15ch13 commented Jan 16, 2019

Made some changes to the output and add get_hash() function:

diff --git a/bin/checkhashes.ps1 b/bin/checkhashes.ps1
index cd5c7cb8e..31ae4111b 100644
--- a/bin/checkhashes.ps1
+++ b/bin/checkhashes.ps1
@@ -65,36 +65,38 @@ foreach ($single in Get-ChildItem $Dir "$App.json") {
     $manifest = parse_json "$Dir\$($single.Name)"
 
     # Skip nighly manifests, since their hash validation is skipped
-    if (($manifest.version -ne 'nightly')) {
-        $urls = @()
-        $hashes = @()
-
-        if ($manifest.architecture) {
-            # First handle 64bit
-            url $manifest '64bit' | ForEach-Object { $urls += $_ }
-            hash $manifest '64bit' | ForEach-Object { $hashes += $_ }
-            url $manifest '32bit' | ForEach-Object { $urls += $_ }
-            hash $manifest '32bit' | ForEach-Object { $hashes += $_ }
-        } elseif ($manifest.url) {
-            $manifest.url | ForEach-Object { $urls += $_ }
-            $manifest.hash | ForEach-Object { $hashes += $_ }
-        } else {
-            err $name 'Manifest does not contain URL property.'
-            continue
-        }
+    if ($manifest.version -eq 'nightly') {
+        continue
+    }
 
-        # Number of URLS and Hashes is different
-        if ($urls.Length -ne $hashes.Length) {
-            err $name 'URLS and hashes count mismatch.'
-            continue
-        }
+    $urls = @()
+    $hashes = @()
+
+    if ($manifest.architecture) {
+        # First handle 64bit
+        url $manifest '64bit' | ForEach-Object { $urls += $_ }
+        hash $manifest '64bit' | ForEach-Object { $hashes += $_ }
+        url $manifest '32bit' | ForEach-Object { $urls += $_ }
+        hash $manifest '32bit' | ForEach-Object { $hashes += $_ }
+    } elseif ($manifest.url) {
+        $manifest.url | ForEach-Object { $urls += $_ }
+        $manifest.hash | ForEach-Object { $hashes += $_ }
+    } else {
+        err $name 'Manifest does not contain URL property.'
+        continue
+    }
 
-        $MANIFESTS += New-Object psobject @{
-            app      = $name
-            manifest = $manifest
-            urls     = $urls
-            hashes   = $hashes
-        }
+    # Number of URLS and Hashes is different
+    if ($urls.Length -ne $hashes.Length) {
+        err $name 'URLS and hashes count mismatch.'
+        continue
+    }
+
+    $MANIFESTS += New-Object psobject @{
+        app      = $name
+        manifest = $manifest
+        urls     = $urls
+        hashes   = $hashes
     }
 }
 
@@ -114,27 +116,16 @@ $MANIFESTS | ForEach-Object {
     $actuals = @()
 
     $current.urls | ForEach-Object {
-        $expected_hash = $current.hashes[$count]
-        $algorithm = 'sha256'
-        $name = $current.app
+        $algorithm, $expected = get_hash $current.hashes[$count]
         $version = 'HASH_CHECK'
-        $tmp = $expected_hash -split ':'
-
-        if ($tmp.Length -eq 2) {
-            $algorithm = $tmp[0]
-            $expected_hash = $tmp[1]
-        }
 
-        dl_with_cache $name $version $_ $null $null -use_cache:$UseCache
+        dl_with_cache $current.app $version $_ $null $null -use_cache:$UseCache
 
-        $to_check = fullpath (cache_path $name $version $_)
+        $to_check = fullpath (cache_path $current.app $version $_)
         $actual_hash = compute_hash $to_check $algorithm
         $actuals += $actual_hash
-        if ($actual_hash -ne $expected_hash) {
+        if ($actual_hash -ne $expected) {
             $mismatched += $count
-            Write-Host 'Wrong' -ForegroundColor Red
-        } else {
-            if (!$SkipCorrect) { Write-Host 'OK' -ForegroundColor Green }
         }
         $count++
     }
@@ -145,11 +136,16 @@ $MANIFESTS | ForEach-Object {
             Write-Host 'OK' -ForegroundColor Green
         }
     } else {
-        Write-Host "$($current.app): "
+        Write-Host "$($current.app): " -NoNewline
+        Write-Host 'Mismatch found ' -ForegroundColor Red
         $mismatched | ForEach-Object {
-            Write-Host "`t$($current.urls[$_])" -ForegroundColor Red
-            Write-Host "`t`tExp:" $current.hashes[$_] -ForegroundColor Green
-            Write-Host "`t`tAct:" $actuals[$_] -ForegroundColor Red
+            $file = fullpath (cache_path $current.app $version $current.urls[$_])
+            Write-Host  "`tURL:`t`t$($current.urls[$_])"
+            if(Test-Path $file) {
+                Write-Host  "`tFirst bytes:`t$((get_magic_bytes_pretty $file ' ').ToUpper())"
+            }
+            Write-Host  "`tExpected:`t$($current.urls[$_])" -ForegroundColor Green
+            Write-Host  "`tActual:`t`t$($actuals[$_])" -ForegroundColor Red
         }
     }
 
diff --git a/lib/core.ps1 b/lib/core.ps1
index d2c2f1aff..8c63e224d 100644
--- a/lib/core.ps1
+++ b/lib/core.ps1
@@ -657,6 +657,19 @@ function format_hash_aria2([String] $hash) {
     return $hash
 }
 
+function get_hash([String] $multihash) {
+    $type, $hash = $multihash.split(':')
+    if(!$hash) {
+        # no type specified, assume sha256
+        $type, $hash = 'sha256', $multihash
+    }
+
+    if(@('md5','sha1','sha256', 'sha512') -notcontains $type) {
+        return $false, "Hash type '$type' isn't supported."
+    }
+    return $type, $hash
+}
+
 function handle_special_urls($url)
 {
     # FossHub.com
diff --git a/lib/install.ps1 b/lib/install.ps1
index 845a5f776..c446a1c06 100644
--- a/lib/install.ps1
+++ b/lib/install.ps1
@@ -672,17 +672,13 @@ function check_hash($file, $hash, $app_name) {
     Write-Host "Checking hash of " -NoNewline
     Write-Host $(url_remote_filename $url) -f Cyan -NoNewline
     Write-Host " ... " -nonewline
-    $type, $expected = $hash.split(':')
-    if(!$expected) {
-        # no type specified, assume sha256
-        $type, $expected = 'sha256', $type
-    }
 
-    if(@('md5','sha1','sha256', 'sha512') -notcontains $type) {
-        return $false, "Hash type '$type' isn't supported."
+    $algorithm, $expected = get_hash $hash
+    if($algorithm -eq $false) {
+        return $false, "Hash type '$algorithm' isn't supported."
     }
 
-    $actual = (compute_hash $file $type).ToLower()
+    $actual = (compute_hash $file $algorithm).ToLower()
     $expected = $expected.ToLower()
 
     if($actual -ne $expected) {

@Ash258
Copy link
Contributor Author

Ash258 commented Jan 17, 2019

Made some changes to your suggestion:

    if(@('md5','sha1','sha256', 'sha512') -notcontains $type) {
        return $false, "Hash type '$type' isn't supported."
    }

I rather return $null instead of $false. As $false implies, it's boolean always, but it's only in one use case.

Also fixed my bug, when I striped algorithm type, when there was any other than sha256, it just got saved as is without type.

Best manifests to test: python-exp, coreutils, ant

@Ash258
Copy link
Contributor Author

Ash258 commented Mar 8, 2019

Should be ready to merge 🎉

@r15ch13 r15ch13 merged commit ad01bff into ScoopInstaller:master Mar 11, 2019
@Ash258 Ash258 deleted the checkhashes branch March 11, 2019 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants