From 7f4ea0204782ca3d076b1cf68972b295538f2668 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Thu, 2 Aug 2018 11:16:36 +0200 Subject: [PATCH 1/2] Merge `curl` options used by casks into download strategies. --- Library/Homebrew/download_strategy.rb | 47 ++++++++++++++++--- .../Homebrew/test/download_strategies_spec.rb | 2 +- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 5d9d3f25c0..f16ee9ad92 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -1,3 +1,4 @@ +require "cgi" require "json" require "rexml/document" require "time" @@ -263,14 +264,29 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy # Curl options to be always passed to curl, # with raw head calls (`curl --head`) or with actual `fetch`. + def _curl_args + args = [] + + if meta.key?(:cookies) + escape_cookie = ->(k, v) { "#{CGI.escape(k.to_s)}=#{CGI.escape(v.to_s)}" } + args += ["-b", meta.fetch(:cookies).map(&escape_cookie).join(";")] + end + + args += ["-e", meta.fetch(:referer)] if meta.key?(:referer) + + args += ["--user", meta.fetch(:user)] if meta.key?(:user) + + args + end + def _curl_opts - return ["--user", meta.fetch(:user)] if meta.key?(:user) - [] + return { user_agent: meta.fetch(:user_agent) } if meta.key?(:user_agent) + {} end def resolved_url(url) redirect_url, _, status = curl_output( - *_curl_opts, "--silent", "--head", + "--silent", "--head", "--write-out", "%{redirect_url}", "--output", "/dev/null", url.to_s @@ -289,17 +305,20 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy redirect_url end + def curl_output(*args, **options) + super(*_curl_args, *args, **_curl_opts, **options) + end + def curl(*args, **options) - args.concat _curl_opts args << "--connect-timeout" << "5" unless mirrors.empty? - super(*args, **options) + super(*_curl_args, *args, **_curl_opts, **options) end end # Detect and download from Apache Mirror class CurlApacheMirrorDownloadStrategy < CurlDownloadStrategy def apache_mirrors - mirrors, = curl_output(*_curl_opts, "--silent", "--location", "#{@url}&asjson=1") + mirrors, = curl_output("--silent", "--location", "#{@url}&asjson=1") JSON.parse(mirrors) end @@ -323,7 +342,13 @@ end # Query parameters on the URL are converted into POST parameters class CurlPostDownloadStrategy < CurlDownloadStrategy def _fetch - base_url, data = @url.split("?") + base_url, data = if meta.key?(:data) + escape_data = ->(k, v) { ["-d", "#{CGI.escape(k.to_s)}=#{CGI.escape(v.to_s)}"] } + [@url, meta[:data].flat_map(&escape_data)] + else + @url.split("?", 2) + end + curl_download base_url, "--data", data, to: temporary_path end end @@ -584,11 +609,19 @@ class SubversionDownloadStrategy < VCSDownloadStrategy # This saves on bandwidth and will have a similar effect to verifying the # cache as it will make any changes to get the right revision. args = [] + if revision ohai "Checking out #{@ref}" args << "-r" << revision end + args << "--ignore-externals" if ignore_externals + + if meta[:trust_cert] == true + args << "--trust-server-cert" + args << "--non-interactive" + end + if target.directory? system_command("svn", args: ["update", *args], chdir: target.to_s) else diff --git a/Library/Homebrew/test/download_strategies_spec.rb b/Library/Homebrew/test/download_strategies_spec.rb index d78ae2437c..3dcd3fee55 100644 --- a/Library/Homebrew/test/download_strategies_spec.rb +++ b/Library/Homebrew/test/download_strategies_spec.rb @@ -239,7 +239,7 @@ describe CurlDownloadStrategy do let(:resource) { double(Resource, url: url, mirrors: [], specs: { user: "download:123456" }) } it "parses the opts and sets the corresponding args" do - expect(subject.send(:_curl_opts)).to eq(["--user", "download:123456"]) + expect(subject.send(:_curl_args)).to eq(["--user", "download:123456"]) end describe "#tarball_path" do From 64bb92d95d93494e6041d1b119a91d8a945c4477 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Thu, 2 Aug 2018 15:41:44 +0200 Subject: [PATCH 2/2] Use `URI.encode_www_form` instead of `CGI.escape`. --- Library/Homebrew/download_strategy.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index f16ee9ad92..ab5fd6543e 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -1,4 +1,3 @@ -require "cgi" require "json" require "rexml/document" require "time" @@ -268,7 +267,7 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy args = [] if meta.key?(:cookies) - escape_cookie = ->(k, v) { "#{CGI.escape(k.to_s)}=#{CGI.escape(v.to_s)}" } + escape_cookie = ->(cookie) { URI.encode_www_form([cookie]) } args += ["-b", meta.fetch(:cookies).map(&escape_cookie).join(";")] end @@ -343,7 +342,7 @@ end class CurlPostDownloadStrategy < CurlDownloadStrategy def _fetch base_url, data = if meta.key?(:data) - escape_data = ->(k, v) { ["-d", "#{CGI.escape(k.to_s)}=#{CGI.escape(v.to_s)}"] } + escape_data = ->(d) { ["-d", URI.encode_www_form([d])] } [@url, meta[:data].flat_map(&escape_data)] else @url.split("?", 2)