diff --git a/Library/Homebrew/api.rb b/Library/Homebrew/api.rb index 9ab55cbca1..ff26cbcb5a 100644 --- a/Library/Homebrew/api.rb +++ b/Library/Homebrew/api.rb @@ -56,6 +56,7 @@ module Homebrew retry_count = 0 url = "#{Homebrew::EnvConfig.api_domain}/#{endpoint}" default_url = "#{HOMEBREW_API_DEFAULT_DOMAIN}/#{endpoint}" + primary_url = url if Homebrew.running_as_root_but_not_owned_by_root? && (!target.exist? || target.empty?) @@ -68,14 +69,13 @@ module Homebrew "--speed-time", ENV.fetch("HOMEBREW_CURL_SPEED_TIME"), # This is a Curl format token, not a Ruby one. # rubocop:disable Style/FormatStringToken - "--write-out", "%{stderr}HTTP status: %{http_code}" + "--write-out", "%{http_code}" # rubocop:enable Style/FormatStringToken ] insecure_download = DevelopmentTools.ca_file_substitution_required? || DevelopmentTools.curl_substitution_required? skip_download = skip_download?(target:, stale_seconds:) - if enqueue unless skip_download require "download_queue" @@ -87,22 +87,61 @@ module Homebrew return [{}, false] end + etag_path = Pathname("#{target}.etag") + etag_url_path = Pathname("#{etag_path}.url") + new_etag_path = Pathname("#{target}.etag.new") + new_target = Pathname("#{target}.new") + json_data, from_payload_cache = begin + unless skip_download + require "lock_file" + # cmd/update.sh takes the same lock before replacing the body and validators. + download_lock ||= DownloadLock.new(target) + download_lock.lock_or_wait(quiet: Context.current.quiet?) + skip_download = skip_download?(target:, stale_seconds:) + end download_succeeded = T.let(false, T::Boolean) + unconditional_retry = T.let(false, T::Boolean) begin args = curl_args.dup - args.prepend("--time-cond", target.to_s) if target.exist? && !target.empty? + # Keep conditional requests in sync with utils/api.sh and cmd/update.sh. + use_etag = HOMEBREW_CURL_SUPPORTS_ETAGS && url == primary_url + if !unconditional_retry && target.exist? && !target.empty? + if use_etag && etag_path.exist? && !etag_path.empty? && + etag_url_path.exist? && etag_url_path.read == url + args.prepend("--etag-compare", etag_path.to_s) + elsif !HOMEBREW_CURL_SUPPORTS_ETAGS && url == primary_url + args.prepend("--time-cond", target.to_s) + end + end + args.prepend("--etag-save", new_etag_path.to_s) if use_etag if insecure_download opoo DevelopmentTools.insecure_download_warning(endpoint) args.append("--insecure") end unless skip_download + FileUtils.rm_f([new_target, new_etag_path]) ohai "Downloading #{url}" if $stdout.tty? && !Context.current.quiet? # Disable retries here, we handle them ourselves below. - Utils::Curl.curl_download(*args, url, to: target, retries: 0, show_error: false) + result = Utils::Curl.curl_download(*args, url, to: new_target, retries: 0, show_error: false, + print_stdout: false) download_succeeded = true + if new_target.exist? + FileUtils.rm_f([etag_path, etag_url_path]) + FileUtils.mv(new_target, target) + end + if use_etag && new_etag_path.exist? && !new_etag_path.empty? + FileUtils.mv(new_etag_path, etag_path) + File.write(etag_url_path, url) + elsif etag_path.exist? && result&.stdout != "304" + FileUtils.rm_f([etag_path, etag_url_path]) + end + end + rescue ErrorDuringExecution => e + if e.exitstatus == 56 && args && (args.include?("--etag-compare") || args.include?("--time-cond")) + unconditional_retry = true + retry end - rescue ErrorDuringExecution if url == default_url raise unless target.exist? raise if target.empty? @@ -117,6 +156,8 @@ module Homebrew end opoo "#{target.basename}: update failed, falling back to cached version." + ensure + FileUtils.rm_f([new_target, new_etag_path]) unless skip_download end # Only refresh the cache mtime after a successful curl revalidation/download. @@ -166,6 +207,8 @@ module Homebrew else [json_data, !skip_download] end + ensure + download_lock&.unlock end sig { diff --git a/Library/Homebrew/brew.sh b/Library/Homebrew/brew.sh index b1e854d881..89fd26c75c 100644 --- a/Library/Homebrew/brew.sh +++ b/Library/Homebrew/brew.sh @@ -393,6 +393,7 @@ export HOMEBREW_CURL export HOMEBREW_BREWED_CURL_PATH export HOMEBREW_CURL_WARNING export HOMEBREW_SYSTEM_CURL_TOO_OLD +export HOMEBREW_CURL_SUPPORTS_ETAGS export HOMEBREW_GIT export HOMEBREW_GIT_WARNING export HOMEBREW_MINIMUM_GIT_VERSION diff --git a/Library/Homebrew/cleanup.rb b/Library/Homebrew/cleanup.rb index 06af8f62eb..e1fead1cff 100644 --- a/Library/Homebrew/cleanup.rb +++ b/Library/Homebrew/cleanup.rb @@ -662,14 +662,15 @@ module Homebrew api_internal = cache/"api/internal" api_package_files = if scrub? && api_internal.directory? current_api_package_basename = Homebrew::API::Internal.cached_packages_json_file_path.basename.to_s - # Keep only the current OS's envelope and its `.payload` and - # `.payload.index` sidecars and scrub the rest, including orphaned - # sidecars and temp files. + # Keep only the current OS's envelope and its sidecars and scrub the + # rest, including orphaned sidecars and temp files. # Keep in sync with the previous-OS-version removal in cmd/update.sh. kept_basenames = [ current_api_package_basename, "#{current_api_package_basename}.payload", "#{current_api_package_basename}.payload.index", + "#{current_api_package_basename}.etag", + "#{current_api_package_basename}.etag.url", ] api_internal.glob("packages.*.jws.json*").reject do |path| kept_basenames.include?(path.basename.to_s) diff --git a/Library/Homebrew/cmd/update.sh b/Library/Homebrew/cmd/update.sh index 51d027603b..534142fa9c 100644 --- a/Library/Homebrew/cmd/update.sh +++ b/Library/Homebrew/cmd/update.sh @@ -9,7 +9,7 @@ # HOMEBREW_CASK_REPOSITORY, HOMEBREW_CELLAR, HOMEBREW_CORE_DEFAULT_GIT_REMOTE, HOMEBREW_CORE_GIT_REMOTE, # HOMEBREW_CORE_REPOSITORY, HOMEBREW_CURL, HOMEBREW_DEV_CMD_RUN, HOMEBREW_FORCE_BREWED_CA_CERTIFICATES, # HOMEBREW_FORCE_BREWED_CURL, HOMEBREW_FORCE_BREWED_GIT, -# HOMEBREW_SYSTEM_CURL_TOO_OLD, HOMEBREW_USER_AGENT_CURL are set by brew.sh +# HOMEBREW_CURL_SUPPORTS_ETAGS, HOMEBREW_SYSTEM_CURL_TOO_OLD, HOMEBREW_USER_AGENT_CURL are set by brew.sh # shellcheck disable=SC2154 source "${HOMEBREW_LIBRARY}/Homebrew/utils/cmd.sh" source "${HOMEBREW_LIBRARY}/Homebrew/utils/api.sh" @@ -409,6 +409,7 @@ api_curl_download() { --fail --compressed --silent \ --speed-limit "${HOMEBREW_CURL_SPEED_LIMIT}" --speed-time "${HOMEBREW_CURL_SPEED_TIME}" \ --location --remote-time --output "${cache_path}" \ + --write-out "%{http_code}" \ "$@" \ --user-agent "${HOMEBREW_USER_AGENT_CURL}" \ "${json_url}" @@ -423,6 +424,15 @@ fetch_api_file() { local cache_path="${api_cache}/${filename}" mkdir -p "$(dirname "${cache_path}")" + # Match DownloadLock in Homebrew::API.fetch_json_api_file (api.rb). + local lock_path="${HOMEBREW_PREFIX}/var/homebrew/locks/${cache_path##*/}.download.lock" + if ! { mkdir -p "${lock_path%/*}" && exec 201>"${lock_path}"; } || ! _create_lock 201 "API download ${filename}" + then + echo "Cannot lock ${filename} for download!" >>"${update_failed_file}" + exec 201>&- + return 1 + fi + if [[ "${filename}" == "formula.jws.json" ]] || [[ "${filename}" == "internal/packages.$(bottle_tag).jws.json" ]] then local is_formula_file=1 @@ -443,28 +453,68 @@ fetch_api_file() { echo "Checking if we need to fetch ${filename}..." fi - local arg curl_exit_code json_url last_json_url - local -a time_cond + local etag_path="${cache_path}.etag" + local new_etag_path="${cache_path}.etag.new" + local new_cache_path="${cache_path}.new" + local arg compare_etag_path curl_exit_code http_code json_url last_json_url + local -a etag_args conditional_args while read -r json_url do - time_cond=() - while read -r arg - do - time_cond+=("${arg}") - done < <(api_time_cond_args "${cache_path}") - api_curl_download "${json_url}" "${cache_path}" "${time_cond[@]}" + # Keep conditional requests in sync with Homebrew::API.fetch_json_api_file in api.rb. + compare_etag_path="" + etag_args=() + if [[ "${HOMEBREW_CURL_SUPPORTS_ETAGS}" == "1" && -z "${last_json_url}" ]] + then + compare_etag_path="${etag_path}" + etag_args=(--etag-save "${new_etag_path}") + fi + conditional_args=() + if [[ -z "${last_json_url}" ]] + then + while read -r arg + do + conditional_args+=("${arg}") + done < <(api_conditional_args "${cache_path}" "${compare_etag_path}" "${json_url}") + fi + rm -f "${new_cache_path}" "${new_etag_path}" + http_code="$(api_curl_download "${json_url}" "${new_cache_path}" "${etag_args[@]}" "${conditional_args[@]}")" curl_exit_code=$? # A conditional request can fail with a receive error (curl exit code 56) when # an unconditional request for the same URL succeeds, so retry exactly once. - if [[ ${curl_exit_code} -eq 56 ]] && [[ ${#time_cond[@]} -gt 0 ]] + if [[ ${curl_exit_code} -eq 56 ]] && [[ ${#conditional_args[@]} -gt 0 ]] then - api_curl_download "${json_url}" "${cache_path}" + rm -f "${new_cache_path}" "${new_etag_path}" + http_code="$(api_curl_download "${json_url}" "${new_cache_path}" "${etag_args[@]}")" curl_exit_code=$? fi last_json_url="${json_url}" [[ ${curl_exit_code} -eq 0 ]] && break done < <(api_urls "${filename}") + if [[ ${curl_exit_code} -eq 0 ]] + then + if [[ -f "${new_cache_path}" ]] + then + rm -f "${etag_path}" "${etag_path}.url" + if ! mv -f "${new_cache_path}" "${cache_path}" + then + echo "Failed to replace ${cache_path}!" >>"${update_failed_file}" + rm -f "${new_cache_path}" "${new_etag_path}" + exec 201>&- + return 1 + fi + fi + if [[ -s "${new_etag_path}" ]] + then + mv -f "${new_etag_path}" "${etag_path}" + printf '%s' "${last_json_url}" >"${etag_path}.url" + elif [[ "${http_code}" != "304" ]] + then + rm -f "${etag_path}" "${etag_path}.url" + fi + fi + rm -f "${new_cache_path}" "${new_etag_path}" + if [[ -n ${is_formula_file} ]] && [[ -f "${api_cache}/formula_names.txt" ]] then mv -f "${api_cache}/formula_names.txt" "${api_cache}/formula_names.before.txt" @@ -496,6 +546,7 @@ fetch_api_file() { else echo "Failed to download ${last_json_url}!" >>"${update_failed_file}" fi + exec 201>&- } homebrew-update() { @@ -1070,7 +1121,7 @@ EOS rm -f "${HOMEBREW_CACHE}"/api/internal/formula.*.jws.json rm -f "${HOMEBREW_CACHE}"/api/internal/cask.*.jws.json - # Remove API files (and their `.payload` and `.payload.index` sidecars) + # Remove API files (and their sidecars) # from previous OS versions, keeping the current OS's so `brew # update-report`'s API data load stays or becomes prewarmed. Keep in # sync with `cache_files` in Library/Homebrew/cleanup.rb. @@ -1080,6 +1131,8 @@ EOS "${HOMEBREW_CACHE}/api/internal/packages.$(bottle_tag).jws.json") ;; "${HOMEBREW_CACHE}/api/internal/packages.$(bottle_tag).jws.json.payload") ;; "${HOMEBREW_CACHE}/api/internal/packages.$(bottle_tag).jws.json.payload.index") ;; + "${HOMEBREW_CACHE}/api/internal/packages.$(bottle_tag).jws.json.etag") ;; + "${HOMEBREW_CACHE}/api/internal/packages.$(bottle_tag).jws.json.etag.url") ;; *) rm -f "${f}" ;; esac done diff --git a/Library/Homebrew/global.rb b/Library/Homebrew/global.rb index 5a8b0014a7..a243110b52 100644 --- a/Library/Homebrew/global.rb +++ b/Library/Homebrew/global.rb @@ -25,6 +25,7 @@ HOMEBREW_PROCESSOR = T.let(ENV.fetch("HOMEBREW_PROCESSOR").freeze, String) HOMEBREW_PHYSICAL_PROCESSOR = T.let(ENV.fetch("HOMEBREW_PHYSICAL_PROCESSOR").freeze, String) HOMEBREW_BREWED_CURL_PATH = Pathname.new(ENV.fetch("HOMEBREW_BREWED_CURL_PATH")).freeze +HOMEBREW_CURL_SUPPORTS_ETAGS = T.let(ENV["HOMEBREW_CURL_SUPPORTS_ETAGS"] == "1", T::Boolean) HOMEBREW_USER_AGENT_CURL = T.let(ENV.fetch("HOMEBREW_USER_AGENT_CURL").freeze, String) HOMEBREW_USER_AGENT_FAKE_SAFARI = # Don't update this beyond 10.15.7 until Safari actually updates their diff --git a/Library/Homebrew/test/api_spec.rb b/Library/Homebrew/test/api_spec.rb index bdb93d813f..3111ee8263 100644 --- a/Library/Homebrew/test/api_spec.rb +++ b/Library/Homebrew/test/api_spec.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true require "api" +require "lock_file" require "openssl" RSpec.describe Homebrew::API do @@ -103,6 +104,209 @@ RSpec.describe Homebrew::API do expect(target.mtime.to_i).to eq stale_mtime.to_i end + context "with a stale cached file" do + let(:target) { cache_dir/"bar.json" } + let(:etag_path) { Pathname("#{target}.etag") } + let(:etag_url_path) { Pathname("#{etag_path}.url") } + let(:new_etag_path) { Pathname("#{target}.etag.new") } + let(:curl_args) { [] } + + before do + stub_const("HOMEBREW_CURL_SUPPORTS_ETAGS", true) + target.write json + FileUtils.touch(target, mtime: Time.now - 7200) + etag_url_path.write "#{Homebrew::EnvConfig.api_domain}/bar.json" + end + + def stub_curl(status:, etag: "") + allow(Utils::Curl).to receive(:curl_download) do |*args, to:, **| + curl_args.replace(args) + File.write(args.fetch(args.index("--etag-save") + 1), etag) if etag && args.include?("--etag-save") + to.write json if status == 200 + SystemCommand::Result.new(["curl"], [[:stdout, status.to_s]], + instance_double(Process::Status, success?: true, exitstatus: 0), + secrets: []) + end + end + + def fetch + described_class.fetch_json_api_file("bar.json", target:, stale_seconds: 3600) + end + + it "sends the saved ETag instead of the cache mtime and stores the new one" do + etag_path.write 'W/"old"' + stub_curl(status: 200, etag: 'W/"new"') + fetch + expect(curl_args).to include("--etag-compare", etag_path.to_s) + expect(curl_args).not_to include("--time-cond") + expect(etag_path.read).to eq 'W/"new"' + end + + it "downloads a fresh body and saves the ETag when none is saved" do + etag_url_path.unlink + stub_curl(status: 200, etag: 'W/"new"') + fetch + expect([curl_args.grep(/--(?:time-cond|etag-compare)/), etag_path.read, etag_url_path.read]) + .to eq [[], 'W/"new"', "#{Homebrew::EnvConfig.api_domain}/bar.json"] + end + + it "downloads a fresh body when the saved ETag is empty" do + etag_path.write "" + stub_curl(status: 200, etag: 'W/"new"') + fetch + expect(curl_args).not_to include("--time-cond", "--etag-compare") + end + + it "keeps the saved ETag, refreshes the mtime and leaves no temp file on a 304" do + etag_path.write 'W/"old"' + stub_curl(status: 304) + fetch + expect(etag_path.read).to eq 'W/"old"' + expect(new_etag_path).not_to exist + expect(target.mtime).to be > Time.now - 60 + end + + it "ignores a leftover temp ETag from an earlier run" do + etag_path.write 'W/"old"' + new_etag_path.write 'W/"leftover"' + stub_curl(status: 304, etag: nil) + fetch + expect(etag_path.read).to eq 'W/"old"' + end + + it "preserves an in-progress ETag when using a fresh cached file" do + FileUtils.touch(target) + new_etag_path.write 'W/"new"' + fetch + expect(new_etag_path).to exist + end + + test_each([false, true]) do |corrupt| + it "holds the shared download lock when refreshing a #{corrupt ? "corrupt fresh" : "stale"} cache" do + if corrupt + target.write json_invalid + FileUtils.touch(target) + end + lock = DownloadLock.new(target) + allow(Utils::Curl).to receive(:curl_download) do |*_, to:, **| + expect { lock.lock }.to raise_error(OperationInProgressError) + to.write json + end + fetch + ensure + lock&.unlock + end + end + + it "removes the saved ETag when a 200 carries none" do + etag_path.write 'W/"old"' + stub_curl(status: 200) + fetch + expect(etag_path).not_to exist + end + + it "keeps the saved ETag when the download fails" do + etag_path.write 'W/"old"' + allow(Utils::Curl).to receive(:curl_download).and_raise(ErrorDuringExecution.new(["curl"], status: 1)) + expect { fetch }.to output(/falling back to cached version/).to_stderr + expect(etag_path.read).to eq 'W/"old"' + end + + it "preserves the cached body, ETag and mtime after a partial download" do + etag_path.write 'W/"old"' + mtime = target.mtime + allow(described_class).to receive(:opoo) + allow(Utils::Curl).to receive(:curl_download) do |*args, to:, **| + to.write '{"partial":' + File.write(args.fetch(args.index("--etag-save") + 1), 'W/"partial"') + raise ErrorDuringExecution.new(["curl"], status: 18) + end + + fetch + + expect([target.read, etag_path.read, target.mtime, new_etag_path.exist?, Pathname("#{target}.new").exist?]) + .to eq [json, 'W/"old"', mtime, false, false] + end + + test_each([false, true]) do |supports_etags| + it "retries a receive error without conditions with ETag support #{supports_etags}" do + stub_const("HOMEBREW_CURL_SUPPORTS_ETAGS", supports_etags) + etag_path.write 'W/"old"' + requests = [] + allow(Utils::Curl).to receive(:curl_download) do |*args, to:, **| + requests << args + if args.include?("--etag-compare") || args.include?("--time-cond") + raise ErrorDuringExecution.new(["curl"], status: 56) + end + + to.write '{"foo":"updated"}' + SystemCommand::Result.new(["curl"], [[:stdout, "200"]], + instance_double(Process::Status, success?: true, exitstatus: 0), + secrets: []) + end + + fetch + + expect([requests.length, target.read, etag_path.exist?]).to eq [2, '{"foo":"updated"}', false] + end + end + + it "ignores an ETag saved for a different API domain" do + etag_path.write 'W/"old"' + etag_url_path.write "https://mirror.example/api/bar.json" + stub_curl(status: 200, etag: 'W/"new"') + fetch + expect(curl_args).not_to include("--time-cond", "--etag-compare") + end + + it "ignores an ETag whose origin is unknown" do + etag_path.write 'W/"old"' + etag_url_path.unlink + stub_curl(status: 200, etag: 'W/"new"') + fetch + expect(curl_args).not_to include("--time-cond", "--etag-compare") + end + + it "keeps the saved ETag after a 304 with curl older than 7.68.0" do + stub_const("HOMEBREW_CURL_SUPPORTS_ETAGS", false) + etag_path.write 'W/"old"' + stub_curl(status: 304) + fetch + expect(etag_path).to exist + end + + it "uses only --time-cond and removes the saved ETag after a 200 when curl is older than 7.68.0" do + stub_const("HOMEBREW_CURL_SUPPORTS_ETAGS", false) + etag_path.write 'W/"old"' + stub_curl(status: 200) + fetch + expect(curl_args).to include("--time-cond", target.to_s) + expect(curl_args.grep(/etag/)).to be_empty + expect(etag_path).not_to exist + end + + it "downloads unconditionally from the fallback API domain" do + allow(Homebrew::EnvConfig).to receive(:api_domain).and_return("https://example.invalid/api") + etag_url_path.write "https://example.invalid/api/bar.json" + etag_path.write 'W/"old"' + requests = [] + allow(Utils::Curl).to receive(:curl_download) do |*args, to:, **| + requests << args.dup + raise ErrorDuringExecution.new(["curl"], status: 1) if requests.length == 1 + + to.write json + SystemCommand::Result.new(["curl"], [[:stdout, "200"]], + instance_double(Process::Status, success?: true, exitstatus: 0), + secrets: []) + end + fetch + expect(requests.fetch(0)).to include("--etag-compare", "--etag-save") + expect(requests.fetch(1)).not_to include("--time-cond") + expect(requests.fetch(1).grep(/etag/)).to be_empty + expect(etag_path).not_to exist + end + end + it "refreshes the cache mtime when a fallback to the default API domain succeeds" do target = cache_dir/"bar.json" target.write json diff --git a/Library/Homebrew/test/cleanup_spec.rb b/Library/Homebrew/test/cleanup_spec.rb index 3b09bf0345..3fb323e558 100644 --- a/Library/Homebrew/test/cleanup_spec.rb +++ b/Library/Homebrew/test/cleanup_spec.rb @@ -874,6 +874,8 @@ RSpec.describe Homebrew::Cleanup do api_internal/current_basename, api_internal/"#{current_basename}.payload", api_internal/"#{current_basename}.payload.index", + api_internal/"#{current_basename}.etag", + api_internal/"#{current_basename}.etag.url", ] scrubbed_files = [ api_internal/"packages.stale.jws.json.payload", @@ -887,7 +889,7 @@ RSpec.describe Homebrew::Cleanup do described_class.new(scrub: true, cache:).cleanup_cache - expect((kept_files + scrubbed_files).map(&:exist?)).to eq([true, true, true, false, false, false]) + expect((kept_files + scrubbed_files).map(&:exist?)).to eq([true, true, true, true, true, false, false, false]) end it "cleans up API source files and symlinks at any depth without cleaning directories" do diff --git a/Library/Homebrew/test/cmd/update_spec.rb b/Library/Homebrew/test/cmd/update_spec.rb index 05cdb1f0d4..8046d8f143 100644 --- a/Library/Homebrew/test/cmd/update_spec.rb +++ b/Library/Homebrew/test/cmd/update_spec.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true require "open3" +require "lock_file" require "cmd/shared_examples/args_parse" require "cmd/update" @@ -32,10 +33,11 @@ RSpec.describe Homebrew::Cmd::Update do def setup_update_utils (test_root/"Library/Homebrew/utils").mkpath FileUtils.ln_s repository_root/"Library/Homebrew/utils.sh", test_root/"Library/Homebrew/utils.sh" - %w[api cmd executables formatter lock tty].each do |name| + %w[api cmd curl executables formatter lock tty].each do |name| FileUtils.ln_s repository_root/"Library/Homebrew/utils/#{name}.sh", test_root/"Library/Homebrew/utils/#{name}.sh" end + FileUtils.ln_s repository_root/"Library/Homebrew/utils/lock_sh", test_root/"Library/Homebrew/utils/lock_sh" end it "stops Git from finding the enclosing repository" do @@ -138,13 +140,15 @@ RSpec.describe Homebrew::Cmd::Update do fetch_api_file formula.jws.json "#{update_failed_file}" SH { - "HOMEBREW_API_DEFAULT_DOMAIN" => "https://formulae.example/api", - "HOMEBREW_API_DOMAIN" => nil, - "HOMEBREW_CACHE" => (test_root/"cache").to_s, - "HOMEBREW_CURL_SPEED_LIMIT" => "100", - "HOMEBREW_CURL_SPEED_TIME" => "5", - "HOMEBREW_LIBRARY" => (test_root/"Library").to_s, - "HOMEBREW_USER_AGENT_CURL" => "Homebrew/test", + "HOMEBREW_API_DEFAULT_DOMAIN" => "https://formulae.example/api", + "HOMEBREW_API_DOMAIN" => nil, + "HOMEBREW_CACHE" => (test_root/"cache").to_s, + "HOMEBREW_CURL_SPEED_LIMIT" => "100", + "HOMEBREW_CURL_SPEED_TIME" => "5", + "HOMEBREW_CURL_SUPPORTS_ETAGS" => nil, + "HOMEBREW_LIBRARY" => (test_root/"Library").to_s, + "HOMEBREW_PREFIX" => HOMEBREW_PREFIX.to_s, + "HOMEBREW_USER_AGENT_CURL" => "Homebrew/test", }, ) @@ -153,6 +157,182 @@ RSpec.describe Homebrew::Cmd::Update do ) end + describe "API ETag revalidation" do + let(:cache_path) { test_root/"cache/api/formula.jws.json" } + let(:etag_path) { test_root/"cache/api/formula.jws.json.etag" } + let(:new_etag_path) { test_root/"cache/api/formula.jws.json.etag.new" } + let(:args_file) { test_root/"curl-args.txt" } + let(:update_failed_file) { test_root/"update_failed.txt" } + + def fetch_with_curl_response(status_code:, body: "", etag: "", saved_etag: '"old"', curl_supports_etags: "1", + api_domain: nil, saved_url: nil, exit_code: 0) + setup_update_utils + cache_path.dirname.mkpath + cache_path.write "cached" + args_file.write "" + etag_path.write saved_etag if saved_etag + Pathname("#{etag_path}.url").write saved_url || "#{api_domain || "https://formulae.example/api"}/formula.jws.json" + + _stdout, stderr, status = run_update_shell( + <<~SH, + source "#{update_script}" + curl() { + printf '%s\\n' "$@" >> "#{args_file}" + [[ "${!#}" == "#{api_domain}/"* ]] && return 22 + local output etag_save + while [[ $# -gt 0 ]] + do + case "$1" in + --output) output="$2"; shift ;; + --etag-save) etag_save="$2"; shift ;; + esac + shift + done + [[ -n "${etag_save}" ]] && printf '%s' '#{etag}' > "${etag_save}" + [[ -n '#{body}' ]] && printf '%s' '#{body}' > "${output}" + printf '%s' '#{status_code}' + return #{exit_code} + } + fetch_api_file formula.jws.json "#{update_failed_file}" + SH + { + "HOMEBREW_API_DEFAULT_DOMAIN" => "https://formulae.example/api", + "HOMEBREW_API_DOMAIN" => api_domain, + "HOMEBREW_CACHE" => (test_root/"cache").to_s, + "HOMEBREW_CURL_SPEED_LIMIT" => "100", + "HOMEBREW_CURL_SPEED_TIME" => "5", + "HOMEBREW_CURL_SUPPORTS_ETAGS" => curl_supports_etags, + "HOMEBREW_LIBRARY" => (test_root/"Library").to_s, + "HOMEBREW_PREFIX" => HOMEBREW_PREFIX.to_s, + "HOMEBREW_USER_AGENT_CURL" => "Homebrew/test", + }, + ) + + conditional_flags = %w[--etag-compare --etag-save --time-cond] + { + success: status.success? && stderr.empty? && !update_failed_file.exist?, + curl_args: args_file.read.lines(chomp: true).each_cons(2).filter_map do |flag, value| + [flag, value] if conditional_flags.include?(flag) + end, + body: cache_path.read, + etag: (etag_path.read if etag_path.exist?), + temp_etag: new_etag_path.exist?, + } + end + + it "revalidates with the saved ETag and saves the new one with the new body" do + expect(fetch_with_curl_response(status_code: 200, body: "fresh", etag: '"new"')).to eq( + success: true, + curl_args: [ + ["--etag-save", new_etag_path.to_s], + ["--etag-compare", etag_path.to_s], + ], + body: "fresh", + etag: '"new"', + temp_etag: false, + ) + end + + it "keeps the saved ETag when the server answers 304" do + expect(fetch_with_curl_response(status_code: 304).except(:curl_args)).to eq( + success: true, body: "cached", etag: '"old"', temp_etag: false, + ) + end + + it "removes the saved ETag after a 200 that carries none" do + expect(fetch_with_curl_response(status_code: 200, body: "fresh").except(:curl_args)).to eq( + success: true, body: "fresh", etag: nil, temp_etag: false, + ) + end + + it "downloads a fresh body without a saved ETag" do + expect(fetch_with_curl_response(status_code: 200, body: "fresh", etag: '"new"', saved_etag: nil)).to eq( + success: true, + curl_args: [["--etag-save", new_etag_path.to_s]], + body: "fresh", + etag: '"new"', + temp_etag: false, + ) + end + + it "uses only the time condition when curl is older than 7.68.0" do + expect(fetch_with_curl_response(status_code: 200, body: "fresh", curl_supports_etags: nil)).to eq( + success: true, + curl_args: [["--time-cond", cache_path.to_s]], + body: "fresh", + etag: nil, + temp_etag: false, + ) + end + + it "downloads unconditionally from the fallback API domain" do + result = fetch_with_curl_response(status_code: 200, body: "fresh", etag: '"new"', + api_domain: "https://mirror.example/api") + expect(result).to eq( + success: true, + curl_args: [ + ["--etag-save", new_etag_path.to_s], + ["--etag-compare", etag_path.to_s], + ], + body: "fresh", + etag: nil, + temp_etag: false, + ) + end + + it "preserves the cached body and ETag after a partial download" do + expect(fetch_with_curl_response(status_code: 200, body: "partial", etag: '"partial"', exit_code: 18) + .except(:curl_args)).to eq(success: false, body: "cached", etag: '"old"', temp_etag: false) + end + + it "ignores an ETag saved for a different API domain" do + expect(fetch_with_curl_response(status_code: 200, body: "fresh", etag: '"new"', + saved_url: "https://mirror.example/api/formula.jws.json")[:curl_args]) + .to eq [["--etag-save", new_etag_path.to_s]] + expect(Pathname("#{etag_path}.url").read).to eq "https://formulae.example/api/formula.jws.json" + end + + it "keeps the saved ETag after a 304 with curl older than 7.68.0" do + expect(fetch_with_curl_response(status_code: 304, curl_supports_etags: nil).except(:curl_args)).to eq( + success: true, body: "cached", etag: '"old"', temp_etag: false, + ) + end + + it "does not overwrite a cache being refreshed by Ruby" do + DownloadLock.new(cache_path).with_lock do + expect(fetch_with_curl_response(status_code: 200, body: "fresh", etag: '"new"').except(:curl_args)).to eq( + success: false, body: "cached", etag: '"old"', temp_etag: false, + ) + end + end + end + + it "redetects curl ETag support when the flag is inherited" do + setup_update_utils + + stdout, stderr, status = run_update_shell( + <<~SH, + source "#{test_root}/Library/Homebrew/utils.sh" + source "#{test_root}/Library/Homebrew/utils/curl.sh" + HOMEBREW_CURL=curl + for version in 7.41.0 7.67.0 7.68.0 8.7.1 + do + curl() { echo "curl ${version} (x86_64-apple-darwin) libcurl/${version}"; } + setup-user-agents + [[ -n "${HOMEBREW_CURL_SUPPORTS_ETAGS}" ]] && echo "${version}: yes" || echo "${version}: no" + done + SH + { + "HOMEBREW_CURL_SUPPORTS_ETAGS" => "1", + "HOMEBREW_LIBRARY" => (test_root/"Library").to_s, + }, + ) + + expect([status.success?, stderr, stdout]).to eq( + [true, "", "7.41.0: no\n7.67.0: no\n7.68.0: yes\n8.7.1: yes\n"], + ) + end + it "passes all arguments through to delegated upgrades" do args_file = test_root/"brew-args.txt" brew_wrapper = test_root/"brew-wrapper" @@ -241,6 +421,58 @@ RSpec.describe Homebrew::Cmd::Update do expect(args_file.read).to eq("update-report\n--auto-update\n") end + it "keeps the current API file's saved ETag when removing other OS versions' API files" do + api_internal = test_root/"cache/api/internal" + setup_update_utils + api_internal.mkpath + (test_root/"repository").mkpath + %w[ + packages.arm64_tahoe.jws.json + packages.arm64_tahoe.jws.json.etag + packages.arm64_tahoe.jws.json.etag.url + packages.arm64_sequoia.jws.json + packages.arm64_sequoia.jws.json.etag + packages.arm64_sequoia.jws.json.etag.url + ].each { |name| (api_internal/name).write "x" } + + _stdout, stderr, status = run_update_shell( + <<~SH, + source "#{update_script}" + bottle_tag() { echo arm64_tahoe; } + brew() { :; } + fetch_api_file() { :; } + git_init_if_necessary() { :; } + git() { + [[ "$1" == "--version" ]] && return 0 + return 1 + } + lock() { :; } + odie() { echo "Error: $*" >&2; exit 1; } + ohai() { :; } + onoe() { echo "Error: $*" >&2; } + safe_cd() { cd "$1" >/dev/null || exit 1; } + setup_ca_certificates() { :; } + setup_curl() { :; } + setup_git() { :; } + homebrew-update --auto-update + SH + { + "HOMEBREW_BREW_GIT_REMOTE" => "https://github.com/Homebrew/brew", + "HOMEBREW_CACHE" => (test_root/"cache").to_s, + "HOMEBREW_CELLAR" => (test_root/"cellar").to_s, + "HOMEBREW_LIBRARY" => (test_root/"Library").to_s, + "HOMEBREW_NO_INSTALL_FROM_API" => nil, + "HOMEBREW_PREFIX" => (test_root/"prefix").to_s, + "HOMEBREW_REPOSITORY" => (test_root/"repository").to_s, + }, + ) + + expect([status.success?, stderr]).to eq([true, ""]) + expect(api_internal.children.map { |path| path.basename.to_s }.sort).to eq( + %w[packages.arm64_tahoe.jws.json packages.arm64_tahoe.jws.json.etag packages.arm64_tahoe.jws.json.etag.url], + ) + end + it "does not query redirected remote metadata for no-op tap updates" do args_file = test_root/"brew-args.txt" fetches_file = test_root/"fetches.txt" diff --git a/Library/Homebrew/utils/api.sh b/Library/Homebrew/utils/api.sh index f84640bbce..cd5d5db60a 100644 --- a/Library/Homebrew/utils/api.sh +++ b/Library/Homebrew/utils/api.sh @@ -25,12 +25,24 @@ api_curlrc_args() { fi } -api_time_cond_args() { +# Keep conditional requests in sync with Homebrew::API.fetch_json_api_file in api.rb. +api_conditional_args() { local cache_path="$1" + local etag_path="$2" + local json_url="$3" if [[ -s "${cache_path}" ]] then - echo "--time-cond" - echo "${cache_path}" + if [[ -n "${etag_path}" ]] + then + if [[ -s "${etag_path}" && -f "${etag_path}.url" && "$(<"${etag_path}.url")" == "${json_url}" ]] + then + echo "--etag-compare" + echo "${etag_path}" + fi + else + echo "--time-cond" + echo "${cache_path}" + fi fi } diff --git a/Library/Homebrew/utils/curl.sh b/Library/Homebrew/utils/curl.sh index 182b4c382d..a0c08a9c03 100644 --- a/Library/Homebrew/utils/curl.sh +++ b/Library/Homebrew/utils/curl.sh @@ -73,4 +73,10 @@ setup-user-agents() { curl_version_output="$("${HOMEBREW_CURL}" --version 2>/dev/null)" curl_name_and_version="${curl_version_output%% (*}" HOMEBREW_USER_AGENT_CURL="${HOMEBREW_USER_AGENT} ${curl_name_and_version// //}" + + unset HOMEBREW_CURL_SUPPORTS_ETAGS + if [[ "$(numeric "${curl_name_and_version##* }")" -ge "$(numeric "7.68.0")" ]] + then + HOMEBREW_CURL_SUPPORTS_ETAGS=1 + fi } diff --git a/docs/Tips-and-Tricks.md b/docs/Tips-and-Tricks.md index 16dcc846dc..4a09901225 100644 --- a/docs/Tips-and-Tricks.md +++ b/docs/Tips-and-Tricks.md @@ -227,6 +227,10 @@ sudo chown -R "${USER}" /usr/local/etc You can configure Homebrew to retrieve bottles and the JSON API from a caching proxy or mirror. +API mirrors should preserve `ETag` headers: with curl 7.68.0 or later, Homebrew revalidates cached JSON using the ETag saved for that URL. +Without a matching validator or when falling back to another API domain, Homebrew downloads a fresh response. +Older curl versions use time-based checks on the configured API domain. + For example, in JFrog's Artifactory, accessible at `https://artifacts.example.com`: - For the bottles, configure a new "remote" repository with `homebrew` as the "repository key" and `https://ghcr.io` (GitHub Packages) as the URL.