This is an automated email from the ASF dual-hosted git repository. quantranhong1999 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-jspf.git
commit 6f8432051786e6ebdf51829340a80a0adaf5a3d1 Author: Benoit TELLIER <[email protected]> AuthorDate: Wed Oct 28 22:11:22 2026 +0100 [FIX] AsynchronousSPFExecutor: always complete the SPF result A failed lookup (NXDOMAIN/NODATA) is handed to the listener as an empty list. ExpModifier only expected null, so a "fail" record whose exp= target has no TXT record threw an IndexOutOfBoundsException inside the future callback: the result was never set and getResult() hung forever. - ExpModifier treats an empty answer as no answer (RFC 7208 6.2). - The executor hands any exception from a listener to the error handling, and sets a temperror result if no catcher handles it, so the future always completes. - The result is no longer set right after a failed or timed out lookup starts a follow-up lookup, which published it too early. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../jspf/executor/AsynchronousSPFExecutor.java | 62 +++++++++------------- .../org/apache/james/jspf/terms/ExpModifier.java | 2 +- .../org/apache/james/jspf/SPFExecutorBaseTest.java | 13 +++++ 3 files changed, 40 insertions(+), 37 deletions(-) diff --git a/resolver/src/main/java/org/apache/james/jspf/executor/AsynchronousSPFExecutor.java b/resolver/src/main/java/org/apache/james/jspf/executor/AsynchronousSPFExecutor.java index 5560917..00e6a11 100644 --- a/resolver/src/main/java/org/apache/james/jspf/executor/AsynchronousSPFExecutor.java +++ b/resolver/src/main/java/org/apache/james/jspf/executor/AsynchronousSPFExecutor.java @@ -29,11 +29,8 @@ import org.apache.james.jspf.core.DNSService; import org.apache.james.jspf.core.SPFChecker; import org.apache.james.jspf.core.SPFCheckerExceptionCatcher; import org.apache.james.jspf.core.SPFSession; -import org.apache.james.jspf.core.exceptions.NeutralException; -import org.apache.james.jspf.core.exceptions.NoneException; -import org.apache.james.jspf.core.exceptions.PermErrorException; +import org.apache.james.jspf.core.exceptions.SPFErrorConstants; import org.apache.james.jspf.core.exceptions.SPFResultException; -import org.apache.james.jspf.core.exceptions.TempErrorException; import org.apache.james.jspf.core.exceptions.TimeoutException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -66,8 +63,7 @@ public class AsynchronousSPFExecutor implements SPFExecutor { DNSLookupContinuation cont = checker.checkSPF(session); handleCont(session, result, cont, checker); } catch (Exception e) { - handleError(session, e); - result.setSPFResult(session); + handleErrorAndComplete(session, result, e); } } @@ -75,15 +71,7 @@ public class AsynchronousSPFExecutor implements SPFExecutor { if (cont != null) { // if the checker returns a continuation we return it dnsProbe.getRecordsAsync(cont.getRequest()) - .thenAccept(results -> { - try { - DNSLookupContinuation dnsLookupContinuation = cont.getListener().onDNSResponse(new DNSResponse(results), session); - handleCont(session, result, dnsLookupContinuation, checker); - } catch (PermErrorException | NoneException | TempErrorException | NeutralException e) { - handleError(session, e); - result.setSPFResult(session); - } - }) + .thenAccept(results -> onResponse(session, result, cont, new DNSResponse(results), checker)) .exceptionally(e -> { if (e instanceof CompletionException && e.getCause() != null) { e = e.getCause(); @@ -92,24 +80,12 @@ public class AsynchronousSPFExecutor implements SPFExecutor { e = new TimeoutException(e.getMessage()); } if (e instanceof LookupFailedException) { - try { - DNSLookupContinuation dnsLookupContinuation = cont.getListener().onDNSResponse(new DNSResponse(new ArrayList<>()), session); - handleCont(session, result, dnsLookupContinuation, checker); - result.setSPFResult(session); - return null; - } catch (PermErrorException | NoneException | TempErrorException | NeutralException ex2) { - handleError(session, ex2); - result.setSPFResult(session); - return null; - } + onResponse(session, result, cont, new DNSResponse(new ArrayList<>()), checker); + } else if (e instanceof TimeoutException) { + onResponse(session, result, cont, new DNSResponse((TimeoutException) e), checker); + } else { + handleErrorAndComplete(session, result, e); } - if (e instanceof TimeoutException) { - handleTimeout(cont, new DNSResponse((TimeoutException) e), session, result, checker); - result.setSPFResult(session); - return null; - } - handleError(session, e); - result.setSPFResult(session); return null; }); } else { @@ -117,12 +93,26 @@ public class AsynchronousSPFExecutor implements SPFExecutor { } } - private void handleTimeout(DNSLookupContinuation cont, DNSResponse e, SPFSession session, FutureSPFResult result, SPFChecker checker) { + private void onResponse(SPFSession session, FutureSPFResult result, DNSLookupContinuation cont, DNSResponse response, SPFChecker checker) { try { - DNSLookupContinuation dnsLookupContinuation = cont.getListener().onDNSResponse(e, session); + DNSLookupContinuation dnsLookupContinuation = cont.getListener().onDNSResponse(response, session); + // The result is set once no checker is left, or by the error handling handleCont(session, result, dnsLookupContinuation, checker); - } catch (PermErrorException | NoneException | TempErrorException | NeutralException ex2) { - handleError(session, ex2); + } catch (Exception e) { + handleErrorAndComplete(session, result, e); + } + } + + private void handleErrorAndComplete(SPFSession session, FutureSPFResult result, Throwable e) { + try { + handleError(session, e); + } catch (RuntimeException ex) { + // Never leave the future pending: an unexpected error is a temperror + LOGGER.error("Unexpected error during the asynchronous SPF check", ex); + session.setCurrentResult(SPFErrorConstants.TEMP_ERROR_CONV); + session.setCurrentResultExpanded(SPFErrorConstants.TEMP_ERROR_CONV); + } finally { + result.setSPFResult(session); } } diff --git a/resolver/src/main/java/org/apache/james/jspf/terms/ExpModifier.java b/resolver/src/main/java/org/apache/james/jspf/terms/ExpModifier.java index 5c664b0..8059c88 100644 --- a/resolver/src/main/java/org/apache/james/jspf/terms/ExpModifier.java +++ b/resolver/src/main/java/org/apache/james/jspf/terms/ExpModifier.java @@ -158,7 +158,7 @@ public class ExpModifier extends GenericModifier implements MacroExpandEnabled, try { List<String> records = lookup.getResponse(); - if (records == null) { + if (records == null || records.isEmpty()) { return null; } diff --git a/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java b/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java index 4369ee3..07f97a2 100644 --- a/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java @@ -334,4 +334,17 @@ public abstract class SPFExecutorBaseTest { assertEquals(SPFErrorConstants.FAIL_CONV, result.getResult()); assertEquals(10, addressLookups.get()); } + + @Test(timeout = 10000) + public void shouldReturnFailWhenExplanationDomainHasNoTxtRecord() throws IOException { + String hostname = "shouldReturnFailWhenExplanationDomainHasNoTxtRecord.spfexecutor.fake"; + FakeResolver fakeResolver = new FakeResolver(); + fakeResolver.addRecord(new TXTRecord(Name.fromString(hostname + "."), DClass.IN, 30L, + "v=spf1 -all exp=explain." + hostname)); + + SPF spf = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)); + SPFResult result = spf.checkSPF("192.0.2.127", "a_user@" + hostname, hostname); + + assertEquals(SPFErrorConstants.FAIL_CONV, result.getResult()); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
