JAMES-2257 All MailKooks should use Optional sender
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/93be34c3 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/93be34c3 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/93be34c3 Branch: refs/heads/master Commit: 93be34c312e63d71d7b0bab0c6d3083ff642ae44 Parents: 215f99d Author: Benoit Tellier <btell...@linagora.com> Authored: Tue Oct 30 10:56:16 2018 +0700 Committer: Benoit Tellier <btell...@linagora.com> Committed: Thu Nov 1 10:56:32 2018 +0700 ---------------------------------------------------------------------- .../protocols/smtp/core/MailCmdHandler.java | 6 ++-- .../core/fastfail/ValidSenderDomainHandler.java | 6 ++-- .../james/protocols/smtp/hook/SimpleHook.java | 3 +- .../protocols/smtp/AbstractSMTPServerTest.java | 5 +-- .../fastfail/ValidSenderDomainHandlerTest.java | 6 ++-- .../james/smtpserver/fastfail/SPFHandler.java | 11 ++++--- .../apache/james/smtpserver/SPFHandlerTest.java | 33 ++++++++++---------- 7 files changed, 37 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/93be34c3/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/MailCmdHandler.java ---------------------------------------------------------------------- diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/MailCmdHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/MailCmdHandler.java index 806d730..5a18476 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/MailCmdHandler.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/MailCmdHandler.java @@ -31,6 +31,7 @@ import javax.inject.Inject; import org.apache.commons.configuration.Configuration; import org.apache.commons.configuration.ConfigurationException; import org.apache.james.core.MailAddress; +import org.apache.james.core.MaybeSender; import org.apache.james.metrics.api.MetricFactory; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.api.Request; @@ -257,10 +258,7 @@ public class MailCmdHandler extends AbstractHookableCmdHandler<MailHook> { @Override protected HookResult callHook(MailHook rawHook, SMTPSession session, String parameters) { MailAddress sender = (MailAddress) session.getAttachment(SMTPSession.SENDER, State.Transaction); - if (sender.isNullSender()) { - sender = null; - } - return rawHook.doMail(session, sender); + return rawHook.doMail(session, MaybeSender.of(sender)); } http://git-wip-us.apache.org/repos/asf/james-project/blob/93be34c3/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/ValidSenderDomainHandler.java ---------------------------------------------------------------------- diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/ValidSenderDomainHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/ValidSenderDomainHandler.java index db70aa5..efb0384 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/ValidSenderDomainHandler.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/ValidSenderDomainHandler.java @@ -18,7 +18,7 @@ ****************************************************************/ package org.apache.james.protocols.smtp.core.fastfail; -import org.apache.james.core.MailAddress; +import org.apache.james.core.MaybeSender; import org.apache.james.protocols.smtp.SMTPRetCode; import org.apache.james.protocols.smtp.SMTPSession; import org.apache.james.protocols.smtp.dsn.DSNStatus; @@ -34,8 +34,8 @@ public abstract class ValidSenderDomainHandler implements MailHook { @Override - public HookResult doMail(SMTPSession session, MailAddress sender) { - if (sender != null && !hasMXRecord(session,sender.getDomain().name())) { + public HookResult doMail(SMTPSession session, MaybeSender sender) { + if (!sender.isNullSender() && !hasMXRecord(session,sender.get().getDomain().name())) { return HookResult.builder() .hookReturnCode(HookReturnCode.deny()) .smtpReturnCode(SMTPRetCode.SYNTAX_ERROR_ARGUMENTS) http://git-wip-us.apache.org/repos/asf/james-project/blob/93be34c3/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/hook/SimpleHook.java ---------------------------------------------------------------------- diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/hook/SimpleHook.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/hook/SimpleHook.java index 2364253..f226e17 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/hook/SimpleHook.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/hook/SimpleHook.java @@ -22,6 +22,7 @@ package org.apache.james.protocols.smtp.hook; import org.apache.commons.configuration.Configuration; import org.apache.commons.configuration.ConfigurationException; import org.apache.james.core.MailAddress; +import org.apache.james.core.MaybeSender; import org.apache.james.protocols.smtp.MailEnvelope; import org.apache.james.protocols.smtp.SMTPSession; @@ -65,7 +66,7 @@ public class SimpleHook implements HeloHook, MailHook, RcptHook, MessageHook { * Return {@link HookResult} with {@link HookReturnCode#DECLINED} */ @Override - public HookResult doMail(SMTPSession session, MailAddress sender) { + public HookResult doMail(SMTPSession session, MaybeSender sender) { return HookResult.DECLINED; } http://git-wip-us.apache.org/repos/asf/james-project/blob/93be34c3/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/AbstractSMTPServerTest.java ---------------------------------------------------------------------- diff --git a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/AbstractSMTPServerTest.java b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/AbstractSMTPServerTest.java index faeca83..7bcb07f 100644 --- a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/AbstractSMTPServerTest.java +++ b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/AbstractSMTPServerTest.java @@ -37,6 +37,7 @@ import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.net.smtp.SMTPClient; import org.apache.commons.net.smtp.SMTPReply; import org.apache.james.core.MailAddress; +import org.apache.james.core.MaybeSender; import org.apache.james.metrics.api.NoopMetricFactory; import org.apache.james.protocols.api.Protocol; import org.apache.james.protocols.api.ProtocolServer; @@ -593,7 +594,7 @@ public abstract class AbstractSMTPServerTest { } @Override - public HookResult doMail(SMTPSession session, MailAddress sender) { + public HookResult doMail(SMTPSession session, MaybeSender sender) { return HookResult.DENY; } }; @@ -643,7 +644,7 @@ public abstract class AbstractSMTPServerTest { } @Override - public HookResult doMail(SMTPSession session, MailAddress sender) { + public HookResult doMail(SMTPSession session, MaybeSender sender) { return HookResult.DENYSOFT; } }; http://git-wip-us.apache.org/repos/asf/james-project/blob/93be34c3/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/ValidSenderDomainHandlerTest.java ---------------------------------------------------------------------- diff --git a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/ValidSenderDomainHandlerTest.java b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/ValidSenderDomainHandlerTest.java index 82d8d4b..3d5da19 100644 --- a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/ValidSenderDomainHandlerTest.java +++ b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/ValidSenderDomainHandlerTest.java @@ -27,6 +27,7 @@ import java.util.Map; import org.apache.commons.configuration.Configuration; import org.apache.commons.configuration.ConfigurationException; import org.apache.james.core.MailAddress; +import org.apache.james.core.MaybeSender; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.smtp.SMTPSession; import org.apache.james.protocols.smtp.hook.HookReturnCode; @@ -106,7 +107,7 @@ public class ValidSenderDomainHandlerTest { @Test public void testNullSenderNotReject() { ValidSenderDomainHandler handler = createHandler(); - HookReturnCode response = handler.doMail(setupMockedSession(null),null).getResult(); + HookReturnCode response = handler.doMail(setupMockedSession(null), MaybeSender.nullSender()).getResult(); assertThat(HookReturnCode.declined()).describedAs("Not blocked cause its a nullsender").isEqualTo(response); } @@ -115,7 +116,8 @@ public class ValidSenderDomainHandlerTest { public void testInvalidSenderDomainReject() throws Exception { ValidSenderDomainHandler handler = createHandler(); SMTPSession session = setupMockedSession(new MailAddress("invalid@invalid")); - HookReturnCode response = handler.doMail(session,(MailAddress) session.getAttachment(SMTPSession.SENDER, State.Transaction)).getResult(); + MailAddress sender = (MailAddress) session.getAttachment(SMTPSession.SENDER, State.Transaction); + HookReturnCode response = handler.doMail(session, MaybeSender.of(sender)).getResult(); assertThat(HookReturnCode.deny()).describedAs("Blocked cause we use reject action").isEqualTo(response); } http://git-wip-us.apache.org/repos/asf/james-project/blob/93be34c3/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/SPFHandler.java ---------------------------------------------------------------------- diff --git a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/SPFHandler.java b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/SPFHandler.java index 7a6e2b2..2c86b5f 100644 --- a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/SPFHandler.java +++ b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/SPFHandler.java @@ -23,6 +23,7 @@ import javax.inject.Inject; import org.apache.commons.configuration.Configuration; import org.apache.commons.configuration.ConfigurationException; import org.apache.james.core.MailAddress; +import org.apache.james.core.MaybeSender; import org.apache.james.jspf.core.DNSService; import org.apache.james.jspf.core.exceptions.SPFErrorConstants; import org.apache.james.jspf.executor.SPFResult; @@ -109,17 +110,17 @@ public class SPFHandler implements JamesMessageHook, MailHook, RcptHook, Protoco * @param session * SMTP session object */ - private void doSPFCheck(SMTPSession session, MailAddress sender) { + private void doSPFCheck(SMTPSession session, MaybeSender sender) { String heloEhlo = (String) session.getAttachment(SMTPSession.CURRENT_HELO_NAME, State.Transaction); // We have no Sender or HELO/EHLO yet return false - if (sender == null || heloEhlo == null) { + if (sender.isNullSender() || heloEhlo == null) { LOGGER.info("No Sender or HELO/EHLO present"); } else { String ip = session.getRemoteAddress().getAddress().getHostAddress(); - SPFResult result = spf.checkSPF(ip, sender.toString(), heloEhlo); + SPFResult result = spf.checkSPF(ip, sender.asString(), heloEhlo); String spfResult = result.getResult(); @@ -128,7 +129,7 @@ public class SPFHandler implements JamesMessageHook, MailHook, RcptHook, Protoco // Store the header session.setAttachment(SPF_HEADER, result.getHeaderText(), State.Transaction); - LOGGER.info("Result for {} - {} - {} = {}", ip, sender, heloEhlo, spfResult); + LOGGER.info("Result for {} - {} - {} = {}", ip, sender.asString(), heloEhlo, spfResult); // Check if we should block! if ((spfResult.equals(SPFErrorConstants.FAIL_CONV)) || (spfResult.equals(SPFErrorConstants.SOFTFAIL_CONV) && blockSoftFail) || (spfResult.equals(SPFErrorConstants.PERM_ERROR_CONV) && blockPermError)) { @@ -169,7 +170,7 @@ public class SPFHandler implements JamesMessageHook, MailHook, RcptHook, Protoco } @Override - public HookResult doMail(SMTPSession session, MailAddress sender) { + public HookResult doMail(SMTPSession session, MaybeSender sender) { doSPFCheck(session, sender); return HookResult.DECLINED; } http://git-wip-us.apache.org/repos/asf/james-project/blob/93be34c3/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SPFHandlerTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SPFHandlerTest.java b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SPFHandlerTest.java index 04481cd..d7221ff 100644 --- a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SPFHandlerTest.java +++ b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SPFHandlerTest.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.List; import org.apache.james.core.MailAddress; +import org.apache.james.core.MaybeSender; import org.apache.james.jspf.core.DNSRequest; import org.apache.james.jspf.core.DNSService; import org.apache.james.jspf.core.exceptions.TimeoutException; @@ -169,7 +170,7 @@ public class SPFHandlerTest { @Test public void testSPFpass() throws Exception { - MailAddress sender = new MailAddress("t...@spf1.james.apache.org"); + MaybeSender sender = MaybeSender.of(new MailAddress("t...@spf1.james.apache.org")); MailAddress rcpt = new MailAddress("test@localhost"); setupMockedSMTPSession("192.168.100.1", "spf1.james.apache.org"); SPFHandler spf = new SPFHandler(); @@ -177,12 +178,12 @@ public class SPFHandlerTest { spf.setDNSService(mockedDnsService); assertThat(spf.doMail(mockedSMTPSession, sender).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); - assertThat(spf.doRcpt(mockedSMTPSession, sender, rcpt).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); + assertThat(spf.doRcpt(mockedSMTPSession, sender.get(), rcpt).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); } @Test public void testSPFfail() throws Exception { - MailAddress sender = new MailAddress("t...@spf2.james.apache.org"); + MaybeSender sender = MaybeSender.of(new MailAddress("t...@spf2.james.apache.org")); MailAddress rcpt = new MailAddress("test@localhost"); setupMockedSMTPSession("192.168.100.1", "spf2.james.apache.org"); SPFHandler spf = new SPFHandler(); @@ -190,12 +191,12 @@ public class SPFHandlerTest { spf.setDNSService(mockedDnsService); assertThat(spf.doMail(mockedSMTPSession, sender).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); - assertThat(spf.doRcpt(mockedSMTPSession, sender, rcpt).getResult()).describedAs("fail").isEqualTo(HookReturnCode.deny()); + assertThat(spf.doRcpt(mockedSMTPSession, sender.get(), rcpt).getResult()).describedAs("fail").isEqualTo(HookReturnCode.deny()); } @Test public void testSPFsoftFail() throws Exception { - MailAddress sender = new MailAddress("t...@spf3.james.apache.org"); + MaybeSender sender = MaybeSender.of(new MailAddress("t...@spf3.james.apache.org")); MailAddress rcpt = new MailAddress("test@localhost"); setupMockedSMTPSession("192.168.100.1", "spf3.james.apache.org"); SPFHandler spf = new SPFHandler(); @@ -203,12 +204,12 @@ public class SPFHandlerTest { spf.setDNSService(mockedDnsService); assertThat(spf.doMail(mockedSMTPSession, sender).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); - assertThat(spf.doRcpt(mockedSMTPSession, sender, rcpt).getResult()).describedAs("softfail declined").isEqualTo(HookReturnCode.declined()); + assertThat(spf.doRcpt(mockedSMTPSession, sender.get(), rcpt).getResult()).describedAs("softfail declined").isEqualTo(HookReturnCode.declined()); } @Test public void testSPFsoftFailRejectEnabled() throws Exception { - MailAddress sender = new MailAddress("t...@spf3.james.apache.org"); + MaybeSender sender = MaybeSender.of(new MailAddress("t...@spf3.james.apache.org")); MailAddress rcpt = new MailAddress("test@localhost"); setupMockedSMTPSession("192.168.100.1", "spf3.james.apache.org"); @@ -219,12 +220,12 @@ public class SPFHandlerTest { spf.setBlockSoftFail(true); assertThat(spf.doMail(mockedSMTPSession, sender).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); - assertThat(spf.doRcpt(mockedSMTPSession, sender, rcpt).getResult()).describedAs("softfail reject").isEqualTo(HookReturnCode.deny()); + assertThat(spf.doRcpt(mockedSMTPSession, sender.get(), rcpt).getResult()).describedAs("softfail reject").isEqualTo(HookReturnCode.deny()); } @Test public void testSPFpermError() throws Exception { - MailAddress sender = new MailAddress("t...@spf4.james.apache.org"); + MaybeSender sender = MaybeSender.of(new MailAddress("t...@spf4.james.apache.org")); MailAddress rcpt = new MailAddress("test@localhost"); setupMockedSMTPSession("192.168.100.1", "spf4.james.apache.org"); @@ -235,12 +236,12 @@ public class SPFHandlerTest { spf.setBlockSoftFail(true); assertThat(spf.doMail(mockedSMTPSession, sender).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); - assertThat(spf.doRcpt(mockedSMTPSession, sender, rcpt).getResult()).describedAs("permerror reject").isEqualTo(HookReturnCode.deny()); + assertThat(spf.doRcpt(mockedSMTPSession, sender.get(), rcpt).getResult()).describedAs("permerror reject").isEqualTo(HookReturnCode.deny()); } @Test public void testSPFtempError() throws Exception { - MailAddress sender = new MailAddress("t...@spf5.james.apache.org"); + MaybeSender sender = MaybeSender.of(new MailAddress("t...@spf5.james.apache.org")); MailAddress rcpt = new MailAddress("test@localhost"); setupMockedSMTPSession("192.168.100.1", "spf5.james.apache.org"); @@ -250,12 +251,12 @@ public class SPFHandlerTest { spf.setDNSService(mockedDnsService); assertThat(spf.doMail(mockedSMTPSession, sender).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); - assertThat(spf.doRcpt(mockedSMTPSession, sender, rcpt).getResult()).describedAs("temperror denysoft").isEqualTo(HookReturnCode.denySoft()); + assertThat(spf.doRcpt(mockedSMTPSession, sender.get(), rcpt).getResult()).describedAs("temperror denysoft").isEqualTo(HookReturnCode.denySoft()); } @Test public void testSPFNoRecord() throws Exception { - MailAddress sender = new MailAddress("t...@spf6.james.apache.org"); + MaybeSender sender = MaybeSender.of(new MailAddress("t...@spf6.james.apache.org")); MailAddress rcpt = new MailAddress("test@localhost"); setupMockedSMTPSession("192.168.100.1", "spf6.james.apache.org"); @@ -264,12 +265,12 @@ public class SPFHandlerTest { spf.setDNSService(mockedDnsService); assertThat(spf.doMail(mockedSMTPSession, sender).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); - assertThat(spf.doRcpt(mockedSMTPSession, sender, rcpt).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); + assertThat(spf.doRcpt(mockedSMTPSession, sender.get(), rcpt).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); } @Test public void testSPFpermErrorRejectDisabled() throws Exception { - MailAddress sender = new MailAddress("t...@spf4.james.apache.org"); + MaybeSender sender = MaybeSender.of(new MailAddress("t...@spf4.james.apache.org")); MailAddress rcpt = new MailAddress("test@localhost"); setupMockedSMTPSession("192.168.100.1", "spf4.james.apache.org"); SPFHandler spf = new SPFHandler(); @@ -279,6 +280,6 @@ public class SPFHandlerTest { spf.setBlockPermError(false); assertThat(spf.doMail(mockedSMTPSession, sender).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); - assertThat(spf.doRcpt(mockedSMTPSession, sender, rcpt).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); + assertThat(spf.doRcpt(mockedSMTPSession, sender.get(), rcpt).getResult()).describedAs("declined").isEqualTo(HookReturnCode.declined()); } } --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
