JAVA-6235: Configurable DNS domain validation for SRV records by apmasell · Pull Request #2054 · mongodb/mongo-java-driver

apmasell · 2026-09-15T17:56:24Z

Add support for srvAllowedHostsSuffix using the public suffix list as a deny list as a safeguard and add docs explaining the use of this parameter (and its potential security implications when used).

Add a MongoClient-level `srvAllowedHostsSuffix` option that overrides the domain inferred from the SRV host name when validating the hosts returned by an SRV lookup. The value is normalized at configuration time (a leading "." is prepended if absent, so the stored and returned value always begins with "."), and every resolved host must end with it, matched case-insensitively; otherwise the existing inferred-domain validation is unchanged. The option is parsed from the connection string, carried on ClusterSettings, rejected on non-mongodb+srv URIs, rejected when it contains whitespace, no domain label, or an empty domain label, and threaded through the internal DNS resolution path. It is supported with both multi-server and load-balanced SRV connection strings, which now also apply srvServiceName consistently. JAVA-6233

Copilot

🟡 Changes recommended

Case, IDN, and platform-charset handling currently permit public-suffix deny-list bypasses.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds configurable SRV hostname-suffix validation across connection parsing, cluster configuration, DNS resolution, and monitoring.

Changes:

Adds srvAllowedHostsSuffix to public configuration APIs.
Propagates and enforces the suffix during SRV discovery.
Adds unit, functional, and prose-test coverage.

File summaries

File	Description
`driver-sync/src/test/functional/com/mongodb/client/InitialDnsSeedlistDiscoveryTest.java`	Supports the new option in functional assertions.
`driver-core/src/test/unit/com/mongodb/internal/dns/DefaultDnsResolverTest.java`	Tests suffix-based DNS validation.
`driver-core/src/test/unit/com/mongodb/internal/connection/SrvPollingProseTests.java`	Updates resolver and monitor signatures.
`driver-core/src/test/unit/com/mongodb/internal/connection/LoadBalancedClusterTest.java`	Updates monitor factory mocks.
`driver-core/src/test/unit/com/mongodb/internal/connection/InitialDnsSeedListDiscoveryProseTest.java`	Tests configured suffix overrides.
`driver-core/src/test/unit/com/mongodb/internal/connection/DnsMultiServerClusterSpecification.groovy`	Updates factory implementation signature.
`driver-core/src/test/unit/com/mongodb/internal/connection/DefaultDnsSrvRecordMonitorSpecification.groovy`	Updates monitor tests for suffix propagation.
`driver-core/src/test/unit/com/mongodb/ConnectionStringUnitTest.java`	Tests parsing and validation.
`driver-core/src/test/unit/com/mongodb/connection/ClusterSettingsSpecification.groovy`	Tests builder and URI application.
`driver-core/src/test/unit/com/mongodb/AbstractConnectionStringTest.java`	Supports the option in shared tests.
`driver-core/src/main/com/mongodb/internal/dns/DnsResolver.java`	Extends SRV resolution contract.
`driver-core/src/main/com/mongodb/internal/dns/DefaultDnsResolver.java`	Enforces suffix validation.
`driver-core/src/main/com/mongodb/internal/connection/LoadBalancedCluster.java`	Propagates suffix for load-balanced clusters.
`driver-core/src/main/com/mongodb/internal/connection/DomainNameUtils.java`	Normalizes suffixes and checks the PSL.
`driver-core/src/main/com/mongodb/internal/connection/DnsSrvRecordMonitorFactory.java`	Extends monitor factory contract.
`driver-core/src/main/com/mongodb/internal/connection/DnsMultiServerCluster.java`	Propagates suffix for multi-server clusters.
`driver-core/src/main/com/mongodb/internal/connection/DefaultDnsSrvRecordMonitorFactory.java`	Passes suffix into monitors.
`driver-core/src/main/com/mongodb/internal/connection/DefaultDnsSrvRecordMonitor.java`	Applies suffix during polling.
`driver-core/src/main/com/mongodb/ConnectionString.java`	Adds URI parsing and public accessor.
`driver-core/src/main/com/mongodb/connection/ClusterSettings.java`	Adds builder and settings support.

Review details

Suppressed comments (2)

driver-core/src/main/com/mongodb/internal/connection/DomainNameUtils.java:79

The bundled PSL is UTF-8 and contains non-ASCII rules such as 公司.cn, but this constructor decodes it with the JVM default charset. On non-UTF-8 Java 8 environments those rules are corrupted, so equivalent Unicode public suffixes can bypass the deny list. Specify UTF-8 explicitly.

        try (Scanner scanner = new Scanner(Objects.requireNonNull(
                DomainNameUtils.class.getResourceAsStream("public_suffix_list.dat"), "Missing DNS suffix list"))) {

driver-core/src/main/com/mongodb/internal/connection/DomainNameUtils.java:63

This condition rejects multi-label and private public-suffix-list entries as well as actual top-level domains, so this message is misleading for inputs such as .co.uk. Describe the rejected value as a public suffix and update the assertions that currently expect this text.

            throw new IllegalArgumentException("srvAllowedHostsSuffix must not be a top-level domain");

Files reviewed: 20/21 changed files
Comments generated: 3
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+                throw new IllegalArgumentException("srvAllowedHostsSuffix must not contain empty domain labels");
+            }
+        }
+        if (isTopLevelDomain(hasLeadingDot ? srvAllowedHostsSuffix.substring(1) : srvAllowedHostsSuffix)) {


+        ConnectionString first = new ConnectionString("mongodb+srv://test12.test.build.10gen.cc/?srvAllowedHostsSuffix=.build.10gen.cc");
+        ConnectionString second = new ConnectionString("mongodb+srv://test12.test.build.10gen.cc/?srvAllowedHostsSuffix=.other.10gen.cc");
+        assertNotEquals(first, second);
+        assertNotEquals(first.hashCode(), second.hashCode());


Copilot

🟡 Changes recommended

Case-sensitive public-suffix validation permits uppercase deny-list bypasses, and the API documentation contradicts implemented restrictions.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

driver-core/src/main/com/mongodb/internal/connection/DomainNameUtils.java:62

The public-suffix lookup is case-sensitive even though DNS suffixes are case-insensitive. A URI using .COM bypasses this safeguard, while the resolver later accepts .COM against any .com target. Normalize the value for the PSL lookup so uppercase variants are rejected too.

        if (isTopLevelDomain(hasLeadingDot ? srvAllowedHostsSuffix.substring(1) : srvAllowedHostsSuffix)) {

driver-core/src/main/com/mongodb/connection/ClusterSettings.java:199

The public builder documentation says a bare TLD is accepted at the caller's risk, but normalizeSrvAllowedHostsSuffix rejects it (and every other public suffix). This makes the documented API contract disagree with actual behavior.

         * used with SRV. Specifying an overly broad suffix (for example a bare TLD) weakens SRV host name validation and
         * is the responsibility of the caller.</p>

Files reviewed: 20/21 changed files
Comments generated: 2
Review effort level: Balanced

+            }
+        }
+        if (isTopLevelDomain(hasLeadingDot ? srvAllowedHostsSuffix.substring(1) : srvAllowedHostsSuffix)) {
+            throw new IllegalArgumentException("srvAllowedHostsSuffix must not be a top-level domain");


+     * is stored and returned to callers. The suffix must contain at least one non-empty domain label and no
+     * whitespace; an overly broad suffix is permitted and is the caller's responsibility.


nhachicha

Thanks for taking this over! The implementation in general is ok, a couple of things left:

1. The official spec tests never run. The submodule isn't bumped.
1. public_suffix_list.dat The PSL spec says drivers MUST obtain it from the specifications repo, where it's pre-stripped of comments (partially my fault since I pointed you to mongodb/mongo-python-driver#2903 instead of mongodb/mongo-python-driver#2868 which contains the correct file)
1. Claude /driver-code-review caught some additional points, please assess and pushback / ignore if they're irrelevant

Claude Opus (1M Context) review

🔴 Blocking

DOMAIN_PATTERN widening is a regression. Loosening the TLD class from [a-zA-Z]{2,63} to [a-zA-Z0-9\-]{2,63} to admit xn-- labels changes a shared method. xn--frosch-6ya.w23 is an existing negative case in DomainNameUtilsTest, so that test now fails; and SocksSocket:135 uses isDomainName for SOCKS5 proxy hosts, which now accepts foo.123, foo.--, evil.-a-. Suggest validating with the original pattern before IDN conversion, or special-casing the xn-- prefix.
MPL-2.0 attribution missing. The vendored list ships in the Apache-2.0 driver-core jar; neither THIRD-PARTY-NOTICES nor sbom.json is updated. Needs a notice entry and legal/human sign-off.

🟡 Important

Trailing/leading dot not stripped (spec step 1). .build.10gen.cc. currently throws; the spec requires stripping, and srvAllowedHostsSuffix-trailing-dot expects success. The current test enshrines the wrong behaviour.
isPublicSuffix isn't the publicsuffix.org algorithm. The invalidMatchWildcard latch converts to return true on the next ordinary line and is never reset, so results depend on upstream line ordering (it works only because !www.ck sits immediately after *.ck). Suggest a rule index + the documented algorithm: exceptions win, then longest match.
Per-call cost got worse. IDN.toASCII now runs on every rule line — ~16.4k conversions per validation, doubled because ConnectionString and ClusterSettings.Builder both normalize. An ASCII fast-path plus a lazily-built index fixes both.
Misleading error messages. .com fails with "is not a valid domain" (it trips isDomainName as a single label) rather than the public-suffix message; for bare TLDs the PSL check is effectively unreachable.
ClusterSettings.Builder javadoc still contradicts the code (Copilot's comment, unaddressed) — says a bare TLD is the caller's responsibility, but public suffixes are rejected. The ConnectionString option docs also carry no security warning, which is the path most users take.
New Spock tests in ClusterSettingsSpecification.groovy — .agents/references/testing-guide.md: "Do not add new Spock tests." Mechanical signature updates are fine; the new blocks should be JUnit 5.
No direct test for the reworked matcher. DomainNameUtilsTest already exists — add uppercase, multi-label public suffixes, wildcard/exception ordering, U-label vs A-label, and missing-resource cases.
Out-of-scope behaviour changes. applySrvConnectionStringOptions now applies srvServiceName on the LOAD_BALANCED path for the first time (genuine fix, but unrelated), and the unwrapped MongoConfigurationException rethrow changes messages on the pre-existing inferred-domain path. Please split or call out in the description.
ClusterSettings.build() doesn't reject the option without SRV, unlike the URI path.

🟢 Nit

assertNotEquals(first.hashCode(), second.hashCode()) asserts more than the hashCode contract (Copilot's comment, unaddressed).
DOMAIN_PATTERN allows localhost, so srvAllowedHostsSuffix=localhost slips past the ≥2-label rule.
Duplicated comment on the two wildcard tests — inaccurate on the first.
Stray double blank line in DefaultDnsResolverTest; over-length lines in LoadBalancedClusterTest.
Builder.srvAllowedHostsSuffix can't be reset to null; field lacks @Nullable though the getter has it.

nhachicha · 2026-09-17T11:05:21Z

+     * always begins with {@code "."}.</p>
+     *
+     * @return the normalized SRV allowed hosts suffix, always beginning with {@code "."}. Defaults to null.
+     * @since 5.9


The feature will probably be released in 5.13 (next minor)

Suggested change

* @since 5.9

* @since 5.13

nhachicha · 2026-09-17T11:42:25Z

+        if (labels.isEmpty()) {
+            throw new IllegalArgumentException("srvAllowedHostsSuffix must contain at least one domain label");
+        }
+        String[] parts = labels.split("\\.", -1);


I believe this is redundant since the check below isDomainName rejects "build..10gen.cc" you can keep it if you think this gives more clarity (specific error message)

nhachicha · 2026-09-17T11:45:16Z

+    private static boolean isPublicSuffix(final String suffix) {
+        try (Scanner scanner = new Scanner(
+                Objects.requireNonNull(
+                        DomainNameUtils.class.getResourceAsStream("public_suffix_list.dat"), "Missing DNS suffix list"),


Suggested change

DomainNameUtils.class.getResourceAsStream("public_suffix_list.dat"), "Missing DNS suffix list"),

DomainNameUtils.class.getResourceAsStream("public_suffix_list.dat"), "Missing DNS public suffix list"),

nhachicha · 2026-09-17T11:56:49Z

+    private static boolean isPublicSuffix(final String suffix) {
+        try (Scanner scanner = new Scanner(
+                Objects.requireNonNull(
+                        DomainNameUtils.class.getResourceAsStream("public_suffix_list.dat"), "Missing DNS suffix list"),


Caught by Claude:

GraalVM: public_suffix_list.dat isn't registered as a resource

Native image drops unregistered classpath resources, so getResourceAsStream("public_suffix_list.dat") will NPE for anyone setting srvAllowedHostsSuffix. We hand-maintain this metadata. Add to driver-core/src/main/resources/META-INF/native-image/resource-config.json:

{ "pattern":"\\Qcom/mongodb/internal/connection/public_suffix_list.dat\\E" }

Note graalvm-native-image-app-task passes either way today — missing resources fail at runtime on access, not at build, and NativeImageApp never hits this path. Worth adding a small component beside DnsSpi that builds a ConnectionString with the option set (no server needed) so CI can actually catch this.

nhachicha · 2026-09-17T12:00:42Z

+            boolean invalidMatchWildcard = false;
+            while (scanner.hasNextLine()) {
+                String line = scanner.nextLine();
+                if (line.startsWith("//") || line.isEmpty()) {


You shouldn't need this logic if we're using the public_suffix_list.data provisioned from spec

Drivers SHOULD NOT fetch the list from the network at runtime, and SHOULD instead resolve it from a copy shipped with the driver.

nhachicha · 2026-09-17T12:48:42Z

+         *
+         * @param srvAllowedHostsSuffix the SRV allowed hosts suffix; may not be null or empty
+         * @return this
+         * @since 5.9


Suggested change

* @since 5.9

* @since 5.13

nhachicha · 2026-09-17T13:07:18Z

+    @Test
+    public void shouldThrowWhenAnyResolvedHostDoesNotEndWithSrvAllowedHostsSuffix() {
+        DefaultDnsResolver resolver = resolverReturning("ok.build.10gen.cc.", "bad.evil.example.com.");
+        assertThrows(MongoConfigurationException.class,


should we assert the message include bad.evil.example.com.? Making sure the exception include it for investigation/Root Cause Analysis...

rozza and others added 2 commits September 14, 2026 15:51

fixup: check TLD list

14542d5

apmasell requested a review from nhachicha September 15, 2026 17:56

apmasell requested a review from a team as a code owner September 15, 2026 17:56

apmasell added 2 commits September 15, 2026 15:43

fixup

ed6a147

style

28ef6b9

nhachicha requested a balanced review from Copilot September 16, 2026 16:05

nhachicha assigned apmasell Sep 16, 2026

Copilot started reviewing on behalf of nhachicha September 16, 2026 16:05 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

nhachicha requested a balanced review from Copilot September 16, 2026 17:51

Copilot started reviewing on behalf of nhachicha September 16, 2026 17:51 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

apmasell added 6 commits September 16, 2026 14:39

fixup:idn

061d7e9

fixup:idn

8a4afe9

fixup:idn

ad44f13

fixup:idn

46628ce

fixup:idn

21d5248

fixup:idn

c034d01

nhachicha requested changes Sep 17, 2026

View reviewed changes

Navigation Menu

JAVA-6235: Configurable DNS domain validation for SRV records - #2054

JAVA-6235: Configurable DNS domain validation for SRV records#2054
apmasell wants to merge 10 commits into
mongodb:mainfrom
apmasell:JAVA-6235

apmasell commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

nhachicha left a comment

Uh oh!

nhachicha Sep 17, 2026 •

edited

Loading

Uh oh!

nhachicha Sep 17, 2026

Uh oh!

nhachicha Sep 17, 2026

Uh oh!

nhachicha Sep 17, 2026

Uh oh!

nhachicha Sep 17, 2026

Uh oh!

nhachicha Sep 17, 2026 •

edited

Loading

Uh oh!

nhachicha Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		* is stored and returned to callers. The suffix must contain at least one non-empty domain label and no
		* whitespace; an overly broad suffix is permitted and is the caller's responsibility.

	DomainNameUtils.class.getResourceAsStream("public_suffix_list.dat"), "Missing DNS suffix list"),
	DomainNameUtils.class.getResourceAsStream("public_suffix_list.dat"), "Missing DNS public suffix list"),

Conversation

apmasell commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

nhachicha left a comment

Choose a reason for hiding this comment

🔴 Blocking

🟡 Important

🟢 Nit

Uh oh!

nhachicha Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nhachicha Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

nhachicha Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

nhachicha Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

nhachicha Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

nhachicha Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nhachicha Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nhachicha Sep 17, 2026 •

edited

Loading

nhachicha Sep 17, 2026 •

edited

Loading