Follow-up to MimeMessage efficiency improvements.

I missed a case where message-id should not be set locally, which is
the case where the Mime parser clears all headers *and* does not find
a message-id.  The parsed MimeMessage should accurately reflect this.

In the old code, the local id was created at construction time and then
immediately discarded by the parser (calling headers.clear()).

In the new code, I was generating a message-id any time I couldn't find
one.  Now, when explicitly cleared or removed, I set a boolean to inhibit
automatic generation of a new one.

I also missed the fact that a missing message-id no longer throws an
exception, it simply returns null, and so I changed the code that was
catching that exception to simply check for null.

(Note:  Clearly, modeling of legacy behavior is becoming annoying here;
It would be better to do away with all of the automatic logic, and simply
generate message-id locally when appropriate:  On locally-generated
messages.  I don't want to touch this for the current release, but I left
a note in the code to this effect.)

Bug: 2504774
Change-Id: Ibfcbd2363c7ae39ee6d44e4c3295f88258cb4945
This commit is contained in:
Andrew Stadler 2010-03-10 16:42:49 -08:00
parent 44552da606
commit dfd53b0e82
3 changed files with 33 additions and 15 deletions

View File

@ -128,15 +128,11 @@ public class LegacyConversions {
}
// public String mClientId;
// Absorb a MessagingException here in the case of messages that were delivered without
// a proper message-id. This is seen in some ISP's but it is non-fatal -- (we'll just use
// the locally-generated message-id.)
try {
localMessage.mMessageId = ((MimeMessage)message).getMessageId();
} catch (MessagingException me) {
if (Email.DEBUG) {
Log.d(Email.LOG_TAG, "Missing message-id for UID=" + localMessage.mServerId);
}
// Only replace the local message-id if a new one was found. This is seen in some ISP's
// which may deliver messages w/o a message-id header.
String messageId = ((MimeMessage)message).getMessageId();
if (messageId != null) {
localMessage.mMessageId = messageId;
}
// public long mBodyKey;

View File

@ -44,8 +44,11 @@ import java.util.Stack;
import java.util.regex.Pattern;
/**
* An implementation of Message that stores all of it's metadata in RFC 822 and
* An implementation of Message that stores all of its metadata in RFC 822 and
* RFC 2045 style headers.
*
* NOTE: Automatic generation of a local message-id is becoming unwieldy and should be removed.
* It would be better to simply do it explicitly on local creation of new outgoing messages.
*/
public class MimeMessage extends Message {
private MimeHeader mHeader;
@ -62,6 +65,7 @@ public class MimeMessage extends Message {
private Date mSentDate;
private Body mBody;
protected int mSize;
private boolean mInhibitLocalMessageId = false;
// Shared random source for generating local message-id values
private static java.util.Random sRandom = new java.util.Random();
@ -118,6 +122,7 @@ public class MimeMessage extends Message {
// Before parsing the input stream, clear all local fields that may be superceded by
// the new incoming message.
getMimeHeaders().clear();
mInhibitLocalMessageId = true;
mFrom = null;
mTo = null;
mCc = null;
@ -323,14 +328,14 @@ public class MimeMessage extends Message {
/**
* Get the mime "Message-ID" header. This value will be preloaded with a locally-generated
* random ID, if the value has not previously been set.
* @return the Message-ID header string
* @throws MessagingException
* random ID, if the value has not previously been set. Local generation can be inhibited/
* overridden by explicitly clearing the headers, removing the message-id header, etc.
* @return the Message-ID header string, or null if explicitly has been set to null
*/
@Override
public String getMessageId() throws MessagingException {
String messageId = getFirstHeader("Message-ID");
if (messageId == null) {
if (messageId == null && !mInhibitLocalMessageId) {
messageId = generateMessageId();
setMessageId(messageId);
}
@ -378,6 +383,9 @@ public class MimeMessage extends Message {
public void removeHeader(String name) throws MessagingException {
getMimeHeaders().removeHeader(name);
if ("Message-ID".equalsIgnoreCase(name)) {
mInhibitLocalMessageId = true;
}
}
/**

View File

@ -503,7 +503,21 @@ public class MimeMessageTest extends TestCase {
assertEquals(1, mm.getRecipients(MimeMessage.RecipientType.CC).length);
assertEquals("a@b.com", mm.getRecipients(MimeMessage.RecipientType.CC)[0].getAddress());
assertEquals(0, mm.getRecipients(MimeMessage.RecipientType.BCC).length);
assertEquals("<testabcd.1234@silly.test>",mm.getMessageId());
assertEquals("<testabcd.1234@silly.test>", mm.getMessageId());
}
/**
* Confirm parser w/o a message-id inhibits a local message-id from being generated
*/
public void testParseNoMessageId() throws MessagingException, IOException {
String entireMessage =
"To: user@domain.com\r\n" +
"\r\n" +
"Testing\r\n";
MimeMessage mm = null;
mm = new MimeMessage(new ByteArrayInputStream(entireMessage.getBytes("us-ascii")));
assertNull(mm.getMessageId());
}
// TODO more test for writeTo()