JAMES-2363 Use MimeMessageWrapper usages to ensure MessageID is not overridden
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/d4a75754 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/d4a75754 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/d4a75754 Branch: refs/heads/master Commit: d4a75754906200fdc586b2bfb3c3caacc33da74e Parents: 3d8724e Author: benwa <[email protected]> Authored: Mon Mar 26 15:37:36 2018 +0700 Committer: benwa <[email protected]> Committed: Thu Mar 29 16:43:03 2018 +0700 ---------------------------------------------------------------------- .../james/core/builder/MimeMessageBuilder.java | 23 ++++- .../james/core/builder/MimeMessageWrapper.java | 70 ++++++++++++++++ .../core/builder/MimeMessageWrapperTest.java | 88 ++++++++++++++++++++ .../src/main/java/org/apache/mailet/Mail.java | 2 +- .../james/transport/mailets/LogMessageTest.java | 13 ++- .../transport/mailets/ReplaceContentTest.java | 9 +- .../transport/mailets/StripAttachmentTest.java | 13 +-- .../org/apache/james/server/core/MailImpl.java | 14 ++-- .../core/MimeMessageCopyOnWriteProxy.java | 2 +- .../server/core/MimeMessageWrapperTest.java | 18 ++++ .../mailrepository/file/MBoxMailRepository.java | 4 +- 11 files changed, 232 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/d4a75754/core/src/main/java/org/apache/james/core/builder/MimeMessageBuilder.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/james/core/builder/MimeMessageBuilder.java b/core/src/main/java/org/apache/james/core/builder/MimeMessageBuilder.java index 7cf181d..303743e 100644 --- a/core/src/main/java/org/apache/james/core/builder/MimeMessageBuilder.java +++ b/core/src/main/java/org/apache/james/core/builder/MimeMessageBuilder.java @@ -25,6 +25,7 @@ import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.Properties; @@ -60,6 +61,22 @@ public class MimeMessageBuilder { this.name = name; this.value = value; } + + @Override + public final boolean equals(Object o) { + if (o instanceof Header) { + Header header = (Header) o; + + return Objects.equals(this.name, header.name) + && Objects.equals(this.value, header.value); + } + return false; + } + + @Override + public final int hashCode() { + return Objects.hash(name, value); + } } public static class MultipartBuilder { @@ -380,8 +397,10 @@ public class MimeMessageBuilder { for (Header header: headerList) { mimeMessage.addHeader(header.name, header.value); } - mimeMessage.saveChanges(); - return mimeMessage; + + MimeMessage wrappedMessage = MimeMessageWrapper.wrap(mimeMessage); + wrappedMessage.saveChanges(); + return wrappedMessage; } } http://git-wip-us.apache.org/repos/asf/james-project/blob/d4a75754/core/src/main/java/org/apache/james/core/builder/MimeMessageWrapper.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/james/core/builder/MimeMessageWrapper.java b/core/src/main/java/org/apache/james/core/builder/MimeMessageWrapper.java new file mode 100644 index 0000000..8e66bec --- /dev/null +++ b/core/src/main/java/org/apache/james/core/builder/MimeMessageWrapper.java @@ -0,0 +1,70 @@ +/**************************************************************** + * 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.builder; + +import java.util.Collections; +import java.util.Properties; + +import javax.mail.Header; +import javax.mail.MessagingException; +import javax.mail.Session; +import javax.mail.internet.MimeMessage; + +import com.github.fge.lambdas.Throwing; +import com.github.fge.lambdas.consumers.ThrowingConsumer; + +public class MimeMessageWrapper extends MimeMessage { + + public static MimeMessageWrapper wrap(MimeMessage mimeMessage) throws MessagingException { + try { + return new MimeMessageWrapper(mimeMessage); + } catch (MessagingException e) { + // Copying a mime message fails when the body is empty + // Copying manually the headers is the best alternative... + + MimeMessageWrapper result = new MimeMessageWrapper(); + ThrowingConsumer<Header> consumer = header -> result.addHeader(header.getName(), header.getValue()); + Collections.list(mimeMessage.getAllHeaders()) + .forEach(Throwing.consumer(consumer).sneakyThrow()); + return result; + } + } + + private MimeMessageWrapper() { + super(Session.getDefaultInstance(new Properties())); + } + + private MimeMessageWrapper(MimeMessage mimeMessage) throws MessagingException { + super(mimeMessage); + } + + /** + * Overrides default javamail behaviour by not altering the Message-ID by + * default, see <a href="https://issues.apache.org/jira/browse/JAMES-875";>JAMES-875</a> and + * <a href="https://issues.apache.org/jira/browse/JAMES-1010";>JAMES-1010</a> + */ + @Override + protected void updateMessageID() throws MessagingException { + if (getMessageID() == null) { + super.updateMessageID(); + } + } + +} http://git-wip-us.apache.org/repos/asf/james-project/blob/d4a75754/core/src/test/java/org/apache/james/core/builder/MimeMessageWrapperTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/james/core/builder/MimeMessageWrapperTest.java b/core/src/test/java/org/apache/james/core/builder/MimeMessageWrapperTest.java new file mode 100644 index 0000000..230763c --- /dev/null +++ b/core/src/test/java/org/apache/james/core/builder/MimeMessageWrapperTest.java @@ -0,0 +1,88 @@ +/**************************************************************** + * 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.builder; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.Properties; + +import javax.mail.Session; +import javax.mail.internet.MimeMessage; + +import org.junit.Test; + +public class MimeMessageWrapperTest { + + @Test + public void saveChangesShouldPreserveMessageId() throws Exception { + String messageId = "<[email protected]>"; + String messageText = "Message-ID: " + messageId + "\r\n" + + "Subject: test\r\n" + + "\r\n" + + "Content!"; + InputStream inputStream = new ByteArrayInputStream(messageText.getBytes(StandardCharsets.UTF_8)); + MimeMessage message = new MimeMessage(Session.getDefaultInstance(new Properties()), inputStream); + MimeMessageWrapper mimeMessageWrapper = MimeMessageWrapper.wrap(message); + + mimeMessageWrapper.saveChanges(); + + assertThat(mimeMessageWrapper.getMessageID()) + .isEqualTo(messageId); + } + + @Test + public void wrapShouldPreserveBody() throws Exception { + String messageAsText = "header1: <[email protected]>\r\n" + + "Subject: test\r\n" + + "\r\n" + + "Content!"; + InputStream inputStream = new ByteArrayInputStream(messageAsText.getBytes(StandardCharsets.UTF_8)); + MimeMessage message = new MimeMessage(Session.getDefaultInstance(new Properties()), inputStream); + MimeMessageWrapper mimeMessageWrapper = MimeMessageWrapper.wrap(message); + + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + + mimeMessageWrapper.writeTo(outputStream); + + assertThat(new String(outputStream.toByteArray(), StandardCharsets.UTF_8)) + .isEqualTo(messageAsText); + } + + @Test + public void wrapShouldNotThrowWhenNoBody() throws Exception { + MimeMessage originalMessage = new MimeMessage(Session.getDefaultInstance(new Properties())); + originalMessage.addHeader("header1", "value1"); + originalMessage.addHeader("header2", "value2"); + originalMessage.addHeader("header2", "value3"); + MimeMessageWrapper mimeMessageWrapper = MimeMessageWrapper.wrap(originalMessage); + + assertThat(Collections.list(mimeMessageWrapper.getAllHeaders())) + .extracting(javaxHeader -> new MimeMessageBuilder.Header(javaxHeader.getName(), javaxHeader.getValue())) + .contains(new MimeMessageBuilder.Header("header1", "value1"), + new MimeMessageBuilder.Header("header2", "value2"), + new MimeMessageBuilder.Header("header2", "value3")); + } + +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/d4a75754/mailet/api/src/main/java/org/apache/mailet/Mail.java ---------------------------------------------------------------------- diff --git a/mailet/api/src/main/java/org/apache/mailet/Mail.java b/mailet/api/src/main/java/org/apache/mailet/Mail.java index 3ffcaa4..23a0918 100644 --- a/mailet/api/src/main/java/org/apache/mailet/Mail.java +++ b/mailet/api/src/main/java/org/apache/mailet/Mail.java @@ -165,7 +165,7 @@ public interface Mail extends Serializable, Cloneable { * * @param message the new message that this Mail instance will wrap */ - void setMessage(MimeMessage message); + void setMessage(MimeMessage message) throws MessagingException; /** * Sets the state of this message. http://git-wip-us.apache.org/repos/asf/james-project/blob/d4a75754/mailet/standard/src/test/java/org/apache/james/transport/mailets/LogMessageTest.java ---------------------------------------------------------------------- diff --git a/mailet/standard/src/test/java/org/apache/james/transport/mailets/LogMessageTest.java b/mailet/standard/src/test/java/org/apache/james/transport/mailets/LogMessageTest.java index ef03a8f..033231e 100644 --- a/mailet/standard/src/test/java/org/apache/james/transport/mailets/LogMessageTest.java +++ b/mailet/standard/src/test/java/org/apache/james/transport/mailets/LogMessageTest.java @@ -28,7 +28,10 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import java.util.Properties; + import javax.mail.MessagingException; +import javax.mail.Session; import javax.mail.internet.MimeMessage; import org.apache.james.core.builder.MimeMessageBuilder; @@ -81,11 +84,13 @@ class LogMessageTest { .build(); mailet.init(mailetConfig); + MimeMessage message = new MimeMessage(Session.getDefaultInstance(new Properties())); + message.addHeader("Date", "Tue, 16 Jan 2018 10:23:03 +0100"); + message.setSubject("subject"); + message.setText("This is a fake mail"); + mailet.service(FakeMail.builder() - .mimeMessage(MimeMessageBuilder.mimeMessageBuilder() - .addHeader("Date", "Tue, 16 Jan 2018 10:23:03 +0100") - .setSubject("subject") - .setText("This is a fake mail")) + .mimeMessage(message) .build()); verify(logger).info("Logging mail {}", (Object) null); http://git-wip-us.apache.org/repos/asf/james-project/blob/d4a75754/mailet/standard/src/test/java/org/apache/james/transport/mailets/ReplaceContentTest.java ---------------------------------------------------------------------- diff --git a/mailet/standard/src/test/java/org/apache/james/transport/mailets/ReplaceContentTest.java b/mailet/standard/src/test/java/org/apache/james/transport/mailets/ReplaceContentTest.java index 508c4c1..7631b33 100644 --- a/mailet/standard/src/test/java/org/apache/james/transport/mailets/ReplaceContentTest.java +++ b/mailet/standard/src/test/java/org/apache/james/transport/mailets/ReplaceContentTest.java @@ -22,7 +22,9 @@ package org.apache.james.transport.mailets; import static org.assertj.core.api.Assertions.assertThat; import java.nio.charset.StandardCharsets; +import java.util.Properties; +import javax.mail.Session; import javax.mail.internet.MimeMessage; import org.apache.james.core.builder.MimeMessageBuilder; @@ -78,10 +80,11 @@ public class ReplaceContentTest { .build(); mailet.init(mailetConfig); + MimeMessage message = new MimeMessage(Session.getDefaultInstance(new Properties())); + message.setText("This is one simple test/ è one simple test.\n" + + "Blo blo blo blo.\n"); Mail mail = FakeMail.builder() - .mimeMessage(MimeMessageBuilder.mimeMessageBuilder() - .setText("This is one simple test/ è one simple test.\n" - + "Blo blo blo blo.\n")) + .mimeMessage(message) .build(); mailet.service(mail); http://git-wip-us.apache.org/repos/asf/james-project/blob/d4a75754/mailet/standard/src/test/java/org/apache/james/transport/mailets/StripAttachmentTest.java ---------------------------------------------------------------------- diff --git a/mailet/standard/src/test/java/org/apache/james/transport/mailets/StripAttachmentTest.java b/mailet/standard/src/test/java/org/apache/james/transport/mailets/StripAttachmentTest.java index 71ec927..850561c 100644 --- a/mailet/standard/src/test/java/org/apache/james/transport/mailets/StripAttachmentTest.java +++ b/mailet/standard/src/test/java/org/apache/james/transport/mailets/StripAttachmentTest.java @@ -34,6 +34,7 @@ import java.util.Map; import java.util.Optional; import javax.mail.MessagingException; +import javax.mail.Multipart; import javax.mail.Part; import javax.mail.internet.MimeBodyPart; import javax.mail.internet.MimeMessage; @@ -686,19 +687,19 @@ class StripAttachmentTest { .build(); mailet.init(mci); - MimeMultipart mimeMultipart = MimeMessageBuilder.multipartBuilder() - .addBody(MimeMessageBuilder.bodyPartBuilder() - .filename("removeMe.tmp")) - .build(); MimeMessage mimeMessage = MimeMessageBuilder.mimeMessageBuilder() - .setContent(mimeMultipart) + .setContent(MimeMessageBuilder.multipartBuilder() + .addBody(MimeMessageBuilder.bodyPartBuilder() + .filename("removeMe.tmp"))) .build(); Mail mail = mock(Mail.class); //When mailet.processMultipartPartMessage(mimeMessage, mail); //Then - assertThat(mimeMultipart.getCount()).isZero(); + assertThat(mimeMessage.getContent()).isInstanceOf(MimeMultipart.class); + MimeMultipart multipart = (MimeMultipart) mimeMessage.getContent(); + assertThat(multipart.getCount()).isZero(); } @Test http://git-wip-us.apache.org/repos/asf/james-project/blob/d4a75754/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java ---------------------------------------------------------------------- diff --git a/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java b/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java index 9521784..816c9e6 100644 --- a/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java +++ b/server/container/core/src/main/java/org/apache/james/server/core/MailImpl.java @@ -231,7 +231,7 @@ public class MailImpl implements Disposable, Mail { public MailImpl build() { MailImpl mail = new MailImpl(); - mimeMessage.ifPresent(mail::setMessage); + mimeMessage.ifPresent(Throwing.consumer(mail::setMessage).sneakyThrow()); name.ifPresent(mail::setName); sender.ifPresent(mail::setSender); mail.setRecipients(recipients); @@ -323,7 +323,7 @@ public class MailImpl implements Disposable, Mail { /** * The MimeMessage that holds the mail data. */ - private MimeMessage message; + private MimeMessageCopyOnWriteProxy message; /** * The sender of this mail. */ @@ -437,7 +437,7 @@ public class MailImpl implements Disposable, Mail { * A constructor that creates a MailImpl with the specified name, sender, * recipients, and MimeMessage. */ - public MailImpl(String name, MailAddress sender, Collection<MailAddress> recipients, MimeMessage message) { + public MailImpl(String name, MailAddress sender, Collection<MailAddress> recipients, MimeMessage message) throws MessagingException { this(name, sender, recipients); this.setMessage(new MimeMessageCopyOnWriteProxy(message)); } @@ -538,7 +538,7 @@ public class MailImpl implements Disposable, Mail { * @param message the new MimeMessage associated with this MailImpl */ @Override - public void setMessage(MimeMessage message) { + public void setMessage(MimeMessage message) throws MessagingException { // TODO: We should use the MimeMessageCopyOnWriteProxy // everytime we set the MimeMessage. We should @@ -551,7 +551,11 @@ public class MailImpl implements Disposable, Mail { if (this.message != null) { LifecycleUtil.dispose(this.message); } - this.message = message; + if (message instanceof MimeMessageCopyOnWriteProxy) { + this.message = (MimeMessageCopyOnWriteProxy) message; + } else { + this.message = new MimeMessageCopyOnWriteProxy(message); + } } } http://git-wip-us.apache.org/repos/asf/james-project/blob/d4a75754/server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java ---------------------------------------------------------------------- diff --git a/server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java b/server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java index 3a8ede4..cc375e9 100644 --- a/server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java +++ b/server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java @@ -91,7 +91,7 @@ public class MimeMessageCopyOnWriteProxy extends MimeMessage implements Disposab protected MessageReferenceTracker refCount; - public MimeMessageCopyOnWriteProxy(MimeMessage original) { + public MimeMessageCopyOnWriteProxy(MimeMessage original) throws MessagingException { this(original, false); } http://git-wip-us.apache.org/repos/asf/james-project/blob/d4a75754/server/container/core/src/test/java/org/apache/james/server/core/MimeMessageWrapperTest.java ---------------------------------------------------------------------- diff --git a/server/container/core/src/test/java/org/apache/james/server/core/MimeMessageWrapperTest.java b/server/container/core/src/test/java/org/apache/james/server/core/MimeMessageWrapperTest.java index 0385a1b..617ca4d 100644 --- a/server/container/core/src/test/java/org/apache/james/server/core/MimeMessageWrapperTest.java +++ b/server/container/core/src/test/java/org/apache/james/server/core/MimeMessageWrapperTest.java @@ -18,6 +18,7 @@ ****************************************************************/ package org.apache.james.server.core; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -36,6 +37,7 @@ import javax.mail.internet.MimeMessage; import javax.mail.util.SharedByteArrayInputStream; import org.apache.james.lifecycle.api.LifecycleUtil; +import org.apache.james.util.MimeMessageUtil; import org.apache.mailet.base.RFC2822Headers; import org.junit.After; import org.junit.Before; @@ -283,4 +285,20 @@ public class MimeMessageWrapperTest extends MimeMessageFromStreamTest { MimeMessageWrapper wrapper = new MimeMessageWrapper(message); assertEquals("\"base64\"", wrapper.getEncoding()); } + + @Test + public void saveChangesShouldPreserveMessageId() throws Exception { + String messageId = "<[email protected]>"; + MimeMessage message = MimeMessageUtil.mimeMessageFromString("Message-ID: " + messageId + "\r\n" + + "Subject: test\r\n" + + "\r\n" + + "Content!"); + + MimeMessageWrapper mimeMessageWrapper = new MimeMessageWrapper(message); + + mimeMessageWrapper.saveChanges(); + + assertThat(mimeMessageWrapper.getMessageID()) + .isEqualTo(messageId); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/d4a75754/server/data/data-file/src/main/java/org/apache/james/mailrepository/file/MBoxMailRepository.java ---------------------------------------------------------------------- diff --git a/server/data/data-file/src/main/java/org/apache/james/mailrepository/file/MBoxMailRepository.java b/server/data/data-file/src/main/java/org/apache/james/mailrepository/file/MBoxMailRepository.java index caed732..bcc69bb 100755 --- a/server/data/data-file/src/main/java/org/apache/james/mailrepository/file/MBoxMailRepository.java +++ b/server/data/data-file/src/main/java/org/apache/james/mailrepository/file/MBoxMailRepository.java @@ -526,7 +526,7 @@ public class MBoxMailRepository implements MailRepository, Configurable { } @Override - public Mail retrieve(String key) { + public Mail retrieve(String key) throws MessagingException { loadKeys(); MailImpl res; @@ -669,7 +669,7 @@ public class MBoxMailRepository implements MailRepository, Configurable { } @Override - public void remove(String key) { + public void remove(String key) throws MessagingException { loadKeys(); try { lockMBox(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
