Merge pull request #2932 from GauthamGoli/audit_urls_rubocop_part_2
audit: Port audit_urls to rubocop and add corresponding tests Part 2
This commit is contained in:
Mike McQuaid
GitHub
commit
733abc7e7d
@ -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"
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user