Skip to content
Permalink

Comparing changes

This is a direct comparison between two commits made in this repository or its related repositories. View the default comparison for this range or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: Zimbra/zm-mailbox
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: fe7c3fbbdc033560322f2331f9e310f0532b3c80
Choose a base ref
..
head repository: Zimbra/zm-mailbox
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 35866450742086bdd07d75e57a9b2f7a1b154523
Choose a head ref
Original file line number Diff line number Diff line change
@@ -16,8 +16,6 @@
*/
package com.zimbra.cs.service;

import static org.junit.Assert.assertTrue;

import java.net.URL;
import java.util.HashMap;
import java.util.List;
@@ -27,6 +25,7 @@

import javax.servlet.http.Cookie;

import org.apache.tika.mime.MediaType;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
@@ -52,6 +51,12 @@
import com.zimbra.cs.servlet.CsrfFilter;
import com.zimbra.soap.SoapServlet;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;

public class FileUploadServletTest {
private static FileUploadServlet servlet;
private static Account testAccount;
@@ -374,4 +379,75 @@ public void testFileUploadAuthTokenCsrfEnabled2() throws Exception {
String respStrg = resp.output.toString();
assertTrue(respStrg.contains("200"));
}

@Test
public void testMediaTypeIsPlainNullOrEmpty() {
assertTrue(FileUploadServlet.Upload.mediaTypeIsPlainNullOrEmpty(null));
assertTrue(FileUploadServlet.Upload.mediaTypeIsPlainNullOrEmpty(""));
assertTrue(FileUploadServlet.Upload.mediaTypeIsPlainNullOrEmpty(" "));
assertTrue(FileUploadServlet.Upload.mediaTypeIsPlainNullOrEmpty("text/plain"));
assertFalse(FileUploadServlet.Upload.mediaTypeIsPlainNullOrEmpty("application/json"));
assertFalse(FileUploadServlet.Upload.mediaTypeIsPlainNullOrEmpty("text/xml"));
}

@Test
public void testIsContentTypeXML() {
assertFalse(FileUploadServlet.Upload.isContentTypeXML(null));
assertFalse(FileUploadServlet.Upload.isContentTypeXML(""));
assertFalse(FileUploadServlet.Upload.isContentTypeXML(" "));
assertTrue(FileUploadServlet.Upload.isContentTypeXML("text/xml"));
assertTrue(FileUploadServlet.Upload.isContentTypeXML("application/xml"));
assertFalse(FileUploadServlet.Upload.isContentTypeXML("application/json"));
assertFalse(FileUploadServlet.Upload.isContentTypeXML("text/plain"));
}

@Test
public void testForSpecialCase_text_xml() {
Object[][] scenarios = {
// {initialMediaType, contentType, expectedMediaType}
// case 1
{"text/plain", "text/xml", "text/xml"},
// case 2
{"text/plain", "application/json", null},
// case 3
{"application/json", "text/xml", null},
// case 4
{"application/json", "application/json", null},
// case 5
{null,"text/xml","text/xml"},
// case 6
{"","text/xml","text/xml"},
// case 7
{"text/plain", null, null},
// case 8
{"application/json", null, null}
};

for (Object[] scenario : scenarios) {
String initialMediaType = (String) scenario[0];
String contentType = (String) scenario[1];
String expectedMediaType = (String) scenario[2];
MediaType mediaType = null;

// Actual logic under test
if (FileUploadServlet.Upload.mediaTypeIsPlainNullOrEmpty(initialMediaType) &&
FileUploadServlet.Upload.isContentTypeXML(contentType)) {
mediaType = MediaType.parse(contentType);
}

// Assertions
if (expectedMediaType == null) {
assertNull("Failed for scenario: " + scenarioToString(scenario), mediaType);
} else {
assertNotNull("Failed for scenario: " + scenarioToString(scenario), mediaType);
assertEquals("Failed for scenario: " + scenarioToString(scenario), expectedMediaType, mediaType.toString());
}
}
}

// Helper method to format scenario data for debugging
private String scenarioToString(Object[] scenario) {
return String.format("initialMediaType='%s', contentType='%s', expectedMediaType='%s'",
scenario[0], scenario[1], scenario[2]);
}
}
15 changes: 15 additions & 0 deletions store/src/java/com/zimbra/cs/service/FileUploadServlet.java
Original file line number Diff line number Diff line change
@@ -214,6 +214,7 @@ public static MimeType getMimeType(FileItem fileItem) {
metadata.add(Metadata.RESOURCE_NAME_KEY, fileItem.getName());
String customMimeTypesPath = LC.custom_mimetypes.value();
MediaType mediaType = null;
String contentType = fileItem.getContentType();
try {
TikaInputStream stream = TikaInputStream.get(fileItem.getInputStream());
if (new File(customMimeTypesPath).isFile()) {
@@ -222,6 +223,10 @@ public static MimeType getMimeType(FileItem fileItem) {
} else {
mediaType = detector.detect(stream, metadata);
}
// special-case text/xml to avoid detection of text/plain
if (mediaTypeIsPlainNullOrEmpty(String.valueOf(mediaType)) && isContentTypeXML(contentType)) {
mediaType = MediaType.parse(contentType);
}
mimeType = tikaConfig.getMimeRepository().forName(mediaType.toString());
mLog.debug("Content type detected by tika: %s.", mimeType.toString());
} catch (MimeTypeException mexp) {
@@ -232,6 +237,16 @@ public static MimeType getMimeType(FileItem fileItem) {
return mimeType;
}

public static boolean mediaTypeIsPlainNullOrEmpty(String mediaType) {
return StringUtils.isBlank(mediaType) || MimeConstants.CT_TEXT_PLAIN.equals(mediaType);
}

public static boolean isContentTypeXML(String contentType) {
return !StringUtil.isNullOrEmpty(contentType) &&
(MimeConstants.CT_TEXT_XML.equals(contentType) ||
MimeConstants.CT_TEXT_XML_LEGACY.equals(contentType));
}

public static String getExtension(MimeType mimeType) {
String fileExtension = null;
fileExtension = mimeType == null ? "" : StringUtils.strip(mimeType.getExtension(), ".");
9 changes: 8 additions & 1 deletion store/src/java/com/zimbra/cs/service/mail/SaveDraft.java
Original file line number Diff line number Diff line change
@@ -24,6 +24,7 @@

import javax.mail.MessagingException;
import javax.mail.SendFailedException;
import javax.mail.Session;
import javax.mail.internet.MimeMessage;

import com.zimbra.common.account.Key.AccountBy;
@@ -69,6 +70,7 @@ public class SaveDraft extends MailDocumentHandler {
private static final String[] RESPONSE_ITEM_PATH = new String[] { };
public static final String IS_DELEGATED_REQUEST = "isDelegatedReq";
public static final String DELEGATEE_ACCOUNT_ID = "delegateeAccountId";
private static final String MAIL_SMTP_FROM = "mail.smtp.from";

@Override
protected String[] getProxiedIdPath(Element request) {
@@ -170,10 +172,15 @@ public Element handle(Element request, Map<String, Object> context) throws Servi

if (autoSendTime != 0) {
AccountUtil.checkQuotaWhenSendMail(mbox);
Session mSession = null;
try {
mSession = JMSession.getSmtpSession(mbox.getAccount());
if (null != mailAddress) {
mSession.getProperties().setProperty(MAIL_SMTP_FROM, mailAddress);
}
// always throws MessagingException. The api call here is checking if MessagingException is an instance of SendFailedException
// then get the list of invalid e-mail addresses.
MailUtil.validateRcptAddresses(JMSession.getSmtpSession(mbox.getAccount()), mm.getAllRecipients());
MailUtil.validateRcptAddresses(mSession, mm.getAllRecipients());
} catch (MessagingException mex) {
if (mex instanceof SendFailedException) {
SendFailedException sfex = (SendFailedException) mex;