Test for invalid URL strings as well as bad schemes in ReferralChaser

Author: Dave Sims

lib/github/ldap/referral_chaser.rb

@@ -53,11 +53,13 @@ def search(options)
         unless referral_entries.empty?
           entry = referral_entries.first
           referral_string = entry[:search_referrals].first
-          referral = Referral.new(referral_string, admin_user, admin_password, port)
-          search_results = referral.search(options)
+          if GitHub::Ldap::URL.valid?(referral_string)
+            referral = Referral.new(referral_string, admin_user, admin_password, port)
+            search_results = referral.search(options)
+          end
         end
 
-        search_results
+        Array(search_results)
       end
 
       private

lib/github/ldap/url.rb

@@ -49,16 +49,20 @@ class URL
       #      ldap://dc4.ghe.local:456/CN=Maggie,DC=dc4,DC=ghe,DC=local?cn,mail?base?(cn=Charlie)
       #
       def initialize(url_string)
-        @uri = URI(url_string)
-        unless ["ldap", "ldaps"].include?(@uri.scheme)
-          raise InvalidSchemeException.new("Invalid scheme: #{@uri.scheme}")
+        if !self.class.valid?(url_string)
+          raise InvalidLdapURLException.new("Invalid LDAP URL: #{url_string}")
         end
+        @uri = URI(url_string)
         @dn = URI.unescape(@uri.path.sub(/^\//, ""))
         if @uri.query
           @attributes, @scope, @filter = @uri.query.split("?")
         end
       end
 
+      def self.valid?(url_string)
+        url_string =~ URI::regexp && ["ldap", "ldaps"].include?(URI(url_string).scheme)
+      end
+
       # Maps the returned scope value from the URL to one of Net::LDAP::Scopes
       #
       # The URL scope value can be one of:
@@ -76,7 +80,7 @@ def net_ldap_scope
         Net::LDAP::SearchScopes[SCOPES[scope]]
       end
 
-      class InvalidSchemeException < Exception; end
+      class InvalidLdapURLException < Exception; end
     end
   end
 end

test/referral_chaser_test.rb

@@ -8,13 +8,13 @@ def setup
   end
 
   def test_creates_referral_with_connection_credentials
-    @ldap.expects(:search).yields({ search_referrals: ["referral string"]}).returns([])
+    @ldap.expects(:search).yields({ search_referrals: ["ldap://dc1.ghe.local/"]}).returns([])
 
     referral = mock("GitHub::Ldap::ReferralChaser::Referral")
     referral.stubs(:search).returns([])
 
     GitHub::Ldap::ReferralChaser::Referral.expects(:new)
-      .with("referral string", "uid=admin,dc=github,dc=com", "passworD1", options[:port])
+      .with("ldap://dc1.ghe.local/", "uid=admin,dc=github,dc=com", "passworD1", options[:port])
       .returns(referral)
 
     @chaser.search({})
@@ -81,6 +81,13 @@ def test_returns_referral_search_results
     assert_equal(["result", "result"], results)
   end
 
+  def test_handle_blank_url_string_in_referral
+    @mock_connection.expects(:search).yields({ search_referrals: [""] })
+
+    results = @chaser.search({})
+    assert_equal([], results)
+  end
+
   def test_returns_referral_search_results
     @ldap.expects(:search).yields({ foo: ["not a referral"] })
 

test/url_test.rb

@@ -35,10 +35,17 @@ def test_simple_url
   end
 
   def test_invalid_scheme
-    ex = assert_raises(GitHub::Ldap::URL::InvalidSchemeException) do
+    ex = assert_raises(GitHub::Ldap::URL::InvalidLdapURLException) do
       GitHub::Ldap::URL.new("http://dc4.ghe.local")
     end
-    assert_equal("Invalid scheme: http", ex.message)
+    assert_equal("Invalid LDAP URL: http://dc4.ghe.local", ex.message)
+  end
+
+  def test_invalid_url
+    ex = assert_raises(GitHub::Ldap::URL::InvalidLdapURLException) do
+      GitHub::Ldap::URL.new("not a url")
+    end
+    assert_equal("Invalid LDAP URL: not a url", ex.message)
   end
 
   def test_parse_dn