Make Sparoid::Instance public-IP resolution thread-safe and self-healing (#25)

* Make Sparoid::Instance public-IP resolution thread-safe and empty-aware

Sparoid::Instance memoized with `@public_ips ||= super`, which is neither
thread-safe nor empty-aware: `[]` is truthy, so a transient empty resolution
(icanhazip down or empty body) was pinned for the process lifetime and every
later knock then sent zero SPA packets. Under a shared instance hit by a thread
pool, the first wave of knocks also resolved concurrently and one slow/empty
response clobbered a good value via last-writer-wins.

Move the caching into Instance#cached_public_ips: resolve behind a per-instance
mutex with a double-check (kills the thundering herd and the clobber) and cache
only a non-empty result, so a transient empty resolution self-heals on the next
knock instead of pinning the instance.

Also raise when generate_messages would otherwise produce no messages: an empty
public-IP resolution with no IPv6 fallback used to let auth report success
while sending zero packets. It now raises Sparoid::PublicIPError (a sibling of
ResolvError under Error) so the caller can tell 'I cannot resolve my own public
IP' apart from 'the target host is unreachable', instead of a silent no-op.

* Stub icanhazip in tests instead of allowing real network

Drop WebMock.allow_net_connect! and stub icanhazip in test_it_resolves_public_ip
so the suite no longer depends on a working internet connection or icanhazip
being up (which would make CI flaky and let mistyped stubs fall through to the
network).

* Initialize @public_ips to [] and drop safe-navigation checks

@public_ips is now always an array, so the empty/cached check is a plain
.any? instead of &.any?.

Author: oskgu360

Gemfile

@@ -11,3 +11,4 @@ gem "rake", "~> 13.0"
 gem "rubocop", "~> 1.7"
 gem "rubocop-minitest", require: false
 gem "rubocop-rake", require: false
+gem "webmock"

lib/sparoid.rb

@@ -103,6 +103,8 @@ def generate_messages(ip)
         ips = ips.grep_v(Resolv::IPv6)
         ips << Resolv::IPv6.create(native_ipv6)
       end
+      raise PublicIPError, "Sparoid could not resolve a public IP to knock from" if ips.empty?
+
       ips.map { |i| message(i) }
     end
   end
@@ -253,16 +255,31 @@ class Error < StandardError; end
 
   class ResolvError < Error; end
 
-  # Instance of SPAroid that only resolved public_ips once
+  class PublicIPError < Error; end
+
+  # Instance of SPAroid that resolves its public IP once and reuses it
   class Instance
     include Sparoid
 
-    def public_ips(*args)
-      @public_ips ||= super
+    def initialize
+      @resolve_mutex = Mutex.new
+      @public_ips = []
     end
 
     def cached_public_ips
-      public_ips
+      return @public_ips if @public_ips.any?
+
+      @resolve_mutex.synchronize do
+        return @public_ips if @public_ips.any?
+
+        ips = public_ips
+        if ips.empty?
+          warn "Sparoid: Failed to retrieve public IPs"
+        else
+          @public_ips = ips
+        end
+        ips
+      end
     end
   end
 end

test/sparoid_test.rb

@@ -8,6 +8,7 @@ def test_that_it_has_a_version_number
   end
 
   def test_it_resolves_public_ip
+    stub_icanhazip(ipv4_body: "203.0.113.7\n", ipv6_body: "2001:db8::1\n")
     addresses = Sparoid.send(:public_ips)
     assert(addresses.any? { |ip| ip.is_a?(Resolv::IPv4) || ip.is_a?(Resolv::IPv6) })
   end
@@ -228,6 +229,38 @@ def test_keygen_outputs_keys
     assert_match(/^hmac-key = [0-9a-f]{64}$/, output.lines[1].chomp)
   end
 
+  def stub_icanhazip(ipv4_body:, ipv6_body: "")
+    stub_request(:get, "http://ipv6.icanhazip.com/").to_return(status: 200, body: ipv6_body)
+    stub_request(:get, "http://ipv4.icanhazip.com/").to_return(status: 200, body: ipv4_body)
+  end
+
+  def test_instance_retries_and_recovers_instead_of_pinning_empty
+    instance = Sparoid::Instance.new
+
+    stub_icanhazip(ipv4_body: "", ipv6_body: "")
+    _out, err = capture_io { assert_empty instance.cached_public_ips }
+    assert_match(/Failed to retrieve public IPs/, err)
+    assert_empty instance.instance_variable_get(:@public_ips), "empty result must not be memoized"
+
+    stub_icanhazip(ipv4_body: "203.0.113.7\n")
+    assert_equal ["203.0.113.7"], instance.cached_public_ips.map(&:to_s)
+  end
+
+  def test_auth_raises_when_no_public_ip_resolved
+    key = "0000000000000000000000000000000000000000000000000000000000000000"
+    hmac_key = "0000000000000000000000000000000000000000000000000000000000000000"
+    s = Sparoid::Instance.new
+
+    s.stub(:cached_public_ips, []) do
+      s.stub(:public_ipv6_by_udp, nil) do
+        err = assert_raises(Sparoid::PublicIPError) do
+          s.auth(key, hmac_key, "127.0.0.1", 1337)
+        end
+        assert_match(/could not resolve a public IP/, err.message)
+      end
+    end
+  end
+
   def test_fdpass_closes_socket_when_connect_fails_synchronously
     addr = Addrinfo.tcp("127.0.0.1", 0)
     closed = false

test/test_helper.rb

@@ -9,3 +9,4 @@
 
 require "minitest/stub_const"
 require "minitest/autorun"
+require "webmock/minitest"