From c27eed4606f32a88723cb8be1022c531abcc51eb Mon Sep 17 00:00:00 2001 From: Frederick Zhang Date: Fri, 25 Nov 2022 12:09:59 +1100 Subject: [PATCH] Curl: Fix following redirections when base changes Update base URL when there is an absolute location, so that following relative locations are considered relative to the new base. Consider below cURL output for https://example_one.com: HTTP/1.1 302 Moved Temporarily Location: https://example_two.com HTTP/1.1 302 Moved Temporarily Location: /foo/ HTTP/1.1 200 OK The final URL should be https://example_two.com/foo/ rather than https://example_one.com/foo/. --- Library/Homebrew/download_strategy.rb | 2 +- Library/Homebrew/test/utils/curl_spec.rb | 54 ++++++++++++++++++++++++ Library/Homebrew/utils/curl.rb | 24 +++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index c10858a353..acce289519 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -471,7 +471,7 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy lines = output.to_s.lines.map(&:chomp) - final_url = curl_response_last_location(parsed_output[:responses], absolutize: true, base_url: url) + final_url = curl_response_follow_redirections(parsed_output[:responses], url) final_url ||= url content_disposition_parser = Mechanize::HTTP::ContentDispositionParser.new diff --git a/Library/Homebrew/test/utils/curl_spec.rb b/Library/Homebrew/test/utils/curl_spec.rb index 841f29c21c..e098e642eb 100644 --- a/Library/Homebrew/test/utils/curl_spec.rb +++ b/Library/Homebrew/test/utils/curl_spec.rb @@ -556,4 +556,58 @@ describe "Utils::Curl" do expect(curl_response_last_location([response_hash[:ok]])).to be_nil end end + + describe "#curl_response_follow_redirections" do + it "returns the original URL when there are no location headers" do + expect( + curl_response_follow_redirections( + [response_hash[:ok]], + "https://brew.sh/test1/test2", + ), + ).to eq("https://brew.sh/test1/test2") + end + + it "returns the URL relative to base when locations are relative" do + expect( + curl_response_follow_redirections( + [response_hash[:redirection_root_relative], response_hash[:ok]], + "https://brew.sh/test1/test2", + ), + ).to eq("https://brew.sh/example/") + + expect( + curl_response_follow_redirections( + [response_hash[:redirection_parent_relative], response_hash[:ok]], + "https://brew.sh/test1/test2", + ), + ).to eq("https://brew.sh/test1/example/") + + expect( + curl_response_follow_redirections( + [ + response_hash[:redirection_parent_relative], + response_hash[:redirection_parent_relative], + response_hash[:ok], + ], + "https://brew.sh/test1/test2", + ), + ).to eq("https://brew.sh/test1/example/example/") + end + + it "returns new base when there are absolute location(s)" do + expect( + curl_response_follow_redirections( + [response_hash[:redirection], response_hash[:ok]], + "https://brew.sh/test1/test2", + ), + ).to eq(location_urls[0]) + + expect( + curl_response_follow_redirections( + [response_hash[:redirection], response_hash[:redirection_parent_relative], response_hash[:ok]], + "https://brew.sh/test1/test2", + ), + ).to eq("#{location_urls[0]}example/") + end + end end diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 8fe6ca9772..93e23254b6 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -506,6 +506,30 @@ module Utils nil end + # Returns the final URL by following location headers in cURL responses. + # @param responses [Array] An array of hashes containing response + # status information and headers from `#parse_curl_response`. + # @param base_url [String] The URL to use as a base. + # @return [String] The final absolute URL after redirections. + sig { + params( + responses: T::Array[T::Hash[Symbol, T.untyped]], + base_url: String, + ).returns(String) + } + def curl_response_follow_redirections(responses, base_url) + responses.each do |response| + next if response[:headers].blank? + + location = response[:headers]["location"] + next if location.blank? + + base_url = URI.join(base_url, location).to_s + end + + base_url + end + private # Parses HTTP response text from `curl` output into a hash containing the