DRIVERS 2922 Allow valid SRV hostnames with less than 3 parts

[kyleburgess2025]

No description provided.

[Copilot]

Pull request overview

This PR removes the requirement for SRV hostnames to have at least 3 parts (hostname.domain.tld), allowing valid SRV hostnames with 1 or 2 parts. The change updates validation logic to accept hostnames like "example.com" or even "example" as valid SRV hostnames.

Key Changes:

• Relaxed SRV hostname validation from requiring minimum 3 parts to requiring at least 1 part
• Updated domain matching logic to handle SRV hostnames with fewer than 3 parts
• Removed test cases that expected errors for hostnames with fewer than 3 parts

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
spec/spec_tests/data/connection_string/invalid-uris.yml	Removed test case for invalid scheme
spec/mongo/uri/srv_protocol_spec.rb	Removed test cases expecting errors for 1-2 part hostnames and updated expectations to not raise errors
spec/mongo/srv/result_spec.rb	Added comprehensive test coverage for domain matching with 1, 2, and 3-part SRV hostnames
lib/mongo/uri/srv_protocol.rb	Updated validation to require minimum 1 component instead of 3, changed error message
lib/mongo/srv/result.rb	Updated domain matching logic to handle SRV hostnames with fewer than 3 parts

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kyleburgess2025]

Just removed this test, don't think this should throw under the new requirements.

[kyleburgess2025]

this should be valid under the new SRV requirements, right?

[jamis]

The new SRV requirements only apply to mongodb+srv schemes. Note the scheme for this test is mongo, which is not a supported scheme. (We only support mongodb:// and mongodb+srv:// -- which is what this test is ensuring.)

So, this test should remain unchanged -- and it should be passing. If it is failing with your changes, then you might have touched more than you intended...

[kyleburgess2025]

fixed! thanks!

[jamis]

Great job digging into this, and locating where the changes need to be made. You're on the right track. 👍

[jamis]

For efficiency's sake, I'd recommend splitting the query_hostname once and saving the parts, and then referring to them as needed.

Suggested change

	srv_is_less_than_three_parts = query_hostname.split('.').length < 3
	srv_host_domain = if srv_is_less_than_three_parts
	query_hostname.split('.')
	else
	query_hostname.split('.')[1..-1]
	end
	query_hostname_parts = query_hostname.split('.')
	srv_is_less_than_three_parts = query_hostname_parts.length < 3
	srv_host_domain = if srv_is_less_than_three_parts
	query_hostname_parts
	else
	query_hostname_parts[1..-1]
	end

[kyleburgess2025]

fixed!

[jamis]

It's a little more idiomatic to ask parts.empty? than to query on the length directly, but I can see an argument being made for the former here (since we're actually measuring the number of parts).

In that case, though, I think parts.length < 1 might be clearer, since that way we're making it explicit that the number of parts has a minimum of 1.

[kyleburgess2025]

fixed

[jamis]

❤️

[jamis]

When writing tests with rspec, the recommended way of setting up test data is to use let, e.g.

let(:record) do
  double('record').tap do |record|
    allow(record).to receive(:target).and_return(mismatched_host_name)
    allow(record).to receive(:port).and_return(42)
    allow(record).to receive(:ttl).and_return(1)
  end
end

let(:result) { described_class.new(srv_name) }

Then, the test itself reduces to just testing the expectations:

it 'raises MismatchedDomain error' do
  expect { result.add_record(record) }.to raise_error(Mongo::Error::MismatchedDomain)
end

This pattern is especially useful when the setup is repeated for multiple tests, but is still preferred even if it's only used in a single test.

[kyleburgess2025]

done!

[jamis]

The new SRV requirements only apply to mongodb+srv schemes. Note the scheme for this test is mongo, which is not a supported scheme. (We only support mongodb:// and mongodb+srv:// -- which is what this test is ensuring.)

So, this test should remain unchanged -- and it should be passing. If it is failing with your changes, then you might have touched more than you intended...