Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

audit: Port audit_urls partially to rubocop and add corresponding tests #2911

Merged
merged 1 commit into from
Jul 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 0 additions & 49 deletions Library/Homebrew/dev-cmd/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1282,56 +1282,7 @@ def audit_download_strategy
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
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/rubocops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@
require_relative "./rubocops/legacy_patches_cop"
require_relative "./rubocops/conflicts_cop"
require_relative "./rubocops/options_cop"
require_relative "./rubocops/urls_cop"
90 changes: 90 additions & 0 deletions Library/Homebrew/rubocops/urls_cop.rb
Original file line number Diff line number Diff line change
@@ -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
54 changes: 54 additions & 0 deletions Library/Homebrew/test/rubocops/urls_cop_spec.rb
Original file line number Diff line number Diff line change
@@ -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