This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 5c5c9b180cc9bbf63157bc871827a5a513592545 Author: Benoit Tellier <[email protected]> AuthorDate: Fri May 29 12:19:07 2020 +0700 JAMES-3197 Mailet processing should handle NoClassDefFoundError Due to JAMES-3176, we ended up having ExtractMDNOriginalJMAPMessageId throwing NoClassDefFoundError. This error is not handled by the mailet processor, resulting in a mailqueue nack, and generated an infinite loop on top of RabbitMQ. This error can happen too when a user specifies a custom mailet with unsatisfied dependencies. Thus, when a mailet, or a matcher throws a `NoClassDefFoundError` we should execute mailet error handling normally, and prevent that infinite loop. --- .../org/apache/james/mailets/MailetErrorsTest.java | 22 +++++++++++++++ .../mailets/NoClassDefFoundErrorMailet.java | 32 ++++++++++++++++++++++ .../james/mailetcontainer/impl/ProcessorUtil.java | 4 +-- .../mailetcontainer/impl/camel/CamelProcessor.java | 4 +-- .../impl/jmx/JMXStateMailetProcessorListener.java | 2 +- .../lib/AbstractStateMailetProcessor.java | 2 +- .../lib/AbstractStateMailetProcessorTest.java | 10 +++---- 7 files changed, 65 insertions(+), 11 deletions(-) diff --git a/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/MailetErrorsTest.java b/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/MailetErrorsTest.java index c5e13dc..175b4c4 100644 --- a/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/MailetErrorsTest.java +++ b/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/MailetErrorsTest.java @@ -32,8 +32,10 @@ import org.apache.james.mailets.configuration.MailetContainer; import org.apache.james.mailets.configuration.ProcessorConfiguration; import org.apache.james.mailrepository.api.MailRepositoryUrl; import org.apache.james.modules.protocols.SmtpGuiceProbe; +import org.apache.james.transport.mailets.NoClassDefFoundErrorMailet; import org.apache.james.transport.mailets.ErrorMailet; import org.apache.james.transport.mailets.ErrorMatcher; +import org.apache.james.transport.mailets.NoClassDefFoundErrorMatcher; import org.apache.james.transport.mailets.NoopMailet; import org.apache.james.transport.mailets.Null; import org.apache.james.transport.mailets.RuntimeErrorMailet; @@ -86,6 +88,26 @@ public class MailetErrorsTest { } @Test + public void mailetProcessingShouldHandleClassNotFoundException() throws Exception { + jamesServer = TemporaryJamesServer.builder() + .withBase(SMTP_ONLY_MODULE) + .withMailetContainer(MailetContainer.builder() + .putProcessor(CommonProcessors.deliverOnlyTransport()) + .putProcessor(errorProcessor()) + .putProcessor(ProcessorConfiguration.root() + .addMailet(MailetConfiguration.builder() + .matcher(All.class) + .mailet(NoClassDefFoundErrorMailet.class)))) + .build(temporaryFolder.newFolder()); + MailRepositoryProbeImpl probe = jamesServer.getProbe(MailRepositoryProbeImpl.class); + + smtpMessageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort()).sendMessage(FROM, FROM); + + awaitAtMostOneMinute.until(() -> probe.getRepositoryMailCount(ERROR_REPOSITORY) == 1); + } + + + @Test public void mailetProcessorsShouldHandleRuntimeException() throws Exception { jamesServer = TemporaryJamesServer.builder() .withBase(SMTP_ONLY_MODULE) diff --git a/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/NoClassDefFoundErrorMailet.java b/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/NoClassDefFoundErrorMailet.java new file mode 100644 index 0000000..b173183 --- /dev/null +++ b/server/mailet/integration-testing/src/test/java/org/apache/james/transport/mailets/NoClassDefFoundErrorMailet.java @@ -0,0 +1,32 @@ +/**************************************************************** + * 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.transport.mailets; + +import javax.mail.MessagingException; + +import org.apache.mailet.Mail; +import org.apache.mailet.base.GenericMailet; + +public class NoClassDefFoundErrorMailet extends GenericMailet { + @Override + public void service(Mail mail) throws MessagingException { + throw new NoClassDefFoundError(); + } +} diff --git a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/ProcessorUtil.java b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/ProcessorUtil.java index fda19fd..787b464 100644 --- a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/ProcessorUtil.java +++ b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/ProcessorUtil.java @@ -47,13 +47,13 @@ public class ProcessorUtil { * @param nextState * the next state to set */ - public static void handleException(Exception me, Mail mail, String offendersName, String nextState, Logger logger) { + public static void handleException(Throwable me, Mail mail, String offendersName, String nextState, Logger logger) { mail.setState(nextState); StringWriter sout = new StringWriter(); PrintWriter out = new PrintWriter(sout, true); String exceptionBuffer = "Exception calling " + offendersName + ": " + me.getMessage(); out.println(exceptionBuffer); - Exception e = me; + Throwable e = me; while (e != null) { e.printStackTrace(out); if (e instanceof MessagingException) { diff --git a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/CamelProcessor.java b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/CamelProcessor.java index 678b938..a46001a 100644 --- a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/CamelProcessor.java +++ b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/CamelProcessor.java @@ -61,7 +61,7 @@ public class CamelProcessor { public void process(Mail mail) throws Exception { long start = System.currentTimeMillis(); TimeMetric timeMetric = metricFactory.timer(mailet.getClass().getSimpleName()); - Exception ex = null; + Throwable ex = null; try (Closeable closeable = MDCBuilder.create() .addContext(MDCBuilder.PROTOCOL, "MAILET") @@ -75,7 +75,7 @@ public class CamelProcessor { .build()) { MailetPipelineLogging.logBeginOfMailetProcess(mailet, mail); mailet.service(mail); - } catch (Exception me) { + } catch (Exception | NoClassDefFoundError me) { ex = me; String onMailetException = null; diff --git a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/jmx/JMXStateMailetProcessorListener.java b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/jmx/JMXStateMailetProcessorListener.java index 38fe438..7840c4d 100644 --- a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/jmx/JMXStateMailetProcessorListener.java +++ b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/jmx/JMXStateMailetProcessorListener.java @@ -61,7 +61,7 @@ public class JMXStateMailetProcessorListener implements MailetProcessorListener, } @Override - public void afterMailet(Mailet m, String mailName, String state, long processTime, Exception e) { + public void afterMailet(Mailet m, String mailName, String state, long processTime, Throwable e) { MailetManagement mgmt = mailetMap.get(m); if (mgmt != null) { mgmt.update(processTime, e == null); diff --git a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessor.java b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessor.java index 45db8ad..0d6a399 100644 --- a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessor.java +++ b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessor.java @@ -404,7 +404,7 @@ public abstract class AbstractStateMailetProcessor implements MailProcessor, Con * @param e * or null if no Exception was thrown */ - void afterMailet(Mailet m, String mailName, String state, long processTime, Exception e); + void afterMailet(Mailet m, String mailName, String state, long processTime, Throwable e); /** * Get called after each {@link Matcher} call was complete diff --git a/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessorTest.java b/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessorTest.java index 4894fa7..c76a019 100644 --- a/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessorTest.java +++ b/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessorTest.java @@ -85,7 +85,7 @@ public abstract class AbstractStateMailetProcessorTest { } @Override - public void afterMailet(Mailet m, String mailName, String state, long processTime, Exception e) { + public void afterMailet(Mailet m, String mailName, String state, long processTime, Throwable e) { // check for class name as the terminating mailet will kick in too if (MockMailet.class.equals(m.getClass())) { @@ -130,7 +130,7 @@ public abstract class AbstractStateMailetProcessorTest { } @Override - public void afterMailet(Mailet m, String mailName, String state, long processTime, Exception e) { + public void afterMailet(Mailet m, String mailName, String state, long processTime, Throwable e) { // check for class name as the terminating mailet will kick in too if (MockMailet.class.equals(m.getClass())) { @@ -177,7 +177,7 @@ public abstract class AbstractStateMailetProcessorTest { } @Override - public void afterMailet(Mailet m, String mailName, String state, long processTime, Exception e) { + public void afterMailet(Mailet m, String mailName, String state, long processTime, Throwable e) { throw new RuntimeException("Should not call any mailet!"); } }); @@ -212,7 +212,7 @@ public abstract class AbstractStateMailetProcessorTest { } @Override - public void afterMailet(Mailet m, String mailName, String state, long processTime, Exception e) { + public void afterMailet(Mailet m, String mailName, String state, long processTime, Throwable e) { throw new RuntimeException("Should not call any mailet!"); } }); @@ -249,7 +249,7 @@ public abstract class AbstractStateMailetProcessorTest { } @Override - public void afterMailet(Mailet m, String mailName, String state, long processTime, Exception e) { + public void afterMailet(Mailet m, String mailName, String state, long processTime, Throwable e) { if (ExceptionThrowingMailet.class.equals(m.getClass())) { // the name should be not the same as we have a part match assertThat(mail.getName()).isNotEqualTo(mailName); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
