Skip to content

Commit ec104ed

Browse files
committed
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.
1 parent 3cb9766 commit ec104ed

File tree

2 files changed

+27
-3
lines changed

2 files changed

+27
-3
lines changed

lib/sshkit/host.rb

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ module SSHKit
66
UnparsableHostStringError = Class.new(SSHKit::StandardError)
77

88
class Host
9+
DEFAULT_SSH_PORT = 22
910

1011
attr_accessor :password, :hostname, :port, :user, :ssh_options
1112

@@ -69,10 +70,11 @@ def username
6970
end
7071

7172
def eql?(other_host)
72-
other_host.hash == hash
73+
user == other_host.user &&
74+
hostname == other_host.hostname &&
75+
port_with_default == other_host.send(:port_with_default)
7376
end
7477
alias :== :eql?
75-
alias :equal? :eql?
7678

7779
def to_s
7880
hostname
@@ -93,6 +95,12 @@ def properties
9395
@properties ||= OpenStruct.new
9496
end
9597

98+
private
99+
100+
def port_with_default
101+
port || DEFAULT_SSH_PORT
102+
end
103+
96104
end
97105

98106
# @private

test/unit/test_host.rb

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,23 @@ def test_assert_hosts_hash_equally
6767
def test_assert_hosts_compare_equal
6868
assert Host.new('example.com') == Host.new('example.com')
6969
assert Host.new('example.com').eql? Host.new('example.com')
70-
assert Host.new('example.com').equal? Host.new('example.com')
70+
71+
assert Host.new('example.com:22') == Host.new('example.com')
72+
assert Host.new('example.com:22').eql? Host.new('example.com')
73+
74+
assert Host.new('example.com:22') != Host.new('example.com:23')
75+
assert !Host.new('example.com:22').eql?(Host.new('example.com:23'))
76+
77+
assert Host.new('[email protected]') == Host.new('[email protected]')
78+
assert Host.new('[email protected]').eql? Host.new('[email protected]')
79+
80+
assert Host.new('[email protected]') != Host.new('[email protected]')
81+
assert !Host.new('[email protected]').eql?(Host.new('[email protected]'))
82+
83+
a = Host.new('example.com')
84+
b = Host.new('example.com')
85+
assert a.equal? a
86+
assert !a.equal?(b)
7187
end
7288

7389
def test_arbitrary_host_properties

0 commit comments

Comments
 (0)