diff --git a/Library/Homebrew/test/download_strategies_spec.rb b/Library/Homebrew/test/download_strategies_spec.rb index 07a3f7bc8d..3f5f6c07a8 100644 --- a/Library/Homebrew/test/download_strategies_spec.rb +++ b/Library/Homebrew/test/download_strategies_spec.rb @@ -183,9 +183,12 @@ describe CurlDownloadStrategy do let(:specs) { { user_agent: "Mozilla/25.0.1" } } it "adds the appropriate curl args" do - expect(subject).to receive(:system_command!) { |*, args:, **| - expect(args.each_cons(2)).to include(["--user-agent", "Mozilla/25.0.1"]) - } + expect(subject).to receive(:system_command).with( + /curl/, + hash_including(args: array_including_cons("--user-agent", "Mozilla/25.0.1")), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) subject.fetch end @@ -197,14 +200,15 @@ describe CurlDownloadStrategy do let(:specs) { { user_agent: :fake } } it "adds the appropriate curl args" do - expect(subject).to receive(:system_command!) { |*, args:, **| - expect(args.each_cons(2).to_a).to include( - [ - "--user-agent", - a_string_matching(/Mozilla.*Mac OS X 10.*AppleWebKit/), - ], - ) - } + expect(subject).to receive(:system_command).with( + /curl/, + hash_including(args: array_including_cons( + "--user-agent", + a_string_matching(/Mozilla.*Mac OS X 10.*AppleWebKit/), + )), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) subject.fetch end @@ -221,9 +225,12 @@ describe CurlDownloadStrategy do } it "adds the appropriate curl args and does not URL-encode the cookies" do - expect(subject).to receive(:system_command!) { |*, args:, **| - expect(args.each_cons(2)).to include(["-b", "coo=k/e;mon=ster"]) - } + expect(subject).to receive(:system_command).with( + /curl/, + hash_including(args: array_including_cons("-b", "coo=k/e;mon=ster")), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) subject.fetch end @@ -233,9 +240,12 @@ describe CurlDownloadStrategy do let(:specs) { { referer: "https://somehost/also" } } it "adds the appropriate curl args" do - expect(subject).to receive(:system_command!) { |*, args:, **| - expect(args.each_cons(2)).to include(["-e", "https://somehost/also"]) - } + expect(subject).to receive(:system_command).with( + /curl/, + hash_including(args: array_including_cons("-e", "https://somehost/also")), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) subject.fetch end @@ -247,10 +257,12 @@ describe CurlDownloadStrategy do let(:specs) { { headers: ["foo", "bar"] } } it "adds the appropriate curl args" do - expect(subject).to receive(:system_command!) { |*, args:, **| - expect(args.each_cons(2).to_a).to include(["--header", "foo"]) - expect(args.each_cons(2).to_a).to include(["--header", "bar"]) - } + expect(subject).to receive(:system_command).with( + /curl/, + hash_including(args: array_including_cons("--header", "foo").and(array_including_cons("--header", "bar"))), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) subject.fetch end @@ -327,10 +339,12 @@ describe CurlPostDownloadStrategy do } it "adds the appropriate curl args" do - expect(subject).to receive(:system_command!) { |*, args:, **| - expect(args.each_cons(2)).to include(["-d", "form=data"]) - expect(args.each_cons(2)).to include(["-d", "is=good"]) - } + expect(subject).to receive(:system_command).with( + /curl/, + hash_including(args: array_including_cons("-d", "form=data").and(array_including_cons("-d", "is=good"))), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) subject.fetch end @@ -340,9 +354,12 @@ describe CurlPostDownloadStrategy do let(:specs) { { using: :post } } it "adds the appropriate curl args" do - expect(subject).to receive(:system_command!) { |*, args:, **| - expect(args.each_cons(2)).to include(["-X", "POST"]) - } + expect(subject).to receive(:system_command).with( + /curl/, + hash_including(args: array_including_cons("-X", "POST")), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) subject.fetch end @@ -364,8 +381,7 @@ describe SubversionDownloadStrategy do it "adds the appropriate svn args" do expect(subject).to receive(:system_command!) - .with("svn", args: array_including("--trust-server-cert", - "--non-interactive")) + .with("svn", hash_including(args: array_including("--trust-server-cert", "--non-interactive"))) subject.fetch end end @@ -374,9 +390,8 @@ describe SubversionDownloadStrategy do let(:specs) { { revision: "10" } } it "adds svn arguments for :revision" do - expect(subject).to receive(:system_command!) { |*, args:, **| - expect(args.each_cons(2)).to include(["-r", "10"]) - } + expect(subject).to receive(:system_command!) + .with("svn", hash_including(args: array_including_cons("-r", "10"))) subject.fetch end diff --git a/Library/Homebrew/test/spec_helper.rb b/Library/Homebrew/test/spec_helper.rb index a37f306f58..7d878b0989 100644 --- a/Library/Homebrew/test/spec_helper.rb +++ b/Library/Homebrew/test/spec_helper.rb @@ -257,3 +257,10 @@ RSpec::Matchers.define :a_json_string do false end end + +# Match consecutive elements in an array. +RSpec::Matchers.define :array_including_cons do |*cons| + match do |actual| + expect(actual.each_cons(cons.size)).to include(cons) + end +end diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 2318579ee0..e8a05fcfbd 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -44,14 +44,47 @@ def curl_args(*extra_args, show_output: false, user_agent: :default) args + extra_args end -def curl(*args, secrets: [], **options) +def curl_with_workarounds(*args, secrets: nil, print_stdout: nil, print_stderr: nil, **options) + command_options = { + secrets: secrets, + print_stdout: print_stdout, + print_stderr: print_stderr, + }.compact + # SSL_CERT_FILE can be incorrectly set by users or portable-ruby and screw # with SSL downloads so unset it here. - system_command! curl_executable, - args: curl_args(*args, **options), - print_stdout: true, - env: { "SSL_CERT_FILE" => nil }, - secrets: secrets + result = system_command curl_executable, + args: curl_args(*args, **options), + env: { "SSL_CERT_FILE" => nil }, + **command_options + + if !result.success? && !args.include?("--http1.1") + # This is a workaround for https://github.com/curl/curl/issues/1618. + if result.status.exitstatus == 56 # Unexpected EOF + out = curl_output("-V").stdout + + # If `curl` doesn't support HTTP2, the exception is unrelated to this bug. + return result unless out.include?("HTTP2") + + # The bug is fixed in `curl` >= 7.60.0. + curl_version = out[/curl (\d+(\.\d+)+)/, 1] + return result if Gem::Version.new(curl_version) >= Gem::Version.new("7.60.0") + + return curl_with_workarounds(*args, "--http1.1", **command_options, **options) + end + + if result.status.exitstatus == 16 # Error in the HTTP2 framing layer + return curl_with_workarounds(*args, "--http1.1", **command_options, **options) + end + end + + result +end + +def curl(*args, **options) + result = curl_with_workarounds(*args, print_stdout: true, **options) + result.assert_success! + result end def curl_download(*args, to: nil, partial: true, **options) @@ -82,30 +115,10 @@ def curl_download(*args, to: nil, partial: true, **options) end curl("--location", "--remote-time", "--continue-at", continue_at.to_s, "--output", destination, *args, **options) -rescue ErrorDuringExecution => e - # This is a workaround for https://github.com/curl/curl/issues/1618. - raise unless e.status.exitstatus == 56 # Unexpected EOF - - raise if args.include?("--http1.1") - - out = curl_output("-V").stdout - - # If `curl` doesn't support HTTP2, the exception is unrelated to this bug. - raise unless out.include?("HTTP2") - - # The bug is fixed in `curl` >= 7.60.0. - curl_version = out[/curl (\d+(\.\d+)+)/, 1] - raise if Gem::Version.new(curl_version) >= Gem::Version.new("7.60.0") - - args << "--http1.1" - retry end -def curl_output(*args, secrets: [], **options) - system_command curl_executable, - args: curl_args(*args, show_output: true, **options), - print_stderr: false, - secrets: secrets +def curl_output(*args, **options) + curl_with_workarounds(*args, print_stderr: false, show_output: true, **options) end def curl_check_http_content(url, user_agents: [:default], check_content: false, strict: false)