From f2b4474126c131d3e5c3675a46cd8b8bf01b7cd8 Mon Sep 17 00:00:00 2001 From: "Hongli Lai (Phusion)" Date: Sat, 27 Dec 2014 09:13:36 +0100 Subject: [PATCH] Improve SSHKit::Host comparison * #equal? should test for object identity, not whether the contents are equal, as per the Ruby documentation. * #eql? and #== should check the contents directly instead of comparing the hash. This prevents false positives. --- lib/sshkit/host.rb | 13 +++++++++++-- test/unit/test_host.rb | 18 +++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/sshkit/host.rb b/lib/sshkit/host.rb index e38d02d0..d3987e0c 100644 --- a/lib/sshkit/host.rb +++ b/lib/sshkit/host.rb @@ -5,6 +5,7 @@ module SSHKit UnparsableHostStringError = Class.new(SSHKit::StandardError) class Host + DEFAULT_SSH_PORT = 22 attr_accessor :password, :hostname, :port, :user, :ssh_options @@ -60,10 +61,12 @@ def username end def eql?(other_host) - other_host.hash == hash + other_host && + user == other_host.user && + hostname == other_host.hostname && + port_with_default == other_host.send(:port_with_default) end alias :== :eql? - alias :equal? :eql? def to_s hostname @@ -84,6 +87,12 @@ def properties @properties ||= OpenStruct.new end + private + + def port_with_default + port || DEFAULT_SSH_PORT + end + end # @private diff --git a/test/unit/test_host.rb b/test/unit/test_host.rb index cf3621bd..930b064a 100644 --- a/test/unit/test_host.rb +++ b/test/unit/test_host.rb @@ -58,7 +58,23 @@ def test_assert_hosts_hash_equally def test_assert_hosts_compare_equal assert Host.new('example.com') == Host.new('example.com') assert Host.new('example.com').eql? Host.new('example.com') - assert Host.new('example.com').equal? Host.new('example.com') + + assert Host.new('example.com:22') == Host.new('example.com') + assert Host.new('example.com:22').eql? Host.new('example.com') + + assert Host.new('example.com:22') != Host.new('example.com:23') + assert !Host.new('example.com:22').eql?(Host.new('example.com:23')) + + assert Host.new('foo@example.com') == Host.new('foo@example.com') + assert Host.new('foo@example.com').eql? Host.new('foo@example.com') + + assert Host.new('foo@example.com') != Host.new('bar@example.com') + assert !Host.new('foo@example.com').eql?(Host.new('bar@example.com')) + + a = Host.new('example.com') + b = Host.new('example.com') + assert a.equal? a + assert !a.equal?(b) end def test_arbitrary_host_properties