RUBY-3838 Fix raise_on_invalid handling and protect SRV monitor thread

Author: comandeo-mongo

lib/mongo/srv/monitor.rb

@@ -45,7 +45,10 @@ def initialize(cluster, **opts)
         raise ArgumentError, 'SRV URI is required' unless @srv_uri = opts.delete(:srv_uri)
 
         @options = opts.freeze
-        @resolver = Srv::Resolver.new(**opts)
+        # Per the polling-srv-records spec, mismatched-domain records and empty
+        # results MUST be logged and ignored, not raised. Force the polling
+        # resolver into log-and-continue mode regardless of caller options.
+        @resolver = Srv::Resolver.new(**opts, raise_on_invalid: false)
         @last_result = @srv_uri.srv_result
         @stop_semaphore = Semaphore.new
       end
@@ -82,6 +85,12 @@ def scan!
         rescue Resolv::ResolvError => e
           log_warn("SRV monitor: unable to resolve hostname #{@srv_uri.query_hostname}: #{e.class}: #{e}")
           return
+        rescue Mongo::Error => e
+          # Defense in depth: the polling resolver is configured to log-and-continue
+          # for mismatched-domain and empty-result cases, but if any Mongo::Error
+          # leaks out we MUST NOT let it terminate the SRV monitor thread.
+          log_warn("SRV monitor: error resolving hostname #{@srv_uri.query_hostname}: #{e.class}: #{e}")
+          return
         end
 
         if last_result.empty?

lib/mongo/srv/resolver.rb

@@ -142,7 +142,7 @@ def get_txt_options_string(hostname)
       #
       # @return [ Boolean ] Whether an error should be raised.
       def raise_on_invalid?
-        @raise_on_invalid ||= @options[:raise_on_invalid] || true
+        @options.fetch(:raise_on_invalid, true)
       end
     end
   end

lib/mongo/uri/srv_protocol.rb

@@ -117,14 +117,14 @@ def raise_invalid_error!(details)
         raise Error::InvalidURI.new(@string, details, FORMAT)
       end
 
-      # Gets the SRV resolver.
-      # If domain verification fails or no SRV records are found,
-      # an error must not be raised per the spec; instead, a warning is logged.
+      # Gets the SRV resolver used for initial URI parsing.
+      # Per the Initial DNS Seedlist Discovery spec, the driver MUST raise an
+      # error if domain verification fails or no SRV records are found, so
+      # raise_on_invalid is left at its default of true.
       #
       # @return [ Mongo::Srv::Resolver ]
       def resolver
         @resolver ||= Srv::Resolver.new(
-          raise_on_invalid: false,
           resolv_options: options[:resolv_options],
           timeout: options[:connect_timeout]
         )

spec/mongo/srv/monitor_spec.rb

@@ -229,6 +229,32 @@
           expect(cluster.addresses.map(&:to_s).sort).to eq(hosts.sort)
         end
       end
+
+      context 'when the resolver raises Mongo::Error::MismatchedDomain' do
+        let(:resolver) do
+          double('resolver').tap do |resolver|
+            allow(resolver).to receive(:get_records)
+              .and_raise(Mongo::Error::MismatchedDomain, 'simulated mismatched domain')
+          end
+        end
+
+        it 'does not propagate the error and leaves the topology unchanged' do
+          expect(cluster.addresses.map(&:to_s).sort).to eq(hosts.sort)
+        end
+      end
+
+      context 'when the resolver raises Mongo::Error::NoSRVRecords' do
+        let(:resolver) do
+          double('resolver').tap do |resolver|
+            allow(resolver).to receive(:get_records)
+              .and_raise(Mongo::Error::NoSRVRecords, 'simulated no records')
+          end
+        end
+
+        it 'does not propagate the error and leaves the topology unchanged' do
+          expect(cluster.addresses.map(&:to_s).sort).to eq(hosts.sort)
+        end
+      end
     end
 
     context 'when srv_max_hosts is specified' do

spec/mongo/srv/resolver_spec.rb

@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+require 'lite_spec_helper'
+
+describe Mongo::Srv::Resolver do
+  describe '#get_records' do
+    let(:hostname) { 'foo.example.com' }
+    let(:srv_query) { '_mongodb._tcp.' + hostname }
+
+    let(:mismatched_record) do
+      double('record').tap do |record|
+        allow(record).to receive(:target).and_return('evil.attacker.tld')
+        allow(record).to receive(:port).and_return(27_017)
+        allow(record).to receive(:ttl).and_return(1)
+      end
+    end
+
+    let(:dns_resolver) do
+      instance_double(Resolv::DNS).tap do |dns|
+        allow(dns).to receive(:timeouts=)
+      end
+    end
+
+    before do
+      allow(Resolv::DNS).to receive(:new).and_return(dns_resolver)
+    end
+
+    context 'when raise_on_invalid is false and a mismatched-domain record is returned' do
+      let(:resolver) { described_class.new(raise_on_invalid: false) }
+
+      before do
+        allow(dns_resolver).to receive(:getresources)
+          .with(srv_query, Resolv::DNS::Resource::IN::SRV)
+          .and_return([ mismatched_record ])
+      end
+
+      it 'does not raise and logs a warning' do
+        expect(resolver).to receive(:log_warn).at_least(:once)
+        expect { resolver.get_records(hostname) }.not_to raise_error
+      end
+    end
+
+    context 'when raise_on_invalid is false and no records are returned' do
+      let(:resolver) { described_class.new(raise_on_invalid: false) }
+
+      before do
+        allow(dns_resolver).to receive(:getresources)
+          .with(srv_query, Resolv::DNS::Resource::IN::SRV)
+          .and_return([])
+      end
+
+      it 'does not raise NoSRVRecords' do
+        allow(resolver).to receive(:log_warn)
+        expect { resolver.get_records(hostname) }.not_to raise_error
+      end
+    end
+
+    context 'when raise_on_invalid is true (default) and a mismatched-domain record is returned' do
+      let(:resolver) { described_class.new }
+
+      before do
+        allow(dns_resolver).to receive(:getresources)
+          .with(srv_query, Resolv::DNS::Resource::IN::SRV)
+          .and_return([ mismatched_record ])
+      end
+
+      it 'raises MismatchedDomain' do
+        expect { resolver.get_records(hostname) }
+          .to raise_error(Mongo::Error::MismatchedDomain)
+      end
+    end
+
+    context 'when raise_on_invalid is true (default) and no records are returned' do
+      let(:resolver) { described_class.new }
+
+      before do
+        allow(dns_resolver).to receive(:getresources)
+          .with(srv_query, Resolv::DNS::Resource::IN::SRV)
+          .and_return([])
+      end
+
+      it 'raises NoSRVRecords' do
+        expect { resolver.get_records(hostname) }
+          .to raise_error(Mongo::Error::NoSRVRecords)
+      end
+    end
+  end
+end