curl_download_strategy: fix caching edgecase

Library/Homebrew/download_strategy/curl_download_strategy.rb

@@ -249,30 +249,25 @@ def resolve_url_basename_time_file_size(url, timeout: nil)
       [*parse_content_disposition.call("Content-Disposition: #{header}")]
     end
 
-    time =  parsed_headers
-            .flat_map { |headers| [*headers["last-modified"]] }
-            .filter_map do |t|
-              t.match?(/^\d+$/) ? Time.at(t.to_i) : Time.parse(t)
-            rescue ArgumentError # When `Time.parse` gets a badly formatted date.
-              nil
-            end
-
-    file_size = parsed_headers
-                .flat_map { |headers| [*headers["content-length"]&.to_i] }
-                .last
+    final_headers = parsed_headers.last || {}
+
+    time = [*final_headers["last-modified"]].filter_map do |t|
+      t.match?(/^\d+$/) ? Time.at(t.to_i) : Time.parse(t)
+    rescue ArgumentError # When `Time.parse` gets a badly formatted date.
+      nil
+    end
+
+    file_size = [*final_headers["content-length"]].last&.to_i
 
     # Fallback to content-range header if content-length is not available.
     # Content-Range format: "bytes start-end/total" or "bytes */total" or "bytes start-end/*"
     if file_size.nil? || file_size.zero?
-      file_size = parsed_headers
-                  .flat_map { |headers| [*headers["content-range"]] }
+      file_size = [*final_headers["content-range"]]
                   .filter_map { |range| Integer(range.split("/").last, 10, exception: false) }
                   .last
     end
 
-    content_type = parsed_headers
-                   .flat_map { |headers| [*headers["content-type"]] }
-                   .last
+    content_type = [*final_headers["content-type"]].last
 
     is_redirection = url != final_url
     basename = filenames.last || parse_basename(final_url, search_query: !is_redirection)

Library/Homebrew/test/download_strategies/curl_spec.rb

@@ -18,9 +18,11 @@
     }
   end
 
+  let(:responses) { [{ headers: }] }
+
   before do
     allow(strategy).to receive(:curl_headers).with(any_args)
-                                             .and_return({ responses: [{ headers: }] })
+                                             .and_return({ responses: })
   end
 
   it "parses the opts and sets the corresponding args" do
@@ -231,6 +233,53 @@
       end
     end
 
+    context "with an HTML redirect before the downloaded file" do
+      let(:final_headers) { headers }
+      let(:responses) do
+        [
+          { headers: { "content-type"   => "text/html; charset=UTF-8",
+                       "content-length" => "100",
+                       "location"       => "https://example.com/media/foo.tar.gz" } },
+          { headers: final_headers },
+        ]
+      end
+
+      before do
+        allow(strategy).to receive(:curl)
+
+        strategy.cached_location.dirname.mkpath
+        strategy.cached_location.write("cached")
+      end
+
+      it "ignores a cached download of a different size" do
+        expect { strategy.fetch }.to output(/differs from Content-Length header: 37182/).to_stdout
+      end
+
+      context "when the file is newer than the cached download" do
+        let(:final_headers) { { "last-modified" => (Time.now + 3600).httpdate } }
+
+        it "ignores the cached download" do
+          expect { strategy.fetch }.to output(/is before Last-Modified header/).to_stdout
+        end
+      end
+
+      context "when the redirect ends on a web page" do
+        let(:final_headers) { headers.merge("content-type" => "text/html; charset=UTF-8") }
+
+        it "keeps the cached download" do
+          expect { strategy.fetch }.to output(/Already downloaded/).to_stdout
+        end
+      end
+
+      context "when the file is sent without a size or a modification time" do
+        let(:final_headers) { { "transfer-encoding" => "chunked" } }
+
+        it "keeps the cached download" do
+          expect { strategy.fetch }.to output(/Already downloaded/).to_stdout
+        end
+      end
+    end
+
     context "when a redirect target names a variable the download did not declare" do
       let(:redirect_url) do
         "https://example.com/elsewhere/foo.tar.gz?leak=" \