Handle invalid names in download strategies

When subformulae are initialized without a name parameter, Homebrew
assigns the name "__UNKNOWN__". This may cause collisions in the cache.

Currently CurlDownloadStrategy and its descendants handles this by
extracting the basename form the URL and using that as the cached
filename. However, other strategies simply raise an exception.

We can improve the other strategies by URL-encoding the URL string and
using that as the cached directory name.

Note that this happens very rarely, especially now that resources (which
always have a name) are preferred to subformulae. The most common case
is a subformula that specifies a head download.

Closes Homebrew/homebrew#22949.

1 files changed, 29 insertions, 3 deletions

diff --git a/Library/Homebrew/test/test_download_strategies.rb b/Library/Homebrew/test/test_download_strategies.rb
index 3a330e797..cdcb92dc9 100644
--- a/Library/Homebrew/test/test_download_strategies.rb
+++ b/Library/Homebrew/test/test_download_strategies.rb
@@ -2,7 +2,7 @@ require 'testing_env'
 require 'download_strategy'
 require 'bottles' # XXX: hoist these regexps into constants in Pathname?
 
-class SoftwareSpecDouble
+class ResourceDouble
   attr_reader :url, :specs
 
   def initialize(url="http://foo.com/bar.tar.gz", specs={})
@@ -14,8 +14,8 @@ end
 class AbstractDownloadStrategyTests < Test::Unit::TestCase
   def setup
     @name = "foo"
-    @package = SoftwareSpecDouble.new
-    @strategy = AbstractDownloadStrategy.new(@name, @package)
+    @resource = ResourceDouble.new
+    @strategy = AbstractDownloadStrategy.new(@name, @resource)
     @args = %w{foo bar baz}
   end
 
@@ -37,6 +37,32 @@ class AbstractDownloadStrategyTests < Test::Unit::TestCase
   end
 end
 
+class DownloadStrategyCheckoutNameTests < Test::Unit::TestCase
+  def setup
+    @resource = ResourceDouble.new("http://foo.com/bar")
+    @strategy = AbstractDownloadStrategy
+  end
+
+  def escaped(tag)
+    "#{ERB::Util.url_encode(@resource.url)}--#{tag}"
+  end
+
+  def test_explicit_name
+    downloader = @strategy.new("baz", @resource)
+    assert_equal "baz--foo", downloader.checkout_name("foo")
+  end
+
+  def test_empty_name
+    downloader = @strategy.new("", @resource)
+    assert_equal escaped("foo"), downloader.checkout_name("foo")
+  end
+
+  def test_unknown_name
+    downloader = @strategy.new("__UNKNOWN__", @resource)
+    assert_equal escaped("foo"), downloader.checkout_name("foo")
+  end
+end
+
 class DownloadStrategyDetectorTests < Test::Unit::TestCase
   def setup
     @d = DownloadStrategyDetector.new