admin/brew
1
0
Fork 0
Code Issues Packages Projects Releases 6 Wiki Activity

Merge pull request #11972 from samford/fix-curl_args_options

Browse Source 
Curl: Fix #curl_args options
This commit is contained in:
Sam Ford 2021-09-07 15:27:25 -04:00 committed by GitHub
commit c4a91cdc95
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 116 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Library/Homebrew/api.rb
View File

@ -27,7 +27,7 @@ module Homebrew
return cache[endpoint] if cache.present? && cache.key?(endpoint)
api_url = "#{API_DOMAIN}/#{endpoint}"
output = Utils::Curl.curl_output("--fail", "--max-time", "5", api_url)
output = Utils::Curl.curl_output("--fail", api_url, max_time: 5)
raise ArgumentError, "No file found at #{Tty.underline}#{api_url}#{Tty.reset}" unless output.success?
cache[endpoint] = if json

2
Library/Homebrew/download_strategy.rb
View File

@ -558,7 +558,7 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy
end
def curl(*args, **options)
args << "--connect-timeout" << "15" unless mirrors.empty?
options[:connect_timeout] = 15 unless mirrors.empty?
super(*_curl_args, *args, **_curl_opts, **command_output_options, **options)
end
end

16
Library/Homebrew/livecheck/strategy.rb
View File

@ -39,8 +39,6 @@ module Homebrew
DEFAULT_CURL_ARGS = [
# Follow redirections to handle mirrors, relocations, etc.
"--location",
"--connect-timeout", CURL_CONNECT_TIMEOUT,
"--max-time", CURL_MAX_TIME
].freeze
# `curl` arguments used in `Strategy#page_headers` method.
@ -62,12 +60,14 @@ module Homebrew
# Baseline `curl` options used in {Strategy} methods.
DEFAULT_CURL_OPTIONS = {
print_stdout: false,
print_stderr: false,
debug: false,
verbose: false,
timeout: CURL_PROCESS_TIMEOUT,
retry: false,
print_stdout: false,
print_stderr: false,
debug: false,
verbose: false,
timeout: CURL_PROCESS_TIMEOUT,
connect_timeout: CURL_CONNECT_TIMEOUT,
max_time: CURL_MAX_TIME,
retries: 0,
}.freeze
# HTTP response head(s) and body are typically separated by a double

53
Library/Homebrew/test/utils/curl_spec.rb
View File

@ -5,10 +5,10 @@ require "utils/curl"
describe "Utils::Curl" do
describe "curl_args" do
let(:args) { "foo" }
let(:args) { ["foo"] }
let(:user_agent_string) { "Lorem ipsum dolor sit amet" }
it "returns --disable as the first argument when HOMEBREW_CURLRC is not set" do
it "returns `--disable` as the first argument when HOMEBREW_CURLRC is not set" do
# --disable must be the first argument according to "man curl"
expect(curl_args(*args).first).to eq("--disable")
end
@ -18,6 +18,26 @@ describe "Utils::Curl" do
expect(curl_args(*args).first).not_to eq("--disable")
end
it "uses `--connect-timeout` when `:connect_timeout` is Numeric" do
expect(curl_args(*args, connect_timeout: 123).join(" ")).to include("--connect-timeout 123")
expect(curl_args(*args, connect_timeout: 123.4).join(" ")).to include("--connect-timeout 123.4")
expect(curl_args(*args, connect_timeout: 123.4567).join(" ")).to include("--connect-timeout 123.457")
end
it "errors when `:connect_timeout` is not Numeric" do
expect { curl_args(*args, connect_timeout: "test") }.to raise_error(TypeError)
end
it "uses `--max-time` when `:max_time` is Numeric" do
expect(curl_args(*args, max_time: 123).join(" ")).to include("--max-time 123")
expect(curl_args(*args, max_time: 123.4).join(" ")).to include("--max-time 123.4")
expect(curl_args(*args, max_time: 123.4567).join(" ")).to include("--max-time 123.457")
end
it "errors when `:max_time` is not Numeric" do
expect { curl_args(*args, max_time: "test") }.to raise_error(TypeError)
end
it "uses `--retry 3` when HOMEBREW_CURL_RETRIES is unset" do
expect(curl_args(*args).join(" ")).to include("--retry 3")
end
@ -27,12 +47,27 @@ describe "Utils::Curl" do
expect(curl_args(*args).join(" ")).to include("--retry 10")
end
it "doesn't use `--retry` when `:retry` == `false`" do
expect(curl_args(*args, retry: false).join(" ")).not_to include("--retry")
it "uses `--retry` when `:retries` is a positive Integer" do
expect(curl_args(*args, retries: 5).join(" ")).to include("--retry 5")
end
it "uses `--retry 3` when `:retry` == `true`" do
expect(curl_args(*args, retry: true).join(" ")).to include("--retry 3")
it "doesn't use `--retry` when `:retries` is nil or a non-positive Integer" do
expect(curl_args(*args, retries: nil).join(" ")).not_to include("--retry")
expect(curl_args(*args, retries: 0).join(" ")).not_to include("--retry")
expect(curl_args(*args, retries: -1).join(" ")).not_to include("--retry")
end
it "errors when `:retries` is not Numeric" do
expect { curl_args(*args, retries: "test") }.to raise_error(TypeError)
end
it "uses `--retry-max-time` when `:retry_max_time` is Numeric" do
expect(curl_args(*args, retry_max_time: 123).join(" ")).to include("--retry-max-time 123")
expect(curl_args(*args, retry_max_time: 123.4).join(" ")).to include("--retry-max-time 123")
end
it "errors when `:retry_max_time` is not Numeric" do
expect { curl_args(*args, retry_max_time: "test") }.to raise_error(TypeError)
end
it "uses HOMEBREW_USER_AGENT_FAKE_SAFARI when `:user_agent` is `:browser` or `:fake`" do
@ -53,6 +88,12 @@ describe "Utils::Curl" do
.to include("--user-agent #{user_agent_string}")
end
it "errors when `:user_agent` is not a String or supported Symbol" do
expect { curl_args(*args, user_agent: :an_unsupported_symbol) }
.to raise_error(TypeError, ":user_agent must be :browser/:fake, :default, or a String")
expect { curl_args(*args, user_agent: 123) }.to raise_error(TypeError)
end
it "uses `--fail` unless `:show_output` is `true`" do
expect(curl_args(*args, show_output: false).join(" ")).to include("--fail")
expect(curl_args(*args, show_output: nil).join(" ")).to include("--fail")

77
Library/Homebrew/utils/curl.rb
View File

@ -10,6 +10,8 @@ module Utils
#
# @api private
module Curl
extend T::Sig
using TimeRemaining
module_function
@ -27,7 +29,26 @@ module Utils
@curl
end
def curl_args(*extra_args, **options)
sig {
params(
extra_args: T.untyped,
connect_timeout: T.any(Integer, Float, NilClass),
max_time: T.any(Integer, Float, NilClass),
retries: T.nilable(Integer),
retry_max_time: T.any(Integer, Float, NilClass),
show_output: T.nilable(T::Boolean),
user_agent: T.any(String, Symbol, NilClass),
).returns(T::Array[T.untyped])
}
def curl_args(
*extra_args,
connect_timeout: nil,
max_time: nil,
retries: Homebrew::EnvConfig.curl_retries.to_i,
retry_max_time: nil,
show_output: false,
user_agent: nil
)
args = []
# do not load .curlrc unless requested (must be the first argument)
@ -40,28 +61,33 @@ module Utils
args << "--show-error"
args << "--user-agent" << case options[:user_agent]
args << "--user-agent" << case user_agent
when :browser, :fake
HOMEBREW_USER_AGENT_FAKE_SAFARI
when :default, nil
HOMEBREW_USER_AGENT_CURL
when String
options[:user_agent]
user_agent
else
raise TypeError, ":user_agent must be :browser/:fake, :default, or a String"
end
args << "--header" << "Accept-Language: en"
unless options[:show_output] == true
unless show_output == true
args << "--fail"
args << "--progress-bar" unless Context.current.verbose?
args << "--verbose" if Homebrew::EnvConfig.curl_verbose?
args << "--silent" unless $stdout.tty?
end
args << "--connect-timeout" << connect_timeout.round(3) if options[:connect_timeout]
args << "--max-time" << max_time.round(3) if options[:max_time]
args << "--retry" << Homebrew::EnvConfig.curl_retries unless options[:retry] == false
args << "--retry-max-time" << retry_max_time.round if options[:retry_max_time]
args << "--connect-timeout" << connect_timeout.round(3) if connect_timeout.present?
args << "--max-time" << max_time.round(3) if max_time.present?
# A non-positive integer (e.g., 0) or `nil` will omit this argument
args << "--retry" << retries if retries&.positive?
args << "--retry-max-time" << retry_max_time.round if retry_max_time.present?
args + extra_args
end
@ -188,8 +214,13 @@ module Utils
if url != secure_url
user_agents.each do |user_agent|
secure_details = begin
curl_http_content_headers_and_checksum(secure_url, specs: specs, hash_needed: true,
user_agent: user_agent, use_homebrew_curl: use_homebrew_curl)
curl_http_content_headers_and_checksum(
secure_url,
specs: specs,
hash_needed: true,
use_homebrew_curl: use_homebrew_curl,
user_agent: user_agent,
)
rescue Timeout::Error
next
end
@ -205,8 +236,13 @@ module Utils
details = nil
user_agents.each do |user_agent|
details =
curl_http_content_headers_and_checksum(url, specs: specs, hash_needed: hash_needed,
user_agent: user_agent, use_homebrew_curl: use_homebrew_curl)
curl_http_content_headers_and_checksum(
url,
specs: specs,
hash_needed: hash_needed,
use_homebrew_curl: use_homebrew_curl,
user_agent: user_agent,
)
break if http_status_ok?(details[:status])
end
@ -271,16 +307,21 @@ module Utils
"The #{url_type} #{url} may be able to use HTTPS rather than HTTP. Please verify it in a browser."
end
def curl_http_content_headers_and_checksum(url, specs: {}, hash_needed: false,
user_agent: :default, use_homebrew_curl: false)
def curl_http_content_headers_and_checksum(
url, specs: {}, hash_needed: false,
use_homebrew_curl: false, user_agent: :default
)
file = Tempfile.new.tap(&:close)
specs = specs.flat_map { |option, argument| ["--#{option.to_s.tr("_", "-")}", argument] }
max_time = hash_needed ? "600" : "25"
max_time = hash_needed ? 600 : 25
output, _, status = curl_output(
*specs, "--dump-header", "-", "--output", file.path, "--location",
"--connect-timeout", "15", "--max-time", max_time, "--retry-max-time", max_time, url,
user_agent: user_agent, use_homebrew_curl: use_homebrew_curl
*specs, "--dump-header", "-", "--output", file.path, "--location", url,
use_homebrew_curl: use_homebrew_curl,
connect_timeout: 15,
max_time: max_time,
retry_max_time: max_time,
user_agent: user_agent
)
status_code = :unknown