From 05690689bcac96bc1e6e28667a15e52cf789714c Mon Sep 17 00:00:00 2001 From: Roland Walker Date: Tue, 23 Dec 2014 11:08:45 -0500 Subject: [PATCH] Remove homebrew-fork Checksum class and refactor * perform actual checksum calculation within download.rb * use an accessor named sha256 instead of sums for checksum data * migrate checksum-specific exceptions out of homebrew-fork --- lib/cask/audit.rb | 11 ++---- lib/cask/cli.rb | 2 +- lib/cask/cli/internal_stanza.rb | 6 +--- lib/cask/download.rb | 36 +++++++++++-------- lib/cask/dsl.rb | 33 +++++++++-------- lib/cask/exceptions.rb | 22 ++++++++++++ lib/cask/utils.rb | 3 +- .../Library/Homebrew/checksum.rb | 19 ---------- .../Library/Homebrew/exceptions.rb | 21 ----------- .../Library/Homebrew/extend/pathname.rb | 22 ------------ .../Library/Homebrew/resource.rb | 21 ++--------- test/cask/dsl_test.rb | 4 +-- test/cask/installer_test.rb | 4 +-- 13 files changed, 70 insertions(+), 134 deletions(-) delete mode 100644 lib/homebrew-fork/Library/Homebrew/checksum.rb diff --git a/lib/cask/audit.rb b/lib/cask/audit.rb index 67893f0decb..539a4a435eb 100644 --- a/lib/cask/audit.rb +++ b/lib/cask/audit.rb @@ -13,7 +13,6 @@ class Cask::Audit def run!(download = false) _check_required_stanzas _check_no_string_version_latest - _check_checksums _check_sha256_no_check_if_latest _check_sourceforge_download_url_format _check_download(download) if download @@ -27,7 +26,7 @@ class Cask::Audit def _check_required_stanzas odebug "Auditing required stanzas" - %i{version url homepage}.each do |sym| + %i{version sha256 url homepage}.each do |sym| add_error "a #{sym} stanza is required" unless cask.send(sym) end add_error 'a license value is required (:unknown is OK)' unless cask.license @@ -37,12 +36,6 @@ class Cask::Audit add_error 'at least one activatable artifact stanza is required' unless installable_artifacts.size > 0 end - def _check_checksums - odebug "Auditing checksums" - return if cask.sums == :no_check - add_error 'a sha256 stanza is required' unless cask.sums.is_a?(Array) && cask.sums.length > 0 - end - def _check_no_string_version_latest odebug "Verifying version :latest does not appear as a string ('latest')" if (cask.version == 'latest') @@ -52,7 +45,7 @@ class Cask::Audit def _check_sha256_no_check_if_latest odebug "Verifying sha256 :no_check with version :latest" - if cask.version == :latest and cask.sums.is_a?(Array) + if cask.version == :latest and cask.sha256 != :no_check add_error "you should use sha256 :no_check when version is :latest" end end diff --git a/lib/cask/cli.rb b/lib/cask/cli.rb index c781f8c544f..199198ba277 100644 --- a/lib/cask/cli.rb +++ b/lib/cask/cli.rb @@ -117,7 +117,7 @@ class Cask::CLI Cask.init command = lookup_command(command_string) run_command(command, *rest) - rescue CaskError, ChecksumMismatchError => e + rescue CaskError, CaskSha256MismatchError => e onoe e $stderr.puts e.backtrace if Cask.debug exit 1 diff --git a/lib/cask/cli/internal_stanza.rb b/lib/cask/cli/internal_stanza.rb index eab90a6e389..9a99b054a88 100644 --- a/lib/cask/cli/internal_stanza.rb +++ b/lib/cask/cli/internal_stanza.rb @@ -6,7 +6,7 @@ class Cask::CLI::InternalStanza < Cask::CLI::InternalUseBase # # If no tokens are given, then data for all Casks is returned. # - # The pseudo-stanzas "artifacts" and "sums" are available. + # The pseudo-stanza "artifacts" is available. # # On failure, a blank line is returned on the standard output. # @@ -70,7 +70,6 @@ class Cask::CLI::InternalStanza < Cask::CLI::InternalUseBase def self.print_stanzas(stanza, format=nil, table=nil, quiet=nil, *cask_tokens) count = 0 - stanza = :sums if stanza == :sha256 stanza = :full_name if stanza == :name if ARTIFACTS.include?(stanza) artifact_name = stanza @@ -118,9 +117,6 @@ class Cask::CLI::InternalStanza < Cask::CLI::InternalUseBase else if artifact_name or value.is_a?(Symbol) puts value.inspect - elsif stanza == :sums - # hack. why does "to_s" equal "inspect" for checksum objects? - puts value else puts value.to_s end diff --git a/lib/cask/download.rb b/lib/cask/download.rb index 1a90a629927..9949c7fd1b0 100644 --- a/lib/cask/download.rb +++ b/lib/cask/download.rb @@ -27,25 +27,31 @@ class Cask::Download HOMEBREW_CACHE_CASKS.join(downloaded_path.basename) rescue StandardError => e end - _check_sums(downloaded_path, cask.sums) unless cask.sums === :no_check + _compare_sha256(downloaded_path, cask) downloaded_path end private - def _check_sums(path, sums) - has_sum = false - sums.each do |sum| - unless sum.empty? - computed = Checksum.new(sum.hash_type, Digest.const_get(sum.hash_type.to_s.upcase).file(path).hexdigest) - if sum == computed - odebug "Checksums match" - else - ohai 'Note: running "brew update" may fix checksum errors' - raise ChecksumMismatchError.new(path, sum, computed) - end - has_sum = true - end + def _calc_sha256(path) + require 'digest/sha2' + Digest::SHA2.file(path).hexdigest + end + + def _compare_sha256(path, cask) + begin + expected = cask.sha256 + rescue RuntimeError => e + end + return if expected == :no_check + computed = _calc_sha256(path) + if expected.nil? or expected.empty? + raise CaskSha256MissingError.new("sha256 required: sha256 '#{computed}'") + end + if expected == computed + odebug "SHA256 checksums match" + else + ohai 'Note: running "brew update" may fix sha256 checksum errors' + raise CaskSha256MismatchError.new(path, expected, computed) end - raise ChecksumMissingError.new("Checksum required: sha256 '#{Digest::SHA256.file(path).hexdigest}'") unless has_sum end end diff --git a/lib/cask/dsl.rb b/lib/cask/dsl.rb index fc7a88778c5..6a4caa56d59 100644 --- a/lib/cask/dsl.rb +++ b/lib/cask/dsl.rb @@ -1,4 +1,3 @@ -require 'checksum' require 'set' module Cask::DSL; end @@ -44,7 +43,7 @@ module Cask::DSL def tags; self.class.tags; end - def sums; self.class.sums || []; end + def sha256; self.class.sha256; end def artifacts; self.class.artifacts; end @@ -149,6 +148,21 @@ module Cask::DSL @version ||= arg end + SYMBOLIC_SHA256S = Set.new [ + :no_check, + ] + + def sha256(arg=nil) + if arg.nil? + @sha256 + elsif @sha256 + raise CaskInvalidError.new(self.token, "'sha256' stanza may only appear once") + elsif !arg.is_a?(String) and !SYMBOLIC_SHA256S.include?(arg) + raise CaskInvalidError.new(self.token, "invalid 'sha256' value: '#{arg.inspect}'") + end + @sha256 ||= arg + end + def tags(*args) return @tags if args.empty? if @tags and !args.empty? @@ -296,21 +310,6 @@ module Cask::DSL end end - attr_reader :sums - - def hash_name(hash_type) - hash_type.to_s == 'sha2' ? 'sha256' : hash_type.to_s - end - - def sha256(sha2=nil) - if sha2 == :no_check - @sums = sha2 - else - @sums ||= [] - @sums << Checksum.new(:sha2, sha2) unless sha2.nil? - end - end - def method_missing(method, *args) Cask::Utils.method_missing_message(method, self.token) return nil diff --git a/lib/cask/exceptions.rb b/lib/cask/exceptions.rb index 81ab7ed51d7..f187a7678b5 100644 --- a/lib/cask/exceptions.rb +++ b/lib/cask/exceptions.rb @@ -103,3 +103,25 @@ class CaskInvalidError < CaskError "Cask '#{token}' definition is invalid" + (submsg.length > 0 ? ": #{submsg}" : '') end end + +class CaskSha256MissingError < ArgumentError +end + +class CaskSha256MismatchError < RuntimeError + attr_reader :path, :expected, :actual + def initialize(path, expected, actual) + @path = path + @expected = expected + @actual = actual + end + + def to_s + <<-EOS.undent + sha256 mismatch + Expected: #{expected} + Actual: #{actual} + File: #{path} + To retry an incomplete download, remove the file above. + EOS + end +end diff --git a/lib/cask/utils.rb b/lib/cask/utils.rb index c3a674f172c..4107f743bd6 100644 --- a/lib/cask/utils.rb +++ b/lib/cask/utils.rb @@ -1,3 +1,4 @@ +# coding: utf-8 # see Homebrew Library/Homebrew/utils.rb require 'yaml' @@ -101,7 +102,7 @@ module Cask::Utils :version, :license, :tags, - :sums, + :sha256, :artifacts, :caveats, :depends_on, diff --git a/lib/homebrew-fork/Library/Homebrew/checksum.rb b/lib/homebrew-fork/Library/Homebrew/checksum.rb deleted file mode 100644 index a4e38e0adad..00000000000 --- a/lib/homebrew-fork/Library/Homebrew/checksum.rb +++ /dev/null @@ -1,19 +0,0 @@ -class Checksum - attr_reader :hash_type, :hexdigest - alias_method :to_s, :hexdigest - - TYPES = [:sha1, :sha256] - - def initialize(hash_type, hexdigest) - @hash_type = hash_type - @hexdigest = hexdigest - end - - def empty? - hexdigest.empty? - end - - def ==(other) - hash_type == other.hash_type && hexdigest == other.hexdigest - end -end diff --git a/lib/homebrew-fork/Library/Homebrew/exceptions.rb b/lib/homebrew-fork/Library/Homebrew/exceptions.rb index 9d09b8b860e..7d7043beb6d 100644 --- a/lib/homebrew-fork/Library/Homebrew/exceptions.rb +++ b/lib/homebrew-fork/Library/Homebrew/exceptions.rb @@ -8,24 +8,3 @@ end # raised in CurlDownloadStrategy.fetch class CurlDownloadStrategyError < RuntimeError; end - -# raised by Pathname#verify_checksum when "expected" is nil or empty -class ChecksumMissingError < ArgumentError; end - -# raised by Pathname#verify_checksum when verification fails -class ChecksumMismatchError < RuntimeError - attr_reader :expected, :hash_type - - def initialize fn, expected, actual - @expected = expected - @hash_type = expected.hash_type.to_s.upcase - - super <<-EOS.undent - #{@hash_type} mismatch - Expected: #{expected} - Actual: #{actual} - Archive: #{fn} - To retry an incomplete download, remove the file above. - EOS - end -end diff --git a/lib/homebrew-fork/Library/Homebrew/extend/pathname.rb b/lib/homebrew-fork/Library/Homebrew/extend/pathname.rb index 119e22557a9..19ed1d2d777 100644 --- a/lib/homebrew-fork/Library/Homebrew/extend/pathname.rb +++ b/lib/homebrew-fork/Library/Homebrew/extend/pathname.rb @@ -57,28 +57,6 @@ class Pathname %r[^#!\s*\S+] === open('r') { |f| f.read(1024) } end - def incremental_hash(klass) - digest = klass.new - if digest.respond_to?(:file) - digest.file(self) - else - buf = "" - open("rb") { |f| digest << buf while f.read(1024, buf) } - end - digest.hexdigest - end - - def sha256 - require 'digest/sha2' - incremental_hash(Digest::SHA2) - end - - def verify_checksum expected - raise ChecksumMissingError if expected.nil? or expected.empty? - actual = Checksum.new(expected.hash_type, send(expected.hash_type).downcase) - raise ChecksumMismatchError.new(self, expected, actual) unless expected == actual - end - # FIXME eliminate the places where we rely on this method alias_method :to_str, :to_s unless method_defined?(:to_str) diff --git a/lib/homebrew-fork/Library/Homebrew/resource.rb b/lib/homebrew-fork/Library/Homebrew/resource.rb index c1d10b68a4c..7ad61ab5347 100644 --- a/lib/homebrew-fork/Library/Homebrew/resource.rb +++ b/lib/homebrew-fork/Library/Homebrew/resource.rb @@ -1,5 +1,4 @@ require 'download_strategy' -require 'checksum' require 'version' # Resource is the fundamental representation of an external resource. The @@ -7,8 +6,8 @@ require 'version' # of this class. class Resource - attr_reader :checksum, :mirrors, :specs, :using - attr_writer :url, :checksum, :version + attr_reader :mirrors, :specs, :using + attr_writer :url, :version attr_accessor :download_strategy # Formula name must be set after the DSL, as we have no access to the @@ -21,7 +20,6 @@ class Resource @version = nil @mirrors = [] @specs = {} - @checksum = nil @using = nil instance_eval(&block) if block_given? end @@ -87,21 +85,6 @@ class Resource cached_download end - def verify_download_integrity fn - if fn.respond_to?(:file?) && fn.file? - ohai "Verifying #{fn.basename} checksum" if ARGV.verbose? - fn.verify_checksum(checksum) - end - rescue ChecksumMissingError - opoo "Cannot verify integrity of #{fn.basename}" - puts "A checksum was not provided for this resource" - puts "For your reference the SHA1 is: #{fn.sha1}" - end - - Checksum::TYPES.each do |type| - define_method(type) { |val| @checksum = Checksum.new(type, val) } - end - def url val=nil, specs={} return @url if val.nil? @url = val diff --git a/test/cask/dsl_test.rb b/test/cask/dsl_test.rb index 50c7c023830..c07a5d381ce 100644 --- a/test/cask/dsl_test.rb +++ b/test/cask/dsl_test.rb @@ -114,9 +114,7 @@ describe Cask::DSL do sha256 'imasha2' end instance = ChecksumCask.new - instance.sums.must_equal [ - Checksum.new(:sha2, 'imasha2') - ] + instance.sha256.must_equal 'imasha2' end end diff --git a/test/cask/installer_test.rb b/test/cask/installer_test.rb index aef57eab9ed..7cf5f729dc0 100644 --- a/test/cask/installer_test.rb +++ b/test/cask/installer_test.rb @@ -162,7 +162,7 @@ describe Cask::Installer do shutup do Cask::Installer.new(bad_checksum).install end - }.must_raise(ChecksumMismatchError) + }.must_raise(CaskSha256MismatchError) end it "blows up on a missing checksum" do @@ -171,7 +171,7 @@ describe Cask::Installer do shutup do Cask::Installer.new(missing_checksum).install end - }.must_raise(ChecksumMissingError) + }.must_raise(CaskSha256MissingError) end it "installs fine if sha256 :no_check is used" do