From b7ddd271e1d3b983b0fffbb6d87f0b45a3abcb9e Mon Sep 17 00:00:00 2001 From: Gautham Goli Date: Wed, 19 Jul 2017 01:14:14 +0530 Subject: [PATCH] audit: Port audit_urls partially to rubocop and add corresponding tests --- Library/Homebrew/dev-cmd/audit.rb | 49 ---------- Library/Homebrew/rubocops.rb | 1 + Library/Homebrew/rubocops/urls_cop.rb | 90 +++++++++++++++++++ .../Homebrew/test/rubocops/urls_cop_spec.rb | 54 +++++++++++ 4 files changed, 145 insertions(+), 49 deletions(-) create mode 100644 Library/Homebrew/rubocops/urls_cop.rb create mode 100644 Library/Homebrew/test/rubocops/urls_cop_spec.rb diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 4bcfdd1284..327c3e8116 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -1282,56 +1282,7 @@ class ResourceAuditor end def audit_urls - # Check GNU urls; doesn't apply to mirrors - if url =~ %r{^(?:https?|ftp)://ftpmirror.gnu.org/(.*)} - problem "Please use \"https://ftp.gnu.org/gnu/#{Regexp.last_match(1)}\" instead of #{url}." - end - - # Fossies upstream requests they aren't used as primary URLs - # https://github.com/Homebrew/homebrew-core/issues/14486#issuecomment-307753234 - if url =~ %r{^https?://fossies\.org/} - problem "Please don't use fossies.org in the url (using as a mirror is fine)" - end - - if mirrors.include?(url) - problem "URL should not be duplicated as a mirror: #{url}" - end - urls = [url] + mirrors - - # Check a variety of SSL/TLS URLs that don't consistently auto-redirect - # or are overly common errors that need to be reduced & fixed over time. - urls.each do |p| - case p - when %r{^http://ftp\.gnu\.org/}, - %r{^http://ftpmirror\.gnu\.org/}, - %r{^http://download\.savannah\.gnu\.org/}, - %r{^http://download-mirror\.savannah\.gnu\.org/}, - %r{^http://[^/]*\.apache\.org/}, - %r{^http://code\.google\.com/}, - %r{^http://fossies\.org/}, - %r{^http://mirrors\.kernel\.org/}, - %r{^http://(?:[^/]*\.)?bintray\.com/}, - %r{^http://tools\.ietf\.org/}, - %r{^http://launchpad\.net/}, - %r{^http://github\.com/}, - %r{^http://bitbucket\.org/}, - %r{^http://anonscm\.debian\.org/}, - %r{^http://cpan\.metacpan\.org/}, - %r{^http://hackage\.haskell\.org/}, - %r{^http://(?:[^/]*\.)?archive\.org}, - %r{^http://(?:[^/]*\.)?freedesktop\.org}, - %r{^http://(?:[^/]*\.)?mirrorservice\.org/} - problem "Please use https:// for #{p}" - when %r{^http://search\.mcpan\.org/CPAN/(.*)}i - problem "#{p} should be `https://cpan.metacpan.org/#{Regexp.last_match(1)}`" - when %r{^(http|ftp)://ftp\.gnome\.org/pub/gnome/(.*)}i - problem "#{p} should be `https://download.gnome.org/#{Regexp.last_match(2)}`" - when %r{^git://anonscm\.debian\.org/users/(.*)}i - problem "#{p} should be `https://anonscm.debian.org/git/users/#{Regexp.last_match(1)}`" - end - end - # Prefer HTTP/S when possible over FTP protocol due to possible firewalls. urls.each do |p| case p diff --git a/Library/Homebrew/rubocops.rb b/Library/Homebrew/rubocops.rb index f673720a64..7d104ad9eb 100644 --- a/Library/Homebrew/rubocops.rb +++ b/Library/Homebrew/rubocops.rb @@ -9,3 +9,4 @@ require_relative "./rubocops/checksum_cop" require_relative "./rubocops/legacy_patches_cop" require_relative "./rubocops/conflicts_cop" require_relative "./rubocops/options_cop" +require_relative "./rubocops/urls_cop" diff --git a/Library/Homebrew/rubocops/urls_cop.rb b/Library/Homebrew/rubocops/urls_cop.rb new file mode 100644 index 0000000000..830b68ead1 --- /dev/null +++ b/Library/Homebrew/rubocops/urls_cop.rb @@ -0,0 +1,90 @@ +require_relative "./extend/formula_cop" + +module RuboCop + module Cop + module FormulaAudit + # This cop audits urls and mirrors in Formulae + class Urls < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body_node) + urls = find_every_method_call_by_name(body_node, :url) + mirrors = find_every_method_call_by_name(body_node, :mirror) + + # GNU urls; doesn't apply to mirrors + gnu_pattern = %r{^(?:https?|ftp)://ftpmirror.gnu.org/(.*)} + audit_urls(urls, gnu_pattern) do |match, url| + problem "Please use \"https://ftp.gnu.org/gnu/#{match[1]}\" instead of #{url}." + end + + # Fossies upstream requests they aren't used as primary URLs + # https://github.com/Homebrew/homebrew-core/issues/14486#issuecomment-307753234 + fossies_pattern = %r{^https?://fossies\.org/} + audit_urls(urls, fossies_pattern) do + problem "Please don't use fossies.org in the url (using as a mirror is fine)" + end + + audit_urls(mirrors, /.*/) do |_, mirror| + urls.each do |url| + url_string = string_content(parameters(url).first) + next unless url_string.eql?(mirror) + problem "URL should not be duplicated as a mirror: #{url_string}" + end + end + + urls += mirrors + + # Check a variety of SSL/TLS URLs that don't consistently auto-redirect + # or are overly common errors that need to be reduced & fixed over time. + http_to_https_patterns = Regexp.union([%r{^http://ftp\.gnu\.org/}, + %r{^http://ftpmirror\.gnu\.org/}, + %r{^http://download\.savannah\.gnu\.org/}, + %r{^http://download-mirror\.savannah\.gnu\.org/}, + %r{^http://[^/]*\.apache\.org/}, + %r{^http://code\.google\.com/}, + %r{^http://fossies\.org/}, + %r{^http://mirrors\.kernel\.org/}, + %r{^http://(?:[^/]*\.)?bintray\.com/}, + %r{^http://tools\.ietf\.org/}, + %r{^http://launchpad\.net/}, + %r{^http://github\.com/}, + %r{^http://bitbucket\.org/}, + %r{^http://anonscm\.debian\.org/}, + %r{^http://cpan\.metacpan\.org/}, + %r{^http://hackage\.haskell\.org/}, + %r{^http://(?:[^/]*\.)?archive\.org}, + %r{^http://(?:[^/]*\.)?freedesktop\.org}, + %r{^http://(?:[^/]*\.)?mirrorservice\.org/}]) + audit_urls(urls, http_to_https_patterns) do |_, url| + problem "Please use https:// for #{url}" + end + + cpan_pattern = %r{^http://search\.mcpan\.org/CPAN/(.*)}i + audit_urls(urls, cpan_pattern) do |match, url| + problem "#{url} should be `https://cpan.metacpan.org/#{match[1]}`" + end + + gnome_pattern = %r{^(http|ftp)://ftp\.gnome\.org/pub/gnome/(.*)}i + audit_urls(urls, gnome_pattern) do |match, url| + problem "#{url} should be `https://download.gnome.org/#{match[2]}`" + end + + debian_pattern = %r{^git://anonscm\.debian\.org/users/(.*)}i + audit_urls(urls, debian_pattern) do |match, url| + problem "#{url} should be `https://anonscm.debian.org/git/users/#{match[1]}`" + end + end + + private + + def audit_urls(urls, regex) + urls.each do |url_node| + url_string_node = parameters(url_node).first + url_string = string_content(url_string_node) + match_object = regex_match_group(url_string_node, regex) + offending_node(url_string_node.parent) if match_object + yield match_object, url_string if match_object + end + end + end + end + end +end diff --git a/Library/Homebrew/test/rubocops/urls_cop_spec.rb b/Library/Homebrew/test/rubocops/urls_cop_spec.rb new file mode 100644 index 0000000000..2e56dbf03b --- /dev/null +++ b/Library/Homebrew/test/rubocops/urls_cop_spec.rb @@ -0,0 +1,54 @@ +require "rubocop" +require "rubocop/rspec/support" +require_relative "../../extend/string" +require_relative "../../rubocops/urls_cop" + +describe RuboCop::Cop::FormulaAudit::Urls do + subject(:cop) { described_class.new } + + context "When auditing urls" do + it "with offenses" do + formulas = [{ + "url" => "https://ftpmirror.gnu.org/lightning/lightning-2.1.0.tar.gz", + "msg" => 'Please use "https://ftp.gnu.org/gnu/lightning/lightning-2.1.0.tar.gz" instead of https://ftpmirror.gnu.org/lightning/lightning-2.1.0.tar.gz.', + "col" => 2, + }, { + "url" => "https://fossies.org/linux/privat/monit-5.23.0.tar.gz", + "msg" => "Please don't use fossies.org in the url (using as a mirror is fine)", + "col" => 2, + }, { + "url" => "http://tools.ietf.org/tools/rfcmarkup/rfcmarkup-1.119.tgz", + "msg" => "Please use https:// for http://tools.ietf.org/tools/rfcmarkup/rfcmarkup-1.119.tgz", + "col" => 2, + }, { + "url" => "http://search.mcpan.org/CPAN/authors/id/Z/ZE/ZEFRAM/Perl4-CoreLibs-0.003.tar.gz", + "msg" => "http://search.mcpan.org/CPAN/authors/id/Z/ZE/ZEFRAM/Perl4-CoreLibs-0.003.tar.gz should be `https://cpan.metacpan.org/authors/id/Z/ZE/ZEFRAM/Perl4-CoreLibs-0.003.tar.gz`", + "col" => 2, + }, { + "url" => "http://ftp.gnome.org/pub/GNOME/binaries/mac/banshee/banshee-2.macosx.intel.dmg", + "msg" => "http://ftp.gnome.org/pub/GNOME/binaries/mac/banshee/banshee-2.macosx.intel.dmg should be `https://download.gnome.org/binaries/mac/banshee/banshee-2.macosx.intel.dmg`", + "col" => 2, + }] + + formulas.each do |formula| + source = <<-EOS.undent + class Foo < Formula + desc "foo" + url "#{formula["url"]}" + end + EOS + expected_offenses = [{ message: formula["msg"], + severity: :convention, + line: 3, + column: formula["col"], + source: source }] + + inspect_source(cop, source) + + expected_offenses.zip(cop.offenses.reverse).each do |expected, actual| + expect_offense(expected, actual) + end + end + end + end +end