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 58b1d879abfd679758c95725375ab3f7f5598af7 Author: Gautier DI FOLCO <[email protected]> AuthorDate: Wed Mar 18 13:08:57 2020 +0100 JAMES-3119 Strong type ProtocolSession --- .../james/protocols/api/ProtocolSession.java | 83 +++++++++++++++++++--- .../james/protocols/api/ProtocolSessionImpl.java | 45 +++++++----- .../james/protocols/api/ProtocolSessionTest.java | 22 +++--- .../lmtp/core/DataLineMessageHookHandler.java | 4 +- .../apache/james/protocols/pop3/POP3Session.java | 11 ++- .../pop3/core/AbstractApopCmdHandler.java | 7 +- .../james/protocols/pop3/core/DeleCmdHandler.java | 8 ++- .../james/protocols/pop3/core/ListCmdHandler.java | 11 +-- .../protocols/pop3/core/MessageMetaDataUtils.java | 12 ++-- .../james/protocols/pop3/core/QuitCmdHandler.java | 4 +- .../james/protocols/pop3/core/RetrCmdHandler.java | 5 +- .../james/protocols/pop3/core/RsetCmdHandler.java | 2 +- .../james/protocols/pop3/core/StatCmdHandler.java | 10 +-- .../james/protocols/pop3/core/TopCmdHandler.java | 4 +- .../james/protocols/pop3/core/UidlCmdHandler.java | 8 +-- .../apache/james/protocols/smtp/SMTPSession.java | 13 ++-- .../james/protocols/smtp/SMTPSessionImpl.java | 22 ++---- .../smtp/core/AbstractAddHeadersFilter.java | 9 +-- ...ractSenderAuthIdentifyVerificationRcptHook.java | 2 +- .../james/protocols/smtp/core/DataCmdHandler.java | 14 ++-- .../smtp/core/DataLineMessageHookHandler.java | 20 ++++-- .../james/protocols/smtp/core/MailCmdHandler.java | 20 +++--- .../james/protocols/smtp/core/RcptCmdHandler.java | 31 ++++---- .../smtp/core/ReceivedDataLineFilter.java | 18 +++-- .../smtp/core/SeparatingDataLineFilter.java | 5 +- .../protocols/smtp/core/UnknownCmdHandler.java | 7 +- .../smtp/core/esmtp/MailSizeEsmtpExtension.java | 29 ++++---- .../smtp/core/fastfail/DNSRBLHandler.java | 21 +++--- .../smtp/core/fastfail/MaxUnknownCmdHandler.java | 13 ++-- .../core/fastfail/ResolvableEhloHeloHandler.java | 12 ++-- .../core/fastfail/SupressDuplicateRcptHandler.java | 38 +++++----- .../smtp/core/fastfail/DNSRBLHandlerTest.java | 73 ++++++++++--------- .../smtp/core/fastfail/MaxRcptHandlerTest.java | 4 +- .../core/fastfail/MaxUnknownCmdHandlerTest.java | 32 ++++++--- .../fastfail/ResolvableEhloHeloHandlerTest.java | 60 +++++++++------- .../fastfail/ValidSenderDomainHandlerTest.java | 35 +++++---- .../protocols/smtp/utils/BaseFakeSMTPSession.java | 14 ++-- .../DataLineJamesMessageHookHandler.java | 11 +-- .../org/apache/james/smtpserver/SMTPConstants.java | 8 ++- .../james/smtpserver/fastfail/SPFHandler.java | 29 ++++---- .../james/smtpserver/fastfail/URIRBLHandler.java | 18 ++--- .../netty/SMTPChannelUpstreamHandler.java | 4 +- .../apache/james/smtpserver/SPFHandlerTest.java | 41 +++++++---- .../james/smtpserver/SpamAssassinHandlerTest.java | 41 +++++++---- .../apache/james/smtpserver/URIRBLHandlerTest.java | 41 +++++++---- .../james/smtpserver/ValidRcptHandlerTest.java | 39 ++++++---- .../apache/james/smtpserver/ValidRcptMXTest.java | 38 ++++++---- upgrade-instructions.md | 18 +++++ 48 files changed, 614 insertions(+), 402 deletions(-) diff --git a/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSession.java b/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSession.java index 0937b4b..b483985 100644 --- a/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSession.java +++ b/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSession.java @@ -22,10 +22,16 @@ package org.apache.james.protocols.api; import java.net.InetSocketAddress; import java.nio.charset.Charset; import java.util.Map; +import java.util.Objects; +import java.util.Optional; import org.apache.james.core.Username; import org.apache.james.protocols.api.handler.LineHandler; +import com.google.common.base.MoreObjects; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; + /** * Session for a protocol. Every new connection generates a new session */ @@ -36,16 +42,75 @@ public interface ProtocolSession { Transaction } + class AttachmentKey<T> { + public static <U> AttachmentKey<U> of(String value, Class<U> type) { + Preconditions.checkArgument(!Strings.isNullOrEmpty(value), "An attachment key should not be empty or null"); + + return new AttachmentKey<>(value, type); + } + + private final String value; + private final Class<T> type; + + private AttachmentKey(String value, Class<T> type) { + this.value = value; + this.type = type; + } + + public String asString() { + return value; + } + + public Optional<T> convert(Object object) { + return Optional.ofNullable(object) + .filter(type::isInstance) + .map(type::cast); + } + + @Override + public final boolean equals(Object o) { + if (o instanceof AttachmentKey) { + AttachmentKey<?> that = (AttachmentKey<?>) o; + + return Objects.equals(this.value, that.value) + && Objects.equals(this.type, that.type); + } + return false; + } + + @Override + public final int hashCode() { + return Objects.hash(value, type); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("value", value) + .add("type", type.getName()) + .toString(); + } + } + /** - * Store the given value with the given key in the specified {@link State}. If you want to remove a value you need to use <code>null</code> as value + * Store the given value with the given key in the specified {@link State}. * * @param key the key under which the value should get stored - * @param value the value which will get stored under the given key or <code>null</code> if you want to remove any value which is stored under the key + * @param value the value which will get stored under the given key * @param state the {@link State} to which the mapping belongs * @return oldValue the value which was stored before for this key or <code>null</code> if non was stored before. */ - Object setAttachment(String key, Object value, State state); - + <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state); + + /** + * Remove a value stored for the given key in the specified {@link State}. + * + * @param key the key under which the value should get stored + * @param state the {@link State} to which the mapping belongs + * @return oldValue the value which was stored before for this key or <code>null</code> if non was stored before. + */ + <T> Optional<T> removeAttachment(AttachmentKey<T> key,State state); + /** * Return the value which is stored for the given key in the specified {@link State} or <code>null</code> if non was stored before. * @@ -53,27 +118,27 @@ public interface ProtocolSession { * @param state the {@link State} in which the value was stored for the key * @return value the stored value for the key */ - Object getAttachment(String key, State state); + <T> Optional<T> getAttachment(AttachmentKey<T> key, State state); /** * Return Map which can be used to store objects within a session * * @return state - * @deprecated use {@link #setAttachment(String, Object, State)} + * @deprecated use {@link #setAttachment(AttachmentKey, Object, State)} */ @Deprecated - Map<String, Object> getState(); + Map<AttachmentKey<?>, Object> getState(); /** * Returns Map that consists of the state of the {@link ProtocolSession} per connection * * @return map of the current {@link ProtocolSession} state per connection - * @deprecated use {@link #getAttachment(String, State)} + * @deprecated use {@link #getAttachment(AttachmentKey, State)} */ @Deprecated - Map<String,Object> getConnectionState(); + Map<AttachmentKey<?>, Object> getConnectionState(); /** diff --git a/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSessionImpl.java b/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSessionImpl.java index 266aeba..ec27d9e 100644 --- a/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSessionImpl.java +++ b/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSessionImpl.java @@ -23,10 +23,13 @@ import java.net.InetSocketAddress; import java.nio.charset.Charset; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.apache.james.core.Username; import org.apache.james.protocols.api.handler.LineHandler; +import com.google.common.base.Preconditions; + /** * Basic implementation of {@link ProtocolSession} * @@ -34,8 +37,8 @@ import org.apache.james.protocols.api.handler.LineHandler; */ public class ProtocolSessionImpl implements ProtocolSession { private final ProtocolTransport transport; - private final Map<String, Object> connectionState; - private final Map<String, Object> sessionState; + private final Map<AttachmentKey<?>, Object> connectionState; + private final Map<AttachmentKey<?>, Object> sessionState; private Username username; protected final ProtocolConfiguration config; private static final Charset CHARSET = Charset.forName("US-ASCII"); @@ -92,12 +95,12 @@ public class ProtocolSessionImpl implements ProtocolSession { @Override - public Map<String, Object> getConnectionState() { + public Map<AttachmentKey<?>, Object> getConnectionState() { return connectionState; } @Override - public Map<String, Object> getState() { + public Map<AttachmentKey<?>, Object> getState() { return sessionState; } @@ -134,28 +137,34 @@ public class ProtocolSessionImpl implements ProtocolSession { } @Override - public Object setAttachment(String key, Object value, State state) { + public <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + + if (state == State.Connection) { + return key.convert(connectionState.put(key, value)); + } else { + return key.convert(sessionState.put(key, value)); + } + } + + @Override + public <T> Optional<T> removeAttachment(AttachmentKey<T> key, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + if (state == State.Connection) { - if (value == null) { - return connectionState.remove(key); - } else { - return connectionState.put(key, value); - } + return key.convert(connectionState.remove(key)); } else { - if (value == null) { - return sessionState.remove(key); - } else { - return sessionState.put(key, value); - } + return key.convert(sessionState.remove(key)); } } @Override - public Object getAttachment(String key, State state) { + public <T> Optional<T> getAttachment(AttachmentKey<T> key, State state) { if (state == State.Connection) { - return connectionState.get(key); + return key.convert(connectionState.get(key)); } else { - return sessionState.get(key); + return key.convert(sessionState.get(key)); } } diff --git a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/SMTPConstants.java b/protocols/api/src/test/java/org/apache/james/protocols/api/ProtocolSessionTest.java similarity index 75% copy from server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/SMTPConstants.java copy to protocols/api/src/test/java/org/apache/james/protocols/api/ProtocolSessionTest.java index b401721..584735f 100644 --- a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/SMTPConstants.java +++ b/protocols/api/src/test/java/org/apache/james/protocols/api/ProtocolSessionTest.java @@ -7,7 +7,7 @@ * "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 * + * 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 * @@ -15,16 +15,18 @@ * KIND, either express or implied. See the License for the * * specific language governing permissions and limitations * * under the License. * - ****************************************************************/ + ***************************************************************/ -package org.apache.james.smtpserver; +package org.apache.james.protocols.api; -/** - * Constants which are used within SMTP Session - */ -public interface SMTPConstants { +import org.junit.jupiter.api.Test; - String DATA_MIMEMESSAGE_STREAMSOURCE = "org.apache.james.core.DataCmdHandler.DATA_MIMEMESSAGE_STREAMSOURCE"; - String MAIL = "MAIL"; +import nl.jqno.equalsverifier.EqualsVerifier; -} +class ProtocolSessionTest { + + @Test + void attachmentKeyShouldRespectBeanContract() { + EqualsVerifier.forClass(ProtocolSession.AttachmentKey.class).verify(); + } +} \ No newline at end of file diff --git a/protocols/lmtp/src/main/java/org/apache/james/protocols/lmtp/core/DataLineMessageHookHandler.java b/protocols/lmtp/src/main/java/org/apache/james/protocols/lmtp/core/DataLineMessageHookHandler.java index 1e971cf..cc69dab 100644 --- a/protocols/lmtp/src/main/java/org/apache/james/protocols/lmtp/core/DataLineMessageHookHandler.java +++ b/protocols/lmtp/src/main/java/org/apache/james/protocols/lmtp/core/DataLineMessageHookHandler.java @@ -27,7 +27,7 @@ import org.apache.james.protocols.api.Response; import org.apache.james.protocols.api.handler.WiringException; import org.apache.james.protocols.lmtp.LMTPMultiResponse; import org.apache.james.protocols.lmtp.hook.DeliverToRecipientHook; -import org.apache.james.protocols.smtp.MailEnvelopeImpl; +import org.apache.james.protocols.smtp.MailEnvelope; import org.apache.james.protocols.smtp.SMTPResponse; import org.apache.james.protocols.smtp.SMTPRetCode; import org.apache.james.protocols.smtp.SMTPSession; @@ -43,7 +43,7 @@ public class DataLineMessageHookHandler extends org.apache.james.protocols.smtp. @Override - protected Response processExtensions(SMTPSession session, MailEnvelopeImpl mail) { + protected Response processExtensions(SMTPSession session, MailEnvelope mail) { LMTPMultiResponse mResponse = null; for (MailAddress recipient : mail.getRecipients()) { diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/POP3Session.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/POP3Session.java index 332cef4..1be4efd 100644 --- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/POP3Session.java +++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/POP3Session.java @@ -19,8 +19,11 @@ package org.apache.james.protocols.pop3; +import java.util.List; + import org.apache.james.protocols.api.ProtocolSession; import org.apache.james.protocols.pop3.mailbox.Mailbox; +import org.apache.james.protocols.pop3.mailbox.MessageMetaData; /** * All the handlers access this interface to communicate with POP3Handler object @@ -28,9 +31,11 @@ import org.apache.james.protocols.pop3.mailbox.Mailbox; public interface POP3Session extends ProtocolSession { - String UID_LIST = "UID_LIST"; - String DELETED_UID_LIST = "DELETED_UID_LIST"; - String APOP_TIMESTAMP = "APOP_TIMESTAMP"; + @SuppressWarnings("unchecked") + AttachmentKey<List<MessageMetaData>> UID_LIST = AttachmentKey.of("UID_LIST", (Class<List<MessageMetaData>>) (Object) List.class); + @SuppressWarnings("unchecked") + AttachmentKey<List<String>> DELETED_UID_LIST = AttachmentKey.of("DELETED_UID_LIST", (Class<List<String>>) (Object) List.class); + AttachmentKey<String> APOP_TIMESTAMP = AttachmentKey.of("APOP_TIMESTAMP", String.class); // Authentication states for the POP3 interaction /** Waiting for user id */ diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/AbstractApopCmdHandler.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/AbstractApopCmdHandler.java index 656339a..521c1df 100644 --- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/AbstractApopCmdHandler.java +++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/AbstractApopCmdHandler.java @@ -37,10 +37,11 @@ import com.google.common.collect.ImmutableSet; public abstract class AbstractApopCmdHandler extends AbstractPassCmdHandler { private static final Collection<String> COMMANDS = ImmutableSet.of("APOP"); - + private static final String MISSING_APOP_TIMESTAMP = ""; + @Override public Response onCommand(POP3Session session, Request request) { - if (session.getAttachment(POP3Session.APOP_TIMESTAMP, State.Connection) == null) { + if (!session.getAttachment(POP3Session.APOP_TIMESTAMP, State.Connection).isPresent()) { // APOP timestamp was not found in the session so APOP is not supported return POP3Response.ERR; } @@ -81,7 +82,7 @@ public abstract class AbstractApopCmdHandler extends AbstractPassCmdHandler { @Override protected final Mailbox auth(POP3Session session, Username username, String password) throws Exception { - return auth(session, (String)session.getAttachment(POP3Session.APOP_TIMESTAMP, State.Connection), username, password); + return auth(session, session.getAttachment(POP3Session.APOP_TIMESTAMP, State.Connection).orElse(MISSING_APOP_TIMESTAMP), username, password); } diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/DeleCmdHandler.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/DeleCmdHandler.java index 93263aa..ce499d7 100644 --- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/DeleCmdHandler.java +++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/DeleCmdHandler.java @@ -19,6 +19,7 @@ package org.apache.james.protocols.pop3.core; +import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -61,7 +62,12 @@ public class DeleCmdHandler implements CommandHandler<POP3Session> { StringBuilder responseBuffer = new StringBuilder(64).append("Message (").append(num).append(") does not exist."); return new POP3Response(POP3Response.ERR_RESPONSE, responseBuffer.toString()); } - List<String> deletedUidList = (List<String>) session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction); + List<String> deletedUidList = session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction) + .orElseGet(() -> { + ArrayList<String> uidList = new ArrayList<>(); + session.setAttachment(POP3Session.DELETED_UID_LIST, uidList, State.Transaction); + return uidList; + }); String uid = meta.getUid(); diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/ListCmdHandler.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/ListCmdHandler.java index 7cfeaf9..b706146 100644 --- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/ListCmdHandler.java +++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/ListCmdHandler.java @@ -31,6 +31,7 @@ import org.apache.james.protocols.pop3.POP3Response; import org.apache.james.protocols.pop3.POP3Session; import org.apache.james.protocols.pop3.mailbox.MessageMetaData; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; /** @@ -55,8 +56,8 @@ public class ListCmdHandler implements CommandHandler<POP3Session> { @SuppressWarnings("unchecked") public Response onCommand(POP3Session session, Request request) { String parameters = request.getArgument(); - List<MessageMetaData> uidList = (List<MessageMetaData>) session.getAttachment(POP3Session.UID_LIST, State.Transaction); - List<String> deletedUidList = (List<String>) session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction); + List<MessageMetaData> uidList = session.getAttachment(POP3Session.UID_LIST, State.Transaction).orElse(ImmutableList.of()); + List<String> deletedUidList = session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction).orElse(ImmutableList.of()); if (session.getHandlerState() == POP3Session.TRANSACTION) { POP3Response response = null; @@ -66,10 +67,10 @@ public class ListCmdHandler implements CommandHandler<POP3Session> { long size = 0; int count = 0; List<MessageMetaData> validResults = new ArrayList<>(); - if (uidList.isEmpty() == false) { + if (!uidList.isEmpty()) { for (MessageMetaData data : uidList) { - if (deletedUidList.contains(data.getUid()) == false) { + if (!deletedUidList.contains(data.getUid())) { size += data.getSize(); count++; validResults.add(data); @@ -95,7 +96,7 @@ public class ListCmdHandler implements CommandHandler<POP3Session> { return new POP3Response(POP3Response.ERR_RESPONSE, responseBuffer.toString()); } - if (deletedUidList.contains(data.getUid()) == false) { + if (!deletedUidList.contains(data.getUid())) { StringBuilder responseBuffer = new StringBuilder(64).append(num).append(" ").append(data.getSize()); response = new POP3Response(POP3Response.OK_RESPONSE, responseBuffer.toString()); } else { diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/MessageMetaDataUtils.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/MessageMetaDataUtils.java index 477f5cf..339c758 100644 --- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/MessageMetaDataUtils.java +++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/MessageMetaDataUtils.java @@ -19,7 +19,6 @@ package org.apache.james.protocols.pop3.core; -import java.util.List; import java.util.stream.IntStream; import org.apache.james.protocols.api.ProtocolSession.State; @@ -33,13 +32,10 @@ public class MessageMetaDataUtils { * found. */ public static MessageMetaData getMetaData(POP3Session session, int number) { - @SuppressWarnings("unchecked") - List<MessageMetaData> uidList = (List<MessageMetaData>) session.getAttachment(POP3Session.UID_LIST, State.Transaction); - if (uidList == null || number > uidList.size()) { - return null; - } else { - return uidList.get(number - 1); - } + return session.getAttachment(POP3Session.UID_LIST, State.Transaction) + .filter(uidList -> number <= uidList.size()) + .map(uidList -> uidList.get(number - 1)) + .orElse(null); } /** diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/QuitCmdHandler.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/QuitCmdHandler.java index 791c062..f61714d 100644 --- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/QuitCmdHandler.java +++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/QuitCmdHandler.java @@ -33,6 +33,7 @@ import org.apache.james.protocols.pop3.mailbox.Mailbox; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; /** @@ -59,13 +60,12 @@ public class QuitCmdHandler implements CommandHandler<POP3Session> { * cleanup of the POP3Handler state. */ @Override - @SuppressWarnings("unchecked") public Response onCommand(POP3Session session, Request request) { Response response = null; if (session.getHandlerState() == POP3Session.AUTHENTICATION_READY || session.getHandlerState() == POP3Session.AUTHENTICATION_USERSET) { return SIGN_OFF; } - List<String> toBeRemoved = (List<String>) session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction); + List<String> toBeRemoved = session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction).orElse(ImmutableList.of()); Mailbox mailbox = session.getUserMailbox(); try { String[] uids = toBeRemoved.toArray(new String[toBeRemoved.size()]); diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/RetrCmdHandler.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/RetrCmdHandler.java index f87d604..99d050c 100644 --- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/RetrCmdHandler.java +++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/RetrCmdHandler.java @@ -34,6 +34,7 @@ import org.apache.james.protocols.pop3.POP3StreamResponse; import org.apache.james.protocols.pop3.mailbox.MessageMetaData; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; /** @@ -70,10 +71,10 @@ public class RetrCmdHandler implements CommandHandler<POP3Session> { response = new POP3Response(POP3Response.ERR_RESPONSE, responseBuffer.toString()); return response; } - List<String> deletedUidList = (List<String>) session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction); + List<String> deletedUidList = session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction).orElse(ImmutableList.of()); String uid = data.getUid(); - if (deletedUidList.contains(uid) == false) { + if (!deletedUidList.contains(uid)) { InputStream content = session.getUserMailbox().getMessage(uid); if (content != null) { diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/RsetCmdHandler.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/RsetCmdHandler.java index 74601ad..f88ec0b 100644 --- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/RsetCmdHandler.java +++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/RsetCmdHandler.java @@ -69,7 +69,7 @@ public class RsetCmdHandler implements CommandHandler<POP3Session> { List<MessageMetaData> messages = session.getUserMailbox().getMessages(); session.setAttachment(POP3Session.UID_LIST, messages, State.Transaction); - session.setAttachment(POP3Session.DELETED_UID_LIST, new ArrayList<String>(), State.Transaction); + session.setAttachment(POP3Session.DELETED_UID_LIST, new ArrayList<>(), State.Transaction); } catch (IOException e) { // In the event of an exception being thrown there may or may not be // anything in userMailbox diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/StatCmdHandler.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/StatCmdHandler.java index 89105f9..f942124 100644 --- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/StatCmdHandler.java +++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/StatCmdHandler.java @@ -31,6 +31,7 @@ import org.apache.james.protocols.pop3.POP3Response; import org.apache.james.protocols.pop3.POP3Session; import org.apache.james.protocols.pop3.mailbox.MessageMetaData; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; /** @@ -44,18 +45,17 @@ public class StatCmdHandler implements CommandHandler<POP3Session> { * of messages in the mailbox and its aggregate size. */ @Override - @SuppressWarnings("unchecked") public Response onCommand(POP3Session session, Request request) { if (session.getHandlerState() == POP3Session.TRANSACTION) { - List<MessageMetaData> uidList = (List<MessageMetaData>) session.getAttachment(POP3Session.UID_LIST, State.Transaction); - List<String> deletedUidList = (List<String>) session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction); + List<MessageMetaData> uidList = session.getAttachment(POP3Session.UID_LIST, State.Transaction).orElse(ImmutableList.of()); + List<String> deletedUidList = session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction).orElse(ImmutableList.of()); long size = 0; int count = 0; - if (uidList.isEmpty() == false) { + if (!uidList.isEmpty()) { List<MessageMetaData> validResults = new ArrayList<>(); for (MessageMetaData data : uidList) { - if (deletedUidList.contains(data.getUid()) == false) { + if (!deletedUidList.contains(data.getUid())) { size += data.getSize(); count++; validResults.add(data); diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/TopCmdHandler.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/TopCmdHandler.java index b77f3ff..5931a2b 100644 --- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/TopCmdHandler.java +++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/TopCmdHandler.java @@ -88,10 +88,10 @@ public class TopCmdHandler extends RetrCmdHandler implements CapaCapability { return new POP3Response(POP3Response.ERR_RESPONSE, responseBuffer.toString()); } - List<String> deletedUidList = (List<String>) session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction); + List<String> deletedUidList = session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction).orElse(ImmutableList.of()); String uid = data.getUid(); - if (deletedUidList.contains(uid) == false) { + if (!deletedUidList.contains(uid)) { InputStream message = new CountingBodyInputStream(new ExtraDotInputStream(new CRLFTerminatedInputStream(session.getUserMailbox().getMessage(uid))), lines); return new POP3StreamResponse(POP3Response.OK_RESPONSE, "Message follows", message); diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/UidlCmdHandler.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/UidlCmdHandler.java index 7f33832..4155a65 100644 --- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/UidlCmdHandler.java +++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/core/UidlCmdHandler.java @@ -33,6 +33,7 @@ import org.apache.james.protocols.pop3.POP3Response; import org.apache.james.protocols.pop3.POP3Session; import org.apache.james.protocols.pop3.mailbox.MessageMetaData; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; /** @@ -47,13 +48,12 @@ public class UidlCmdHandler implements CommandHandler<POP3Session>, CapaCapabili * of message ids to the client. */ @Override - @SuppressWarnings("unchecked") public Response onCommand(POP3Session session, Request request) { POP3Response response = null; String parameters = request.getArgument(); if (session.getHandlerState() == POP3Session.TRANSACTION) { - List<MessageMetaData> uidList = (List<MessageMetaData>) session.getAttachment(POP3Session.UID_LIST, State.Transaction); - List<String> deletedUidList = (List<String>) session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction); + List<MessageMetaData> uidList = session.getAttachment(POP3Session.UID_LIST, State.Transaction).orElse(ImmutableList.of()); + List<String> deletedUidList = session.getAttachment(POP3Session.DELETED_UID_LIST, State.Transaction).orElse(ImmutableList.of()); try { String identifier = session.getUserMailbox().getIdentifier(); if (parameters == null) { @@ -61,7 +61,7 @@ public class UidlCmdHandler implements CommandHandler<POP3Session>, CapaCapabili for (int i = 0; i < uidList.size(); i++) { MessageMetaData metadata = uidList.get(i); - if (deletedUidList.contains(metadata.getUid()) == false) { + if (!deletedUidList.contains(metadata.getUid())) { StringBuilder responseBuffer = new StringBuilder().append(i + 1).append(" ").append(metadata.getUid(identifier)); response.appendLine(responseBuffer.toString()); } diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSession.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSession.java index 02c84c2..1167750 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSession.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSession.java @@ -19,6 +19,10 @@ package org.apache.james.protocols.smtp; +import java.util.List; + +import org.apache.james.core.MailAddress; +import org.apache.james.core.MaybeSender; import org.apache.james.protocols.api.ProtocolSession; /** @@ -30,12 +34,13 @@ public interface SMTPSession extends ProtocolSession { // Keys used to store/lookup data in the internal state hash map /** Sender's email address */ - String SENDER = "SENDER_ADDRESS"; + AttachmentKey<MaybeSender> SENDER = AttachmentKey.of("SENDER_ADDRESS", MaybeSender.class); /** The message recipients */ - String RCPT_LIST = "RCPT_LIST"; + @SuppressWarnings("unchecked") + AttachmentKey<List<MailAddress>> RCPT_LIST = AttachmentKey.of("RCPT_LIST", (Class<List<MailAddress>>) (Object) List.class); /** HELO or EHLO */ - String CURRENT_HELO_MODE = "CURRENT_HELO_MODE"; - String CURRENT_HELO_NAME = "CURRENT_HELO_NAME"; + AttachmentKey<String> CURRENT_HELO_MODE = AttachmentKey.of("CURRENT_HELO_MODE", String.class); + AttachmentKey<String> CURRENT_HELO_NAME = AttachmentKey.of("CURRENT_HELO_NAME", String.class); /** * Returns the service wide configuration diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java index 91cde64..6b02cf4 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java @@ -18,9 +18,9 @@ ****************************************************************/ package org.apache.james.protocols.smtp; -import java.util.Collection; +import java.util.List; +import java.util.Optional; -import org.apache.james.core.MailAddress; import org.apache.james.protocols.api.ProtocolSessionImpl; import org.apache.james.protocols.api.ProtocolTransport; import org.apache.james.protocols.api.Response; @@ -65,27 +65,19 @@ public class SMTPSessionImpl extends ProtocolSessionImpl implements SMTPSession @Override public void resetState() { // remember the ehlo mode between resets - Object currentHeloMode = getState().get(CURRENT_HELO_MODE); + Optional<String> currentHeloMode = getAttachment(CURRENT_HELO_MODE, State.Transaction); getState().clear(); // start again with the old helo mode - if (currentHeloMode != null) { - getState().put(CURRENT_HELO_MODE, currentHeloMode); - } + currentHeloMode.ifPresent(heloMode -> setAttachment(CURRENT_HELO_MODE, heloMode, State.Transaction)); } @Override - @SuppressWarnings("unchecked") public int getRcptCount() { - int count = 0; - - // check if the key exists - if (getState().get(SMTPSession.RCPT_LIST) != null) { - count = ((Collection<MailAddress>) getState().get(SMTPSession.RCPT_LIST)).size(); - } - - return count; + return getAttachment(SMTPSession.RCPT_LIST, State.Transaction) + .map(List::size) + .orElse(0); } @Override diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractAddHeadersFilter.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractAddHeadersFilter.java index 9c29ffd..8f0e8e3 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractAddHeadersFilter.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractAddHeadersFilter.java @@ -25,6 +25,7 @@ import java.util.Collection; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; +import org.apache.james.protocols.api.ProtocolSession; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.api.Response; import org.apache.james.protocols.api.handler.LineHandler; @@ -39,8 +40,8 @@ public abstract class AbstractAddHeadersFilter extends SeparatingDataLineFilter private static final AtomicInteger COUNTER = new AtomicInteger(0); - private final String headersPrefixAdded = "HEADERS_PREFIX_ADDED" + COUNTER.incrementAndGet(); - private final String headersSuffixAdded = "HEADERS_SUFFIX_ADDED" + COUNTER.incrementAndGet(); + private final ProtocolSession.AttachmentKey<Boolean> headersPrefixAdded = ProtocolSession.AttachmentKey.of("HEADERS_PREFIX_ADDED" + COUNTER.incrementAndGet(), Boolean.class); + private final ProtocolSession.AttachmentKey<Boolean> headersSuffixAdded = ProtocolSession.AttachmentKey.of("HEADERS_SUFFIX_ADDED" + COUNTER.incrementAndGet(), Boolean.class); enum Location { Prefix, @@ -57,7 +58,7 @@ public abstract class AbstractAddHeadersFilter extends SeparatingDataLineFilter @Override protected Response onSeparatorLine(SMTPSession session, ByteBuffer line, LineHandler<SMTPSession> next) { - if (getLocation() == Location.Suffix && session.getAttachment(headersSuffixAdded, State.Transaction) == null) { + if (getLocation() == Location.Suffix && !session.getAttachment(headersSuffixAdded, State.Transaction).isPresent()) { session.setAttachment(headersSuffixAdded, Boolean.TRUE, State.Transaction); return addHeaders(session, line, next); } @@ -66,7 +67,7 @@ public abstract class AbstractAddHeadersFilter extends SeparatingDataLineFilter @Override protected Response onHeadersLine(SMTPSession session, ByteBuffer line, LineHandler<SMTPSession> next) { - if (getLocation() == Location.Prefix && session.getAttachment(headersPrefixAdded, State.Transaction) == null) { + if (getLocation() == Location.Prefix && !session.getAttachment(headersPrefixAdded, State.Transaction).isPresent()) { session.setAttachment(headersPrefixAdded, Boolean.TRUE, State.Transaction); return addHeaders(session, line, next); } diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractSenderAuthIdentifyVerificationRcptHook.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractSenderAuthIdentifyVerificationRcptHook.java index 487ec8c..e709b3e 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractSenderAuthIdentifyVerificationRcptHook.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractSenderAuthIdentifyVerificationRcptHook.java @@ -46,7 +46,7 @@ public abstract class AbstractSenderAuthIdentifyVerificationRcptHook implements @Override public HookResult doRcpt(SMTPSession session, MaybeSender sender, MailAddress rcpt) { if (session.getUsername() != null) { - MaybeSender senderAddress = (MaybeSender) session.getAttachment(SMTPSession.SENDER, ProtocolSession.State.Transaction); + MaybeSender senderAddress = session.getAttachment(SMTPSession.SENDER, ProtocolSession.State.Transaction).orElse(MaybeSender.nullSender()); // Check if the sender address is the same as the user which was used to authenticate. // Its important to ignore case here to fix JAMES-837. This is save todo because if the handler is called diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataCmdHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataCmdHandler.java index 8bebea0..d93b22f 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataCmdHandler.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataCmdHandler.java @@ -21,7 +21,6 @@ package org.apache.james.protocols.smtp.core; import java.io.Closeable; import java.io.IOException; import java.nio.ByteBuffer; -import java.util.ArrayList; import java.util.Collection; import java.util.LinkedList; import java.util.List; @@ -49,6 +48,7 @@ import org.apache.james.protocols.smtp.SMTPSession; import org.apache.james.protocols.smtp.dsn.DSNStatus; import org.apache.james.util.MDCBuilder; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -112,7 +112,7 @@ public class DataCmdHandler implements CommandHandler<SMTPSession>, ExtensibleHa } } - public static final String MAILENV = "MAILENV"; + public static final ProtocolSession.AttachmentKey<MailEnvelope> MAILENV = ProtocolSession.AttachmentKey.of("MAILENV", MailEnvelope.class); private final MetricFactory metricFactory; @@ -161,9 +161,9 @@ public class DataCmdHandler implements CommandHandler<SMTPSession>, ExtensibleHa */ @SuppressWarnings("unchecked") protected Response doDATA(SMTPSession session, String argument) { - MaybeSender sender = (MaybeSender) session.getAttachment(SMTPSession.SENDER, ProtocolSession.State.Transaction); - MailEnvelope env = createEnvelope(session, sender, new ArrayList<>((Collection<MailAddress>) session.getAttachment(SMTPSession.RCPT_LIST, ProtocolSession.State.Transaction))); - session.setAttachment(MAILENV, env,ProtocolSession.State.Transaction); + MaybeSender sender = session.getAttachment(SMTPSession.SENDER, ProtocolSession.State.Transaction).orElse(MaybeSender.nullSender()); + MailEnvelope env = createEnvelope(session, sender, session.getAttachment(SMTPSession.RCPT_LIST, ProtocolSession.State.Transaction).orElse(ImmutableList.of())); + session.setAttachment(MAILENV, env, ProtocolSession.State.Transaction); session.pushLineHandler(lineHandler); return DATA_READY; @@ -208,9 +208,9 @@ public class DataCmdHandler implements CommandHandler<SMTPSession>, ExtensibleHa if ((argument != null) && (argument.length() > 0)) { return UNEXPECTED_ARG; } - if (session.getAttachment(SMTPSession.SENDER, ProtocolSession.State.Transaction) == null) { + if (!session.getAttachment(SMTPSession.SENDER, ProtocolSession.State.Transaction).isPresent()) { return NO_SENDER; - } else if (session.getAttachment(SMTPSession.RCPT_LIST, ProtocolSession.State.Transaction) == null) { + } else if (!session.getAttachment(SMTPSession.RCPT_LIST, ProtocolSession.State.Transaction).isPresent()) { return NO_RECIPIENT; } return null; diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataLineMessageHookHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataLineMessageHookHandler.java index cc645ea..80aded3 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataLineMessageHookHandler.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataLineMessageHookHandler.java @@ -32,7 +32,7 @@ import org.apache.james.protocols.api.Response; import org.apache.james.protocols.api.handler.ExtensibleHandler; import org.apache.james.protocols.api.handler.LineHandler; import org.apache.james.protocols.api.handler.WiringException; -import org.apache.james.protocols.smtp.MailEnvelopeImpl; +import org.apache.james.protocols.smtp.MailEnvelope; import org.apache.james.protocols.smtp.SMTPResponse; import org.apache.james.protocols.smtp.SMTPRetCode; import org.apache.james.protocols.smtp.SMTPSession; @@ -59,8 +59,10 @@ public class DataLineMessageHookHandler implements DataLineFilter, ExtensibleHan @Override public Response onLine(SMTPSession session, ByteBuffer line, LineHandler<SMTPSession> next) { - MailEnvelopeImpl env = (MailEnvelopeImpl) session.getAttachment(DataCmdHandler.MAILENV, ProtocolSession.State.Transaction); - OutputStream out = env.getMessageOutputStream(); + MailEnvelope env = session.getAttachment(DataCmdHandler.MAILENV, ProtocolSession.State.Transaction) + .orElseThrow(() -> new RuntimeException("'" + DataCmdHandler.MAILENV.asString() + "' has not been filled.")); + + OutputStream out = getMessageOutputStream(env); try { // 46 is "." // Stream terminated @@ -95,6 +97,14 @@ public class DataLineMessageHookHandler implements DataLineFilter, ExtensibleHan return null; } + private OutputStream getMessageOutputStream(MailEnvelope env) { + try { + return env.getMessageOutputStream(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + private byte[] readBytes(ByteBuffer line) { line.rewind(); byte[] bline; @@ -107,10 +117,10 @@ public class DataLineMessageHookHandler implements DataLineFilter, ExtensibleHan return bline; } - protected Response processExtensions(SMTPSession session, MailEnvelopeImpl mail) { + protected Response processExtensions(SMTPSession session, MailEnvelope mail) { - if (mail != null && messageHandlers != null) { + if (messageHandlers != null) { for (Object messageHandler : messageHandlers) { MessageHook rawHandler = (MessageHook) messageHandler; LOGGER.debug("executing message handler {}", rawHandler); 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 a9853e4..5f837e7 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 @@ -87,7 +87,7 @@ public class MailCmdHandler extends AbstractHookableCmdHandler<MailHook> { // Check if the response was not ok if (response.getRetCode().equals(SMTPRetCode.MAIL_OK) == false) { // cleanup the session - session.setAttachment(SMTPSession.SENDER, null, State.Transaction); + session.removeAttachment(SMTPSession.SENDER, State.Transaction); } return response; @@ -99,16 +99,14 @@ public class MailCmdHandler extends AbstractHookableCmdHandler<MailHook> { * * @param session * SMTP session object - * @param argument - * the argument passed in with the command by the SMTP client */ - private Response doMAIL(SMTPSession session, String argument) { + private Response doMAIL(SMTPSession session) { StringBuilder responseBuffer = new StringBuilder(); - MaybeSender sender = (MaybeSender) session.getAttachment(SMTPSession.SENDER, State.Transaction); + MaybeSender sender = session.getAttachment(SMTPSession.SENDER, State.Transaction).orElse(MaybeSender.nullSender()); responseBuffer.append( DSNStatus.getStatus(DSNStatus.SUCCESS, DSNStatus.ADDRESS_OTHER)) .append(" Sender <"); - if (sender != null) { + if (!sender.isNullSender()) { responseBuffer.append(sender.asString()); } responseBuffer.append("> OK"); @@ -123,7 +121,7 @@ public class MailCmdHandler extends AbstractHookableCmdHandler<MailHook> { @Override protected Response doCoreCmd(SMTPSession session, String command, String parameters) { - return doMAIL(session, parameters); + return doMAIL(session); } @Override @@ -146,10 +144,10 @@ public class MailCmdHandler extends AbstractHookableCmdHandler<MailHook> { sender = argument.substring(colonIndex + 1); argument = argument.substring(0, colonIndex); } - if (session.getAttachment(SMTPSession.SENDER, State.Transaction) != null) { + if (session.getAttachment(SMTPSession.SENDER, State.Transaction).isPresent()) { return SENDER_ALREADY_SPECIFIED; - } else if (session.getAttachment( - SMTPSession.CURRENT_HELO_MODE, State.Connection) == null + } else if (!session.getAttachment( + SMTPSession.CURRENT_HELO_MODE, State.Connection).isPresent() && session.getConfiguration().useHeloEhloEnforcement()) { return EHLO_HELO_NEEDED; } else if (argument == null @@ -248,7 +246,7 @@ public class MailCmdHandler extends AbstractHookableCmdHandler<MailHook> { @Override protected HookResult callHook(MailHook rawHook, SMTPSession session, String parameters) { - MaybeSender sender = (MaybeSender) session.getAttachment(SMTPSession.SENDER, State.Transaction); + MaybeSender sender = session.getAttachment(SMTPSession.SENDER, State.Transaction).orElse(MaybeSender.nullSender()); return rawHook.doMail(session, sender); } diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/RcptCmdHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/RcptCmdHandler.java index c5e8815..93ce9d6 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/RcptCmdHandler.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/RcptCmdHandler.java @@ -21,6 +21,7 @@ package org.apache.james.protocols.smtp.core; import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.Locale; import java.util.StringTokenizer; @@ -29,6 +30,7 @@ import javax.inject.Inject; 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; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.api.Response; import org.apache.james.protocols.api.handler.CommandHandler; @@ -49,7 +51,7 @@ import com.google.common.collect.ImmutableSet; public class RcptCmdHandler extends AbstractHookableCmdHandler<RcptHook> implements CommandHandler<SMTPSession> { private static final Logger LOGGER = LoggerFactory.getLogger(RcptCmdHandler.class); - public static final String CURRENT_RECIPIENT = "CURRENT_RECIPIENT"; + public static final ProtocolSession.AttachmentKey<MailAddress> CURRENT_RECIPIENT = ProtocolSession.AttachmentKey.of("CURRENT_RECIPIENT", MailAddress.class); private static final Collection<String> COMMANDS = ImmutableSet.of("RCPT"); private static final Response MAIL_NEEDED = new SMTPResponse(SMTPRetCode.BAD_SEQUENCE, DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.DELIVERY_OTHER) + " Need MAIL before RCPT").immutable(); private static final Response SYNTAX_ERROR_ARGS = new SMTPResponse(SMTPRetCode.SYNTAX_ERROR_ARGUMENTS, DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.DELIVERY_SYNTAX) + " Usage: RCPT TO:<recipient>").immutable(); @@ -74,16 +76,9 @@ public class RcptCmdHandler extends AbstractHookableCmdHandler<RcptHook> impleme * parameters passed in with the command by the SMTP client */ @Override - @SuppressWarnings("unchecked") - protected Response doCoreCmd(SMTPSession session, String command, - String parameters) { - Collection<MailAddress> rcptColl = (Collection<MailAddress>) session.getAttachment( - SMTPSession.RCPT_LIST, State.Transaction); - if (rcptColl == null) { - rcptColl = new ArrayList<>(); - } - MailAddress recipientAddress = (MailAddress) session.getAttachment( - CURRENT_RECIPIENT, State.Transaction); + protected Response doCoreCmd(SMTPSession session, String command, String parameters) { + List<MailAddress> rcptColl = session.getAttachment(SMTPSession.RCPT_LIST, State.Transaction).orElseGet(ArrayList::new); + MailAddress recipientAddress = session.getAttachment(CURRENT_RECIPIENT, State.Transaction).orElse(MailAddress.nullSender()); rcptColl.add(recipientAddress); session.setAttachment(SMTPSession.RCPT_LIST, rcptColl, State.Transaction); StringBuilder response = new StringBuilder(); @@ -111,7 +106,7 @@ public class RcptCmdHandler extends AbstractHookableCmdHandler<RcptHook> impleme recipient = argument.substring(colonIndex + 1); argument = argument.substring(0, colonIndex); } - if (session.getAttachment(SMTPSession.SENDER, State.Transaction) == null) { + if (!session.getAttachment(SMTPSession.SENDER, State.Transaction).isPresent()) { return MAIL_NEEDED; } else if (argument == null || !argument.toUpperCase(Locale.US).equals("TO") @@ -187,7 +182,7 @@ public class RcptCmdHandler extends AbstractHookableCmdHandler<RcptHook> impleme optionTokenizer = null; } - session.setAttachment(CURRENT_RECIPIENT,recipientAddress, State.Transaction); + session.setAttachment(CURRENT_RECIPIENT, recipientAddress, State.Transaction); return null; } @@ -199,8 +194,9 @@ public class RcptCmdHandler extends AbstractHookableCmdHandler<RcptHook> impleme } else if (null != recipient) { sb.append(" [to:").append(recipient).append(']'); } - MaybeSender sender = (MaybeSender) session.getAttachment(SMTPSession.SENDER, State.Transaction); - if (null != sender && !sender.isNullSender()) { + + MaybeSender sender = session.getAttachment(SMTPSession.SENDER, State.Transaction).orElse(MaybeSender.nullSender()); + if (!sender.isNullSender()) { sb.append(" [from:").append(sender.asString()).append(']'); } return sb.toString(); @@ -218,9 +214,8 @@ public class RcptCmdHandler extends AbstractHookableCmdHandler<RcptHook> impleme @Override protected HookResult callHook(RcptHook rawHook, SMTPSession session, String parameters) { - MaybeSender sender = (MaybeSender) session.getAttachment(SMTPSession.SENDER, State.Transaction); - return rawHook.doRcpt(session, sender, - (MailAddress) session.getAttachment(CURRENT_RECIPIENT, State.Transaction)); + MaybeSender sender = session.getAttachment(SMTPSession.SENDER, State.Transaction).orElse(MaybeSender.nullSender()); + return rawHook.doRcpt(session, sender, session.getAttachment(CURRENT_RECIPIENT, State.Transaction).orElse(MailAddress.nullSender())); } protected String getDefaultDomain() { diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/ReceivedDataLineFilter.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/ReceivedDataLineFilter.java index 48b0a73..3c24277 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/ReceivedDataLineFilter.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/ReceivedDataLineFilter.java @@ -25,11 +25,14 @@ import java.util.Collection; import java.util.Date; import java.util.List; import java.util.Locale; +import java.util.Optional; import org.apache.james.core.MailAddress; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.smtp.SMTPSession; +import com.google.common.collect.ImmutableList; + /** * {@link AbstractAddHeadersFilter} which adds the Received header for the message. */ @@ -85,30 +88,31 @@ public class ReceivedDataLineFilter extends AbstractAddHeadersFilter { StringBuilder headerLineBuffer = new StringBuilder(); - String heloMode = (String) session.getAttachment(SMTPSession.CURRENT_HELO_MODE, State.Connection); - String heloName = (String) session.getAttachment(SMTPSession.CURRENT_HELO_NAME, State.Connection); + Optional<String> heloMode = session.getAttachment(SMTPSession.CURRENT_HELO_MODE, State.Connection); + Optional<String> heloName = session.getAttachment(SMTPSession.CURRENT_HELO_NAME, State.Connection); // Put our Received header first headerLineBuffer.append("from ").append(session.getRemoteAddress().getHostName()); - if (heloName != null) { - headerLineBuffer.append(" (").append(heloMode).append(" ").append(heloName).append(")"); + if (heloName.isPresent() && heloMode.isPresent()) { + headerLineBuffer.append(" (").append(heloMode.get()).append(" ").append(heloName.get()).append(")"); } headerLineBuffer.append(" ([").append(session.getRemoteAddress().getAddress().getHostAddress()).append("])"); Header header = new Header("Received", headerLineBuffer.toString()); headerLineBuffer = new StringBuilder(); - headerLineBuffer.append("by ").append(session.getConfiguration().getHelloName()).append(" (").append(session.getConfiguration().getSoftwareName()).append(") with ").append(getServiceType(session, heloMode)); + headerLineBuffer.append("by ").append(session.getConfiguration().getHelloName()).append(" (").append(session.getConfiguration().getSoftwareName()).append(") with ").append(getServiceType(session, heloMode.orElse("NOT-DEFINED"))); headerLineBuffer.append(" ID ").append(session.getSessionID()); - if (((Collection<?>) session.getAttachment(SMTPSession.RCPT_LIST, State.Transaction)).size() == 1) { + List<MailAddress> rcptList = session.getAttachment(SMTPSession.RCPT_LIST, State.Transaction).orElse(ImmutableList.of()); + if (rcptList.size() == 1) { // Only indicate a recipient if they're the only recipient // (prevents email address harvesting and large headers in // bulk email) header.add(headerLineBuffer.toString()); headerLineBuffer = new StringBuilder(); - headerLineBuffer.append("for <").append(((List<MailAddress>) session.getAttachment(SMTPSession.RCPT_LIST, State.Transaction)).get(0).toString()).append(">;"); + headerLineBuffer.append("for <").append(rcptList.get(0).toString()).append(">;"); } else { // Put the ; on the end of the 'by' line headerLineBuffer.append(";"); diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/SeparatingDataLineFilter.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/SeparatingDataLineFilter.java index 4e03460..2b18f3a 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/SeparatingDataLineFilter.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/SeparatingDataLineFilter.java @@ -20,6 +20,7 @@ package org.apache.james.protocols.smtp.core; import java.nio.ByteBuffer; +import org.apache.james.protocols.api.ProtocolSession; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.api.Response; import org.apache.james.protocols.api.handler.LineHandler; @@ -46,11 +47,11 @@ import org.apache.james.protocols.smtp.SMTPSession; */ public abstract class SeparatingDataLineFilter implements DataLineFilter { - private static final String HEADERS_COMPLETE = "HEADERS_COMPLETE"; + private static final ProtocolSession.AttachmentKey<Boolean> HEADERS_COMPLETE = ProtocolSession.AttachmentKey.of("HEADERS_COMPLETE", Boolean.class); @Override public final Response onLine(SMTPSession session, ByteBuffer line, LineHandler<SMTPSession> next) { - if (session.getAttachment(HEADERS_COMPLETE, State.Transaction) == null) { + if (!session.getAttachment(HEADERS_COMPLETE, State.Transaction).isPresent()) { if (line.remaining() == 2) { if (line.get() == '\r' && line.get() == '\n') { line.rewind(); diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/UnknownCmdHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/UnknownCmdHandler.java index bc0c414..f50e03b 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/UnknownCmdHandler.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/UnknownCmdHandler.java @@ -26,6 +26,7 @@ import java.util.Collection; import javax.inject.Inject; import org.apache.james.metrics.api.MetricFactory; +import org.apache.james.protocols.api.ProtocolSession; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.api.Response; import org.apache.james.protocols.api.handler.UnknownCommandHandler; @@ -47,6 +48,8 @@ public class UnknownCmdHandler extends AbstractHookableCmdHandler<UnknownHook> { * The name of the command handled by the command handler */ private static final Collection<String> COMMANDS = ImmutableSet.of(UnknownCommandHandler.COMMAND_IDENTIFIER); + private static final String MISSING_CURR_COMMAND = ""; + public static final ProtocolSession.AttachmentKey<String> CURR_COMMAND = ProtocolSession.AttachmentKey.of("CURR_COMMAND", String.class); @Inject public UnknownCmdHandler(MetricFactory metricFactory) { @@ -67,13 +70,13 @@ public class UnknownCmdHandler extends AbstractHookableCmdHandler<UnknownHook> { @Override protected Response doFilterChecks(SMTPSession session, String command, String parameters) { - session.setAttachment("CURR_COMMAND", command, State.Transaction); + session.setAttachment(CURR_COMMAND, command, State.Transaction); return null; } @Override protected HookResult callHook(UnknownHook rawHook, SMTPSession session, String parameters) { - return rawHook.doUnknown(session, (String) session.getAttachment("CURR_COMMAND", State.Transaction)); + return rawHook.doUnknown(session, session.getAttachment(CURR_COMMAND, State.Transaction).orElse(MISSING_CURR_COMMAND)); } @Override diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/esmtp/MailSizeEsmtpExtension.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/esmtp/MailSizeEsmtpExtension.java index e951d71..16a54e9 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/esmtp/MailSizeEsmtpExtension.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/esmtp/MailSizeEsmtpExtension.java @@ -23,8 +23,10 @@ import java.nio.ByteBuffer; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Optional; import org.apache.james.core.MaybeSender; +import org.apache.james.protocols.api.ProtocolSession; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.api.Response; import org.apache.james.protocols.api.handler.LineHandler; @@ -48,8 +50,9 @@ public class MailSizeEsmtpExtension implements MailParametersHook, EhloExtension private static final Logger LOGGER = LoggerFactory.getLogger(MailSizeEsmtpExtension.class); - private static final String MESG_SIZE = "MESG_SIZE"; // The size of the - private static final String MESG_FAILED = "MESG_FAILED"; // Message failed flag + private static final ProtocolSession.AttachmentKey<Integer> MESG_SIZE = ProtocolSession.AttachmentKey.of("MESG_SIZE", Integer.class); // The size of the + private static final ProtocolSession.AttachmentKey<Boolean> MESG_FAILED = ProtocolSession.AttachmentKey.of("MESG_FAILED", Boolean.class); // Message failed flag + public static final ProtocolSession.AttachmentKey<Long> CURRENT_SIZE = ProtocolSession.AttachmentKey.of("CURRENT_SIZE", Long.class); private static final String[] MAIL_PARAMS = { "SIZE" }; private static final HookResult SYNTAX_ERROR = HookResult.builder() @@ -68,7 +71,7 @@ public class MailSizeEsmtpExtension implements MailParametersHook, EhloExtension @Override public HookResult doMailParameter(SMTPSession session, String paramName, String paramValue) { - MaybeSender tempSender = (MaybeSender) session.getAttachment(SMTPSession.SENDER, State.Transaction); + MaybeSender tempSender = session.getAttachment(SMTPSession.SENDER, State.Transaction).orElse(MaybeSender.nullSender()); return doMailSize(session, paramValue, tempSender); } @@ -134,10 +137,10 @@ public class MailSizeEsmtpExtension implements MailParametersHook, EhloExtension @Override public Response onLine(SMTPSession session, ByteBuffer line, LineHandler<SMTPSession> next) { - Boolean failed = (Boolean) session.getAttachment(MESG_FAILED, State.Transaction); + Optional<Boolean> failed = session.getAttachment(MESG_FAILED, State.Transaction); // If we already defined we failed and sent a reply we should simply // wait for a CRLF.CRLF to be sent by the client. - if (failed != null && failed) { + if (failed.isPresent() && failed.get()) { if (isDataTerminated(line)) { line.rewind(); next.onLine(session, line); @@ -151,15 +154,11 @@ public class MailSizeEsmtpExtension implements MailParametersHook, EhloExtension return next.onLine(session, line); } else { line.rewind(); - Long currentSize = (Long) session.getAttachment("CURRENT_SIZE", State.Transaction); - Long newSize; - if (currentSize == null) { - newSize = Long.valueOf(line.remaining()); - } else { - newSize = Long.valueOf(currentSize.intValue() + line.remaining()); - } + Long newSize = session.getAttachment(CURRENT_SIZE, State.Transaction) + .map(currentSize -> Long.valueOf(currentSize.intValue() + line.remaining())) + .orElseGet(() -> Long.valueOf(line.remaining())); - session.setAttachment("CURRENT_SIZE", newSize, State.Transaction); + session.setAttachment(CURRENT_SIZE, newSize, State.Transaction); if (session.getConfiguration().getMaxMessageSize() > 0 && newSize.intValue() > session.getConfiguration().getMaxMessageSize()) { // Add an item to the state to suppress @@ -183,8 +182,8 @@ public class MailSizeEsmtpExtension implements MailParametersHook, EhloExtension @Override public HookResult onMessage(SMTPSession session, MailEnvelope mail) { - Boolean failed = (Boolean) session.getAttachment(MESG_FAILED, State.Transaction); - if (failed != null && failed) { + Optional<Boolean> failed = session.getAttachment(MESG_FAILED, State.Transaction); + if (failed.isPresent() && failed.get()) { LOGGER.error("Rejected message from {} from {} exceeding system maximum message size of {}", session.getAttachment(SMTPSession.SENDER, State.Transaction), session.getRemoteAddress().getAddress().getHostAddress(), session.getConfiguration().getMaxMessageSize()); return QUOTA_EXCEEDED; } else { diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/DNSRBLHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/DNSRBLHandler.java index 2439de8..68d3181 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/DNSRBLHandler.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/DNSRBLHandler.java @@ -22,10 +22,12 @@ import java.net.InetAddress; import java.net.UnknownHostException; import java.util.Collection; import java.util.Collections; +import java.util.Optional; import java.util.StringTokenizer; import org.apache.james.core.MailAddress; import org.apache.james.core.MaybeSender; +import org.apache.james.protocols.api.ProtocolSession; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.smtp.SMTPSession; import org.apache.james.protocols.smtp.dsn.DSNStatus; @@ -49,11 +51,9 @@ public class DNSRBLHandler implements RcptHook { private boolean getDetail = false; - private final String blocklistedDetail = null; + public static final ProtocolSession.AttachmentKey<Boolean> RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME = ProtocolSession.AttachmentKey.of("org.apache.james.smtpserver.rbl.blocklisted", Boolean.class); - public static final String RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME = "org.apache.james.smtpserver.rbl.blocklisted"; - - public static final String RBL_DETAIL_MAIL_ATTRIBUTE_NAME = "org.apache.james.smtpserver.rbl.detail"; + public static final ProtocolSession.AttachmentKey<String> RBL_DETAIL_MAIL_ATTRIBUTE_NAME = ProtocolSession.AttachmentKey.of("org.apache.james.smtpserver.rbl.detail", String.class); /** * Set the whitelist array @@ -154,7 +154,7 @@ public class DNSRBLHandler implements RcptHook { } } - session.setAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, "true", State.Connection); + session.setAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, true, State.Connection); return; } else { // if it is unknown, it isn't blocked @@ -171,10 +171,11 @@ public class DNSRBLHandler implements RcptHook { checkDNSRBL(session, session.getRemoteAddress().getAddress().getHostAddress()); if (!session.isRelayingAllowed()) { - String blocklisted = (String) session.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, State.Connection); - - if (blocklisted != null) { // was found in the RBL - if (blocklistedDetail == null) { + Optional<Boolean> blocklisted = session.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, State.Connection); + Optional<String> blocklistedDetail = session.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, State.Connection); + + if (blocklisted.isPresent()) { // was found in the RBL + if (!blocklistedDetail.isPresent()) { return HookResult.builder() .hookReturnCode(HookReturnCode.deny()) .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.SECURITY_AUTH) @@ -185,7 +186,7 @@ public class DNSRBLHandler implements RcptHook { return HookResult.builder() .hookReturnCode(HookReturnCode.deny()) - .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT,DSNStatus.SECURITY_AUTH) + " " + blocklistedDetail) + .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT,DSNStatus.SECURITY_AUTH) + " " + blocklistedDetail.get()) .build(); } diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/MaxUnknownCmdHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/MaxUnknownCmdHandler.java index 1461a8f..f8729ec 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/MaxUnknownCmdHandler.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/MaxUnknownCmdHandler.java @@ -20,6 +20,7 @@ package org.apache.james.protocols.smtp.core.fastfail; +import org.apache.james.protocols.api.ProtocolSession; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.smtp.SMTPSession; import org.apache.james.protocols.smtp.hook.HookResult; @@ -33,7 +34,7 @@ public class MaxUnknownCmdHandler implements UnknownHook { public static final int DEFAULT_MAX_UNKOWN = 5; - private static final String UNKOWN_COMMAND_COUNT = "UNKNOWN_COMMAND_COUNT"; + private static final ProtocolSession.AttachmentKey<Integer> UNKOWN_COMMAND_COUNT = ProtocolSession.AttachmentKey.of("UNKNOWN_COMMAND_COUNT", Integer.class); private int maxUnknown = DEFAULT_MAX_UNKOWN; public void setMaxUnknownCmdCount(int maxUnknown) { @@ -42,12 +43,10 @@ public class MaxUnknownCmdHandler implements UnknownHook { @Override public HookResult doUnknown(SMTPSession session, String command) { - Integer count = (Integer) session.getAttachment(UNKOWN_COMMAND_COUNT, State.Transaction); - if (count == null) { - count = 1; - } else { - count++; - } + Integer count = session.getAttachment(UNKOWN_COMMAND_COUNT, State.Transaction) + .map(fetchedCount -> fetchedCount + 1) + .orElse(1); + session.setAttachment(UNKOWN_COMMAND_COUNT, count, State.Transaction); if (count > maxUnknown) { return HookResult.builder() diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/ResolvableEhloHeloHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/ResolvableEhloHeloHandler.java index 403490a..1abce80 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/ResolvableEhloHeloHandler.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/ResolvableEhloHeloHandler.java @@ -24,6 +24,7 @@ import java.net.UnknownHostException; import org.apache.james.core.MailAddress; import org.apache.james.core.MaybeSender; +import org.apache.james.protocols.api.ProtocolSession; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.smtp.SMTPRetCode; import org.apache.james.protocols.smtp.SMTPSession; @@ -39,7 +40,7 @@ import org.apache.james.protocols.smtp.hook.RcptHook; */ public class ResolvableEhloHeloHandler implements RcptHook, HeloHook { - public static final String BAD_EHLO_HELO = "BAD_EHLO_HELO"; + public static final ProtocolSession.AttachmentKey<Boolean> BAD_EHLO_HELO = ProtocolSession.AttachmentKey.of("BAD_EHLO_HELO", Boolean.class); /** * Check if EHLO/HELO is resolvable @@ -50,9 +51,8 @@ public class ResolvableEhloHeloHandler implements RcptHook, HeloHook { * The argument */ protected void checkEhloHelo(SMTPSession session, String argument) { - if (isBadHelo(session, argument)) { - session.setAttachment(BAD_EHLO_HELO, "true", State.Transaction); + session.setAttachment(BAD_EHLO_HELO, true, State.Transaction); } } @@ -74,11 +74,7 @@ public class ResolvableEhloHeloHandler implements RcptHook, HeloHook { protected boolean check(SMTPSession session,MailAddress rcpt) { // not reject it - if (session.getAttachment(BAD_EHLO_HELO, State.Transaction) == null) { - return false; - } - - return true; + return session.getAttachment(BAD_EHLO_HELO, State.Transaction).isPresent(); } @Override diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/SupressDuplicateRcptHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/SupressDuplicateRcptHandler.java index bd4cf71..f3782f6 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/SupressDuplicateRcptHandler.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/fastfail/SupressDuplicateRcptHandler.java @@ -19,8 +19,6 @@ package org.apache.james.protocols.smtp.core.fastfail; -import java.util.Collection; - import org.apache.james.core.MailAddress; import org.apache.james.core.MaybeSender; import org.apache.james.protocols.api.ProtocolSession.State; @@ -40,25 +38,23 @@ public class SupressDuplicateRcptHandler implements RcptHook { private static final Logger LOGGER = LoggerFactory.getLogger(SupressDuplicateRcptHandler.class); @Override - @SuppressWarnings("unchecked") public HookResult doRcpt(SMTPSession session, MaybeSender sender, MailAddress rcpt) { - Collection<MailAddress> rcptList = (Collection<MailAddress>) session.getAttachment(SMTPSession.RCPT_LIST, State.Transaction); - - // Check if the recipient is already in the rcpt list - if (rcptList != null && rcptList.contains(rcpt)) { - StringBuilder responseBuffer = new StringBuilder(); - - responseBuffer.append(DSNStatus.getStatus(DSNStatus.SUCCESS, DSNStatus.ADDRESS_VALID)) - .append(" Recipient <") - .append(rcpt.toString()) - .append("> OK"); - LOGGER.debug("Duplicate recipient not add to recipient list: {}", rcpt); - return HookResult.builder() - .hookReturnCode(HookReturnCode.ok()) - .smtpReturnCode(SMTPRetCode.MAIL_OK) - .smtpDescription(responseBuffer.toString()) - .build(); - } - return HookResult.DECLINED; + return session.getAttachment(SMTPSession.RCPT_LIST, State.Transaction) + .filter(rcptList -> rcptList.contains(rcpt)) + .map(rcptList -> { + StringBuilder responseBuffer = new StringBuilder(); + + responseBuffer.append(DSNStatus.getStatus(DSNStatus.SUCCESS, DSNStatus.ADDRESS_VALID)) + .append(" Recipient <") + .append(rcpt.toString()) + .append("> OK"); + LOGGER.debug("Duplicate recipient not add to recipient list: {}", rcpt); + return HookResult.builder() + .hookReturnCode(HookReturnCode.ok()) + .smtpReturnCode(SMTPRetCode.MAIL_OK) + .smtpDescription(responseBuffer.toString()) + .build(); + }) + .orElse(HookResult.DECLINED); } } diff --git a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/DNSRBLHandlerTest.java b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/DNSRBLHandlerTest.java index 87c30c4..2265aad 100644 --- a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/DNSRBLHandlerTest.java +++ b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/DNSRBLHandlerTest.java @@ -21,6 +21,8 @@ package org.apache.james.protocols.smtp.core.fastfail; import static org.apache.james.protocols.api.ProtocolSession.State.Connection; +import static org.apache.james.protocols.smtp.core.fastfail.DNSRBLHandler.RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME; +import static org.apache.james.protocols.smtp.core.fastfail.DNSRBLHandler.RBL_DETAIL_MAIL_ATTRIBUTE_NAME; import static org.assertj.core.api.Assertions.assertThat; import java.net.InetSocketAddress; @@ -29,6 +31,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import org.apache.james.core.MailAddress; import org.apache.james.core.MaybeSender; @@ -38,6 +41,8 @@ import org.apache.james.protocols.smtp.utils.BaseFakeSMTPSession; import org.junit.Before; import org.junit.Test; +import com.google.common.base.Preconditions; + public class DNSRBLHandlerTest { private SMTPSession mockedSMTPSession; @@ -45,10 +50,6 @@ public class DNSRBLHandlerTest { private String remoteIp = "127.0.0.2"; private boolean relaying = false; - - public static final String RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME = "org.apache.james.smtpserver.rbl.blocklisted"; - - public static final String RBL_DETAIL_MAIL_ATTRIBUTE_NAME = "org.apache.james.smtpserver.rbl.detail"; @Before public void setUp() throws Exception { @@ -114,8 +115,8 @@ public class DNSRBLHandlerTest { */ private void setupMockedSMTPSession(MailAddress rcpt) { mockedSMTPSession = new BaseFakeSMTPSession() { - HashMap<String,Object> sessionState = new HashMap<>(); - HashMap<String,Object> connectionState = new HashMap<>(); + HashMap<AttachmentKey<?>,Object> sessionState = new HashMap<>(); + HashMap<AttachmentKey<?>,Object> connectionState = new HashMap<>(); @Override public InetSocketAddress getRemoteAddress() { @@ -127,7 +128,7 @@ public class DNSRBLHandlerTest { } @Override - public Map<String,Object> getState() { + public Map<AttachmentKey<?>,Object> getState() { return sessionState; } @@ -147,28 +148,34 @@ public class DNSRBLHandlerTest { } @Override - public Object setAttachment(String key, Object value, State state) { + public <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + + if (state == State.Connection) { + return key.convert(connectionState.put(key, value)); + } else { + return key.convert(sessionState.put(key, value)); + } + } + + @Override + public <T> Optional<T> removeAttachment(AttachmentKey<T> key, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + if (state == State.Connection) { - if (value == null) { - return connectionState.remove(key); - } else { - return connectionState.put(key, value); - } + return key.convert(connectionState.remove(key)); } else { - if (value == null) { - return sessionState.remove(key); - } else { - return sessionState.put(key, value); - } + return key.convert(sessionState.remove(key)); } } @Override - public Object getAttachment(String key, State state) { + public <T> Optional<T> getAttachment(AttachmentKey<T> key, State state) { if (state == State.Connection) { - return connectionState.get(key); + return key.convert(connectionState.get(key)); } else { - return sessionState.get(key); + return key.convert(sessionState.get(key)); } } @@ -185,8 +192,8 @@ public class DNSRBLHandlerTest { rbl.setBlacklist(new String[] { "bl.spamcop.net." }); rbl.setGetDetail(true); rbl.doRcpt(mockedSMTPSession, MaybeSender.nullSender(), new MailAddress("test@localhost")); - assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, State.Connection)).describedAs("Details").isEqualTo("Blocked - see http://www.spamcop.net/bl.shtml?127.0.0.2";); - assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Blocked").isNotNull(); + assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, State.Connection)).describedAs("Details").contains("Blocked - see http://www.spamcop.net/bl.shtml?127.0.0.2";); + assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Blocked").isPresent(); } // ip is blacklisted and has txt details but we don'T want to retrieve the txt record @@ -198,8 +205,8 @@ public class DNSRBLHandlerTest { rbl.setBlacklist(new String[] { "bl.spamcop.net." }); rbl.setGetDetail(false); rbl.doRcpt(mockedSMTPSession, MaybeSender.nullSender(), new MailAddress("test@localhost")); - assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("No details").isNull(); - assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Blocked").isNotNull(); + assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("No details").isEmpty(); + assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Blocked").isPresent(); } // ip is allowed to relay @@ -212,8 +219,8 @@ public class DNSRBLHandlerTest { rbl.setBlacklist(new String[] { "bl.spamcop.net." }); rbl.setGetDetail(true); rbl.doRcpt(mockedSMTPSession, MaybeSender.nullSender(), new MailAddress("test@localhost")); - assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("No details").isNull(); - assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Not blocked").isNull(); + assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("No details").isEmpty(); + assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Not blocked").isEmpty(); } // ip not on blacklist @@ -227,8 +234,8 @@ public class DNSRBLHandlerTest { rbl.setBlacklist(new String[] { "bl.spamcop.net." }); rbl.setGetDetail(true); rbl.doRcpt(mockedSMTPSession, MaybeSender.nullSender(), new MailAddress("test@localhost")); - assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("No details").isNull(); - assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Not blocked").isNull(); + assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("No details").isEmpty(); + assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Not blocked").isEmpty(); } // ip on blacklist without txt details @@ -242,8 +249,8 @@ public class DNSRBLHandlerTest { rbl.setBlacklist(new String[] { "bl.spamcop.net." }); rbl.setGetDetail(true); rbl.doRcpt(mockedSMTPSession, MaybeSender.nullSender(), new MailAddress("test@localhost")); - assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, Connection)).isNull(); - assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Blocked").isNotNull(); + assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, Connection)).isEmpty(); + assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Blocked").isPresent(); } // ip on whitelist @@ -257,8 +264,8 @@ public class DNSRBLHandlerTest { rbl.setWhitelist(new String[] { "bl.spamcop.net." }); rbl.setGetDetail(true); rbl.doRcpt(mockedSMTPSession, MaybeSender.nullSender(), new MailAddress("test@localhost")); - assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, Connection)).isNull(); - assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Not blocked").isNull(); + assertThat(mockedSMTPSession.getAttachment(RBL_DETAIL_MAIL_ATTRIBUTE_NAME, Connection)).isEmpty(); + assertThat(mockedSMTPSession.getAttachment(RBL_BLOCKLISTED_MAIL_ATTRIBUTE_NAME, Connection)).withFailMessage("Not blocked").isEmpty(); } diff --git a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/MaxRcptHandlerTest.java b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/MaxRcptHandlerTest.java index c9eb470..72ee259 100644 --- a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/MaxRcptHandlerTest.java +++ b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/MaxRcptHandlerTest.java @@ -35,10 +35,10 @@ public class MaxRcptHandlerTest { private SMTPSession setupMockedSession(final int rcptCount) { return new BaseFakeSMTPSession() { - HashMap<String,Object> state = new HashMap<>(); + HashMap<AttachmentKey<?>, Object> state = new HashMap<>(); @Override - public Map<String,Object> getState() { + public Map<AttachmentKey<?>, Object> getState() { return state; } diff --git a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/MaxUnknownCmdHandlerTest.java b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/MaxUnknownCmdHandlerTest.java index 128ce7d..98f9412 100644 --- a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/MaxUnknownCmdHandlerTest.java +++ b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/MaxUnknownCmdHandlerTest.java @@ -24,45 +24,57 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.apache.james.protocols.smtp.SMTPSession; import org.apache.james.protocols.smtp.hook.HookReturnCode; import org.apache.james.protocols.smtp.utils.BaseFakeSMTPSession; import org.junit.Test; +import com.google.common.base.Preconditions; + public class MaxUnknownCmdHandlerTest { @Test public void testRejectAndClose() throws Exception { SMTPSession session = new BaseFakeSMTPSession() { - private final HashMap<String,Object> map = new HashMap<>(); + private final HashMap<AttachmentKey<?>, Object> map = new HashMap<>(); @Override - public Map<String,Object> getState() { + public Map<AttachmentKey<?>, Object> getState() { return map; } @Override - public Object setAttachment(String key, Object value, State state) { + public <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + if (state == State.Connection) { throw new UnsupportedOperationException(); + } else { + return key.convert(map.put(key, value)); + } + } + @Override + public <T> Optional<T> removeAttachment(AttachmentKey<T> key, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + + if (state == State.Connection) { + throw new UnsupportedOperationException(); } else { - if (value == null) { - return map.remove(key); - } else { - return map.put(key, value); - } + return key.convert(map.remove(key)); } } @Override - public Object getAttachment(String key, State state) { + public <T> Optional<T> getAttachment(AttachmentKey<T> key, State state) { if (state == State.Connection) { throw new UnsupportedOperationException(); } else { - return map.get(key); + return key.convert(map.get(key)); } } }; diff --git a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/ResolvableEhloHeloHandlerTest.java b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/ResolvableEhloHeloHandlerTest.java index 320f6a1..cf2052b 100644 --- a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/ResolvableEhloHeloHandlerTest.java +++ b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/core/fastfail/ResolvableEhloHeloHandlerTest.java @@ -27,6 +27,7 @@ import java.net.InetAddress; import java.net.UnknownHostException; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.apache.james.core.MailAddress; import org.apache.james.core.MaybeSender; @@ -36,6 +37,8 @@ import org.apache.james.protocols.smtp.hook.HookReturnCode; import org.apache.james.protocols.smtp.utils.BaseFakeSMTPSession; import org.junit.Test; +import com.google.common.base.Preconditions; + public class ResolvableEhloHeloHandlerTest { public static final String INVALID_HOST = "foo.bar"; @@ -48,8 +51,8 @@ public class ResolvableEhloHeloHandlerTest { return new BaseFakeSMTPSession() { - HashMap<String,Object> connectionMap = new HashMap<>(); - HashMap<String,Object> map = new HashMap<>(); + HashMap<AttachmentKey<?>, Object> connectionMap = new HashMap<>(); + HashMap<AttachmentKey<?>, Object> map = new HashMap<>(); @Override public boolean isAuthSupported() { @@ -62,7 +65,7 @@ public class ResolvableEhloHeloHandlerTest { } @Override - public Map<String,Object> getConnectionState() { + public Map<AttachmentKey<?>, Object> getConnectionState() { return connectionMap; } @@ -72,33 +75,39 @@ public class ResolvableEhloHeloHandlerTest { } @Override - public Map<String,Object> getState() { + public Map<AttachmentKey<?>, Object> getState() { return map; } @Override - public Object setAttachment(String key, Object value, State state) { + public <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + + if (state == State.Connection) { + return key.convert(connectionMap.put(key, value)); + } else { + return key.convert(map.put(key, value)); + } + } + + @Override + public <T> Optional<T> removeAttachment(AttachmentKey<T> key, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + if (state == State.Connection) { - if (value == null) { - return connectionMap.remove(key); - } else { - return connectionMap.put(key, value); - } + return key.convert(connectionMap.remove(key)); } else { - if (value == null) { - return map.remove(key); - } else { - return connectionMap.put(key, value); - } + return key.convert(map.remove(key)); } } @Override - public Object getAttachment(String key, State state) { + public <T> Optional<T> getAttachment(AttachmentKey<T> key, State state) { if (state == State.Connection) { - return connectionMap.get(key); + return key.convert(connectionMap.get(key)); } else { - return connectionMap.get(key); + return key.convert(map.get(key)); } } @@ -122,11 +131,11 @@ public class ResolvableEhloHeloHandlerTest { @Test public void testRejectInvalidHelo() throws Exception { MailAddress mailAddress = new MailAddress("test@localhost"); - SMTPSession session = setupMockSession(INVALID_HOST,false,false,null,mailAddress); + SMTPSession session = setupMockSession(INVALID_HOST, false, false, null, mailAddress); ResolvableEhloHeloHandler handler = createHandler(); handler.doHelo(session, INVALID_HOST); - assertThat(session.getAttachment(BAD_EHLO_HELO, Transaction)).withFailMessage("Invalid HELO").isNotNull(); + assertThat(session.getAttachment(BAD_EHLO_HELO, Transaction)).withFailMessage("Invalid HELO").isPresent(); HookReturnCode result = handler.doRcpt(session, MaybeSender.nullSender(), mailAddress).getResult(); assertThat(HookReturnCode.deny()).describedAs("Reject").isEqualTo(result); @@ -135,12 +144,12 @@ public class ResolvableEhloHeloHandlerTest { @Test public void testNotRejectValidHelo() throws Exception { MailAddress mailAddress = new MailAddress("test@localhost"); - SMTPSession session = setupMockSession(VALID_HOST,false,false,null,mailAddress); + SMTPSession session = setupMockSession(VALID_HOST, false, false, null, mailAddress); ResolvableEhloHeloHandler handler = createHandler(); handler.doHelo(session, VALID_HOST); - assertThat(session.getAttachment(BAD_EHLO_HELO, Transaction)).withFailMessage("Valid HELO").isNull(); + assertThat(session.getAttachment(BAD_EHLO_HELO, Transaction)).withFailMessage("Valid HELO").isEmpty(); HookReturnCode result = handler.doRcpt(session, MaybeSender.nullSender(), mailAddress).getResult(); assertThat(HookReturnCode.declined()).describedAs("Not reject").isEqualTo(result); @@ -154,7 +163,7 @@ public class ResolvableEhloHeloHandlerTest { handler.doHelo(session, INVALID_HOST); - assertThat(session.getAttachment(BAD_EHLO_HELO, Transaction)).withFailMessage("Value stored").isNotNull(); + assertThat(session.getAttachment(BAD_EHLO_HELO, Transaction)).withFailMessage("Value stored").isPresent(); HookReturnCode result = handler.doRcpt(session, MaybeSender.nullSender(), mailAddress).getResult(); @@ -165,16 +174,15 @@ public class ResolvableEhloHeloHandlerTest { @Test public void testRejectRelay() throws Exception { MailAddress mailAddress = new MailAddress("test@localhost"); - SMTPSession session = setupMockSession(INVALID_HOST,true,false,null,mailAddress); + SMTPSession session = setupMockSession(INVALID_HOST, true, false, null, mailAddress); ResolvableEhloHeloHandler handler = createHandler(); handler.doHelo(session, INVALID_HOST); - assertThat(session.getAttachment(BAD_EHLO_HELO, Transaction)).withFailMessage("Value stored").isNotNull(); + assertThat(session.getAttachment(BAD_EHLO_HELO, Transaction)).withFailMessage("Value stored").isPresent(); HookReturnCode result = handler.doRcpt(session, MaybeSender.nullSender(), mailAddress).getResult(); assertThat(HookReturnCode.deny()).describedAs("Reject").isEqualTo(result); } } - 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 407f1ca..685b4f2 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 @@ -23,6 +23,7 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.apache.james.core.MailAddress; import org.apache.james.core.MaybeSender; @@ -32,6 +33,8 @@ import org.apache.james.protocols.smtp.hook.HookReturnCode; import org.apache.james.protocols.smtp.utils.BaseFakeSMTPSession; import org.junit.Test; +import com.google.common.base.Preconditions; + public class ValidSenderDomainHandlerTest { private ValidSenderDomainHandler createHandler() { @@ -48,11 +51,10 @@ public class ValidSenderDomainHandlerTest { private SMTPSession setupMockedSession(final MailAddress sender) { return new BaseFakeSMTPSession() { - HashMap<String,Object> map = new HashMap<>(); + Map<AttachmentKey<?>, Object> map = new HashMap<>(); @Override - public Map<String,Object> getState() { - + public Map<AttachmentKey<?>, Object> getState() { map.put(SMTPSession.SENDER, MaybeSender.of(sender)); return map; @@ -64,25 +66,34 @@ public class ValidSenderDomainHandlerTest { } @Override - public Object setAttachment(String key, Object value, State state) { + public <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + if (state == State.Connection) { throw new UnsupportedOperationException(); + } else { + return key.convert(getState().put(key, value)); + } + } + + @Override + public <T> Optional<T> removeAttachment(AttachmentKey<T> key, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + if (state == State.Connection) { + throw new UnsupportedOperationException(); } else { - if (value == null) { - return getState().remove(key); - } else { - return getState().put(key, value); - } + return key.convert(getState().remove(key)); } } @Override - public Object getAttachment(String key, State state) { + public <T> Optional<T> getAttachment(AttachmentKey<T> key, State state) { if (state == State.Connection) { throw new UnsupportedOperationException(); } else { - return getState().get(key); + return key.convert(getState().get(key)); } } @@ -103,7 +114,7 @@ public class ValidSenderDomainHandlerTest { public void testInvalidSenderDomainReject() throws Exception { ValidSenderDomainHandler handler = createHandler(); SMTPSession session = setupMockedSession(new MailAddress("invalid@invalid")); - MaybeSender sender = (MaybeSender) session.getAttachment(SMTPSession.SENDER, State.Transaction); + MaybeSender sender = session.getAttachment(SMTPSession.SENDER, State.Transaction).get(); HookReturnCode response = handler.doMail(session, sender).getResult(); assertThat(HookReturnCode.deny()).describedAs("Blocked cause we use reject action").isEqualTo(response); diff --git a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java index a6c8af1..af819ee 100644 --- a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java +++ b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java @@ -23,6 +23,7 @@ package org.apache.james.protocols.smtp.utils; import java.net.InetSocketAddress; import java.nio.charset.Charset; import java.util.Map; +import java.util.Optional; import org.apache.james.core.Username; import org.apache.james.protocols.api.ProtocolSession; @@ -52,7 +53,7 @@ public class BaseFakeSMTPSession implements SMTPSession { } @Override - public Map<String, Object> getConnectionState() { + public Map<AttachmentKey<?>, Object> getConnectionState() { throw new UnsupportedOperationException("Unimplemented Stub Method"); } @@ -67,7 +68,7 @@ public class BaseFakeSMTPSession implements SMTPSession { } @Override - public Map<String, Object> getState() { + public Map<AttachmentKey<?>, Object> getState() { throw new UnsupportedOperationException("Unimplemented Stub Method"); } @@ -147,12 +148,17 @@ public class BaseFakeSMTPSession implements SMTPSession { } @Override - public Object setAttachment(String key, Object value, State state) { + public <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state) { throw new UnsupportedOperationException("Unimplemented Stub Method"); } @Override - public Object getAttachment(String key, State state) { + public <T> Optional<T> removeAttachment(AttachmentKey<T> key, State state) { + throw new UnsupportedOperationException("Unimplemented Stub Method"); + } + + @Override + public <T> Optional<T> getAttachment(AttachmentKey<T> key, State state) { throw new UnsupportedOperationException("Unimplemented Stub Method"); } diff --git a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/DataLineJamesMessageHookHandler.java b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/DataLineJamesMessageHookHandler.java index 67c1125..fe3ac7c 100644 --- a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/DataLineJamesMessageHookHandler.java +++ b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/DataLineJamesMessageHookHandler.java @@ -74,7 +74,8 @@ public class DataLineJamesMessageHookHandler implements DataLineFilter, Extensib byte[] line = new byte[lineByteBuffer.remaining()]; lineByteBuffer.get(line, 0, line.length); - MimeMessageInputStreamSource mmiss = (MimeMessageInputStreamSource) session.getAttachment(SMTPConstants.DATA_MIMEMESSAGE_STREAMSOURCE, State.Transaction); + MimeMessageInputStreamSource mmiss = session.getAttachment(SMTPConstants.DATA_MIMEMESSAGE_STREAMSOURCE, State.Transaction) + .orElseThrow(() -> new RuntimeException("'" + SMTPConstants.DATA_MIMEMESSAGE_STREAMSOURCE.asString() + "' has not been filled.")); try { OutputStream out = mmiss.getWritableOutputStream(); @@ -85,9 +86,8 @@ public class DataLineJamesMessageHookHandler implements DataLineFilter, Extensib out.flush(); out.close(); - @SuppressWarnings("unchecked") - List<MailAddress> recipientCollection = (List<MailAddress>) session.getAttachment(SMTPSession.RCPT_LIST, State.Transaction); - MaybeSender sender = (MaybeSender) session.getAttachment(SMTPSession.SENDER, State.Transaction); + List<MailAddress> recipientCollection = session.getAttachment(SMTPSession.RCPT_LIST, State.Transaction).orElse(ImmutableList.of()); + MaybeSender sender = session.getAttachment(SMTPSession.SENDER, State.Transaction).orElse(MaybeSender.nullSender()); MailImpl mail = MailImpl.builder() .name(MailImpl.getId()) @@ -140,7 +140,8 @@ public class DataLineJamesMessageHookHandler implements DataLineFilter, Extensib protected Response processExtensions(SMTPSession session, Mail mail) { if (mail != null && messageHandlers != null) { try { - MimeMessageInputStreamSource mmiss = (MimeMessageInputStreamSource) session.getAttachment(SMTPConstants.DATA_MIMEMESSAGE_STREAMSOURCE, State.Transaction); + MimeMessageInputStreamSource mmiss = session.getAttachment(SMTPConstants.DATA_MIMEMESSAGE_STREAMSOURCE, State.Transaction) + .orElseThrow(() -> new RuntimeException("'" + SMTPConstants.DATA_MIMEMESSAGE_STREAMSOURCE.asString() + "' has not been filled.")); OutputStream out; out = mmiss.getWritableOutputStream(); for (MessageHook rawHandler : mHandlers) { diff --git a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/SMTPConstants.java b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/SMTPConstants.java index b401721..3b062e3 100644 --- a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/SMTPConstants.java +++ b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/SMTPConstants.java @@ -19,12 +19,16 @@ package org.apache.james.smtpserver; +import org.apache.james.protocols.api.ProtocolSession; +import org.apache.james.server.core.MimeMessageInputStreamSource; +import org.apache.mailet.Mail; + /** * Constants which are used within SMTP Session */ public interface SMTPConstants { - String DATA_MIMEMESSAGE_STREAMSOURCE = "org.apache.james.core.DataCmdHandler.DATA_MIMEMESSAGE_STREAMSOURCE"; - String MAIL = "MAIL"; + ProtocolSession.AttachmentKey<MimeMessageInputStreamSource> DATA_MIMEMESSAGE_STREAMSOURCE = ProtocolSession.AttachmentKey.of("org.apache.james.core.DataCmdHandler.DATA_MIMEMESSAGE_STREAMSOURCE", MimeMessageInputStreamSource.class); + ProtocolSession.AttachmentKey<Mail> MAIL = ProtocolSession.AttachmentKey.of("MAIL", Mail.class); } 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 ee4edf9..f7c0e5c 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 @@ -18,6 +18,8 @@ ****************************************************************/ package org.apache.james.smtpserver.fastfail; +import java.util.Optional; + import javax.inject.Inject; import org.apache.commons.configuration2.Configuration; @@ -29,6 +31,7 @@ import org.apache.james.jspf.core.exceptions.SPFErrorConstants; import org.apache.james.jspf.executor.SPFResult; import org.apache.james.jspf.impl.DefaultSPF; import org.apache.james.jspf.impl.SPF; +import org.apache.james.protocols.api.ProtocolSession; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.api.handler.ProtocolHandler; import org.apache.james.protocols.smtp.SMTPRetCode; @@ -59,13 +62,13 @@ public class SPFHandler implements JamesMessageHook, MailHook, RcptHook, Protoco */ private final Logger serviceLog = FALLBACK_LOG; - private static final String SPF_BLOCKLISTED = "SPF_BLOCKLISTED"; + private static final ProtocolSession.AttachmentKey<Boolean> SPF_BLOCKLISTED = ProtocolSession.AttachmentKey.of("SPF_BLOCKLISTED", Boolean.class); - private static final String SPF_DETAIL = "SPF_DETAIL"; + private static final ProtocolSession.AttachmentKey<String> SPF_DETAIL = ProtocolSession.AttachmentKey.of("SPF_DETAIL", String.class); - private static final String SPF_TEMPBLOCKLISTED = "SPF_TEMPBLOCKLISTED"; + private static final ProtocolSession.AttachmentKey<Boolean> SPF_TEMPBLOCKLISTED = ProtocolSession.AttachmentKey.of("SPF_TEMPBLOCKLISTED", Boolean.class); - private static final String SPF_HEADER = "SPF_HEADER"; + private static final ProtocolSession.AttachmentKey<String> SPF_HEADER = ProtocolSession.AttachmentKey.of("SPF_HEADER", String.class); private static final AttributeName SPF_HEADER_MAIL_ATTRIBUTE_NAME = AttributeName.of("org.apache.james.spf.header"); @@ -114,16 +117,16 @@ public class SPFHandler implements JamesMessageHook, MailHook, RcptHook, Protoco * SMTP session object */ private void doSPFCheck(SMTPSession session, MaybeSender sender) { - String heloEhlo = (String) session.getAttachment(SMTPSession.CURRENT_HELO_NAME, State.Transaction); + Optional<String> heloEhlo = session.getAttachment(SMTPSession.CURRENT_HELO_NAME, State.Transaction); // We have no Sender or HELO/EHLO yet return false - if (sender.isNullSender() || heloEhlo == null) { + if (sender.isNullSender() || !heloEhlo.isPresent()) { LOGGER.info("No Sender or HELO/EHLO present"); } else { String ip = session.getRemoteAddress().getAddress().getHostAddress(); - SPFResult result = spf.checkSPF(ip, sender.asString(), heloEhlo); + SPFResult result = spf.checkSPF(ip, sender.asString(), heloEhlo.get()); String spfResult = result.getResult(); @@ -141,10 +144,10 @@ public class SPFHandler implements JamesMessageHook, MailHook, RcptHook, Protoco explanation = "Block caused by an invalid SPF record"; } session.setAttachment(SPF_DETAIL, explanation, State.Transaction); - session.setAttachment(SPF_BLOCKLISTED, "true", State.Transaction); + session.setAttachment(SPF_BLOCKLISTED, true, State.Transaction); } else if (spfResult.equals(SPFErrorConstants.TEMP_ERROR_CONV)) { - session.setAttachment(SPF_TEMPBLOCKLISTED, "true", State.Transaction); + session.setAttachment(SPF_TEMPBLOCKLISTED, true, State.Transaction); } } @@ -155,13 +158,13 @@ public class SPFHandler implements JamesMessageHook, MailHook, RcptHook, Protoco public HookResult doRcpt(SMTPSession session, MaybeSender sender, MailAddress rcpt) { if (!session.isRelayingAllowed()) { // Check if session is blocklisted - if (session.getAttachment(SPF_BLOCKLISTED, State.Transaction) != null) { + if (session.getAttachment(SPF_BLOCKLISTED, State.Transaction).isPresent()) { return HookResult.builder() .hookReturnCode(HookReturnCode.deny()) - .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.SECURITY_AUTH) + " " + session.getAttachment(SPF_TEMPBLOCKLISTED, State.Transaction)) + .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.SECURITY_AUTH) + " " + session.getAttachment(SPF_TEMPBLOCKLISTED, State.Transaction).orElse(false)) .build(); - } else if (session.getAttachment(SPF_TEMPBLOCKLISTED, State.Transaction) != null) { + } else if (session.getAttachment(SPF_TEMPBLOCKLISTED, State.Transaction).isPresent()) { return HookResult.builder() .hookReturnCode(HookReturnCode.denySoft()) .smtpReturnCode(SMTPRetCode.LOCAL_ERROR) @@ -277,7 +280,7 @@ public class SPFHandler implements JamesMessageHook, MailHook, RcptHook, Protoco @Override public HookResult onMessage(SMTPSession session, Mail mail) { // Store the spf header as attribute for later using - mail.setAttribute(new Attribute(SPF_HEADER_MAIL_ATTRIBUTE_NAME, AttributeValue.of((String) session.getAttachment(SPF_HEADER, State.Transaction)))); + mail.setAttribute(new Attribute(SPF_HEADER_MAIL_ATTRIBUTE_NAME, AttributeValue.of(session.getAttachment(SPF_HEADER, State.Transaction).get()))); return null; } diff --git a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/URIRBLHandler.java b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/URIRBLHandler.java index d2b9dc6..c48d3a2 100644 --- a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/URIRBLHandler.java +++ b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/URIRBLHandler.java @@ -24,6 +24,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.Iterator; +import java.util.Optional; import javax.inject.Inject; import javax.mail.MessagingException; @@ -35,6 +36,7 @@ import javax.mail.internet.MimePart; import org.apache.commons.configuration2.Configuration; import org.apache.commons.configuration2.ex.ConfigurationException; import org.apache.james.dnsservice.api.DNSService; +import org.apache.james.protocols.api.ProtocolSession; import org.apache.james.protocols.api.ProtocolSession.State; import org.apache.james.protocols.api.handler.ProtocolHandler; import org.apache.james.protocols.smtp.SMTPSession; @@ -55,9 +57,9 @@ public class URIRBLHandler implements JamesMessageHook, ProtocolHandler { /** This log is the fall back shared by all instances */ private static final Logger LOGGER = LoggerFactory.getLogger(URIRBLHandler.class); - private static final String LISTED_DOMAIN = "LISTED_DOMAIN"; + private static final ProtocolSession.AttachmentKey<String> LISTED_DOMAIN = ProtocolSession.AttachmentKey.of("LISTED_DOMAIN", String.class); - private static final String URBLSERVER = "URBL_SERVER"; + private static final ProtocolSession.AttachmentKey<String> URBLSERVER = ProtocolSession.AttachmentKey.of("URBL_SERVER", String.class); private DNSService dnsService; @@ -108,13 +110,13 @@ public class URIRBLHandler implements JamesMessageHook, ProtocolHandler { @Override public HookResult onMessage(SMTPSession session, Mail mail) { if (check(session, mail)) { - String uRblServer = (String) session.getAttachment(URBLSERVER, State.Transaction); - String target = (String) session.getAttachment(LISTED_DOMAIN, State.Transaction); + Optional<String> uRblServer = session.getAttachment(URBLSERVER, State.Transaction); + Optional<String> target = session.getAttachment(LISTED_DOMAIN, State.Transaction); String detail = null; // we should try to retrieve details - if (getDetail) { - Collection<String> txt = dnsService.findTXTRecords(target + "." + uRblServer); + if (uRblServer.isPresent() && target.isPresent() && getDetail) { + Collection<String> txt = dnsService.findTXTRecords(target.get() + "." + uRblServer.get()); // Check if we found a txt record if (!txt.isEmpty()) { @@ -127,12 +129,12 @@ public class URIRBLHandler implements JamesMessageHook, ProtocolHandler { if (detail != null) { return HookResult.builder() .hookReturnCode(HookReturnCode.deny()) - .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.SECURITY_OTHER) + "Rejected: message contains domain " + target + " listed by " + uRblServer + " . Details: " + detail) + .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.SECURITY_OTHER) + "Rejected: message contains domain " + target.get() + " listed by " + uRblServer + " . Details: " + detail) .build(); } else { return HookResult.builder() .hookReturnCode(HookReturnCode.deny()) - .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.SECURITY_OTHER) + " Rejected: message contains domain " + target + " listed by " + uRblServer) + .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.SECURITY_OTHER) + " Rejected: message contains domain " + target.orElse("[target not set]") + " listed by " + uRblServer) .build(); } diff --git a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPChannelUpstreamHandler.java b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPChannelUpstreamHandler.java index 6ad881d..6933c2b 100644 --- a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPChannelUpstreamHandler.java +++ b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPChannelUpstreamHandler.java @@ -76,8 +76,8 @@ public class SMTPChannelUpstreamHandler extends BasicChannelUpstreamHandler { SMTPSession smtpSession = (SMTPSession) ctx.getAttachment(); if (smtpSession != null) { - LifecycleUtil.dispose(smtpSession.getAttachment(SMTPConstants.MAIL, State.Transaction)); - LifecycleUtil.dispose(smtpSession.getAttachment(SMTPConstants.DATA_MIMEMESSAGE_STREAMSOURCE, State.Transaction)); + smtpSession.getAttachment(SMTPConstants.MAIL, State.Transaction).ifPresent(LifecycleUtil::dispose); + smtpSession.getAttachment(SMTPConstants.DATA_MIMEMESSAGE_STREAMSOURCE, State.Transaction).ifPresent(LifecycleUtil::dispose); } super.cleanup(ctx); 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 9db776b..9349d6d 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 @@ -23,6 +23,7 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.ArrayList; import java.util.HashMap; import java.util.List; +import java.util.Optional; import org.apache.james.core.MailAddress; import org.apache.james.core.MaybeSender; @@ -36,6 +37,8 @@ import org.apache.james.smtpserver.fastfail.SPFHandler; import org.junit.Before; import org.junit.Test; +import com.google.common.base.Preconditions; + public class SPFHandlerTest { private DNSService mockedDnsService; @@ -128,32 +131,40 @@ public class SPFHandlerTest { private void setupMockedSMTPSession(String ip, final String helo) { mockedSMTPSession = new BaseFakeSMTPSession() { - private final HashMap<String, Object> sstate = new HashMap<>(); - private final HashMap<String, Object> connectionState = new HashMap<>(); + private final HashMap<AttachmentKey<?>, Object> sessionState = new HashMap<>(); + private final HashMap<AttachmentKey<?>, Object> connectionState = new HashMap<>(); @Override - public Object setAttachment(String key, Object value, State state) { + public <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + + if (state == State.Connection) { + return key.convert(connectionState.put(key, value)); + } else { + return key.convert(sessionState.put(key, value)); + } + } + + @Override + public <T> Optional<T> removeAttachment(AttachmentKey<T> key, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + if (state == State.Connection) { - if (value == null) { - return connectionState.remove(key); - } - return connectionState.put(key, value); + return key.convert(connectionState.remove(key)); } else { - if (value == null) { - return sstate.remove(key); - } - return sstate.put(key, value); + return key.convert(sessionState.remove(key)); } } @Override - public Object getAttachment(String key, State state) { - sstate.put(SMTPSession.CURRENT_HELO_NAME, helo); + public <T> Optional<T> getAttachment(AttachmentKey<T> key, State state) { + sessionState.put(SMTPSession.CURRENT_HELO_NAME, helo); if (state == State.Connection) { - return connectionState.get(key); + return key.convert(connectionState.get(key)); } else { - return sstate.get(key); + return key.convert(sessionState.get(key)); } } diff --git a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SpamAssassinHandlerTest.java b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SpamAssassinHandlerTest.java index e650611..6fe930e 100644 --- a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SpamAssassinHandlerTest.java +++ b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SpamAssassinHandlerTest.java @@ -22,6 +22,7 @@ import static org.apache.james.spamassassin.SpamAssassinResult.STATUS_MAIL; import static org.assertj.core.api.Assertions.assertThat; import java.util.HashMap; +import java.util.Optional; import javax.mail.MessagingException; import javax.mail.internet.AddressException; @@ -46,6 +47,8 @@ import org.apache.mailet.base.test.FakeMail; import org.junit.Rule; import org.junit.Test; +import com.google.common.base.Preconditions; + public class SpamAssassinHandlerTest { private static final String SPAMD_HOST = "localhost"; @@ -59,36 +62,44 @@ public class SpamAssassinHandlerTest { return new BaseFakeSMTPSession() { - private final HashMap<String, Object> sstate = new HashMap<>(); - private final HashMap<String, Object> connectionState = new HashMap<>(); + private final HashMap<AttachmentKey<?>, Object> sessionState = new HashMap<>(); + private final HashMap<AttachmentKey<?>, Object> connectionState = new HashMap<>(); private boolean relayingAllowed; @Override - public Object setAttachment(String key, Object value, State state) { + public <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + + if (state == State.Connection) { + return key.convert(connectionState.put(key, value)); + } else { + return key.convert(sessionState.put(key, value)); + } + } + + @Override + public <T> Optional<T> removeAttachment(AttachmentKey<T> key, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + if (state == State.Connection) { - if (value == null) { - return connectionState.remove(key); - } - return connectionState.put(key, value); + return key.convert(connectionState.remove(key)); } else { - if (value == null) { - return sstate.remove(key); - } - return sstate.put(key, value); + return key.convert(sessionState.remove(key)); } } @Override - public Object getAttachment(String key, State state) { + public <T> Optional<T> getAttachment(AttachmentKey<T> key, State state) { try { - sstate.put(SMTPSession.SENDER, MaybeSender.of(new MailAddress("[email protected]"))); + sessionState.put(SMTPSession.SENDER, MaybeSender.of(new MailAddress("[email protected]"))); } catch (AddressException e) { throw new RuntimeException(e); } if (state == State.Connection) { - return connectionState.get(key); + return key.convert(connectionState.get(key)); } else { - return sstate.get(key); + return key.convert(sessionState.get(key)); } } diff --git a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/URIRBLHandlerTest.java b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/URIRBLHandlerTest.java index b8890aa..af14370 100644 --- a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/URIRBLHandlerTest.java +++ b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/URIRBLHandlerTest.java @@ -27,6 +27,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.List; +import java.util.Optional; import javax.mail.MessagingException; import javax.mail.internet.AddressException; @@ -46,6 +47,8 @@ import org.apache.mailet.Mail; import org.apache.mailet.base.test.FakeMail; import org.junit.Test; +import com.google.common.base.Preconditions; + public class URIRBLHandlerTest { private static final String BAD_DOMAIN1 = "bad.domain.de"; @@ -61,36 +64,44 @@ public class URIRBLHandlerTest { private boolean relayingAllowed; - private final HashMap<String, Object> sstate = new HashMap<>(); - private final HashMap<String, Object> connectionState = new HashMap<>(); + private final HashMap<AttachmentKey<?>, Object> sessionState = new HashMap<>(); + private final HashMap<AttachmentKey<?>, Object> connectionState = new HashMap<>(); @Override - public Object setAttachment(String key, Object value, State state) { + public <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + + if (state == State.Connection) { + return key.convert(connectionState.put(key, value)); + } else { + return key.convert(sessionState.put(key, value)); + } + } + + @Override + public <T> Optional<T> removeAttachment(AttachmentKey<T> key, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + if (state == State.Connection) { - if (value == null) { - return connectionState.remove(key); - } - return connectionState.put(key, value); + return key.convert(connectionState.remove(key)); } else { - if (value == null) { - return sstate.remove(key); - } - return sstate.put(key, value); + return key.convert(sessionState.remove(key)); } } @Override - public Object getAttachment(String key, State state) { + public <T> Optional<T> getAttachment(AttachmentKey<T> key, State state) { try { - sstate.put(SMTPSession.SENDER, MaybeSender.of(new MailAddress("[email protected]"))); + sessionState.put(SMTPSession.SENDER, MaybeSender.of(new MailAddress("[email protected]"))); } catch (AddressException e) { throw new RuntimeException(e); } if (state == State.Connection) { - return connectionState.get(key); + return key.convert(connectionState.get(key)); } else { - return sstate.get(key); + return key.convert(sessionState.get(key)); } } diff --git a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptHandlerTest.java b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptHandlerTest.java index 97ee1a1..45bc31b 100644 --- a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptHandlerTest.java +++ b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptHandlerTest.java @@ -23,6 +23,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import java.util.HashMap; +import java.util.Optional; import org.apache.james.core.Domain; import org.apache.james.core.MailAddress; @@ -43,6 +44,8 @@ import org.apache.james.user.memory.MemoryUsersRepository; import org.junit.Before; import org.junit.Test; +import com.google.common.base.Preconditions; + public class ValidRcptHandlerTest { private static final Username VALID_USER = Username.of("postmaster"); private static final String INVALID_USER = "invalid"; @@ -85,30 +88,38 @@ public class ValidRcptHandlerTest { return relayingAllowed; } - private final HashMap<String, Object> sstate = new HashMap<>(); - private final HashMap<String, Object> connectionState = new HashMap<>(); + private final HashMap<AttachmentKey<?>, Object> sessionState = new HashMap<>(); + private final HashMap<AttachmentKey<?>, Object> connectionState = new HashMap<>(); @Override - public Object setAttachment(String key, Object value, State state) { + public <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + + if (state == State.Connection) { + return key.convert(connectionState.put(key, value)); + } else { + return key.convert(sessionState.put(key, value)); + } + } + + @Override + public <T> Optional<T> removeAttachment(AttachmentKey<T> key, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + if (state == State.Connection) { - if (value == null) { - return connectionState.remove(key); - } - return connectionState.put(key, value); + return key.convert(connectionState.remove(key)); } else { - if (value == null) { - return sstate.remove(key); - } - return sstate.put(key, value); + return key.convert(sessionState.remove(key)); } } @Override - public Object getAttachment(String key, State state) { + public <T> Optional<T> getAttachment(AttachmentKey<T> key, State state) { if (state == State.Connection) { - return connectionState.get(key); + return key.convert(connectionState.get(key)); } else { - return sstate.get(key); + return key.convert(sessionState.get(key)); } } }; diff --git a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptMXTest.java b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptMXTest.java index be784d7..ee29e3f 100644 --- a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptMXTest.java +++ b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/ValidRcptMXTest.java @@ -21,6 +21,7 @@ package org.apache.james.smtpserver; import static org.assertj.core.api.Assertions.assertThat; import java.util.HashMap; +import java.util.Optional; import org.apache.james.core.MailAddress; import org.apache.james.core.MaybeSender; @@ -32,6 +33,7 @@ import org.apache.james.protocols.smtp.utils.BaseFakeSMTPSession; import org.apache.james.smtpserver.fastfail.ValidRcptMX; import org.junit.Test; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; public class ValidRcptMXTest { @@ -41,30 +43,38 @@ public class ValidRcptMXTest { private SMTPSession setupMockedSMTPSession(MailAddress rcpt) { return new BaseFakeSMTPSession() { - private final HashMap<String, Object> sstate = new HashMap<>(); - private final HashMap<String, Object> connectionState = new HashMap<>(); + private final HashMap<AttachmentKey<?>, Object> sessionState = new HashMap<>(); + private final HashMap<AttachmentKey<?>, Object> connectionState = new HashMap<>(); @Override - public Object setAttachment(String key, Object value, State state) { + public <T> Optional<T> setAttachment(AttachmentKey<T> key, T value, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + + if (state == State.Connection) { + return key.convert(connectionState.put(key, value)); + } else { + return key.convert(sessionState.put(key, value)); + } + } + + @Override + public <T> Optional<T> removeAttachment(AttachmentKey<T> key, State state) { + Preconditions.checkNotNull(key, "key cannot be null"); + if (state == State.Connection) { - if (value == null) { - return connectionState.remove(key); - } - return connectionState.put(key, value); + return key.convert(connectionState.remove(key)); } else { - if (value == null) { - return sstate.remove(key); - } - return sstate.put(key, value); + return key.convert(sessionState.remove(key)); } } @Override - public Object getAttachment(String key, State state) { + public <T> Optional<T> getAttachment(AttachmentKey<T> key, State state) { if (state == State.Connection) { - return connectionState.get(key); + return key.convert(connectionState.get(key)); } else { - return sstate.get(key); + return key.convert(sessionState.get(key)); } } }; diff --git a/upgrade-instructions.md b/upgrade-instructions.md index aa2ade8..0853760 100644 --- a/upgrade-instructions.md +++ b/upgrade-instructions.md @@ -30,7 +30,25 @@ Change list: - [UidValidity and maildir](#uid-validity-and-maildir) - [UidValidity and JPA or Cassandra](#uid-validity-and-jpa-or-cassandra) - [Differentiation between domain alias and domain mapping](#differentiation-between-domain-alias-and-domain-mapping) + - [ProtocolSession storng typing](#protocolsession-storng-typing) +### ProtocolSession storng typing + +Date 19/03/2020 + +SHA-1 XXX + +JIRA: https://issues.apache.org/jira/browse/JAMES-3119 + +`ProtocolSession` have been reworked in order to increase type strengh +and reduce errors. + +Now `setAttachment` and `getAttachment` are expecting an `AttachmentKey` as key +and return an `Optional` instead of a `null`-able value. + +Moreover `setAttachment` do not allow `null` values and `removeAttachment` +should be use now to remove elements. + ### Differentiation between domain alias and domain mapping Date 10/03/2020 --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
