You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2021/09/28 02:28:35 UTC
[james-project] branch 3.6.x updated: JAMES-3477 Mail::duplicate
did lead to file leak in various places [BACKPORT 3.6.1] (#669)
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch 3.6.x
in repository https://gitbox.apache.org/repos/asf/james-project.git
The following commit(s) were added to refs/heads/3.6.x by this push:
new e322e13 JAMES-3477 Mail::duplicate did lead to file leak in various places [BACKPORT 3.6.1] (#669)
e322e13 is described below
commit e322e13474a94dbb4927ac9041fd891ae3672561
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Tue Sep 28 09:28:33 2021 +0700
JAMES-3477 Mail::duplicate did lead to file leak in various places [BACKPORT 3.6.1] (#669)
Large mails exceeding 100K would be stored on disk and not reclaimed...
MailImpl should avoid double wrapping upon duplicating mail
This was the root cause of the file leak:
MailImpl::setMessage was doing a defensive cleanup... Hence setMessage lone was not to blame.
---
.../org/apache/james/server/core/MailImpl.java | 14 +++++--
.../apache/james/transport/mailets/DSNBounce.java | 45 +++++++++++++---------
.../mailets/RecipientRewriteTableProcessor.java | 18 +++++----
.../mailets/redirect/ProcessRedirectNotify.java | 11 ++++--
4 files changed, 55 insertions(+), 33 deletions(-)
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 dc84621..84dc716 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
@@ -94,18 +94,22 @@ public class MailImpl implements Disposable, Mail {
* @throws MessagingException when the message is not clonable
*/
public static MailImpl duplicate(Mail mail) throws MessagingException {
+ return duplicateWithoutMessage(mail)
+ .mimeMessage(mail.getMessage())
+ .build();
+ }
+
+ public static MailImpl.Builder duplicateWithoutMessage(Mail mail) throws MessagingException {
return MailImpl.builder()
.name(deriveNewName(mail.getName()))
.sender(mail.getMaybeSender())
.addRecipients(mail.getRecipients())
- .mimeMessage(new MimeMessageWrapper(mail.getMessage()))
.remoteHost(mail.getRemoteHost())
.remoteAddr(mail.getRemoteAddr())
.lastUpdated(mail.getLastUpdated())
.errorMessage(mail.getErrorMessage())
.addAttributes(duplicateAttributes(mail))
- .addAllHeadersForRecipients(mail.getPerRecipientSpecificHeaders())
- .build();
+ .addAllHeadersForRecipients(mail.getPerRecipientSpecificHeaders());
}
private static ImmutableList<Attribute> duplicateAttributes(Mail mail) {
@@ -164,6 +168,10 @@ public class MailImpl implements Disposable, Mail {
perRecipientHeaders = new PerRecipientHeaders();
}
+ public String getName() {
+ return name;
+ }
+
public Builder mimeMessage(MimeMessage mimeMessage) {
this.mimeMessage = Optional.ofNullable(mimeMessage);
return this;
diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/DSNBounce.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/DSNBounce.java
index 3dcfe7b..aa24357 100755
--- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/DSNBounce.java
+++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/DSNBounce.java
@@ -48,7 +48,9 @@ import org.apache.james.core.MailAddress;
import org.apache.james.core.MaybeSender;
import org.apache.james.dnsservice.api.DNSService;
import org.apache.james.javax.MimeMultipartReport;
+import org.apache.james.lifecycle.api.LifecycleUtil;
import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
import org.apache.james.transport.mailets.redirect.InitParameters;
import org.apache.james.transport.mailets.redirect.MailModifier;
import org.apache.james.transport.mailets.redirect.NotifyMailetInitParameters;
@@ -305,25 +307,30 @@ public class DSNBounce extends GenericMailet implements RedirectNotify {
}
private void trySendBounce(Mail originalMail) throws MessagingException {
- MailImpl newMail = MailImpl.duplicate(originalMail);
+ MailImpl.Builder newMailBuilder = MailImpl.duplicateWithoutMessage(originalMail);
+
+ newMailBuilder.remoteHost(getRemoteHost());
+ newMailBuilder.remoteAddr(getRemoteAddr());
+ List<MailAddress> recipients = getSenderAsList(originalMail);
+ newMailBuilder.addRecipients(recipients);
+ newMailBuilder.sender(originalMail.getMaybeSender());
+
+ if (getInitParameters().isDebug()) {
+ LOGGER.debug("New mail - sender: {}, recipients: {}, name: {}, remoteHost: {}, remoteAddr: {}, state: {}, lastUpdated: {}, errorMessage: {}",
+ originalMail.getMaybeSender(), recipients, newMailBuilder.getName(), getRemoteHost(), getRemoteAddr(), originalMail.getState(), originalMail.getLastUpdated(), originalMail.getErrorMessage());
+ }
+
+ MimeMessageWrapper bounceMessage = new MimeMessageWrapper(createBounceMessage(originalMail));
try {
- newMail.setRemoteHost(getRemoteHost());
- newMail.setRemoteAddr(getRemoteAddr());
- newMail.setRecipients(getSenderAsList(originalMail));
-
- if (getInitParameters().isDebug()) {
- LOGGER.debug("New mail - sender: {}, recipients: {}, name: {}, remoteHost: {}, remoteAddr: {}, state: {}, lastUpdated: {}, errorMessage: {}",
- newMail.getMaybeSender(), newMail.getRecipients(), newMail.getName(), newMail.getRemoteHost(), newMail.getRemoteAddr(), newMail.getState(), newMail.getLastUpdated(), newMail.getErrorMessage());
- }
-
- newMail.setMessage(createBounceMessage(originalMail));
-
+ MailImpl newMail = newMailBuilder.build();
+ newMail.setMessageNoCopy(bounceMessage);
+
// Set additional headers
MailModifier mailModifier = MailModifier.builder()
- .mailet(this)
- .mail(newMail)
- .dns(dns)
- .build();
+ .mailet(this)
+ .mail(newMail)
+ .dns(dns)
+ .build();
mailModifier.setRecipients(getRecipients(originalMail));
mailModifier.setTo(getTo(originalMail));
mailModifier.setSubjectPrefix(originalMail);
@@ -331,13 +338,13 @@ public class DSNBounce extends GenericMailet implements RedirectNotify {
mailModifier.setReversePath(getReversePath(originalMail));
mailModifier.setIsReply(getInitParameters().isReply(), originalMail);
mailModifier.setSender(getSender(originalMail));
-
+
newMail.getMessage().setHeader(RFC2822Headers.DATE, getDateHeader(originalMail));
-
+
newMail.getMessage().saveChanges();
getMailetContext().sendMail(newMail);
} finally {
- newMail.dispose();
+ LifecycleUtil.dispose(bounceMessage);
}
}
diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java
index 77660c2..1037152 100644
--- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java
+++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java
@@ -37,6 +37,7 @@ import org.apache.james.core.Domain;
import org.apache.james.core.MailAddress;
import org.apache.james.domainlist.api.DomainList;
import org.apache.james.domainlist.api.DomainListException;
+import org.apache.james.lifecycle.api.LifecycleUtil;
import org.apache.james.rrt.api.RecipientRewriteTable;
import org.apache.james.rrt.api.RecipientRewriteTable.ErrorMappingException;
import org.apache.james.rrt.api.RecipientRewriteTableException;
@@ -276,17 +277,20 @@ public class RecipientRewriteTableProcessor {
private void forwardToRemoteAddress(Mail mail, MailAddress recipient, Collection<MailAddress> remoteRecipients) {
if (!remoteRecipients.isEmpty()) {
+ Mail duplicate = null;
try {
- mailetContext.sendMail(
- MailImpl.builder()
- .name(mail.getName())
- .sender(mail.getMaybeSender())
- .addRecipients(ImmutableList.copyOf(remoteRecipients))
- .mimeMessage(mail.getMessage())
- .build());
+ duplicate = MailImpl.builder()
+ .name(mail.getName())
+ .sender(mail.getMaybeSender())
+ .addRecipients(ImmutableList.copyOf(remoteRecipients))
+ .mimeMessage(mail.getMessage())
+ .build();
+ mailetContext.sendMail(duplicate);
LOGGER.info("Mail for {} forwarded to {}", recipient, remoteRecipients);
} catch (MessagingException ex) {
LOGGER.warn("Error forwarding mail to {}", remoteRecipients);
+ } finally {
+ LifecycleUtil.dispose(duplicate);
}
}
}
diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/redirect/ProcessRedirectNotify.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/redirect/ProcessRedirectNotify.java
index 145b68e..c59b0ff 100644
--- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/redirect/ProcessRedirectNotify.java
+++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/redirect/ProcessRedirectNotify.java
@@ -21,6 +21,7 @@ package org.apache.james.transport.mailets.redirect;
import javax.mail.MessagingException;
import javax.mail.internet.MimeMessage;
+import org.apache.james.lifecycle.api.LifecycleUtil;
import org.apache.james.server.core.MailImpl;
import org.apache.mailet.Mail;
import org.slf4j.Logger;
@@ -118,10 +119,12 @@ public class ProcessRedirectNotify {
if (isDebug) {
LOGGER.debug("Alter message");
}
- newMail.setMessage(
- MessageAlteringUtils.from(mailet)
- .originalMail(originalMail)
- .alteredMessage());
+ MimeMessage oldMessage = newMail.getMessage();
+ MimeMessage newMessage = MessageAlteringUtils.from(mailet)
+ .originalMail(originalMail)
+ .alteredMessage();
+ newMail.setMessage(newMessage);
+ LifecycleUtil.dispose(oldMessage);
}
private void createUnalteredMessage(Mail originalMail, MailImpl newMail) throws MessagingException {
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org