Handle IMAP empty bodies more safely

Some IMAP servers return NIL if you BODY.PEEK[TEXT] a messsage with
no body, instead of the more canonical {0}CRLF.  Instead of messing with the
parser to deal with that, it makes more sense not to try and fetch empty
bodies.  So there are three changes:

* Don't fetch parts when size = 0
* Don't append "null" when there is null body text
* Slight change to attachment handling so size is reported >0
* Unit tests on some of the related lower-level protocol stuff

Bug http://b/issue?id=2160387

Change-Id: Ifb8fb0ed5ce7297908e1ae8d5a02dda5975c4a3c
This commit is contained in:
Andrew Stadler 2009-10-07 11:42:27 -07:00
parent 2c79efd827
commit b6756688b1
6 changed files with 104 additions and 12 deletions

View File

@ -222,7 +222,10 @@ public class LegacyConversions {
* that deals with single strings.
*/
private static StringBuffer appendTextPart(StringBuffer sb, String newText) {
if (sb == null) {
if (newText == null) {
return sb;
}
else if (sb == null) {
sb = new StringBuffer(newText);
} else {
if (sb.length() > 0) {

View File

@ -1866,7 +1866,8 @@ public class MessagingController implements Runnable {
// 3. Generate a shell message in which to retrieve the attachment,
// and a shell BodyPart for the attachment. Then glue them together.
Message storeMessage = remoteFolder.createMessage(message.mServerId);
BodyPart storePart = new MimeBodyPart();
MimeBodyPart storePart = new MimeBodyPart();
storePart.setSize((int)attachment.mSize);
storePart.setHeader(MimeHeader.HEADER_ANDROID_ATTACHMENT_STORE_DATA,
attachment.mLocation);
storePart.setHeader(MimeHeader.HEADER_CONTENT_TYPE,

View File

@ -134,6 +134,10 @@ public class MimeBodyPart extends BodyPart {
return getMimeType().equals(mimeType);
}
public void setSize(int size) {
this.mSize = size;
}
public int getSize() throws MessagingException {
return mSize;
}

View File

@ -755,13 +755,16 @@ public class ImapStore extends Store {
for (Object o : fp) {
if (o instanceof Part) {
Part part = (Part) o;
InputStream bodyStream = fetchList.getLiteral(fetchList.size() - 1);
String contentType = part.getContentType();
String contentTransferEncoding = part.getHeader(
MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING)[0];
part.setBody(MimeUtility.decodeBody(
bodyStream,
contentTransferEncoding));
if (part.getSize() > 0) {
InputStream bodyStream =
fetchList.getLiteral(fetchList.size() - 1);
String contentType = part.getContentType();
String contentTransferEncoding = part.getHeader(
MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING)[0];
part.setBody(MimeUtility.decodeBody(
bodyStream,
contentTransferEncoding));
}
}
}
@ -1298,9 +1301,9 @@ public class ImapStore extends Store {
super();
}
public void setSize(int size) {
this.mSize = size;
}
// public void setSize(int size) {
// this.mSize = size;
// }
}
class ImapException extends MessagingException {

View File

@ -26,6 +26,7 @@ import com.android.email.mail.Part;
import com.android.email.mail.Message.RecipientType;
import com.android.email.mail.MessageTestUtils.MessageBuilder;
import com.android.email.mail.MessageTestUtils.MultipartBuilder;
import com.android.email.mail.internet.MimeBodyPart;
import com.android.email.mail.internet.MimeHeader;
import com.android.email.mail.internet.MimeMessage;
import com.android.email.mail.internet.MimeUtility;
@ -171,6 +172,26 @@ public class LegacyConversionsTests extends ProviderTestCase2<EmailProvider> {
return message;
}
/**
* Basic test of body parts conversion from Store message to Provider message.
* This tests that a null body part simply results in null text, and does not crash
* or return "null".
*
* TODO very incomplete, there are many permutations to be explored
*/
public void testUpdateBodyFieldsNullText() throws MessagingException {
EmailContent.Body localBody = new EmailContent.Body();
EmailContent.Message localMessage = new EmailContent.Message();
ArrayList<Part> viewables = new ArrayList<Part>();
Part emptyTextPart = new MimeBodyPart(null, "text/plain");
viewables.add(emptyTextPart);
// a "null" body part of type text/plain should result in a null mTextContent
boolean result = LegacyConversions.updateBodyFields(localBody, localMessage, viewables);
assertTrue(result);
assertNull(localBody.mTextContent);
}
/**
* Sunny day test of adding attachments from an IMAP message.
*/

View File

@ -16,24 +16,32 @@
package com.android.email.mail.store;
import com.android.email.mail.FetchProfile;
import com.android.email.mail.Flag;
import com.android.email.mail.Folder;
import com.android.email.mail.Message;
import com.android.email.mail.MessagingException;
import com.android.email.mail.Part;
import com.android.email.mail.Transport;
import com.android.email.mail.Folder.FolderType;
import com.android.email.mail.Folder.OpenMode;
import com.android.email.mail.internet.BinaryTempFileBody;
import com.android.email.mail.internet.MimeUtility;
import com.android.email.mail.transport.MockTransport;
import android.test.AndroidTestCase;
import android.test.suitebuilder.annotation.SmallTest;
import java.util.ArrayList;
import java.util.Date;
import java.util.Locale;
/**
* This is a series of unit tests for the ImapStore class. These tests must be locally
* complete - no server(s) required.
*
* To run these tests alone, use:
* $ runtest -c com.android.email.mail.store.ImapStoreUnitTests email
*/
@SmallTest
public class ImapStoreUnitTests extends AndroidTestCase {
@ -224,4 +232,56 @@ public class ImapStoreUnitTests extends AndroidTestCase {
int unreadCount = mFolder.getUnreadMessageCount();
assertEquals("getUnreadMessageCount with literal string", 10, unreadCount);
}
/**
* Test for proper operations on servers that return "NIL" for empty message bodies.
*/
public void testNilMessage() throws MessagingException {
MockTransport mock = openAndInjectMockTransport();
setupOpenFolder(mock);
mFolder.open(OpenMode.READ_WRITE, null);
// Prepare to pull structure and peek body text - this is like the "large message"
// loop in MessagingController.synchronizeMailboxGeneric()
FetchProfile fp = new FetchProfile();fp.clear();
fp.add(FetchProfile.Item.STRUCTURE);
Message message1 = mFolder.createMessage("1");
mock.expect("3 UID FETCH 1 \\(UID BODYSTRUCTURE\\)", new String[] {
"* 1 FETCH (UID 1 BODYSTRUCTURE (TEXT PLAIN NIL NIL NIL 7BIT 0 0 NIL NIL NIL))",
"3 OK SUCCESS"
});
mFolder.fetch(new Message[] { message1 }, fp, null);
// The expected result for an empty body is:
// * 1 FETCH (UID 1 BODY[TEXT] {0})
// But some servers are returning NIL for the empty body:
// * 1 FETCH (UID 1 BODY[TEXT] NIL)
// Because this breaks our little parser, fetch() skips over empty parts.
// The rest of this test is confirming that this is the case.
mock.expect("4 UID FETCH 1 \\(UID BODY.PEEK\\[TEXT\\]\\)", new String[] {
"* 1 FETCH (UID 1 BODY[TEXT] NIL)",
"4 OK SUCCESS"
});
ArrayList<Part> viewables = new ArrayList<Part>();
ArrayList<Part> attachments = new ArrayList<Part>();
MimeUtility.collectParts(message1, viewables, attachments);
assertTrue(viewables.size() == 1);
Part emptyBodyPart = viewables.get(0);
fp.clear();
fp.add(emptyBodyPart);
mFolder.fetch(new Message[] { message1 }, fp, null);
// If this wasn't working properly, there would be an attempted interpretation
// of the empty part's NIL and possibly a crash.
// If this worked properly, the "empty" body can now be retrieved
viewables = new ArrayList<Part>();
attachments = new ArrayList<Part>();
MimeUtility.collectParts(message1, viewables, attachments);
assertTrue(viewables.size() == 1);
emptyBodyPart = viewables.get(0);
String text = MimeUtility.getTextFromPart(emptyBodyPart);
assertNull(text);
}
}