JAMES-2541 Share nullSender mailAddress parsing in MailAddress object
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/61b00312 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/61b00312 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/61b00312 Branch: refs/heads/master Commit: 61b003122ce060460dbc8e0b29354972a0a642ea Parents: c37e9fd Author: Benoit Tellier <[email protected]> Authored: Wed Sep 5 15:32:33 2018 +0700 Committer: Benoit Tellier <[email protected]> Committed: Mon Sep 10 17:19:38 2018 +0700 ---------------------------------------------------------------------- core/pom.xml | 6 +- .../java/org/apache/james/core/MailAddress.java | 21 +- .../org/apache/james/core/MailAddressTest.java | 494 ++++++++++--------- .../apache/james/queue/jms/JMSMailQueue.java | 18 +- 4 files changed, 279 insertions(+), 260 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/61b00312/core/pom.xml ---------------------------------------------------------------------- diff --git a/core/pom.xml b/core/pom.xml index 6c2ec01..244a281 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -86,6 +86,10 @@ <artifactId>junit-vintage-engine</artifactId> <scope>test</scope> </dependency> + <dependency> + <groupId>org.slf4j</groupId> + <artifactId>slf4j-api</artifactId> + </dependency> </dependencies> -</project> +</project> \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/61b00312/core/src/main/java/org/apache/james/core/MailAddress.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/james/core/MailAddress.java b/core/src/main/java/org/apache/james/core/MailAddress.java index cd206ef..02b5ce6 100644 --- a/core/src/main/java/org/apache/james/core/MailAddress.java +++ b/core/src/main/java/org/apache/james/core/MailAddress.java @@ -26,6 +26,9 @@ import java.util.Optional; import javax.mail.internet.AddressException; import javax.mail.internet.InternetAddress; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * A representation of an email address. * <p/> @@ -63,6 +66,7 @@ import javax.mail.internet.InternetAddress; * @version 1.0 */ public class MailAddress implements java.io.Serializable { + public static final Logger LOGGER = LoggerFactory.getLogger(MailAddress.class); /** * We hardcode the serialVersionUID * This version (2779163542539434916L) retains compatibility back to @@ -105,11 +109,26 @@ public class MailAddress implements java.io.Serializable { }; - public static MailAddress nullSender() { return NULL_SENDER; } + public static MailAddress getMailSender(String sender) { + if (sender == null || sender.trim().length() <= 0) { + return null; + } + if (sender.equals(MailAddress.NULL_SENDER_AS_STRING)) { + return MailAddress.nullSender(); + } + try { + return new MailAddress(sender); + } catch (AddressException e) { + // Should never happen as long as the user does not modify the header by himself + LOGGER.error("Unable to parse the sender address {}, so we fallback to a null sender", sender, e); + return MailAddress.nullSender(); + } + } + private final String localPart; private final Domain domain; http://git-wip-us.apache.org/repos/asf/james-project/blob/61b00312/core/src/test/java/org/apache/james/core/MailAddressTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/james/core/MailAddressTest.java b/core/src/test/java/org/apache/james/core/MailAddressTest.java index dceeaed..cf60ddd 100644 --- a/core/src/test/java/org/apache/james/core/MailAddressTest.java +++ b/core/src/test/java/org/apache/james/core/MailAddressTest.java @@ -1,241 +1,253 @@ -/**************************************************************** - * Licensed to the Apache Software Foundation (ASF) under one * - * or more contributor license agreements. See the NOTICE file * - * distributed with this work for additional information * - * regarding copyright ownership. The ASF licenses this file * - * to you under the Apache License, Version 2.0 (the * - * "License"); you may not use this file except in compliance * - * with the License. You may obtain a copy of the License at * - * * - * http://www.apache.org/licenses/LICENSE-2.0 * - * * - * Unless required by applicable law or agreed to in writing, * - * software distributed under the License is distributed on an * - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * - * KIND, either express or implied. See the License for the * - * specific language governing permissions and limitations * - * under the License. * - ****************************************************************/ - -package org.apache.james.core; - -import static org.assertj.core.api.Assertions.assertThat; - -import javax.mail.internet.AddressException; -import javax.mail.internet.InternetAddress; - -import org.assertj.core.api.Assertions; -import org.junit.Assert; -import org.junit.Test; - -import nl.jqno.equalsverifier.EqualsVerifier; - -public class MailAddressTest { - - private static final String GOOD_LOCAL_PART = "\"quoted@local part\""; - private static final String GOOD_QUOTED_LOCAL_PART = "\"quoted@local part\"@james.apache.org"; - private static final String GOOD_ADDRESS = "[email protected]"; - private static final Domain GOOD_DOMAIN = Domain.of("james.apache.org"); - - private static final String[] GOOD_ADDRESSES = { - GOOD_ADDRESS, - GOOD_QUOTED_LOCAL_PART, - "[email protected]", - "server-dev@[127.0.0.1]", - "server-dev@#123", - "server-dev@#123.apache.org", - "[email protected]", - "\\[email protected]", - "server-dev\\[email protected]", - }; - - private static final String[] BAD_ADDRESSES = { - "", - "server-dev", - "server-dev@", - "[]", - "server-dev@[]", - "server-dev@#", - "quoted [email protected]", - "quoted@[email protected]", - "[email protected]", - "[email protected]", - "[email protected]", - "[email protected].", - "[email protected]", - "[email protected]", - "[email protected]", - "server-dev@#james.apache.org", - "server-dev@#123james.apache.org", - "server-dev@#-123.james.apache.org", - "server-dev@james. apache.org", - "server-dev@james\\.apache.org", - "server-dev@[300.0.0.1]", - "server-dev@[127.0.1]", - "server-dev@[0127.0.0.1]", - "server-dev@[127.0.1.1a]", - "server-dev@[127\\.0.1.1]", - "server-dev@[127.0.1.1.1]", - "server-dev@[127.0.1.-1]" - }; - - /** - * Test method for {@link MailAddress#hashCode()}. - * - * @throws AddressException - */ - @Test - public void testHashCode() throws AddressException { - - MailAddress a = new MailAddress(GOOD_ADDRESS); - MailAddress b = new MailAddress(GOOD_ADDRESS); - assertThat(a.hashCode() == b.hashCode()).describedAs(a.hashCode() + " != " + b.hashCode()).isTrue(); - } - - /** - * Test method for {@link MailAddress#MailAddress(java.lang.String)}. - * - * @throws AddressException - */ - @Test - public void testMailAddressString() throws AddressException { - - MailAddress a = new MailAddress(GOOD_ADDRESS); - assertThat(GOOD_ADDRESS.equals(a.toString())).isTrue(); - - for (String goodAddress : GOOD_ADDRESSES) { - try { - a = new MailAddress(goodAddress); - } catch (AddressException e) { - Assert.fail(e.getMessage()); - } - } - - for (String badAddress : BAD_ADDRESSES) { - Assertions.assertThatThrownBy(() -> new MailAddress(badAddress)) - .isInstanceOf(AddressException.class); - } - } - - /** - * Test method for {@link MailAddress#MailAddress(java.lang.String, java.lang.String)}. - */ - @Test - public void testMailAddressStringString() { - - try { - new MailAddress("local-part", "domain"); - } catch (AddressException e) { - assertThat(false).describedAs(e.getMessage()).isTrue(); - } - try { - MailAddress a = new MailAddress("local-part", "-domain"); - assertThat(true).describedAs(a.toString()).isFalse(); - } catch (AddressException e) { - assertThat(true).isTrue(); - } - } - - /** - * Test method for {@link MailAddress#MailAddress(javax.mail.internet.InternetAddress)}. - */ - @Test - public void testMailAddressInternetAddress() { - - try { - new MailAddress(new InternetAddress(GOOD_QUOTED_LOCAL_PART)); - } catch (AddressException e) { - System.out.println("AddressException" + e.getMessage()); - assertThat(false).describedAs(e.getMessage()).isTrue(); - } - } - - /** - * Test method for {@link MailAddress#getDomain()}. - */ - @Test - public void testGetDomain() { - - try { - MailAddress a = new MailAddress(new InternetAddress(GOOD_ADDRESS)); - assertThat(a.getDomain().equals(GOOD_DOMAIN)).describedAs(a.getDomain() + " != " + GOOD_DOMAIN).isTrue(); - } catch (AddressException e) { - System.out.println("AddressException" + e.getMessage()); - assertThat(false).describedAs(e.getMessage()).isTrue(); - } - } - - /** - * Test method for {@link MailAddress#getLocalPart()}. - */ - @Test - public void testGetLocalPart() { - - try { - MailAddress a = new MailAddress(new InternetAddress(GOOD_QUOTED_LOCAL_PART)); - assertThat(a.getLocalPart().equals(GOOD_LOCAL_PART)).describedAs(GOOD_LOCAL_PART + " != " + a.getLocalPart()).isTrue(); - } catch (AddressException e) { - System.out.println("AddressException" + e.getMessage()); - assertThat(false).describedAs(e.getMessage()).isTrue(); - } - } - - /** - * Test method for {@link MailAddress#toString()}. - */ - @Test - public void testToString() { - - try { - MailAddress a = new MailAddress(new InternetAddress(GOOD_ADDRESS)); - assertThat(a.toString().equals(GOOD_ADDRESS)).describedAs(a.toString() + " != " + GOOD_ADDRESS).isTrue(); - } catch (AddressException e) { - System.out.println("AddressException" + e.getMessage()); - assertThat(false).describedAs(e.getMessage()).isTrue(); - } - } - - /** - * Test method for {@link MailAddress#toInternetAddress()}. - */ - @Test - public void testToInternetAddress() { - - try { - InternetAddress b = new InternetAddress(GOOD_ADDRESS); - MailAddress a = new MailAddress(b); - assertThat(a.toInternetAddress().equals(b)).isTrue(); - assertThat(a.toString().equals(GOOD_ADDRESS)).describedAs(a.toString() + " != " + GOOD_ADDRESS).isTrue(); - } catch (AddressException e) { - System.out.println("AddressException" + e.getMessage()); - assertThat(false).describedAs(e.getMessage()).isTrue(); - } - } - - /** - * Test method for {@link MailAddress#equals(java.lang.Object)}. - * - * @throws AddressException - */ - @Test - public void testEqualsObject() throws AddressException { - - MailAddress a = new MailAddress(GOOD_ADDRESS); - MailAddress b = new MailAddress(GOOD_ADDRESS); - - assertThat(a.equals(b)).describedAs(a.toString() + " != " + b.toString()).isTrue(); - assertThat(a.equals(null)).describedAs(a.toString() + " != " + null).isFalse(); - } - - @Test - public void equalsShouldReturnTrueWhenBothNullSender() { - assertThat(MailAddress.nullSender()) - .isEqualTo(MailAddress.nullSender()); - } - - @Test - public void shouldMatchBeanContract() { - EqualsVerifier.forClass(MailAddress.class) - .verify(); - } -} +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.core; + +import static org.assertj.core.api.Assertions.assertThat; + +import javax.mail.internet.AddressException; +import javax.mail.internet.InternetAddress; + +import org.assertj.core.api.Assertions; +import org.junit.Assert; +import org.junit.Test; + +import nl.jqno.equalsverifier.EqualsVerifier; + +public class MailAddressTest { + + private static final String GOOD_LOCAL_PART = "\"quoted@local part\""; + private static final String GOOD_QUOTED_LOCAL_PART = "\"quoted@local part\"@james.apache.org"; + private static final String GOOD_ADDRESS = "[email protected]"; + private static final Domain GOOD_DOMAIN = Domain.of("james.apache.org"); + + private static final String[] GOOD_ADDRESSES = { + GOOD_ADDRESS, + GOOD_QUOTED_LOCAL_PART, + "[email protected]", + "server-dev@[127.0.0.1]", + "server-dev@#123", + "server-dev@#123.apache.org", + "[email protected]", + "\\[email protected]", + "server-dev\\[email protected]", + }; + + private static final String[] BAD_ADDRESSES = { + "", + "server-dev", + "server-dev@", + "[]", + "server-dev@[]", + "server-dev@#", + "quoted [email protected]", + "quoted@[email protected]", + "[email protected]", + "[email protected]", + "[email protected]", + "[email protected].", + "[email protected]", + "[email protected]", + "[email protected]", + "server-dev@#james.apache.org", + "server-dev@#123james.apache.org", + "server-dev@#-123.james.apache.org", + "server-dev@james. apache.org", + "server-dev@james\\.apache.org", + "server-dev@[300.0.0.1]", + "server-dev@[127.0.1]", + "server-dev@[0127.0.0.1]", + "server-dev@[127.0.1.1a]", + "server-dev@[127\\.0.1.1]", + "server-dev@[127.0.1.1.1]", + "server-dev@[127.0.1.-1]" + }; + + /** + * Test method for {@link MailAddress#hashCode()}. + * + * @throws AddressException + */ + @Test + public void testHashCode() throws AddressException { + + MailAddress a = new MailAddress(GOOD_ADDRESS); + MailAddress b = new MailAddress(GOOD_ADDRESS); + assertThat(a.hashCode() == b.hashCode()).describedAs(a.hashCode() + " != " + b.hashCode()).isTrue(); + } + + /** + * Test method for {@link MailAddress#MailAddress(java.lang.String)}. + * + * @throws AddressException + */ + @Test + public void testMailAddressString() throws AddressException { + + MailAddress a = new MailAddress(GOOD_ADDRESS); + assertThat(GOOD_ADDRESS.equals(a.toString())).isTrue(); + + for (String goodAddress : GOOD_ADDRESSES) { + try { + a = new MailAddress(goodAddress); + } catch (AddressException e) { + Assert.fail(e.getMessage()); + } + } + + for (String badAddress : BAD_ADDRESSES) { + Assertions.assertThatThrownBy(() -> new MailAddress(badAddress)) + .isInstanceOf(AddressException.class); + } + } + + /** + * Test method for {@link MailAddress#MailAddress(java.lang.String, java.lang.String)}. + */ + @Test + public void testMailAddressStringString() { + + try { + new MailAddress("local-part", "domain"); + } catch (AddressException e) { + assertThat(false).describedAs(e.getMessage()).isTrue(); + } + try { + MailAddress a = new MailAddress("local-part", "-domain"); + assertThat(true).describedAs(a.toString()).isFalse(); + } catch (AddressException e) { + assertThat(true).isTrue(); + } + } + + /** + * Test method for {@link MailAddress#MailAddress(javax.mail.internet.InternetAddress)}. + */ + @Test + public void testMailAddressInternetAddress() { + + try { + new MailAddress(new InternetAddress(GOOD_QUOTED_LOCAL_PART)); + } catch (AddressException e) { + System.out.println("AddressException" + e.getMessage()); + assertThat(false).describedAs(e.getMessage()).isTrue(); + } + } + + /** + * Test method for {@link MailAddress#getDomain()}. + */ + @Test + public void testGetDomain() { + + try { + MailAddress a = new MailAddress(new InternetAddress(GOOD_ADDRESS)); + assertThat(a.getDomain().equals(GOOD_DOMAIN)).describedAs(a.getDomain() + " != " + GOOD_DOMAIN).isTrue(); + } catch (AddressException e) { + System.out.println("AddressException" + e.getMessage()); + assertThat(false).describedAs(e.getMessage()).isTrue(); + } + } + + /** + * Test method for {@link MailAddress#getLocalPart()}. + */ + @Test + public void testGetLocalPart() { + + try { + MailAddress a = new MailAddress(new InternetAddress(GOOD_QUOTED_LOCAL_PART)); + assertThat(a.getLocalPart().equals(GOOD_LOCAL_PART)).describedAs(GOOD_LOCAL_PART + " != " + a.getLocalPart()).isTrue(); + } catch (AddressException e) { + System.out.println("AddressException" + e.getMessage()); + assertThat(false).describedAs(e.getMessage()).isTrue(); + } + } + + /** + * Test method for {@link MailAddress#toString()}. + */ + @Test + public void testToString() { + + try { + MailAddress a = new MailAddress(new InternetAddress(GOOD_ADDRESS)); + assertThat(a.toString().equals(GOOD_ADDRESS)).describedAs(a.toString() + " != " + GOOD_ADDRESS).isTrue(); + } catch (AddressException e) { + System.out.println("AddressException" + e.getMessage()); + assertThat(false).describedAs(e.getMessage()).isTrue(); + } + } + + /** + * Test method for {@link MailAddress#toInternetAddress()}. + */ + @Test + public void testToInternetAddress() { + + try { + InternetAddress b = new InternetAddress(GOOD_ADDRESS); + MailAddress a = new MailAddress(b); + assertThat(a.toInternetAddress().equals(b)).isTrue(); + assertThat(a.toString().equals(GOOD_ADDRESS)).describedAs(a.toString() + " != " + GOOD_ADDRESS).isTrue(); + } catch (AddressException e) { + System.out.println("AddressException" + e.getMessage()); + assertThat(false).describedAs(e.getMessage()).isTrue(); + } + } + + /** + * Test method for {@link MailAddress#equals(java.lang.Object)}. + * + * @throws AddressException + */ + @Test + public void testEqualsObject() throws AddressException { + + MailAddress a = new MailAddress(GOOD_ADDRESS); + MailAddress b = new MailAddress(GOOD_ADDRESS); + + assertThat(a.equals(b)).describedAs(a.toString() + " != " + b.toString()).isTrue(); + assertThat(a.equals(null)).describedAs(a.toString() + " != " + null).isFalse(); + } + + @Test + public void equalsShouldReturnTrueWhenBothNullSender() { + assertThat(MailAddress.nullSender()) + .isEqualTo(MailAddress.nullSender()); + } + + @Test + public void getMailSenderShouldReturnNullSenderWhenNullSender() { + assertThat(MailAddress.getMailSender(MailAddress.NULL_SENDER_AS_STRING)) + .isEqualTo(MailAddress.nullSender()); + } + + @Test + public void getMailSenderShouldReturnParsedAddressWhenNotNullAddress() throws Exception { + assertThat(MailAddress.getMailSender(GOOD_ADDRESS)) + .isEqualTo(new MailAddress(GOOD_ADDRESS)); + } + + @Test + public void shouldMatchBeanContract() { + EqualsVerifier.forClass(MailAddress.class) + .verify(); + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/61b00312/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java ---------------------------------------------------------------------- diff --git a/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java b/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java index ccb9fd1..9795896 100644 --- a/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java +++ b/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java @@ -419,26 +419,10 @@ public class JMSMailQueue implements ManageableMailQueue, JMSSupport, MailPriori splitter.split(attributeNames) .forEach(name -> setMailAttribute(message, mail, name)); - mail.setSender(getMailSender(message.getStringProperty(JAMES_MAIL_SENDER), mail)); + mail.setSender(MailAddress.getMailSender(message.getStringProperty(JAMES_MAIL_SENDER))); mail.setState(message.getStringProperty(JAMES_MAIL_STATE)); } - private MailAddress getMailSender(String sender, Mail mail) { - if (sender == null || sender.trim().length() <= 0) { - return null; - } - if (sender.equals(MailAddress.NULL_SENDER_AS_STRING)) { - return MailAddress.nullSender(); - } - try { - return new MailAddress(sender); - } catch (AddressException e) { - // Should never happen as long as the user does not modify the header by himself - LOGGER.error("Unable to parse the sender address {} for mail {}, so we fallback to a null sender", sender, mail.getName(), e); - return MailAddress.nullSender(); - } - } - /** * Retrieves the attribute by {@code name} form {@code message} and tries to add it on {@code mail}. * --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
