diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 327c3e8116..b15d719d28 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -1283,120 +1283,6 @@ class ResourceAuditor def audit_urls urls = [url] + mirrors - # Prefer HTTP/S when possible over FTP protocol due to possible firewalls. - urls.each do |p| - case p - when %r{^ftp://ftp\.mirrorservice\.org} - problem "Please use https:// for #{p}" - when %r{^ftp://ftp\.cpan\.org/pub/CPAN(.*)}i - problem "#{p} should be `http://search.cpan.org/CPAN#{Regexp.last_match(1)}`" - end - end - - # Check SourceForge urls - urls.each do |p| - # Skip if the URL looks like a SVN repo - next if p.include? "/svnroot/" - next if p.include? "svn.sourceforge" - - # Is it a sourceforge http(s) URL? - next unless p =~ %r{^https?://.*\b(sourceforge|sf)\.(com|net)} - - if p =~ /(\?|&)use_mirror=/ - problem "Don't use #{Regexp.last_match(1)}use_mirror in SourceForge urls (url is #{p})." - end - - if p.end_with?("/download") - problem "Don't use /download in SourceForge urls (url is #{p})." - end - - if p =~ %r{^https?://sourceforge\.} - problem "Use https://downloads.sourceforge.net to get geolocation (url is #{p})." - end - - if p =~ %r{^https?://prdownloads\.} - problem "Don't use prdownloads in SourceForge urls (url is #{p}).\n" \ - "\tSee: http://librelist.com/browser/homebrew/2011/1/12/prdownloads-is-bad/" - end - - if p =~ %r{^http://\w+\.dl\.} - problem "Don't use specific dl mirrors in SourceForge urls (url is #{p})." - end - - problem "Please use https:// for #{p}" if p.start_with? "http://downloads" - end - - # Debian has an abundance of secure mirrors. Let's not pluck the insecure - # one out of the grab bag. - urls.each do |u| - next unless u =~ %r{^http://http\.debian\.net/debian/(.*)}i - problem <<-EOS.undent - Please use a secure mirror for Debian URLs. - We recommend: - https://mirrors.ocf.berkeley.edu/debian/#{Regexp.last_match(1)} - EOS - end - - # Check for Google Code download urls, https:// is preferred - # Intentionally not extending this to SVN repositories due to certificate - # issues. - urls.grep(%r{^http://.*\.googlecode\.com/files.*}) do |u| - problem "Please use https:// for #{u}" - end - - # Check for new-url Google Code download urls, https:// is preferred - urls.grep(%r{^http://code\.google\.com/}) do |u| - problem "Please use https:// for #{u}" - end - - # Check for git:// GitHub repo urls, https:// is preferred. - urls.grep(%r{^git://[^/]*github\.com/}) do |u| - problem "Please use https:// for #{u}" - end - - # Check for git:// Gitorious repo urls, https:// is preferred. - urls.grep(%r{^git://[^/]*gitorious\.org/}) do |u| - problem "Please use https:// for #{u}" - end - - # Check for http:// GitHub repo urls, https:// is preferred. - urls.grep(%r{^http://github\.com/.*\.git$}) do |u| - problem "Please use https:// for #{u}" - end - - # Check for master branch GitHub archives. - urls.grep(%r{^https://github\.com/.*archive/master\.(tar\.gz|zip)$}) do - problem "Use versioned rather than branch tarballs for stable checksums." - end - - # Use new-style archive downloads - urls.each do |u| - next unless u =~ %r{https://.*github.*/(?:tar|zip)ball/} && u !~ /\.git$/ - problem "Use /archive/ URLs for GitHub tarballs (url is #{u})." - end - - # Don't use GitHub .zip files - urls.each do |u| - next unless u =~ %r{https://.*github.*/(archive|releases)/.*\.zip$} && u !~ %r{releases/download} - problem "Use GitHub tarballs rather than zipballs (url is #{u})." - end - - # Don't use GitHub codeload URLs - urls.each do |u| - next unless u =~ %r{https?://codeload\.github\.com/(.+)/(.+)/(?:tar\.gz|zip)/(.+)} - problem <<-EOS.undent - use GitHub archive URLs: - https://github.com/#{Regexp.last_match(1)}/#{Regexp.last_match(2)}/archive/#{Regexp.last_match(3)}.tar.gz - Rather than codeload: - #{u} - EOS - end - - # Check for Maven Central urls, prefer HTTPS redirector over specific host - urls.each do |u| - next unless u =~ %r{https?://(?:central|repo\d+)\.maven\.org/maven2/(.+)$} - problem "#{u} should be `https://search.maven.org/remotecontent?filepath=#{Regexp.last_match(1)}`" - end if name == "curl" && !urls.find { |u| u.start_with?("http://") } problem "should always include at least one HTTP url" diff --git a/Library/Homebrew/rubocops/urls_cop.rb b/Library/Homebrew/rubocops/urls_cop.rb index 830b68ead1..94f049aed3 100644 --- a/Library/Homebrew/rubocops/urls_cop.rb +++ b/Library/Homebrew/rubocops/urls_cop.rb @@ -71,6 +71,122 @@ module RuboCop audit_urls(urls, debian_pattern) do |match, url| problem "#{url} should be `https://anonscm.debian.org/git/users/#{match[1]}`" end + + # Prefer HTTP/S when possible over FTP protocol due to possible firewalls. + mirror_service_pattern = %r{^ftp://ftp\.mirrorservice\.org} + audit_urls(urls, mirror_service_pattern) do |_, url| + problem "Please use https:// for #{url}" + end + + cpan_ftp_pattern = %r{^ftp://ftp\.cpan\.org/pub/CPAN(.*)}i + audit_urls(urls, cpan_ftp_pattern) do |match_obj, url| + problem "#{url} should be `http://search.cpan.org/CPAN#{match_obj[1]}`" + end + + # SourceForge url patterns + sourceforge_patterns = %r{^https?://.*\b(sourceforge|sf)\.(com|net)} + audit_urls(urls, sourceforge_patterns) do |_, url| + # Skip if the URL looks like a SVN repo + next if url.include? "/svnroot/" + next if url.include? "svn.sourceforge" + next if url.include? "/p/" + + if url =~ /(\?|&)use_mirror=/ + problem "Don't use #{Regexp.last_match(1)}use_mirror in SourceForge urls (url is #{url})." + end + + if url.end_with?("/download") + problem "Don't use /download in SourceForge urls (url is #{url})." + end + + if url =~ %r{^https?://sourceforge\.} + problem "Use https://downloads.sourceforge.net to get geolocation (url is #{url})." + end + + if url =~ %r{^https?://prdownloads\.} + problem "Don't use prdownloads in SourceForge urls (url is #{url}).\n" \ + "\tSee: http://librelist.com/browser/homebrew/2011/1/12/prdownloads-is-bad/" + end + + if url =~ %r{^http://\w+\.dl\.} + problem "Don't use specific dl mirrors in SourceForge urls (url is #{url})." + end + + problem "Please use https:// for #{url}" if url.start_with? "http://downloads" + end + + # Debian has an abundance of secure mirrors. Let's not pluck the insecure + # one out of the grab bag. + unsecure_deb_pattern = %r{^http://http\.debian\.net/debian/(.*)}i + audit_urls(urls, unsecure_deb_pattern) do |match, _| + problem <<-EOS.undent + Please use a secure mirror for Debian URLs. + We recommend: + https://mirrors.ocf.berkeley.edu/debian/#{match[1]} + EOS + end + + # Check for new-url Google Code download urls, https:// is preferred + google_code_pattern = Regexp.union([%r{^http://.*\.googlecode\.com/files.*}, + %r{^http://code\.google\.com/}]) + audit_urls(urls, google_code_pattern) do |_, url| + problem "Please use https:// for #{url}" + end + + # Check for git:// GitHub repo urls, https:// is preferred. + git_gh_pattern = %r{^git://[^/]*github\.com/} + audit_urls(urls, git_gh_pattern) do |_, url| + problem "Please use https:// for #{url}" + end + + # Check for git:// Gitorious repo urls, https:// is preferred. + git_gitorious_pattern = %r{^git://[^/]*gitorious\.org/} + audit_urls(urls, git_gitorious_pattern) do |_, url| + problem "Please use https:// for #{url}" + end + + # Check for http:// GitHub repo urls, https:// is preferred. + gh_pattern = %r{^http://github\.com/.*\.git$} + audit_urls(urls, gh_pattern) do |_, url| + problem "Please use https:// for #{url}" + end + + # Check for master branch GitHub archives. + tarball_gh_pattern = %r{^https://github\.com/.*archive/master\.(tar\.gz|zip)$} + audit_urls(urls, tarball_gh_pattern) do + problem "Use versioned rather than branch tarballs for stable checksums." + end + + # Use new-style archive downloads + archive_gh_pattern = %r{https://.*github.*/(?:tar|zip)ball/} + audit_urls(urls, archive_gh_pattern) do |_, url| + next unless url !~ /\.git$/ + problem "Use /archive/ URLs for GitHub tarballs (url is #{url})." + end + + # Don't use GitHub .zip files + zip_gh_pattern = %r{https://.*github.*/(archive|releases)/.*\.zip$} + audit_urls(urls, zip_gh_pattern) do |_, url| + next unless url !~ %r{releases/download} + problem "Use GitHub tarballs rather than zipballs (url is #{url})." + end + + # Don't use GitHub codeload URLs + codeload_gh_pattern = %r{https?://codeload\.github\.com/(.+)/(.+)/(?:tar\.gz|zip)/(.+)} + audit_urls(urls, codeload_gh_pattern) do |match, url| + problem <<-EOS.undent + Use GitHub archive URLs: + https://github.com/#{match[1]}/#{match[2]}/archive/#{match[3]}.tar.gz + Rather than codeload: + #{url} + EOS + end + + # Check for Maven Central urls, prefer HTTPS redirector over specific host + maven_pattern = %r{https?://(?:central|repo\d+)\.maven\.org/maven2/(.+)$} + audit_urls(urls, maven_pattern) do |match, url| + problem "#{url} should be `https://search.maven.org/remotecontent?filepath=#{match[1]}`" + end end private @@ -80,8 +196,9 @@ module RuboCop 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 + next unless match_object + offending_node(url_string_node.parent) + yield match_object, url_string end end end diff --git a/Library/Homebrew/test/rubocops/urls_cop_spec.rb b/Library/Homebrew/test/rubocops/urls_cop_spec.rb index 2e56dbf03b..733732bd01 100644 --- a/Library/Homebrew/test/rubocops/urls_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/urls_cop_spec.rb @@ -28,8 +28,82 @@ describe RuboCop::Cop::FormulaAudit::Urls do "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, + }, { + "url" => "git://anonscm.debian.org/users/foo/foostrap.git", + "msg" => "git://anonscm.debian.org/users/foo/foostrap.git should be `https://anonscm.debian.org/git/users/foo/foostrap.git`", + "col" => 2, + }, { + "url" => "ftp://ftp.mirrorservice.org/foo-1.tar.gz", + "msg" => "Please use https:// for ftp://ftp.mirrorservice.org/foo-1.tar.gz", + "col" => 2, + }, { + "url" => "ftp://ftp.cpan.org/pub/CPAN/foo-1.tar.gz", + "msg" => "ftp://ftp.cpan.org/pub/CPAN/foo-1.tar.gz should be `http://search.cpan.org/CPAN/foo-1.tar.gz`", + "col" => 2, + }, { + "url" => "http://sourceforge.net/projects/something/files/Something-1.2.3.dmg", + "msg" => "Use https://downloads.sourceforge.net to get geolocation (url is http://sourceforge.net/projects/something/files/Something-1.2.3.dmg).", + "col" => 2, + }, { + "url" => "https://downloads.sourceforge.net/project/foo/download", + "msg" => "Don't use /download in SourceForge urls (url is https://downloads.sourceforge.net/project/foo/download).", + "col" => 2, + }, { + "url" => "https://sourceforge.net/project/foo", + "msg" => "Use https://downloads.sourceforge.net to get geolocation (url is https://sourceforge.net/project/foo).", + "col" => 2, + }, { + "url" => "http://prdownloads.sourceforge.net/foo/foo-1.tar.gz", + "msg" => "Don't use prdownloads in SourceForge urls (url is http://prdownloads.sourceforge.net/foo/foo-1.tar.gz).\n" \ + "\tSee: http://librelist.com/browser/homebrew/2011/1/12/prdownloads-is-bad/", + "col" => 2, + }, { + "url" => "http://foo.dl.sourceforge.net/sourceforge/foozip/foozip_1.0.tar.bz2", + "msg" => "Don't use specific dl mirrors in SourceForge urls (url is http://foo.dl.sourceforge.net/sourceforge/foozip/foozip_1.0.tar.bz2).", + "col" => 2, + }, { + "url" => "http://downloads.sourceforge.net/project/foo/foo/2/foo-2.zip", + "msg" => "Please use https:// for http://downloads.sourceforge.net/project/foo/foo/2/foo-2.zip", + "col" => 2, + }, { + "url" => "http://http.debian.net/debian/dists/foo/", + "msg" => "Please use a secure mirror for Debian URLs.\nWe recommend:\n"\ + " https://mirrors.ocf.berkeley.edu/debian/dists/foo/\n", + "col" => 2, + }, { + "url" => "http://foo.googlecode.com/files/foo-1.0.zip", + "msg" => "Please use https:// for http://foo.googlecode.com/files/foo-1.0.zip", + "col" => 2, + }, { + "url" => "git://github.com/foo.git", + "msg" => "Please use https:// for git://github.com/foo.git", + "col" => 2, + }, { + "url" => "git://gitorious.org/foo/foo5", + "msg" => "Please use https:// for git://gitorious.org/foo/foo5", + "col" => 2, + }, { + "url" => "http://github.com/foo/foo5.git", + "msg" => "Please use https:// for http://github.com/foo/foo5.git", + "col" => 2, + }, { + "url" => "https://github.com/foo/foobar/archive/master.zip", + "msg" => "Use versioned rather than branch tarballs for stable checksums.", + "col" => 2, + }, { + "url" => "https://github.com/foo/bar/tarball/v1.2.3", + "msg" => "Use /archive/ URLs for GitHub tarballs (url is https://github.com/foo/bar/tarball/v1.2.3).", + "col" => 2, + }, { + "url" => "https://codeload.github.com/foo/bar/tar.gz/v0.1.1", + "msg" => "Use GitHub archive URLs:\n https://github.com/foo/bar/archive/v0.1.1.tar.gz\n"\ + "Rather than codeload:\n https://codeload.github.com/foo/bar/tar.gz/v0.1.1\n", + "col" => 2, + }, { + "url" => "https://central.maven.org/maven2/com/bar/foo/1.1/foo-1.1.jar", + "msg" => "https://central.maven.org/maven2/com/bar/foo/1.1/foo-1.1.jar should be `https://search.maven.org/remotecontent?filepath=com/bar/foo/1.1/foo-1.1.jar`", + "col" => 2, }] - formulas.each do |formula| source = <<-EOS.undent class Foo < Formula @@ -50,5 +124,61 @@ describe RuboCop::Cop::FormulaAudit::Urls do end end end + + it "with offenses in stable/devel/head block" do + formulas = [{ + "url" => "git://github.com/foo.git", + "msg" => "Please use https:// for git://github.com/foo.git", + "col" => 4, + }] + formulas.each do |formula| + source = <<-EOS.undent + class Foo < Formula + desc "foo" + url "https://foo.com" + + devel do + url "#{formula["url"]}", + :tag => "v1.0.0-alpha.1", + :revision => "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + version "1.0.0-alpha.1" + end + end + EOS + expected_offenses = [{ message: formula["msg"], + severity: :convention, + line: 6, + 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 + + it "with duplicate mirror" do + source = <<-EOS.undent + class Foo < Formula + desc "foo" + url "https://ftpmirror.fnu.org/foo/foo-1.0.tar.gz" + mirror "https://ftpmirror.fnu.org/foo/foo-1.0.tar.gz" + end + EOS + + expected_offenses = [{ message: "URL should not be duplicated as a mirror: https://ftpmirror.fnu.org/foo/foo-1.0.tar.gz", + severity: :convention, + line: 4, + column: 2, + source: source }] + + inspect_source(cop, source) + + expected_offenses.zip(cop.offenses.reverse).each do |expected, actual| + expect_offense(expected, actual) + end + end end end