JAMES-2644 Fix the Content-Type parsing fragility
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/a4d524d8 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/a4d524d8 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/a4d524d8 Branch: refs/heads/master Commit: a4d524d8609de97ed77f222ac5eada022227a359 Parents: 7329ac6 Author: Gautier DI FOLCO <gdifo...@linagora.com> Authored: Wed Jan 16 14:13:10 2019 +0100 Committer: Benoit Tellier <btell...@linagora.com> Committed: Wed Jan 30 17:55:50 2019 +0700 ---------------------------------------------------------------------- server/container/core/pom.xml | 2 +- .../james/server/core/ContentTypeCleaner.java | 51 +++++++ .../server/core/ContentTypeCleanerTest.java | 55 +++++++ .../core/MimeMessageCopyOnWriteProxyTest.java | 35 ++++- .../james/modules/CommonServicesModule.java | 1 + .../apache/james/modules/MimeMessageModule.java | 33 ++++ .../apache/james/smtp/SmtpContentTypeTest.java | 151 +++++++++++++++++++ 7 files changed, 319 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/a4d524d8/server/container/core/pom.xml ---------------------------------------------------------------------- diff --git a/server/container/core/pom.xml b/server/container/core/pom.xml index d37c834..22991c2 100644 --- a/server/container/core/pom.xml +++ b/server/container/core/pom.xml @@ -141,7 +141,7 @@ <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-surefire-plugin</artifactId> <configuration> - <reuseForks>true</reuseForks> + <reuseForks>false</reuseForks> <forkCount>1C</forkCount> </configuration> </plugin> http://git-wip-us.apache.org/repos/asf/james-project/blob/a4d524d8/server/container/core/src/main/java/org/apache/james/server/core/ContentTypeCleaner.java ---------------------------------------------------------------------- diff --git a/server/container/core/src/main/java/org/apache/james/server/core/ContentTypeCleaner.java b/server/container/core/src/main/java/org/apache/james/server/core/ContentTypeCleaner.java new file mode 100644 index 0000000..5d43314 --- /dev/null +++ b/server/container/core/src/main/java/org/apache/james/server/core/ContentTypeCleaner.java @@ -0,0 +1,51 @@ +/**************************************************************** + * 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.server.core; + +import java.util.regex.Pattern; + +import javax.mail.internet.MimePart; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.base.Strings; + +public class ContentTypeCleaner { + private static final Logger LOGGER = LoggerFactory.getLogger(ContentTypeCleaner.class); + private static final Pattern REGEX = Pattern.compile("^[\\w\\-]+/[\\w\\-]+"); + private static final String HANDLER_CLASS_PROPERTY = "mail.mime.contenttypehandler"; + + public static void initialize() { + System.setProperty(HANDLER_CLASS_PROPERTY, ContentTypeCleaner.class.getName()); + } + + public static String cleanContentType(MimePart mimePart, String contentType) { + if (Strings.isNullOrEmpty(contentType)) { + return null; + } + + if (REGEX.matcher(contentType).find()) { + return contentType; + } + + LOGGER.warn("Can not parse Content-Type: " + contentType); + return null; + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/a4d524d8/server/container/core/src/test/java/org/apache/james/server/core/ContentTypeCleanerTest.java ---------------------------------------------------------------------- diff --git a/server/container/core/src/test/java/org/apache/james/server/core/ContentTypeCleanerTest.java b/server/container/core/src/test/java/org/apache/james/server/core/ContentTypeCleanerTest.java new file mode 100644 index 0000000..c1d1bf1 --- /dev/null +++ b/server/container/core/src/test/java/org/apache/james/server/core/ContentTypeCleanerTest.java @@ -0,0 +1,55 @@ +/**************************************************************** + * 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.server.core; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +public class ContentTypeCleanerTest { + @Test + void nullContentTypeShouldReturnNull() { + assertThat(ContentTypeCleaner + .cleanContentType(null, null)) + .isNull(); + } + + @Test + void emptyContentTypeShouldReturnNull() { + assertThat(ContentTypeCleaner + .cleanContentType(null, "")) + .isNull(); + } + + @Test + void invalidContentTypeShouldReturnNull() { + assertThat(ContentTypeCleaner + .cleanContentType(null, "I'mNotValid")) + .isNull(); + } + + @Test + void validContentTypeShouldReturnTheRawInput() { + String contentType = "application/pdf"; + assertThat(ContentTypeCleaner + .cleanContentType(null, contentType)) + .isEqualTo(contentType); + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/a4d524d8/server/container/core/src/test/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxyTest.java ---------------------------------------------------------------------- diff --git a/server/container/core/src/test/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxyTest.java b/server/container/core/src/test/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxyTest.java index dfa46e5..0d0291c 100644 --- a/server/container/core/src/test/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxyTest.java +++ b/server/container/core/src/test/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxyTest.java @@ -31,14 +31,22 @@ import org.apache.james.core.MailAddress; import org.apache.james.core.builder.MimeMessageBuilder; import org.apache.james.lifecycle.api.LifecycleUtil; import org.apache.mailet.Mail; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import com.google.common.collect.ImmutableList; + public class MimeMessageCopyOnWriteProxyTest extends MimeMessageFromStreamTest { final String content = "Subject: foo\r\nContent-Transfer-Encoding2: plain"; final String sep = "\r\n\r\n"; final String body = "bar\r\n.\r\n"; + @BeforeAll + static void setUp() { + ContentTypeCleaner.initialize(); + } + @Override protected MimeMessage getMessageFromSources(String sources) throws Exception { MimeMessageInputStreamSource mmis = new MimeMessageInputStreamSource("test", new SharedByteArrayInputStream(sources.getBytes())); @@ -46,7 +54,7 @@ public class MimeMessageCopyOnWriteProxyTest extends MimeMessageFromStreamTest { } @Test - public void testMessageCloning1() throws Exception { + void testMessageCloning1() throws Exception { ArrayList<MailAddress> r = new ArrayList<>(); r.add(new MailAddress("recipi...@test.com")); MimeMessageCopyOnWriteProxy messageFromSources = (MimeMessageCopyOnWriteProxy) getMessageFromSources( @@ -72,7 +80,7 @@ public class MimeMessageCopyOnWriteProxyTest extends MimeMessageFromStreamTest { } @Test - public void testMessageCloning2() throws Exception { + void testMessageCloning2() throws Exception { ArrayList<MailAddress> r = new ArrayList<>(); r.add(new MailAddress("recipi...@test.com")); MimeMessageCopyOnWriteProxy messageFromSources = (MimeMessageCopyOnWriteProxy) getMessageFromSources( @@ -122,7 +130,7 @@ public class MimeMessageCopyOnWriteProxyTest extends MimeMessageFromStreamTest { * change the second, then it should not clone */ @Test - public void testMessageAvoidCloning() throws Exception { + void testMessageAvoidCloning() throws Exception { ArrayList<MailAddress> r = new ArrayList<>(); r.add(new MailAddress("recipi...@test.com")); MimeMessageCopyOnWriteProxy messageFromSources = (MimeMessageCopyOnWriteProxy) getMessageFromSources( @@ -158,7 +166,7 @@ public class MimeMessageCopyOnWriteProxyTest extends MimeMessageFromStreamTest { * should clone the message. */ @Test - public void testMessageCloning3() throws Exception { + void testMessageCloning3() throws Exception { ArrayList<MailAddress> r = new ArrayList<>(); r.add(new MailAddress("recipi...@test.com")); MimeMessage mimeMessage = MimeMessageBuilder.mimeMessageBuilder() @@ -181,7 +189,7 @@ public class MimeMessageCopyOnWriteProxyTest extends MimeMessageFromStreamTest { } @Test - public void testMessageDisposing() throws Exception { + void testMessageDisposing() throws Exception { ArrayList<MailAddress> r = new ArrayList<>(); r.add(new MailAddress("recipi...@test.com")); MimeMessageCopyOnWriteProxy messageFromSources = (MimeMessageCopyOnWriteProxy) getMessageFromSources( @@ -203,7 +211,7 @@ public class MimeMessageCopyOnWriteProxyTest extends MimeMessageFromStreamTest { } @Test - public void testNPE1() throws MessagingException, InterruptedException { + void testNPE1() throws MessagingException, InterruptedException { ArrayList<MailAddress> recipients = new ArrayList<>(); recipients.add(new MailAddress("recipi...@test.com")); MimeMessageCopyOnWriteProxy mw = new MimeMessageCopyOnWriteProxy(new MimeMessageInputStreamSource("test", @@ -225,7 +233,7 @@ public class MimeMessageCopyOnWriteProxyTest extends MimeMessageFromStreamTest { * created by a MimeMessageInputStreamSource. */ @Test - public void testMessageCloningViaCoW3() throws Exception { + void testMessageCloningViaCoW3() throws Exception { MimeMessage mmorig = getSimpleMessage(); MimeMessage mm = new MimeMessageCopyOnWriteProxy(mmorig); @@ -245,6 +253,18 @@ public class MimeMessageCopyOnWriteProxyTest extends MimeMessageFromStreamTest { LifecycleUtil.dispose(mm); } + @Test + void testMessageWithWrongContentTypeShouldNotThrow() throws Exception { + ImmutableList<MailAddress> recipients = ImmutableList.of(new MailAddress("recipi...@test.com")); + MimeMessageCopyOnWriteProxy messageFromSources = (MimeMessageCopyOnWriteProxy) getMessageFromSources( + content + sep + body); + MailImpl mail = new MailImpl("test", new MailAddress("t...@test.com"), recipients, messageFromSources); + mail.getMessage().addHeader("Content-Type", "file;name=\"malformed.pdf\""); + mail.getMessage().saveChanges(); + LifecycleUtil.dispose(mail); + LifecycleUtil.dispose(messageFromSources); + } + private static String getReferences(MimeMessage m) { StringBuilder ref = new StringBuilder("/"); while (m instanceof MimeMessageCopyOnWriteProxy) { @@ -268,6 +288,5 @@ public class MimeMessageCopyOnWriteProxyTest extends MimeMessageFromStreamTest { private static boolean isSameMimeMessage(MimeMessage first, MimeMessage second) { return getWrappedMessage(first) == getWrappedMessage(second); - } } http://git-wip-us.apache.org/repos/asf/james-project/blob/a4d524d8/server/container/guice/guice-common/src/main/java/org/apache/james/modules/CommonServicesModule.java ---------------------------------------------------------------------- diff --git a/server/container/guice/guice-common/src/main/java/org/apache/james/modules/CommonServicesModule.java b/server/container/guice/guice-common/src/main/java/org/apache/james/modules/CommonServicesModule.java index f079fa7..3b5739b 100644 --- a/server/container/guice/guice-common/src/main/java/org/apache/james/modules/CommonServicesModule.java +++ b/server/container/guice/guice-common/src/main/java/org/apache/james/modules/CommonServicesModule.java @@ -61,6 +61,7 @@ public class CommonServicesModule extends AbstractModule { install(new DropWizardMetricsModule()); install(new TaskManagerModule()); install(new CleanupTaskModule()); + install(new MimeMessageModule()); bind(FileSystem.class).toInstance(fileSystem); bind(Configuration.class).toInstance(configuration); http://git-wip-us.apache.org/repos/asf/james-project/blob/a4d524d8/server/container/guice/guice-common/src/main/java/org/apache/james/modules/MimeMessageModule.java ---------------------------------------------------------------------- diff --git a/server/container/guice/guice-common/src/main/java/org/apache/james/modules/MimeMessageModule.java b/server/container/guice/guice-common/src/main/java/org/apache/james/modules/MimeMessageModule.java new file mode 100644 index 0000000..294f230 --- /dev/null +++ b/server/container/guice/guice-common/src/main/java/org/apache/james/modules/MimeMessageModule.java @@ -0,0 +1,33 @@ +/**************************************************************** + * 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.modules; + +import org.apache.james.server.core.ContentTypeCleaner; + +import com.google.inject.AbstractModule; + +public class MimeMessageModule extends AbstractModule { + + @Override + protected void configure() { + ContentTypeCleaner.initialize(); + } + +} http://git-wip-us.apache.org/repos/asf/james-project/blob/a4d524d8/server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpContentTypeTest.java ---------------------------------------------------------------------- diff --git a/server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpContentTypeTest.java b/server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpContentTypeTest.java new file mode 100644 index 0000000..d72dbca --- /dev/null +++ b/server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpContentTypeTest.java @@ -0,0 +1,151 @@ +/**************************************************************** + * 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.smtp; + +import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN; +import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP; +import static org.apache.james.mailets.configuration.Constants.PASSWORD; +import static org.apache.james.mailets.configuration.Constants.awaitAtMostOneMinute; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; + +import org.apache.james.MemoryJamesServerMain; +import org.apache.james.core.builder.MimeMessageBuilder; +import org.apache.james.mailets.TemporaryJamesServer; +import org.apache.james.mailets.configuration.CommonProcessors; +import org.apache.james.mailets.configuration.MailetConfiguration; +import org.apache.james.mailets.configuration.MailetContainer; +import org.apache.james.mailets.configuration.ProcessorConfiguration; +import org.apache.james.mailets.configuration.SmtpConfiguration; +import org.apache.james.modules.protocols.SmtpGuiceProbe; +import org.apache.james.probe.DataProbe; +import org.apache.james.server.core.MailImpl; +import org.apache.james.transport.matchers.SMTPIsAuthNetwork; +import org.apache.james.utils.DataProbeImpl; +import org.apache.james.utils.FakeSmtp; +import org.apache.james.utils.IMAPMessageReader; +import org.apache.james.utils.SMTPMessageSender; +import org.apache.mailet.Mail; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import javax.mail.MessagingException; + +public class SmtpContentTypeTest { + private static final String FROM = "fromuser@" + DEFAULT_DOMAIN; + private static final String TO = "t...@any.com"; + public static final String SUBJECT = "test"; + + @ClassRule + public static FakeSmtp fakeSmtp = new FakeSmtp(); + @Rule + public IMAPMessageReader imapMessageReader = new IMAPMessageReader(); + @Rule + public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN); + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + private TemporaryJamesServer jamesServer; + + @BeforeClass + public static void setup() { + fakeSmtp.awaitStarted(awaitAtMostOneMinute); + } + + private void createJamesServer(SmtpConfiguration.Builder smtpConfiguration) throws Exception { + MailetContainer.Builder mailetContainer = TemporaryJamesServer.SIMPLE_MAILET_CONTAINER_CONFIGURATION + .putProcessor(ProcessorConfiguration.transport() + .addMailetsFrom(CommonProcessors.deliverOnlyTransport()) + .addMailet(MailetConfiguration.remoteDeliveryBuilder() + .matcher(SMTPIsAuthNetwork.class) + .addProperty("gateway", fakeSmtp.getContainer().getContainerIp())) + .addMailet(MailetConfiguration.TO_BOUNCE)); + + jamesServer = TemporaryJamesServer.builder() + .withBase(MemoryJamesServerMain.SMTP_AND_IMAP_MODULE) + .withSmtpConfiguration(smtpConfiguration) + .withMailetContainer(mailetContainer) + .build(temporaryFolder); + + DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class); + dataProbe.addDomain(DEFAULT_DOMAIN); + dataProbe.addUser(FROM, PASSWORD); + } + + @After + public void tearDown() { + fakeSmtp.clean(); + if (jamesServer != null) { + jamesServer.shutdown(); + } + } + + @Test + public void userShouldBeAbleToReceiveMessagesWithGoodContentType() throws Exception { + createJamesServer(SmtpConfiguration.builder() + .requireAuthentication() + .withAutorizedAddresses("172.0.0.0/8")); + + messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort()) + .authenticate(FROM, PASSWORD) + .sendMessage(mailWithContentType("text/plain;")); + + awaitAtMostOneMinute + .untilAsserted(() -> fakeSmtp.assertEmailReceived(response -> response + .body("", hasSize(1)) + .body("[0].from", equalTo(FROM)) + .body("[0].subject", equalTo(SUBJECT)) + .body("[0].headers.content-type", startsWith("text/plain;")))); + } + + @Test + public void userShouldBeAbleToReceiveMessagesWithBadContentType() throws Exception { + createJamesServer(SmtpConfiguration.builder() + .requireAuthentication() + .withAutorizedAddresses("172.0.0.0/8")); + + messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort()) + .authenticate(FROM, PASSWORD) + .sendMessage(mailWithContentType("wrong|Content-Type;")); + + awaitAtMostOneMinute + .untilAsserted(() -> fakeSmtp.assertEmailReceived(response -> response + .body("", hasSize(1)) + .body("[0].from", equalTo(FROM)) + .body("[0].subject", equalTo(SUBJECT)) + .body("[0].headers.content-type", not(startsWith("wrong|Content-Type;"))))); + } + + private Mail mailWithContentType(String contentType) throws MessagingException { + return MailImpl.builder() + .sender(FROM) + .recipient(TO) + .mimeMessage(MimeMessageBuilder.mimeMessageBuilder() + .setSubject(SUBJECT) + .setText("content", contentType)) + .build(); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
