This is an automated email from the ASF dual-hosted git repository.
chibenwa pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
The following commit(s) were added to refs/heads/master by this push:
new 9eb0bd40d7 [FIX] DNSJavaService: Avoid mutating unmodifiable
collection and skipping fallback on resolution failure (#3211)
9eb0bd40d7 is described below
commit 9eb0bd40d7731a9d0ea8cc98c7888ca1318d7385
Author: ilya terskov <[email protected]>
AuthorDate: Thu Oct 1 16:25:38 2026 +0700
[FIX] DNSJavaService: Avoid mutating unmodifiable collection and skipping
fallback on resolution failure (#3211)
Fixes issues in DNSJavaService.findMXRecords:
- Avoid modifying unmodifiable collection or throwing
UnsupportedOperationException when raw MX records list is empty.
- Do not execute A fallback when TemporaryResolutionException is thrown by
raw MX lookup.
- Include unit and non-regression tests in DNSJavaServiceTest.
---
.../james/dnsservice/dnsjava/DNSJavaService.java | 31 ++++++++--------
.../dnsservice/dnsjava/DNSJavaServiceTest.java | 42 ++++++++++++++++++++++
2 files changed, 58 insertions(+), 15 deletions(-)
diff --git
a/server/dns-service/dnsservice-dnsjava/src/main/java/org/apache/james/dnsservice/dnsjava/DNSJavaService.java
b/server/dns-service/dnsservice-dnsjava/src/main/java/org/apache/james/dnsservice/dnsjava/DNSJavaService.java
index c366934c0c..e82f2ab6a8 100644
---
a/server/dns-service/dnsservice-dnsjava/src/main/java/org/apache/james/dnsservice/dnsjava/DNSJavaService.java
+++
b/server/dns-service/dnsservice-dnsjava/src/main/java/org/apache/james/dnsservice/dnsjava/DNSJavaService.java
@@ -315,25 +315,26 @@ public class DNSJavaService implements DNSService,
DNSServiceMBean, Configurable
@Override
public Collection<String> findMXRecords(String hostname) throws
TemporaryResolutionException {
TimeMetric timeMetric = metricFactory.timer("findMXRecords");
- List<String> servers = new ArrayList<>();
try {
- servers = findMXRecordsRaw(hostname);
- return Collections.unmodifiableCollection(servers);
- } finally {
+ List<String> servers = findMXRecordsRaw(hostname);
+ if (!servers.isEmpty()) {
+ return Collections.unmodifiableCollection(servers);
+ }
+
// If we found no results, we'll add the original domain name if
// it's a valid DNS entry
- if (servers.isEmpty()) {
- LOGGER.info("Couldn't resolve MX records for domain {}.",
hostname);
- try {
- getByName(hostname);
- servers.add(hostname);
- } catch (UnknownHostException uhe) {
- // The original domain name is not a valid host,
- // so we can't add it to the server list. In this
- // case we return an empty list of servers
- LOGGER.error("Couldn't resolve IP address for host {}.",
hostname, uhe);
- }
+ LOGGER.info("Couldn't resolve MX records for domain {}. Falling
back to A/AAAA resolution instead", hostname);
+ try {
+ getByName(hostname);
+ return ImmutableList.of(hostname);
+ } catch (UnknownHostException uhe) {
+ // The original domain name is not a valid host,
+ // so we can't add it to the server list. In this
+ // case we return an empty list of servers
+ LOGGER.info("Couldn't resolve IP address for host {}.",
hostname, uhe);
+ return ImmutableList.of();
}
+ } finally {
timeMetric.stopAndPublish();
}
}
diff --git
a/server/dns-service/dnsservice-dnsjava/src/test/java/org/apache/james/dnsservice/dnsjava/DNSJavaServiceTest.java
b/server/dns-service/dnsservice-dnsjava/src/test/java/org/apache/james/dnsservice/dnsjava/DNSJavaServiceTest.java
index fe3f041a48..b9024c4026 100644
---
a/server/dns-service/dnsservice-dnsjava/src/test/java/org/apache/james/dnsservice/dnsjava/DNSJavaServiceTest.java
+++
b/server/dns-service/dnsservice-dnsjava/src/test/java/org/apache/james/dnsservice/dnsjava/DNSJavaServiceTest.java
@@ -210,6 +210,48 @@ class DNSJavaServiceTest {
assertThat(records.size()).isEqualTo(1);
assertThat(records.contains("mx1.one-mx.bar.")).isTrue();
}
+
+ @Test
+ void testFindMXRecordsShouldReturnFallbackAddressWhenNoMX() throws
Exception {
+ doAnswer(new ZoneCacheLookupRecordsAnswer(loadZone("dnstest.com.")))
+ .when(mockedCache).lookupRecords(any(Name.class), anyInt(),
anyInt());
+ dnsServer.setCache(mockedCache);
+
+ Collection<String> records =
dnsServer.findMXRecords("nomx.dnstest.com.");
+ assertThat(records).containsExactly("nomx.dnstest.com.");
+ }
+
+ @Test
+ void testFindMXRecordsShouldReturnEmptyCollectionWhenNoMXAndUnknownHost()
throws Exception {
+ doAnswer(new ZoneCacheLookupRecordsAnswer(loadZone("dnstest.com.")))
+ .when(mockedCache).lookupRecords(any(Name.class), anyInt(),
anyInt());
+ dnsServer.setCache(mockedCache);
+
+ Collection<String> records =
dnsServer.findMXRecords("nonexistent.dnstest.com.");
+ assertThat(records).isEmpty();
+ }
+
+ @Test
+ void
testFindMXRecordsShouldNotExecuteFallbackWhenTemporaryResolutionExceptionThrown()
{
+ dnsServer.setCache(mockedCache);
+ when(mockedCache.lookupRecords(any(Name.class), anyInt(), anyInt()))
+ .thenThrow(new IllegalStateException("Simulated DNS failure"));
+
+ org.assertj.core.api.Assertions.assertThatThrownBy(() ->
dnsServer.findMXRecords("dnstest.com."))
+
.isInstanceOf(org.apache.james.dnsservice.api.TemporaryResolutionException.class);
+ }
+
+ @Test
+ void testFindMXRecordsShouldReturnUnmodifiableCollection() throws
Exception {
+ doAnswer(new ZoneCacheLookupRecordsAnswer(loadZone("dnstest.com.")))
+ .when(mockedCache).lookupRecords(any(Name.class), anyInt(),
anyInt());
+ dnsServer.setCache(mockedCache);
+
+ Collection<String> records = dnsServer.findMXRecords("dnstest.com.");
+ org.assertj.core.api.Assertions.assertThatThrownBy(() ->
records.add("evil.host.com."))
+ .isInstanceOf(UnsupportedOperationException.class);
+ }
+
/*
* public void testCNAMEasMXrecords() throws Exception { // Zone z =
* loadZone("brandilyncollins.com."); dnsServer.setResolver(null);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
