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-jspf.git
commit 93309395c0ebc2a37f15f9a6559e94a8298de1d6 Author: Quan Tran <[email protected]> AuthorDate: Sat Oct 31 05:17:07 2026 +0700 [FIX] StagedMultipleSPFExecutor: survive an unexpected lookup error DNSServiceAsynchSimulator caught TimeoutException only around the lookup: any other exception thrown by the DNSService ended its thread (or the lookup's own thread when multiThread), so the SPF check, and every later one when single threaded, never completed. DNSServiceXBillImpl can throw so: dnsjava 3.6.0 fails Lookup.run with ClassCastException on a response of opcode UPDATE holding a CNAME or DNAME of RDLENGTH 0. Hand such an exception to the executor as the lookup's error, and let StagedMultipleSPFExecutor give it to the exception catchers, as AsynchronousSPFExecutor does: it cast any lookup error to TimeoutException, and an exception rethrown by a catcher (those of IncludeMechanism and RedirectModifier rethrow an unexpected one) ended its own thread. The next catcher now handles it, and an error left unhandled makes the result a temperror instead of leaving it pending. --- .../jspf/executor/StagedMultipleSPFExecutor.java | 89 ++++++++-------- .../james/jspf/impl/DNSServiceAsynchSimulator.java | 3 +- .../StagedMultipleSPFExecutorIntegrationTest.java | 112 +++++++++++++++++++++ 3 files changed, 160 insertions(+), 44 deletions(-) diff --git a/resolver/src/main/java/org/apache/james/jspf/executor/StagedMultipleSPFExecutor.java b/resolver/src/main/java/org/apache/james/jspf/executor/StagedMultipleSPFExecutor.java index 17fe7a7..51b55b7 100644 --- a/resolver/src/main/java/org/apache/james/jspf/executor/StagedMultipleSPFExecutor.java +++ b/resolver/src/main/java/org/apache/james/jspf/executor/StagedMultipleSPFExecutor.java @@ -29,6 +29,7 @@ import org.apache.james.jspf.core.DNSResponse; 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.SPFErrorConstants; import org.apache.james.jspf.core.exceptions.SPFResultException; import org.apache.james.jspf.core.exceptions.TimeoutException; import org.slf4j.Logger; @@ -127,24 +128,29 @@ public class StagedMultipleSPFExecutor implements SPFExecutor, Runnable { return; } } catch (Exception e) { - while (e != null) { - checker = session.popChecker(c -> c instanceof SPFCheckerExceptionCatcher); - if (checker == null) { - // Error case not handled by JSPF. Throw to avoid infinite loop. See JSPF-110. - throw new RuntimeException("SPFCheckerExceptionCatcher implementation not found, session: " + session, e); - } - try { - ((SPFCheckerExceptionCatcher) checker).onException(e, session); - e = null; - } catch (SPFResultException ex) { - e = ex; - } - } + handleError(session, e); } } result.setSPFResult(session); } + private void handleError(SPFSession session, Exception e) { + while (e != null) { + SPFChecker checker = session.popChecker(c -> c instanceof SPFCheckerExceptionCatcher); + if (checker == null) { + // Error case not handled by JSPF. Throw to avoid infinite loop. See JSPF-110. + throw new RuntimeException("SPFCheckerExceptionCatcher implementation not found, session: " + session, e); + } + try { + ((SPFCheckerExceptionCatcher) checker).onException(e, session); + e = null; + } catch (SPFResultException | RuntimeException ex) { + // IncludeMechanism and RedirectModifier rethrow an unexpected exception: the next catcher handles it + e = ex; + } + } + } + /** * throttle should be true only when the caller thread is the client and not the worker thread. * We could even remove the throttle parameter and check the currentThread. @@ -175,44 +181,41 @@ public class StagedMultipleSPFExecutor implements SPFExecutor, Runnable { Integer respId = (Integer)resp.getId(); SPFSession session = sessions.remove(respId); FutureSPFResult result = results.remove(respId); - - DNSLookupContinuation cont = (DNSLookupContinuation) session.getAttribute(ATTRIBUTE_STAGED_EXECUTOR_CONTINUATION); - + + try { + onResponse(session, result, resp); + } catch (RuntimeException e) { + // Never leave the result pending nor end this thread: an unexpected error is a temperror + LOGGER.error("Unexpected error during the staged SPF check", e); + session.setCurrentResult(SPFErrorConstants.TEMP_ERROR_CONV); + session.setCurrentResultExpanded(SPFErrorConstants.TEMP_ERROR_CONV); + result.setSPFResult(session); + } + } + } + + private void onResponse(SPFSession session, FutureSPFResult result, IResponse resp) { + DNSLookupContinuation cont = (DNSLookupContinuation) session.getAttribute(ATTRIBUTE_STAGED_EXECUTOR_CONTINUATION); + try { DNSResponse response; - if (resp.getException() != null) { + if (resp.getException() instanceof TimeoutException) { response = new DNSResponse((TimeoutException) resp.getException()); + } else if (resp.getException() != null) { + // any other lookup error goes to the exception catcher + throw resp.getException(); } else { response = new DNSResponse(resp.getValue()); } - - - try { - cont = cont.getListener().onDNSResponse(response, session); - - if (cont != null) { - invokeAsynchService(session, result, cont, false); - } else { - execute(session, result, false); - } - } catch (Exception e) { - SPFChecker checker = null; - while (e != null) { - while (checker == null || !(checker instanceof SPFCheckerExceptionCatcher)) { - checker = session.popChecker(); - } - try { - ((SPFCheckerExceptionCatcher) checker).onException(e, session); - e = null; - } catch (SPFResultException ex) { - e = ex; - } finally { - checker = null; - } - } - execute(session, result, false); + cont = cont.getListener().onDNSResponse(response, session); + if (cont != null) { + invokeAsynchService(session, result, cont, false); + return; } + } catch (Exception e) { + handleError(session, e); } + execute(session, result, false); } } diff --git a/resolver/src/main/java/org/apache/james/jspf/impl/DNSServiceAsynchSimulator.java b/resolver/src/main/java/org/apache/james/jspf/impl/DNSServiceAsynchSimulator.java index 38f1487..fc078d1 100644 --- a/resolver/src/main/java/org/apache/james/jspf/impl/DNSServiceAsynchSimulator.java +++ b/resolver/src/main/java/org/apache/james/jspf/impl/DNSServiceAsynchSimulator.java @@ -111,7 +111,8 @@ public class DNSServiceAsynchSimulator implements Runnable, DNSAsynchLookupServi IResponseImpl response; try { response = new IResponseImpl(req.getId(), dnsService.getRecords(req.getValue())); - } catch (TimeoutException e) { + } catch (TimeoutException | RuntimeException e) { + // an unchecked exception would end the thread: no later lookup would get an answer response = new IResponseImpl(req.getId(), e); } diff --git a/resolver/src/test/java/org/apache/james/jspf/executor/StagedMultipleSPFExecutorIntegrationTest.java b/resolver/src/test/java/org/apache/james/jspf/executor/StagedMultipleSPFExecutorIntegrationTest.java new file mode 100644 index 0000000..8ae1999 --- /dev/null +++ b/resolver/src/test/java/org/apache/james/jspf/executor/StagedMultipleSPFExecutorIntegrationTest.java @@ -0,0 +1,112 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.jspf.executor; + +import static org.junit.Assert.assertEquals; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; + +import org.apache.james.jspf.core.DNSRequest; +import org.apache.james.jspf.core.DNSService; +import org.apache.james.jspf.core.exceptions.SPFErrorConstants; +import org.apache.james.jspf.impl.DNSServiceAsynchSimulator; +import org.apache.james.jspf.impl.SPF; +import org.junit.Test; + +public class StagedMultipleSPFExecutorIntegrationTest { + + private static final Map<String, String> RECORDS = Map.of( + "example.com", "v=spf1 +all", + "include.example.com", "v=spf1 include:broken.example.com -all", + "redirect.example.com", "v=spf1 redirect=broken.example.com"); + + // fails the lookups of broken.example.com with an unchecked exception, as dnsjava may do on an answer it + // cannot handle + private static final DNSService DNS = new DNSService() { + public List<String> getRecords(DNSRequest request) { + if (request.getHostname().equals("broken.example.com")) { + throw new IllegalStateException("answer dnsjava cannot handle"); + } + String record = RECORDS.get(request.getHostname()); + return request.getRecordType() == DNSRequest.TXT && record != null ? new ArrayList<>(List.of(record)) : null; + } + + public CompletionStage<List<String>> getRecordsAsync(DNSRequest request) { + return CompletableFuture.completedFuture(getRecords(request)); + } + + public List<String> getLocalDomainNames() { + return List.of("localhost"); + } + + public void setTimeOut(int timeOut) { + } + + public int getRecordLimit() { + return 10; + } + + public void setRecordLimit(int recordLimit) { + } + }; + + @Test(timeout = 10000) + public void shouldEndAsNeutralOnAnUnexpectedLookupError() { + checkUnexpectedLookupError(false, "broken.example.com"); + } + + @Test(timeout = 10000) + public void shouldEndAsNeutralOnAnUnexpectedLookupErrorWhenMultiThreaded() { + checkUnexpectedLookupError(true, "broken.example.com"); + } + + @Test(timeout = 10000) + public void shouldEndAsNeutralOnAnUnexpectedLookupErrorInAnInclude() { + checkUnexpectedLookupError(false, "include.example.com"); + } + + @Test(timeout = 10000) + public void shouldEndAsNeutralOnAnUnexpectedLookupErrorInAnIncludeWhenMultiThreaded() { + checkUnexpectedLookupError(true, "include.example.com"); + } + + @Test(timeout = 10000) + public void shouldEndAsNeutralOnAnUnexpectedLookupErrorInARedirect() { + checkUnexpectedLookupError(false, "redirect.example.com"); + } + + @Test(timeout = 10000) + public void shouldEndAsNeutralOnAnUnexpectedLookupErrorInARedirectWhenMultiThreaded() { + checkUnexpectedLookupError(true, "redirect.example.com"); + } + + private void checkUnexpectedLookupError(boolean multiThread, String domain) { + SPF spf = new SPF(DNS, new StagedMultipleSPFExecutor(new DNSServiceAsynchSimulator(DNS, multiThread))); + + // the exception catcher of SPF makes an unexpected error neutral, as with AsynchronousSPFExecutor + assertEquals(SPFErrorConstants.NEUTRAL_CONV, spf.checkSPF("1.2.3.4", "foo@" + domain, domain).getResult()); + // the executor still checks + assertEquals(SPFErrorConstants.PASS_CONV, spf.checkSPF("1.2.3.4", "[email protected]", "example.com").getResult()); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
