From 76b2eee7771ecf2b339d1023699ef9f30fa3fe65 Mon Sep 17 00:00:00 2001 From: Jack Nagel Date: Mon, 18 Jun 2012 19:58:35 -0500 Subject: [PATCH] Refactor checksumming Signed-off-by: Jack Nagel --- Library/Homebrew/bottles.rb | 2 +- Library/Homebrew/checksums.rb | 22 +++++++++++ Library/Homebrew/cmd/audit.rb | 21 +++++----- Library/Homebrew/exceptions.rb | 26 +++++++++++++ Library/Homebrew/extend/pathname.rb | 7 ++++ Library/Homebrew/formula.rb | 30 ++------------ Library/Homebrew/formula_support.rb | 46 +++++++++++++--------- Library/Homebrew/test/test_checksums.rb | 2 +- Library/Homebrew/test/test_formula.rb | 52 +++++++++++++------------ 9 files changed, 124 insertions(+), 84 deletions(-) create mode 100644 Library/Homebrew/checksums.rb diff --git a/Library/Homebrew/bottles.rb b/Library/Homebrew/bottles.rb index 0164b07ddf..d2abbf03f7 100644 --- a/Library/Homebrew/bottles.rb +++ b/Library/Homebrew/bottles.rb @@ -22,7 +22,7 @@ def built_bottle? f end def bottle_current? f - f.bottle.url && f.bottle.has_checksum? && f.bottle.version == f.stable.version + f.bottle.url && !f.bottle.checksum.empty? && f.bottle.version == f.stable.version end def bottle_file_outdated? f, file diff --git a/Library/Homebrew/checksums.rb b/Library/Homebrew/checksums.rb new file mode 100644 index 0000000000..defaa8bb1e --- /dev/null +++ b/Library/Homebrew/checksums.rb @@ -0,0 +1,22 @@ +class Checksum + attr_reader :hash_type, :hexdigest + + TYPES = [:md5, :sha1, :sha256] + + def initialize type=:sha1, val=nil + @hash_type = type + @hexdigest = val.to_s + end + + def empty? + @hexdigest.empty? + end + + def to_s + @hexdigest + end + + def == other + @hash_type == other.hash_type and @hexdigest == other.hexdigest + end +end diff --git a/Library/Homebrew/cmd/audit.rb b/Library/Homebrew/cmd/audit.rb index 7e05b7b3b5..0fd44580b3 100755 --- a/Library/Homebrew/cmd/audit.rb +++ b/Library/Homebrew/cmd/audit.rb @@ -290,22 +290,21 @@ def audit_formula_specs f end end - cksum = s.checksum_type - unless cksum.nil? - hash = s.send(cksum).strip - len = case cksum + cksum = s.checksum + next if cksum.nil? + + len = case cksum.hash_type when :md5 then 32 when :sha1 then 40 when :sha256 then 64 end - if hash.empty? - problems << " * #{cksum} is empty" - else - problems << " * #{cksum} should be #{len} characters" unless hash.length == len - problems << " * #{cksum} contains invalid characters" unless hash =~ /^[a-fA-F0-9]+$/ - problems << " * #{cksum} should be lowercase" unless hash == hash.downcase - end + if cksum.empty? + problems << " * #{cksum.hash_type} is empty" + else + problems << " * #{cksum.hash_type} should be #{len} characters" unless cksum.hexdigest.length == len + problems << " * #{cksum.hash_type} contains invalid characters" unless cksum.hexdigest =~ /^[a-fA-F0-9]+$/ + problems << " * #{cksum.hash_type} should be lowercase" unless cksum.hexdigest == cksum.hexdigest.downcase end end diff --git a/Library/Homebrew/exceptions.rb b/Library/Homebrew/exceptions.rb index d3e2e89362..2b7f6542c9 100644 --- a/Library/Homebrew/exceptions.rb +++ b/Library/Homebrew/exceptions.rb @@ -148,3 +148,29 @@ end # raised by safe_system in utils.rb class ErrorDuringExecution < RuntimeError end + +# raised by Pathname#verify_checksum when cksum is nil or empty +class ChecksumMissingError < ArgumentError +end + +# raised by Pathname#verify_checksum when verification fails +class ChecksumMismatchError < RuntimeError + attr :advice, true + attr :expected + attr :actual + + def initialize expected, actual + @expected = expected + @actual = actual + + super <<-EOS.undent + #{expected.hash_type.to_s.upcase} mismatch + Expected: #{expected} + Actual: #{actual} + EOS + end + + def to_s + super + advice + end +end diff --git a/Library/Homebrew/extend/pathname.rb b/Library/Homebrew/extend/pathname.rb index 907679f992..5edd8b6649 100644 --- a/Library/Homebrew/extend/pathname.rb +++ b/Library/Homebrew/extend/pathname.rb @@ -301,6 +301,13 @@ class Pathname require 'digest/sha2' incremental_hash(Digest::SHA2) end + alias_method :sha256, :sha2 + + 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(expected, actual) unless expected == actual + end if '1.9' <= RUBY_VERSION alias_method :to_str, :to_s diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 92509f13fc..2cf56581d1 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -38,7 +38,7 @@ class Formula # Ensure the bottle URL is set. If it does not have a checksum, # then a bottle is not available for the current platform. - if @bottle and @bottle.has_checksum? + if @bottle and not (@bottle.checksum.nil? or @bottle.checksum.empty?) @bottle.url ||= bottle_base_url + bottle_filename(self) else @bottle = nil @@ -514,31 +514,7 @@ public # For FormulaInstaller. def verify_download_integrity fn - # Checksums don't apply to HEAD - return if @active_spec == @head - - type = @active_spec.checksum_type || :md5 - supplied = @active_spec.send(type) - type = type.to_s.upcase - - require 'digest' - hasher = Digest.const_get(type) - hash = fn.incremental_hash(hasher) - - if supplied and not supplied.empty? - message = <<-EOS.undent - #{type} mismatch - Expected: #{supplied} - Got: #{hash} - Archive: #{fn} - (To retry an incomplete download, remove the file above.) - EOS - raise message unless supplied.upcase == hash.upcase - else - opoo "Cannot verify package integrity" - puts "The formula did not provide a download checksum" - puts "For your reference the #{type} is: #{hash}" - end + @active_spec.verify_download_integrity(fn) end private @@ -605,7 +581,7 @@ private attr_rw :homepage, :keg_only_reason, :skip_clean_all, :cc_failures - SoftwareSpec::CHECKSUM_TYPES.each do |cksum| + Checksum::TYPES.each do |cksum| class_eval %Q{ def #{cksum}(val=nil) unless val.nil? diff --git a/Library/Homebrew/formula_support.rb b/Library/Homebrew/formula_support.rb index 9bd7b6d6bd..0449094cb0 100644 --- a/Library/Homebrew/formula_support.rb +++ b/Library/Homebrew/formula_support.rb @@ -1,10 +1,9 @@ require 'download_strategy' +require 'checksums' class SoftwareSpec attr_reader :checksum, :mirrors, :specs, :strategy - CHECKSUM_TYPES = [:md5, :sha1, :sha256].freeze - VCS_SYMBOLS = { :bzr => BazaarDownloadStrategy, :curl => CurlDownloadStrategy, @@ -16,17 +15,6 @@ class SoftwareSpec :svn => SubversionDownloadStrategy, } - # Detect which type of checksum is being used, or nil if none - def checksum_type - @checksum_type ||= CHECKSUM_TYPES.detect do |type| - instance_variable_defined?("@#{type}") - end - end - - def has_checksum? - (checksum_type and self.send(checksum_type)) || false - end - # Was the version defined in the DSL, or detected from the URL? def explicit_version? @explicit_version || false @@ -45,11 +33,29 @@ class SoftwareSpec return detected end + def verify_download_integrity fn + fn.verify_checksum @checksum + rescue ChecksumMissingError + opoo "Cannot verify package integrity" + puts "The formula did not provide a download checksum" + puts "For your reference the SHA1 is: #{fn.sha1}" + rescue ChecksumMismatchError => e + e.advice = <<-EOS.undent + Archive: #{fn} + (To retry an incomplete download, remove the file above.) + EOS + raise e + end + # The methods that follow are used in the block-form DSL spec methods - CHECKSUM_TYPES.each do |cksum| + Checksum::TYPES.each do |cksum| class_eval %Q{ def #{cksum}(val=nil) - val.nil? ? @#{cksum} : @#{cksum} = val + if val.nil? + @checksum if @checksum.nil? or @checksum.hash_type == :#{cksum} + else + @checksum = Checksum.new(:#{cksum}, val) + end end } end @@ -84,7 +90,6 @@ class HeadSoftwareSpec < SoftwareSpec def initialize super @version = 'HEAD' - @checksum = nil end def verify_download_integrity fn @@ -97,13 +102,14 @@ class Bottle < SoftwareSpec attr_reader :revision def initialize + super @revision = 0 @strategy = CurlBottleDownloadStrategy end # Checksum methods in the DSL's bottle block optionally take # a Hash, which indicates the platform the checksum applies on. - CHECKSUM_TYPES.each do |cksum| + Checksum::TYPES.each do |cksum| class_eval %Q{ def #{cksum}(val=nil) @#{cksum} ||= Hash.new @@ -111,11 +117,13 @@ class Bottle < SoftwareSpec when nil @#{cksum}[MacOS.cat] when String - @#{cksum}[:lion] = val + @#{cksum}[:lion] = Checksum.new(:#{cksum}, val) when Hash key, value = val.shift - @#{cksum}[value] = key + @#{cksum}[value] = Checksum.new(:#{cksum}, key) end + + @checksum = @#{cksum}[MacOS.cat] if @#{cksum}.has_key? MacOS.cat end } end diff --git a/Library/Homebrew/test/test_checksums.rb b/Library/Homebrew/test/test_checksums.rb index a4c88300c3..c1e20ef282 100644 --- a/Library/Homebrew/test/test_checksums.rb +++ b/Library/Homebrew/test/test_checksums.rb @@ -13,7 +13,7 @@ class ChecksumTests < Test::Unit::TestCase end def bad_checksum f - assert_raises RuntimeError do + assert_raises ChecksumMismatchError do nostdout { f.new.brew {} } end end diff --git a/Library/Homebrew/test/test_formula.rb b/Library/Homebrew/test/test_formula.rb index 55b48e3b03..8f4e5149b8 100644 --- a/Library/Homebrew/test/test_formula.rb +++ b/Library/Homebrew/test/test_formula.rb @@ -99,20 +99,22 @@ class FormulaTests < Test::Unit::TestCase assert_equal CurlDownloadStrategy, f.devel.download_strategy assert_equal GitDownloadStrategy, f.head.download_strategy - assert f.stable.has_checksum? - assert f.bottle.has_checksum? - assert f.devel.has_checksum? - assert !f.head.has_checksum? - assert_equal :sha1, f.stable.checksum_type - assert_equal :sha1, f.bottle.checksum_type - assert_equal :sha256, f.devel.checksum_type - assert_nil f.head.checksum_type + assert_instance_of Checksum, f.stable.checksum + assert_instance_of Checksum, f.bottle.checksum + assert_instance_of Checksum, f.devel.checksum + assert !f.stable.checksum.empty? + assert !f.bottle.checksum.empty? + assert !f.devel.checksum.empty? + assert_nil f.head.checksum + assert_equal :sha1, f.stable.checksum.hash_type + assert_equal :sha1, f.bottle.checksum.hash_type + assert_equal :sha256, f.devel.checksum.hash_type assert_equal case MacOS.cat when :snowleopard then 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef' when :lion then 'baadf00dbaadf00dbaadf00dbaadf00dbaadf00d' - end, f.bottle.sha1 - assert_match /[0-9a-fA-F]{40}/, f.stable.sha1 - assert_match /[0-9a-fA-F]{64}/, f.devel.sha256 + end, f.bottle.checksum.hexdigest + assert_match /[0-9a-fA-F]{40}/, f.stable.checksum.hexdigest + assert_match /[0-9a-fA-F]{64}/, f.devel.checksum.hexdigest assert_nil f.stable.md5 assert_nil f.stable.sha256 @@ -155,11 +157,10 @@ class FormulaTests < Test::Unit::TestCase assert_nil f.stable.specs assert_equal({ :tag => 'foo' }, f.head.specs) - assert f.stable.has_checksum? - assert !f.head.has_checksum? - assert_equal :md5, f.stable.checksum_type - assert_nil f.head.checksum_type - assert_match /[0-9a-fA-F]{32}/, f.stable.md5 + assert_instance_of Checksum, f.stable.checksum + assert_nil f.head.checksum + assert_equal :md5, f.stable.checksum.hash_type + assert_match /[0-9a-fA-F]{32}/, f.stable.checksum.hexdigest assert !f.stable.explicit_version? assert_equal '0.1', f.stable.version @@ -209,9 +210,10 @@ class FormulaTests < Test::Unit::TestCase assert_equal 'file:///foo.com/testball-0.1-bottle.tar.gz', f.bottle.url - assert f.bottle.has_checksum? - assert_equal :sha1, f.bottle.checksum_type - assert_equal 'baadf00dbaadf00dbaadf00dbaadf00dbaadf00d', f.bottle.sha1 + assert_instance_of Checksum, f.bottle.checksum + assert_equal :sha1, f.bottle.checksum.hash_type + assert !f.bottle.checksum.empty? + assert_equal 'baadf00dbaadf00dbaadf00dbaadf00dbaadf00d', f.bottle.sha1.hexdigest assert_nil f.bottle.md5 assert_nil f.bottle.sha256 @@ -238,7 +240,7 @@ class FormulaTests < Test::Unit::TestCase assert_equal f.head, f.active_spec assert_equal 'HEAD', f.version - assert !f.head.has_checksum? + assert_nil f.head.checksum assert_equal 'https://github.com/mxcl/homebrew.git', f.url assert_equal GitDownloadStrategy, f.download_strategy assert_instance_of GitDownloadStrategy, f.downloader @@ -255,7 +257,7 @@ class FormulaTests < Test::Unit::TestCase assert_equal f.head, f.active_spec assert_equal 'HEAD', f.version - assert !f.head.has_checksum? + assert_nil f.head.checksum assert_equal 'https://github.com/mxcl/homebrew.git', f.url assert_equal GitDownloadStrategy, f.download_strategy assert_instance_of GitDownloadStrategy, f.downloader @@ -272,7 +274,7 @@ class FormulaTests < Test::Unit::TestCase assert_equal f.head, f.active_spec assert_equal 'HEAD', f.version - assert !f.head.has_checksum? + assert_nil f.head.checksum assert_equal 'https://github.com/mxcl/homebrew.git', f.url assert_equal GitDownloadStrategy, f.download_strategy assert_instance_of GitDownloadStrategy, f.downloader @@ -288,9 +290,9 @@ class FormulaTests < Test::Unit::TestCase assert_equal f.stable, f.active_spec - assert !f.stable.has_checksum? - assert !f.devel.has_checksum? - assert !f.head.has_checksum? + assert_nil f.stable.checksum + assert_nil f.devel.checksum + assert_nil f.head.checksum assert_equal MercurialDownloadStrategy, f.stable.download_strategy assert_equal BazaarDownloadStrategy, f.devel.download_strategy