diff --git a/Library/Homebrew/livecheck/strategy.rb b/Library/Homebrew/livecheck/strategy.rb index 67f978339b..a20fb70ecd 100644 --- a/Library/Homebrew/livecheck/strategy.rb +++ b/Library/Homebrew/livecheck/strategy.rb @@ -67,7 +67,7 @@ module Homebrew debug: false, verbose: false, timeout: CURL_PROCESS_TIMEOUT, - retry: false, + retries: 0, }.freeze # HTTP response head(s) and body are typically separated by a double diff --git a/Library/Homebrew/test/utils/curl_spec.rb b/Library/Homebrew/test/utils/curl_spec.rb index 2fe1397b10..454d5d1f95 100644 --- a/Library/Homebrew/test/utils/curl_spec.rb +++ b/Library/Homebrew/test/utils/curl_spec.rb @@ -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") diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 9c35957e3d..b3ddc6e57c 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -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