diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 9ffef0f999..e49f65dd25 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -174,30 +174,64 @@ class FormulaAuditor @specs = %w[stable devel head].map { |s| formula.send(s) }.compact end - def url_status_code(url, range: false) - # The system Curl is too old and unreliable with HTTPS homepages on - # Yosemite and below. - return "200" unless DevelopmentTools.curl_handles_most_https_homepages? + def self.check_http_content(url, user_agents: [:default]) + return unless url.start_with? "http" - extra_args = [ - "--connect-timeout", "15", - "--output", "/dev/null", - "--write-out", "%{http_code}" - ] - extra_args << "--range" << "0-0" if range - extra_args << url - - status_code = nil - [:browser, :default].each do |user_agent| - args = curl_args( - extra_args: extra_args, - show_output: true, - user_agent: user_agent, - ) - status_code = Open3.popen3(*args) { |_, stdout, _, _| stdout.read } - break if status_code.start_with? "20" + details = nil + user_agent = nil + user_agents.each do |ua| + details = http_content_headers_and_checksum(url, user_agent: ua) + user_agent = ua + break if details[:status].to_s.start_with?("2") end - status_code + + return "The URL #{url} is not reachable" unless details[:status] + unless details[:status].start_with? "2" + return "The URL #{url} is not reachable (HTTP status code #{details[:status]})" + end + + return unless url.start_with? "http:" + + secure_url = url.sub "http", "https" + secure_details = + http_content_headers_and_checksum(secure_url, user_agent: user_agent) + + if !details[:status].to_s.start_with?("2") || + !secure_details[:status].to_s.start_with?("2") + return + end + + etag_match = details[:etag] && + details[:etag] == secure_details[:etag] + content_length_match = + details[:content_length] && + details[:content_length] == secure_details[:content_length] + file_match = details[:file_hash] == secure_details[:file_hash] + + return if !etag_match && !content_length_match && !file_match + "The URL #{url} could use HTTPS rather than HTTP" + end + + def self.http_content_headers_and_checksum(url, user_agent: :default) + args = curl_args( + extra_args: ["--connect-timeout", "15", "--include", url], + show_output: true, + user_agent: user_agent, + ) + output = Open3.popen3(*args) { |_, stdout, _, _| stdout.read } + + status_code = :unknown + while status_code == :unknown || status_code.to_s.start_with?("3") + headers, _, output = output.partition("\r\n\r\n") + status_code = headers[%r{HTTP\/.* (\d+)}, 1] + end + + { + status: status_code, + etag: headers[%r{ETag: ([wW]\/)?"(([^"]|\\")*)"}, 2], + content_length: headers[/Content-Length: (\d+)/, 1], + file_hash: Digest::SHA256.digest(output), + } end def audit_style @@ -619,9 +653,13 @@ class FormulaAuditor return unless @online - status_code = url_status_code(homepage) - return if status_code.start_with? "20" - problem "The homepage #{homepage} is not reachable (HTTP status code #{status_code})" + # The system Curl is too old and unreliable with HTTPS homepages on + # Yosemite and below. + return unless DevelopmentTools.curl_handles_most_https_homepages? + if http_content_problem = FormulaAuditor.check_http_content(homepage, + user_agents: [:browser, :default]) + problem http_content_problem + end end def audit_bottle_spec @@ -671,11 +709,11 @@ class FormulaAuditor %w[Stable Devel HEAD].each do |name| next unless spec = formula.send(name.downcase) - ra = ResourceAuditor.new(spec, online: @online).audit + ra = ResourceAuditor.new(spec, online: @online, strict: @strict).audit problems.concat ra.problems.map { |problem| "#{name}: #{problem}" } spec.resources.each_value do |resource| - ra = ResourceAuditor.new(resource, online: @online).audit + ra = ResourceAuditor.new(resource, online: @online, strict: @strict).audit problems.concat ra.problems.map { |problem| "#{name} resource #{resource.name.inspect}: #{problem}" } @@ -1231,6 +1269,7 @@ class ResourceAuditor @using = resource.using @specs = resource.specs @online = options[:online] + @strict = options[:strict] @problems = [] end @@ -1490,38 +1529,26 @@ class ResourceAuditor return unless @online urls.each do |url| - check_insecure_mirror(url) if url.start_with? "http:" + next if !@strict && mirrors.include?(url) + + strategy = DownloadStrategyDetector.detect(url, using) + if strategy <= CurlDownloadStrategy && !url.start_with?("file") + if http_content_problem = FormulaAuditor.check_http_content(url) + problem http_content_problem + end + elsif strategy <= GitDownloadStrategy + unless Utils.git_remote_exists url + problem "The URL #{url} is not a valid git URL" + end + elsif strategy <= SubversionDownloadStrategy + unless Utils.svn_remote_exists url + problem "The URL #{url} is not a valid svn URL" + end + end end end - def check_insecure_mirror(url) - details = get_content_details(url) - secure_url = url.sub "http", "https" - secure_details = get_content_details(secure_url) - - return if details[:status].nil? || secure_details[:status].nil? || !details[:status].start_with?("2") || !secure_details[:status].start_with?("2") - - etag_match = details[:etag] && details[:etag] == secure_details[:etag] - content_length_match = details[:content_length] && details[:content_length] == secure_details[:content_length] - file_match = details[:file_hash] == secure_details[:file_hash] - - return if !etag_match && !content_length_match && !file_match - problem "The URL #{url} could use HTTPS rather than HTTP" - end - def problem(text) @problems << text end - - def get_content_details(url) - out = {} - output, = curl_output "--connect-timeout", "15", "--include", url - split = output.partition("\r\n\r\n") - headers = split.first - out[:status] = headers[%r{HTTP\/.* (\d+)}, 1] - out[:etag] = headers[%r{ETag: ([wW]\/)?"(([^"]|\\")*)"}, 2] - out[:content_length] = headers[/Content-Length: (\d+)/, 1] - out[:file_hash] = Digest::SHA256.digest split.last - out - end end diff --git a/Library/Homebrew/test/audit_test.rb b/Library/Homebrew/test/audit_test.rb index 9165edef1e..1d93c31e00 100644 --- a/Library/Homebrew/test/audit_test.rb +++ b/Library/Homebrew/test/audit_test.rb @@ -419,9 +419,8 @@ class FormulaAuditorTests < Homebrew::TestCase EOS fa.audit_homepage - assert_equal ["The homepage should start with http or https " \ - "(URL is #{fa.formula.homepage}).", "The homepage #{fa.formula.homepage} is not reachable " \ - "(HTTP status code 000)"], fa.problems + assert_equal ["The homepage should start with http or https (URL is #{fa.formula.homepage})."], + fa.problems formula_homepages = { "bar" => "http://www.freedesktop.org/wiki/bar", diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb index 70d2787d97..b129c73287 100644 --- a/Library/Homebrew/utils.rb +++ b/Library/Homebrew/utils.rb @@ -10,6 +10,7 @@ require "utils/github" require "utils/hash" require "utils/inreplace" require "utils/popen" +require "utils/svn" require "utils/tty" require "time" diff --git a/Library/Homebrew/utils/git.rb b/Library/Homebrew/utils/git.rb index dfe47f890f..1b4d248946 100644 --- a/Library/Homebrew/utils/git.rb +++ b/Library/Homebrew/utils/git.rb @@ -40,4 +40,9 @@ module Utils @git_path = nil @git_version = nil end + + def self.git_remote_exists(url) + return true unless git_available? + quiet_system "git", "ls-remote", url + end end diff --git a/Library/Homebrew/utils/svn.rb b/Library/Homebrew/utils/svn.rb new file mode 100644 index 0000000000..fb49ac2e99 --- /dev/null +++ b/Library/Homebrew/utils/svn.rb @@ -0,0 +1,11 @@ +module Utils + def self.svn_available? + return @svn if instance_variable_defined?(:@svn) + @svn = quiet_system HOMEBREW_SHIMS_PATH/"scm/svn", "--version" + end + + def self.svn_remote_exists(url) + return true unless svn_available? + quiet_system "svn", "ls", url, "--depth", "empty" + end +end