Raise when given an invalid download strategy spec

When DownloadStrategyDetector.detect is given a second argument, and
that argument is not a symbol or an AbstractDownloadStrategy subclass,
it is silently ignored, and we fall back to guessing the strategy based
on the URL.

This means I can do

  url 'http://foo.com/bar.tar.gz', :using => Class.new

and things will appear to work, even though I have clearly passed an
invalid value for :using.

A more useful behavior is to raise an exception for unknown strategy
specifications.

2 files changed, 13 insertions, 4 deletions

diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb
index 01ff12a71..df903c3bc 100644
--- a/Library/Homebrew/download_strategy.rb
+++ b/Library/Homebrew/download_strategy.rb
@@ -761,12 +761,15 @@ end
 
 class DownloadStrategyDetector
   def self.detect(url, strategy=nil)
-    if strategy.is_a? Class and strategy.ancestors.include? AbstractDownloadStrategy
-      strategy
-    elsif strategy.is_a? Symbol
+    if strategy.nil?
+      detect_from_url(url)
+    elsif Class === strategy && strategy < AbstractDownloadStrategy
+        strategy
+    elsif Symbol === strategy
       detect_from_symbol(strategy)
     else
-      detect_from_url(url)
+      raise TypeError,
+        "Unknown download strategy specification #{strategy.inspect}"
     end
   end
 
diff --git a/Library/Homebrew/test/test_download_strategies.rb b/Library/Homebrew/test/test_download_strategies.rb
index ef330de42..3a330e797 100644
--- a/Library/Homebrew/test/test_download_strategies.rb
+++ b/Library/Homebrew/test/test_download_strategies.rb
@@ -51,4 +51,10 @@ class DownloadStrategyDetectorTests < Test::Unit::TestCase
     @d = DownloadStrategyDetector.detect(Object.new)
     assert_equal CurlDownloadStrategy, @d
   end
+
+  def test_raises_when_passed_unrecognized_strategy
+    assert_raises(TypeError) do
+      DownloadStrategyDetector.detect("foo", Class.new)
+    end
+  end
 end