From c8504427cbc6f0756f59ad722edbe38cc933fb70 Mon Sep 17 00:00:00 2001 From: Issy Long Date: Wed, 26 Jun 2024 19:42:50 +0100 Subject: [PATCH 1/2] CurlDownloadStrategy: Ignore invalid `last-modified` header values - Some download locations return a non-standard formatting of date string for the `Last-Modified` header. This causes `Time.parse` to blow up. The user sees `error: argument out of range`. - In this commit we handle the error and return nil, which `filter_map` (equivalent to `.map.compact`) gets rid of and then `time.last` is as normal. - Fixes https://github.com/Homebrew/brew/issues/ 17556. --- Library/Homebrew/download_strategy.rb | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 656a61823a..9d54d1f2c7 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -518,10 +518,13 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy [*parse_content_disposition.call("Content-Disposition: #{header}")] end - time = parsed_headers - .flat_map { |headers| [*headers["last-modified"]] } - .map { |t| t.match?(/^\d+$/) ? Time.at(t.to_i) : Time.parse(t) } - .last + 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 + nil + end file_size = parsed_headers .flat_map { |headers| [*headers["content-length"]&.to_i] } @@ -530,7 +533,7 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy is_redirection = url != final_url basename = filenames.last || parse_basename(final_url, search_query: !is_redirection) - @resolved_info_cache[url] = [final_url, basename, time, file_size, is_redirection] + @resolved_info_cache[url] = [final_url, basename, time.last, file_size, is_redirection] end def _fetch(url:, resolved_url:, timeout:) From 8c9a6e3379331640b5556dd83732c23428cd724a Mon Sep 17 00:00:00 2001 From: Issy Long Date: Thu, 27 Jun 2024 12:27:30 +0100 Subject: [PATCH 2/2] Add a comment for what and why we're rescuing --- Library/Homebrew/download_strategy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 9d54d1f2c7..9c7574b03f 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -522,7 +522,7 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy .flat_map { |headers| [*headers["last-modified"]] } .filter_map do |t| t.match?(/^\d+$/) ? Time.at(t.to_i) : Time.parse(t) - rescue ArgumentError + rescue ArgumentError # When `Time.parse` gets a badly formatted date. nil end