ADHOC
/
replicant-packages_apps_Email
3
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix Store re-using old data 

The key for the Store cache was not adjusting properly for account
changes (such as port changes, etc...). As such, it was possible to
get an invalid store.

Now, there's problem with leaking Account objects if the store account
changes (see bug 4440839). This is "okay" for now since account changes
are fairly uncommon and Account objects are light. However, this should
be fixed at some point.

Change-Id: I4ddcbc3e2759b7b1374d0300706373678dedec94
This commit is contained in:
Todd Kennedy 2011-05-16 17:47:14 -07:00
parent 7e1fa63d75
commit 581e3c2333
3 changed files with 38 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/com/android/email/Controller.java
View File

@ -1030,10 +1030,11 @@ public class Controller {
}
try {
// Delete Remote store at first.
Store.getInstance(account, context, null).delete();
// Remove the Store instance from cache.
Store.removeInstance(account, context);
// Remove the store instance from cache
Store oldStore = Store.removeInstance(account, context);
if (oldStore != null) {
oldStore.delete(); // If the store was removed, delete it
}
} catch (MessagingException e) {
Log.w(Logging.LOG_TAG, "Failed to delete store", e);
}

38
src/com/android/email/mail/Store.java
View File

@ -31,7 +31,6 @@ import org.xmlpull.v1.XmlPullParserException;
import android.content.Context;
import android.content.res.XmlResourceParser;
import android.os.Bundle;
import android.text.TextUtils;
import android.util.Log;
import java.io.IOException;
@ -64,6 +63,7 @@ public abstract class Store {
* should be returned on FetchProfile.Item.BODY_SANE requests.
*/
public static final int FETCH_BODY_SANE_SUGGESTED_SIZE = (50 * 1024);
@VisibleForTesting
static final HashMap<String, Store> sStores = new HashMap<String, Store>();
@ -162,36 +162,25 @@ public abstract class Store {
}
}
/**
* Gets a unique key for the given account.
* @throws MessagingException If the account is not setup properly (i.e. there is no address
* or login)
*/
protected static String getStoreKey(Context context, Account account)
throws MessagingException {
final StringBuffer key = new StringBuffer();
final HostAuth recvAuth = account.getOrCreateHostAuthRecv(context);
if (recvAuth.mAddress == null) {
throw new MessagingException("Cannot find store for account " + account.mDisplayName);
HostAuth recvAuth = account.getOrCreateHostAuthRecv(context);
String key = recvAuth.getStoreUri();
if (key == null) {
throw new MessagingException("Could not find store for account: " +
account.mDisplayName);
}
final String address = recvAuth.mAddress.trim();
if (TextUtils.isEmpty(address)) {
throw new MessagingException("Cannot find store for account " + account.mDisplayName);
}
key.append(address);
if (recvAuth.mLogin != null) {
key.append(recvAuth.mLogin.trim());
}
return key.toString();
return key;
}
/**
* Get an instance of a mail store for the given account. The account must be valid (i.e. has
* at least an incoming server name).
*
* Username, password, and host are as expected.
* Resource is protocol specific. For example, IMAP uses it as the path prefix. EAS uses it
* as the domain.
* NOTE: The internal algorithm used to find a cached store depends upon the URI of the
* account's HostAuth object. If this ever changes (e.g. such as the user updating the
* host name or port), we will leak entries. This should not be typical, so, it is not
* a critical problem. However, it is something we should consider fixing.
*
* @param account The account of the store.
* @return an initialized store of the appropriate class
@ -233,10 +222,9 @@ public abstract class Store {
*
* @throws MessagingException If the store cannot be removed or if the account is invalid.
*/
public synchronized static void removeInstance(Account account, Context context)
public synchronized static Store removeInstance(Account account, Context context)
throws MessagingException {
final String storeKey = getStoreKey(context, account);
sStores.remove(storeKey);
return sStores.remove(account.mId);
}
/**

24
tests/src/com/android/email/mail/StoreTests.java
View File

@ -44,6 +44,8 @@ public class StoreTests extends AndroidTestCase {
// Make sure to set the host auth; otherwise we create entries in the hostauth db
testAccount.mHostAuthRecv = testAuth;
testAuth.mProtocol = "protocol";
testAuth.mPort = 80;
// No address defined; throws an exception
try {
@ -63,13 +65,15 @@ public class StoreTests extends AndroidTestCase {
// Address defined, no login
testAuth.mAddress = "a.valid.address.com";
testKey = Store.getStoreKey(mContext, testAccount);
assertEquals("a.valid.address.com", testKey);
assertEquals("protocol://a.valid.address.com:80", testKey);
// Address & login defined
testAuth.mAddress = "address.org";
testAuth.mFlags = HostAuth.FLAG_AUTHENTICATE;
testAuth.mLogin = "auser";
testAuth.mPassword = "password";
testKey = Store.getStoreKey(mContext, testAccount);
assertEquals("address.orgauser", testKey);
assertEquals("protocol://auser:password@address.org:80", testKey);
}
public void testGetStoreInfo() {
@ -111,7 +115,7 @@ public class StoreTests extends AndroidTestCase {
testAuth.mProtocol = "pop3";
testStore = Store.getInstance(testAccount, getContext(), null);
assertEquals(1, Store.sStores.size());
assertSame(testStore, Store.sStores.get("pop3.google.com"));
assertSame(testStore, Store.sStores.get(testAuth.getStoreUri()));
Store.sStores.clear();
// IMAP
@ -119,7 +123,19 @@ public class StoreTests extends AndroidTestCase {
testAuth.mProtocol = "imap";
testStore = Store.getInstance(testAccount, getContext(), null);
assertEquals(1, Store.sStores.size());
assertSame(testStore, Store.sStores.get("imap.google.com"));
assertSame(testStore, Store.sStores.get(testAuth.getStoreUri()));
Store.sStores.clear();
// IMAP; host auth changes
testAuth.mAddress = "imap.google.com";
testAuth.mProtocol = "imap";
testStore = Store.getInstance(testAccount, getContext(), null); // cache first store
assertEquals(1, Store.sStores.size());
assertSame(testStore, Store.sStores.get(testAuth.getStoreUri()));
testAuth.mAddress = "mail.google.com"; // change the auth hostname
Store.getInstance(testAccount, getContext(), null); // cache second store
assertEquals(2, Store.sStores.size()); // Now there are two entries ...
assertNotSame(testStore, Store.sStores.get(testAuth.getStoreUri()));
Store.sStores.clear();
// Unknown