From aa989bd55a2013c9858e19455c700b707d65021e Mon Sep 17 00:00:00 2001 From: cnnrmnn Date: Thu, 13 May 2021 11:39:59 -0400 Subject: [PATCH 01/17] Change inititial partial request to HEAD request --- Library/Homebrew/utils/curl.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index eafc5ae81f..67a5630747 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -118,15 +118,15 @@ module Utils result end - def curl_download(*args, to: nil, partial: true, **options) + def curl_download(*args, to: nil, try_partial: true, **options) destination = Pathname(to) destination.dirname.mkpath - if partial + if try_partial range_stdout = curl_output("--location", "--range", "0-1", "--dump-header", "-", "--write-out", "%\{http_code}", - "--output", "/dev/null", *args, **options).stdout + "--head", *args, **options).stdout headers, _, http_status = range_stdout.partition("\r\n\r\n") supports_partial_download = http_status.to_i == 206 # Partial Content From b9b917756c6408f20a341fe47fe0385d067f14d1 Mon Sep 17 00:00:00 2001 From: cnnrmnn Date: Thu, 13 May 2021 11:42:28 -0400 Subject: [PATCH 02/17] Add header parsing --- Library/Homebrew/utils/curl.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 67a5630747..30726f0ab2 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -118,6 +118,13 @@ module Utils result end + def parse_headers(headers) + headers.split("\n").to_h do |h| + partitioned = h.partition(": ") + [partitioned.first, partitioned.last] + end + end + def curl_download(*args, to: nil, try_partial: true, **options) destination = Pathname(to) destination.dirname.mkpath From df0915e33f554cf0139e83359cae867393d31b6f Mon Sep 17 00:00:00 2001 From: cnnrmnn Date: Thu, 13 May 2021 12:11:34 -0400 Subject: [PATCH 03/17] Check partial request support with Accept-Ranges --- Library/Homebrew/utils/curl.rb | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 30726f0ab2..03045dbb43 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -130,23 +130,22 @@ module Utils destination.dirname.mkpath if try_partial - range_stdout = curl_output("--location", "--range", "0-1", - "--dump-header", "-", - "--write-out", "%\{http_code}", + range_stdout = curl_output("--location", "--dump-header", "-", "--head", *args, **options).stdout - headers, _, http_status = range_stdout.partition("\r\n\r\n") + headers = parse_headers(range_stdout.split("\r\n\r\n").first) - supports_partial_download = http_status.to_i == 206 # Partial Content - if supports_partial_download && + # Any value for `accept-ranges` other than none indicates that the server supports range requests. + # Its absence indicates no support. + supports_partial = headers["accept-ranges"] && headers["accept-ranges"] != "none" + + if supports_partial && destination.exist? && - destination.size == %r{^.*Content-Range: bytes \d+-\d+/(\d+)\r\n.*$}m.match(headers)&.[](1)&.to_i + destination.size == headers["content-length"].to_i return # We've already downloaded all the bytes end - else - supports_partial_download = false end - continue_at = if destination.exist? && supports_partial_download + continue_at = if destination.exist? && supports_partial "-" else 0 From 7637fd5366c72ecbed0cfb93ebfb4c5fb449d07a Mon Sep 17 00:00:00 2001 From: cnnrmnn Date: Thu, 13 May 2021 12:27:54 -0400 Subject: [PATCH 04/17] Only use continue-at with partial requests --- Library/Homebrew/utils/curl.rb | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 03045dbb43..65cbdbe92b 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -134,7 +134,7 @@ module Utils "--head", *args, **options).stdout headers = parse_headers(range_stdout.split("\r\n\r\n").first) - # Any value for `accept-ranges` other than none indicates that the server supports range requests. + # Any value for `accept-ranges` other than none indicates that the server supports partial requests. # Its absence indicates no support. supports_partial = headers["accept-ranges"] && headers["accept-ranges"] != "none" @@ -145,15 +145,12 @@ module Utils end end - continue_at = if destination.exist? && supports_partial - "-" - else - 0 - end - curl( - "--location", "--remote-time", "--continue-at", continue_at.to_s, "--output", destination, *args, **options - ) + args += ["--location", "--remote-time", "--output", destination] + # continue-at shouldn't be used with servers that don't support partial requests. + args += ["--continue-at", "-"] if destination.exist? && supports_partial + + curl(*args, **options) end def curl_output(*args, **options) From 16e707254391d50de331489d42ed5c36e4b0c722 Mon Sep 17 00:00:00 2001 From: cnnrmnn Date: Thu, 13 May 2021 12:52:21 -0400 Subject: [PATCH 05/17] Fix argument order --- Library/Homebrew/utils/curl.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 65cbdbe92b..32161e4286 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -119,6 +119,8 @@ module Utils end def parse_headers(headers) + return {} unless headers + headers.split("\n").to_h do |h| partitioned = h.partition(": ") [partitioned.first, partitioned.last] @@ -145,10 +147,9 @@ module Utils end end - - args += ["--location", "--remote-time", "--output", destination] + args = ["--location", "--remote-time", "--output", destination] + args # continue-at shouldn't be used with servers that don't support partial requests. - args += ["--continue-at", "-"] if destination.exist? && supports_partial + args = ["--continue-at", "-"] + args if destination.exist? && supports_partial curl(*args, **options) end From 8ef2d2ecc489cfcd1c08fc99a96ae09ce47f4ffe Mon Sep 17 00:00:00 2001 From: cnnrmnn Date: Thu, 13 May 2021 12:54:17 -0400 Subject: [PATCH 06/17] Update tests --- Library/Homebrew/test/download_strategies/curl_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/test/download_strategies/curl_spec.rb b/Library/Homebrew/test/download_strategies/curl_spec.rb index 09cd6ca07a..31fd493d2e 100644 --- a/Library/Homebrew/test/download_strategies/curl_spec.rb +++ b/Library/Homebrew/test/download_strategies/curl_spec.rb @@ -23,9 +23,10 @@ describe CurlDownloadStrategy do it "calls curl with default arguments" do expect(strategy).to receive(:curl).with( + # example.com supports partial requests. + "--continue-at", "-", "--location", "--remote-time", - "--continue-at", "0", "--output", an_instance_of(Pathname), url, an_instance_of(Hash) From a1566212976072d8b5b95f5b335a3d98f77c713d Mon Sep 17 00:00:00 2001 From: cnnrmnn Date: Fri, 14 May 2021 09:44:05 -0400 Subject: [PATCH 07/17] Simplify header parsing --- Library/Homebrew/utils/curl.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 32161e4286..a2566cf521 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -121,10 +121,8 @@ module Utils def parse_headers(headers) return {} unless headers - headers.split("\n").to_h do |h| - partitioned = h.partition(": ") - [partitioned.first, partitioned.last] - end + # Skip status code + headers.split("\r\n")[2..].to_h { |h| h.split(": ") } end def curl_download(*args, to: nil, try_partial: true, **options) From 86bce7bf1de0e7ef6793493d0a0c674432ea8b6c Mon Sep 17 00:00:00 2001 From: cnnrmnn Date: Fri, 14 May 2021 09:50:57 -0400 Subject: [PATCH 08/17] Stop unnecessarily dumping headers --- Library/Homebrew/utils/curl.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index a2566cf521..ae181c6e27 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -122,7 +122,7 @@ module Utils return {} unless headers # Skip status code - headers.split("\r\n")[2..].to_h { |h| h.split(": ") } + headers.split("\r\n")[1..].to_h { |h| h.split(": ") } end def curl_download(*args, to: nil, try_partial: true, **options) @@ -130,8 +130,7 @@ module Utils destination.dirname.mkpath if try_partial - range_stdout = curl_output("--location", "--dump-header", "-", - "--head", *args, **options).stdout + range_stdout = curl_output("--location", "--head", *args, **options).stdout headers = parse_headers(range_stdout.split("\r\n\r\n").first) # Any value for `accept-ranges` other than none indicates that the server supports partial requests. From b406e0b35a915e45ef8cff84e38f71def6b70f5e Mon Sep 17 00:00:00 2001 From: cnnrmnn Date: Fri, 14 May 2021 09:53:29 -0400 Subject: [PATCH 09/17] Update call sites --- Library/Homebrew/utils/spdx.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/utils/spdx.rb b/Library/Homebrew/utils/spdx.rb index 43e19401f7..93e2d7d1d9 100644 --- a/Library/Homebrew/utils/spdx.rb +++ b/Library/Homebrew/utils/spdx.rb @@ -34,8 +34,8 @@ module SPDX def download_latest_license_data!(to: DATA_PATH) data_url = "https://raw.githubusercontent.com/spdx/license-list-data/#{latest_tag}/json/" - curl_download("#{data_url}licenses.json", to: to/"spdx_licenses.json", partial: false) - curl_download("#{data_url}exceptions.json", to: to/"spdx_exceptions.json", partial: false) + curl_download("#{data_url}licenses.json", to: to/"spdx_licenses.json", try_partial: false) + curl_download("#{data_url}exceptions.json", to: to/"spdx_exceptions.json", try_partial: false) end def parse_license_expression(license_expression) From 85702dfe5358609deaa8b815577902481fdbcf79 Mon Sep 17 00:00:00 2001 From: Connor Mann <34930543+cnnrmnn@users.noreply.github.com> Date: Fri, 14 May 2021 09:55:40 -0400 Subject: [PATCH 10/17] Update Library/Homebrew/utils/curl.rb Co-authored-by: Mike McQuaid --- Library/Homebrew/utils/curl.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index ae181c6e27..e701d4a82b 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -144,7 +144,7 @@ module Utils end end - args = ["--location", "--remote-time", "--output", destination] + args + args = ["--location", "--remote-time", "--output", destination, *args] # continue-at shouldn't be used with servers that don't support partial requests. args = ["--continue-at", "-"] + args if destination.exist? && supports_partial From b79aeab8b22bf18b4906e5d33b40bb62521e56ae Mon Sep 17 00:00:00 2001 From: Connor Mann <34930543+cnnrmnn@users.noreply.github.com> Date: Fri, 14 May 2021 09:55:45 -0400 Subject: [PATCH 11/17] Update Library/Homebrew/utils/curl.rb Co-authored-by: Mike McQuaid --- Library/Homebrew/utils/curl.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index e701d4a82b..04300c34b3 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -146,7 +146,7 @@ module Utils args = ["--location", "--remote-time", "--output", destination, *args] # continue-at shouldn't be used with servers that don't support partial requests. - args = ["--continue-at", "-"] + args if destination.exist? && supports_partial + args = ["--continue-at", "-", *args] if destination.exist? && supports_partial curl(*args, **options) end From ecaaafba203578498fbc0a48493adea1790839f2 Mon Sep 17 00:00:00 2001 From: Connor Mann <34930543+cnnrmnn@users.noreply.github.com> Date: Fri, 14 May 2021 11:16:35 -0400 Subject: [PATCH 12/17] Update Library/Homebrew/utils/curl.rb Co-authored-by: Rylan Polster --- Library/Homebrew/utils/curl.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 04300c34b3..04b9491f95 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -144,9 +144,9 @@ module Utils end end - args = ["--location", "--remote-time", "--output", destination, *args] + args.prepend "--location", "--remote-time", "--output", destination # continue-at shouldn't be used with servers that don't support partial requests. - args = ["--continue-at", "-", *args] if destination.exist? && supports_partial + args.prepend "--continue-at", "-" if destination.exist? && supports_partial curl(*args, **options) end From 2500b8dabae2f7411ec5fea004e27569f7eda940 Mon Sep 17 00:00:00 2001 From: Connor Mann <34930543+cnnrmnn@users.noreply.github.com> Date: Fri, 14 May 2021 11:47:38 -0400 Subject: [PATCH 13/17] Update Library/Homebrew/utils/curl.rb Co-authored-by: Rylan Polster --- Library/Homebrew/utils/curl.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 04b9491f95..eefa0df29f 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -122,7 +122,10 @@ module Utils return {} unless headers # Skip status code - headers.split("\r\n")[1..].to_h { |h| h.split(": ") } + headers.split("\r\n")[1..].to_h do |h| + name, content = h.split(": ") + [name.downcase, content] + end end def curl_download(*args, to: nil, try_partial: true, **options) From f7fe111430014dce9909c00540b87b02518cf7e3 Mon Sep 17 00:00:00 2001 From: Connor Mann <34930543+cnnrmnn@users.noreply.github.com> Date: Fri, 14 May 2021 11:53:49 -0400 Subject: [PATCH 14/17] Use `blank?` instead of checking for nil --- Library/Homebrew/utils/curl.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index eefa0df29f..c9fb1bef64 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -119,7 +119,7 @@ module Utils end def parse_headers(headers) - return {} unless headers + return {} if headers.blank? # Skip status code headers.split("\r\n")[1..].to_h do |h| From b546960da9b737bdb9897a128de24fc867fee5a9 Mon Sep 17 00:00:00 2001 From: Connor Mann Date: Fri, 14 May 2021 15:14:56 -0400 Subject: [PATCH 15/17] Revert "Update Library/Homebrew/utils/curl.rb" This reverts commit ecaaafba203578498fbc0a48493adea1790839f2. --- Library/Homebrew/utils/curl.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index c9fb1bef64..5578d29711 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -147,9 +147,9 @@ module Utils end end - args.prepend "--location", "--remote-time", "--output", destination + args = ["--location", "--remote-time", "--output", destination, *args] # continue-at shouldn't be used with servers that don't support partial requests. - args.prepend "--continue-at", "-" if destination.exist? && supports_partial + args = ["--continue-at", "-", *args] if destination.exist? && supports_partial curl(*args, **options) end From 2a9540b3ab2c5662cca5eea8e991bba528635d3a Mon Sep 17 00:00:00 2001 From: Connor Mann <34930543+cnnrmnn@users.noreply.github.com> Date: Fri, 14 May 2021 15:16:37 -0400 Subject: [PATCH 16/17] Check for key explicitly with `key?` Co-authored-by: Rylan Polster --- Library/Homebrew/utils/curl.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 5578d29711..cb38e9e7d6 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -138,7 +138,7 @@ module Utils # Any value for `accept-ranges` other than none indicates that the server supports partial requests. # Its absence indicates no support. - supports_partial = headers["accept-ranges"] && headers["accept-ranges"] != "none" + supports_partial = headers.key? "accept-ranges" && headers["accept-ranges"] != "none" if supports_partial && destination.exist? && From a079ba9bb389de0d4a849042f45dad6d9ccd31d9 Mon Sep 17 00:00:00 2001 From: Connor Mann Date: Fri, 14 May 2021 15:28:56 -0400 Subject: [PATCH 17/17] Add parentheses for clarity --- Library/Homebrew/utils/curl.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index cb38e9e7d6..517875628c 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -138,7 +138,7 @@ module Utils # Any value for `accept-ranges` other than none indicates that the server supports partial requests. # Its absence indicates no support. - supports_partial = headers.key? "accept-ranges" && headers["accept-ranges"] != "none" + supports_partial = headers.key?("accept-ranges") && headers["accept-ranges"] != "none" if supports_partial && destination.exist? &&