From f88966a8a53198106f01475b7c09ef08ef00e0e9 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Mon, 6 Sep 2021 22:56:25 -0400 Subject: [PATCH] Use curl options where appropriate --- Library/Homebrew/api.rb | 2 +- Library/Homebrew/download_strategy.rb | 2 +- Library/Homebrew/livecheck/strategy.rb | 16 ++++++------ Library/Homebrew/utils/curl.rb | 35 ++++++++++++++++++-------- 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/Library/Homebrew/api.rb b/Library/Homebrew/api.rb index 53b0bf1dd7..4b208fd0ec 100644 --- a/Library/Homebrew/api.rb +++ b/Library/Homebrew/api.rb @@ -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 diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 9718b13ba7..e09bbc73f2 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -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 diff --git a/Library/Homebrew/livecheck/strategy.rb b/Library/Homebrew/livecheck/strategy.rb index a20fb70ecd..148824e245 100644 --- a/Library/Homebrew/livecheck/strategy.rb +++ b/Library/Homebrew/livecheck/strategy.rb @@ -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, - retries: 0, + 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 diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index b3ddc6e57c..6d3049d5d3 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -214,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 @@ -231,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 @@ -297,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