Use curl workarounds for both curl and curl_output.

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(subject).to receive(:system_command).with(
-          expect(args.each_cons(2)).to include(["--user-agent", "Mozilla/25.0.1"])
+          /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(subject).to receive(:system_command).with(
-          expect(args.each_cons(2).to_a).to include(
+          /curl/,
-            [
+          hash_including(args: array_including_cons(
-              "--user-agent",
+            "--user-agent",
-              a_string_matching(/Mozilla.*Mac OS X 10.*AppleWebKit/),
+            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(subject).to receive(:system_command).with(
-          expect(args.each_cons(2)).to include(["-b", "coo=k/e;mon=ster"])
+          /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(subject).to receive(:system_command).with(
-          expect(args.each_cons(2)).to include(["-e", "https://somehost/also"])
+          /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(subject).to receive(:system_command).with(
-          expect(args.each_cons(2).to_a).to include(["--header", "foo"])
+          /curl/,
-          expect(args.each_cons(2).to_a).to include(["--header", "bar"])
+          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(subject).to receive(:system_command).with(
-          expect(args.each_cons(2)).to include(["-d", "form=data"])
+          /curl/,
-          expect(args.each_cons(2)).to include(["-d", "is=good"])
+          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(subject).to receive(:system_command).with(
-          expect(args.each_cons(2)).to include(["-X", "POST"])
+          /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",
+          .with("svn", hash_including(args: array_including("--trust-server-cert", "--non-interactive")))
-                                             "--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(subject).to receive(:system_command!)
-          expect(args.each_cons(2)).to include(["-r", "10"])
+          .with("svn", hash_including(args: array_including_cons("-r", "10")))
-        }
 
         subject.fetch
       end

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

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,
+  result = system_command curl_executable,
-                  args:         curl_args(*args, **options),
+                          args: curl_args(*args, **options),
-                  print_stdout: true,
+                          env:  { "SSL_CERT_FILE" => nil },
-                  env:          { "SSL_CERT_FILE" => nil },
+                          **command_options
-                  secrets:      secrets
+
+  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)
+def curl_output(*args, **options)
-  system_command curl_executable,
+  curl_with_workarounds(*args, print_stderr: false, show_output: true, **options)
-                 args:         curl_args(*args, show_output: true, **options),
-                 print_stderr: false,
-                 secrets:      secrets
 end
 
 def curl_check_http_content(url, user_agents: [:default], check_content: false, strict: false)