From 0e03b4689964425cc623ade699463440219f0899 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 5 Sep 2018 01:39:30 +0200 Subject: [PATCH] Refactor `Hbc::Verify`. --- Library/Homebrew/cask/verify.rb | 35 +++---- Library/Homebrew/cask/verify/checksum.rb | 47 ---------- .../test/cask/verify/checksum_spec.rb | 88 ------------------ Library/Homebrew/test/cask/verify_spec.rb | 91 ++++++++----------- 4 files changed, 54 insertions(+), 207 deletions(-) delete mode 100644 Library/Homebrew/cask/verify/checksum.rb delete mode 100644 Library/Homebrew/test/cask/verify/checksum_spec.rb diff --git a/Library/Homebrew/cask/verify.rb b/Library/Homebrew/cask/verify.rb index 94c11701bd7b0..a46b60755268f 100644 --- a/Library/Homebrew/cask/verify.rb +++ b/Library/Homebrew/cask/verify.rb @@ -1,31 +1,24 @@ -require "cask/verify/checksum" - module Hbc module Verify module_function - def verifications - [ - Hbc::Verify::Checksum, - ] - end - def all(cask, downloaded_path) - odebug "Verifying download" - verifications = for_cask(cask) - odebug "#{verifications.size} verifications defined", verifications - verifications.each do |verification| - odebug "Running verification of class #{verification}" - verification.new(cask, downloaded_path).verify + if cask.sha256 == :no_check + ohai "No SHA-256 checksum defined for Cask '#{cask}', skipping verification." + return end - end - def for_cask(cask) - odebug "Determining which verifications to run for Cask #{cask}" - verifications.select do |verification| - odebug "Checking for verification class #{verification}" - verification.me?(cask) - end + ohai "Verifying SHA-256 checksum for Cask '#{cask}'." + + expected = cask.sha256 + computed = downloaded_path.sha256 + + raise CaskSha256MissingError.new(cask.token, expected, computed) if expected.nil? || expected.empty? + + return if expected == computed + + ohai "Note: Running `brew update` may fix SHA-256 checksum errors." + raise CaskSha256MismatchError.new(cask.token, expected, computed, downloaded_path) end end end diff --git a/Library/Homebrew/cask/verify/checksum.rb b/Library/Homebrew/cask/verify/checksum.rb deleted file mode 100644 index bd169f12aa5cf..0000000000000 --- a/Library/Homebrew/cask/verify/checksum.rb +++ /dev/null @@ -1,47 +0,0 @@ -require "digest" - -module Hbc - module Verify - class Checksum - def self.me?(cask) - return true unless cask.sha256 == :no_check - ohai "No checksum defined for Cask #{cask}, skipping verification" - false - end - - attr_reader :cask, :downloaded_path - - def initialize(cask, downloaded_path) - @cask = cask - @downloaded_path = downloaded_path - end - - def verify - return unless self.class.me?(cask) - ohai "Verifying SHA-256 checksum for Cask '#{cask}'." - verify_checksum - end - - private - - def expected - @expected ||= cask.sha256 - end - - def computed - @computed ||= downloaded_path.sha256 - end - - def verify_checksum - raise CaskSha256MissingError.new(cask.token, expected, computed) if expected.nil? || expected.empty? - - if expected == computed - odebug "SHA-256 checksums match." - else - ohai 'Note: running "brew update" may fix sha256 checksum errors' - raise CaskSha256MismatchError.new(cask.token, expected, computed, downloaded_path) - end - end - end - end -end diff --git a/Library/Homebrew/test/cask/verify/checksum_spec.rb b/Library/Homebrew/test/cask/verify/checksum_spec.rb deleted file mode 100644 index 52c86ffc08107..0000000000000 --- a/Library/Homebrew/test/cask/verify/checksum_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -describe Hbc::Verify::Checksum, :cask do - let(:cask) { double("cask", token: "cask") } - let(:downloaded_path) { instance_double("Pathname") } - let(:verification) { described_class.new(cask, downloaded_path) } - - before do - allow(cask).to receive(:sha256).and_return(expected_sha256) - end - - describe ".me?" do - subject { described_class.me?(cask) } - - context "when expected sha256 is :no_check" do - let(:expected_sha256) { :no_check } - - it { is_expected.to be false } - end - - context "when expected sha256 is nil" do - let(:expected_sha256) { nil } - - it { is_expected.to be true } - end - - context "sha256 is empty" do - let(:expected_sha256) { "" } - - it { is_expected.to be true } - end - - context "when expected sha256 is a valid shasum" do - let(:expected_sha256) { "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef" } - - it { is_expected.to be true } - end - end - - describe "#verify" do - subject { verification.verify } - - before do - allow(cask).to receive(:sha256).and_return(expected_sha256) - allow(downloaded_path).to receive(:sha256).and_return(actual_sha256) - end - - let(:actual_sha256) { "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef" } - - context "when expected matches actual sha256" do - let(:expected_sha256) { actual_sha256 } - - it "does not raise an error" do - expect { subject }.not_to raise_error - end - end - - context "when expected sha256 is :no_check" do - let(:expected_sha256) { :no_check } - - it "does not raise an error" do - expect { subject }.not_to raise_error - end - end - - context "when expected does not match sha256" do - let(:expected_sha256) { "d3adb33fd3adb33fd3adb33fd3adb33fd3adb33fd3adb33fd3adb33fd3adb33f" } - - it "raises an error" do - expect { subject }.to raise_error(Hbc::CaskSha256MismatchError) - end - end - - context "when expected sha256 is nil" do - let(:expected_sha256) { nil } - - it "raises an error" do - expect { subject }.to raise_error(Hbc::CaskSha256MissingError) - end - end - - context "when expected sha256 is empty" do - let(:expected_sha256) { "" } - - it "raises an error" do - expect { subject }.to raise_error(Hbc::CaskSha256MissingError) - end - end - end -end diff --git a/Library/Homebrew/test/cask/verify_spec.rb b/Library/Homebrew/test/cask/verify_spec.rb index 79b23a0e3cf74..2a46df01c87e9 100644 --- a/Library/Homebrew/test/cask/verify_spec.rb +++ b/Library/Homebrew/test/cask/verify_spec.rb @@ -1,63 +1,52 @@ -describe Hbc::Verify, :cask do - let(:cask) { double("cask") } - - let(:verification_classes) { - [ - applicable_verification_class, - inapplicable_verification_class, - ] - } - - let(:applicable_verification_class) { - double("applicable_verification_class", me?: true) - } - - let(:inapplicable_verification_class) { - double("inapplicable_verification_class", me?: false) - } - - before do - allow(described_class).to receive(:verifications) - .and_return(verification_classes) - end +module Hbc + describe Verify, :cask do + describe "::all" do + subject(:verification) { described_class.all(cask, downloaded_path) } + let(:cask) { instance_double(Cask, token: "cask", sha256: expected_sha256) } + let(:cafebabe) { "cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe" } + let(:deadbeef) { "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef" } + let(:computed_sha256) { cafebabe } + let(:downloaded_path) { instance_double(Pathname, sha256: computed_sha256) } + + context "when the expected checksum is :no_check" do + let(:expected_sha256) { :no_check } + + it "skips the check" do + expect { verification }.to output(/skipping verification/).to_stdout + end + end - describe ".for_cask" do - subject { described_class.for_cask(cask) } + context "when expected and computed checksums match" do + let(:expected_sha256) { cafebabe } - it "checks applicability of each verification" do - verification_classes.each do |verify_class| - expect(verify_class).to receive(:me?).with(cask) + it "does not raise an error" do + expect { verification }.not_to raise_error + end end - subject - end - it "includes applicable verifications" do - expect(subject).to include(applicable_verification_class) - end + context "when the expected checksum is nil" do + let(:expected_sha256) { nil } - it "excludes inapplicable verifications" do - expect(subject).not_to include(inapplicable_verification_class) - end - end + it "raises an error" do + expect { verification }.to raise_error CaskSha256MissingError + end + end - describe ".all" do - subject { described_class.all(cask, downloaded_path) } + context "when the expected checksum is empty" do + let(:expected_sha256) { "" } - let(:downloaded_path) { double("downloaded_path") } - let(:applicable_verification) { double("applicable_verification") } - let(:inapplicable_verification) { double("inapplicable_verification") } + it "raises an error" do + expect { verification }.to raise_error CaskSha256MissingError + end + end - before do - allow(applicable_verification_class).to receive(:new) - .and_return(applicable_verification) - allow(inapplicable_verification_class).to receive(:new) - .and_return(inapplicable_verification) - end + context "when expected and computed checksums do not match" do + let(:expected_sha256) { deadbeef } - it "runs only applicable verifications" do - expect(applicable_verification).to receive(:verify) - expect(inapplicable_verification).not_to receive(:verify) - subject + it "raises an error" do + expect { verification }.to raise_error CaskSha256MismatchError + end + end end end end