review follow-up #1

This commit is contained in:
Viktor Szakats 2017-08-29 12:31:07 +00:00
parent 1c2c390c6f
commit c30b941358

View File

@ -201,7 +201,7 @@ class FormulaAuditor
@specs = %w[stable devel head].map { |s| formula.send(s) }.compact @specs = %w[stable devel head].map { |s| formula.send(s) }.compact
end end
def self.check_http_content(type, url, name, user_agents: [:default]) def self.check_http_content(url, name, user_agents: [:default], check_content: false)
return unless url.start_with? "http" return unless url.start_with? "http"
details = nil details = nil
@ -237,23 +237,24 @@ class FormulaAuditor
file_match = details[:file_hash] == secure_details[:file_hash] file_match = details[:file_hash] == secure_details[:file_hash]
if etag_match || content_length_match || file_match if etag_match || content_length_match || file_match
return "The URL #{url} could use HTTPS rather than HTTP" return "The URL #{url} should use HTTPS rather than HTTP"
end end
return if type != "homepage" return unless check_content
details[:file] = details[:file].gsub(%r{https?:\\?\/\\?\/}, "/") no_protocol_file_contents = %r{https?:\\?/\\?/}
secure_details[:file] = secure_details[:file].gsub(%r{https?:\\?\/\\?\/}, "/") details[:file] = details[:file].gsub(no_protocol_file_contents, "/")
secure_details[:file] = secure_details[:file].gsub(no_protocol_file_contents, "/")
# Same content after normalization # Check for the same content after removing all protocols
if details[:file] == secure_details[:file] if details[:file] == secure_details[:file]
return "The URL #{url} could use HTTPS rather than HTTP" return "The URL #{url} should use HTTPS rather than HTTP"
end end
# Same size, different content after normalization # Same size, different content after normalization
# (typical causes: Generated ID, Timestamp, Unix time) # (typical causes: Generated ID, Timestamp, Unix time)
if details[:file].length == secure_details[:file].length if details[:file].length == secure_details[:file].length
return "The URL #{url} could use HTTPS rather than HTTP" return "The URL #{url} may be able to use HTTPS rather than HTTP. Please verify it in a browser."
end end
lenratio = (100 * secure_details[:file].length / details[:file].length).to_i lenratio = (100 * secure_details[:file].length / details[:file].length).to_i
@ -586,10 +587,10 @@ class FormulaAuditor
return unless @online return unless @online
return unless DevelopmentTools.curl_handles_most_https_homepages? return unless DevelopmentTools.curl_handles_most_https_homepages?
if http_content_problem = FormulaAuditor.check_http_content("homepage", if http_content_problem = FormulaAuditor.check_http_content(homepage,
homepage,
formula.name, formula.name,
user_agents: [:browser, :default]) user_agents: [:browser, :default],
check_content: true)
problem http_content_problem problem http_content_problem
end end
end end
@ -1242,8 +1243,7 @@ class ResourceAuditor
# A `brew mirror`'ed URL is usually not yet reachable at the time of # A `brew mirror`'ed URL is usually not yet reachable at the time of
# pull request. # pull request.
next if url =~ %r{^https://dl.bintray.com/homebrew/mirror/} next if url =~ %r{^https://dl.bintray.com/homebrew/mirror/}
if http_content_problem = FormulaAuditor.check_http_content("url", if http_content_problem = FormulaAuditor.check_http_content(url, name)
url, name)
problem http_content_problem problem http_content_problem
end end
elsif strategy <= GitDownloadStrategy elsif strategy <= GitDownloadStrategy