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

DO NOT MERGE: Fix handling IOException in ImapStore 

- mConnection.destroyResponses() should be protected with
if (mConnection != null).
When we get an IOException, we close the connection and null it out in
ioExceptionHandler().  So mConnection can be null at any point after
where ioExceptionHandler() first appears.

- ioExceptionHandler should close its parent ImapFolder only if the argument
connection is mConnection.
Methods like exists() may pass an ImapConnection which is not mConnection
to ioExceptionHandler.  In which case we don't have to close the ImapFolder.

Bug 2898211

Backport of I8f9f45d91f596bb8da1a1575593e652d66deb643

Change-Id: I070458b5535540aba69ad7eee88bd2af8ad5f7b1
This commit is contained in:
Makoto Onuki 2010-08-05 16:43:39 -07:00
parent 29f0638f4d
commit f255081a85
3 changed files with 181 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
src/com/android/email/mail/store/ImapStore.java
View File

@ -510,10 +510,15 @@ public class ImapStore extends Store {
mName = name;
}
private void destroyResponses() {
if (mConnection != null) {
mConnection.destroyResponses();
}
}
@Override
public void open(OpenMode mode, PersistentDataCallbacks callbacks)
throws MessagingException {
try {
if (isOpen()) {
if (mMode == mode) {
@ -526,7 +531,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
} else {
// Return the connection to the pool, if exists.
@ -582,7 +587,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
} catch (MessagingException e) {
mExists = false;
@ -606,6 +611,7 @@ public class ImapStore extends Store {
// TODO implement expunge
mMessageCount = -1;
synchronized (this) {
destroyResponses();
mStore.poolConnection(mConnection);
mConnection = null;
}
@ -707,7 +713,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
}
@ -735,7 +741,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
}
@ -773,7 +779,7 @@ public class ImapStore extends Store {
}
return uids.toArray(Utility.EMPTY_STRINGS);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
}
@ -835,16 +841,16 @@ public class ImapStore extends Store {
fetchInternal(messages, fp, listener);
} catch (RuntimeException e) { // Probably a parser error.
Log.w(Email.LOG_TAG, "Exception detected: " + e.getMessage());
mConnection.logLastDiscourse();
if (mConnection != null) {
mConnection.logLastDiscourse();
}
throw e;
} finally {
mConnection.destroyResponses();
}
}
public void fetchInternal(Message[] messages, FetchProfile fp,
MessageRetrievalListener listener) throws MessagingException {
if (messages == null || messages.length == 0) {
if (messages.length == 0) {
return;
}
checkOpen();
@ -897,7 +903,7 @@ public class ImapStore extends Store {
}
try {
String tag = mConnection.sendCommand(String.format(
mConnection.sendCommand(String.format(
ImapConstants.UID_FETCH + " %s (%s)", joinMessageUids(messages),
Utility.combine(fetchFields.toArray(new String[fetchFields.size()]), ' ')
), false);
@ -991,7 +997,7 @@ public class ImapStore extends Store {
listener.messageRetrieved(message);
}
} finally {
mConnection.destroyResponses();
destroyResponses();
}
} while (!response.isTagged());
} catch (IOException ioe) {
@ -1285,7 +1291,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
}
@ -1297,7 +1303,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
return null;
}
@ -1332,7 +1338,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
}
@ -1344,8 +1350,15 @@ public class ImapStore extends Store {
private MessagingException ioExceptionHandler(ImapConnection connection, IOException ioe)
throws MessagingException {
if (Email.DEBUG) {
Log.d(Email.LOG_TAG, "IO Exception detected: ", ioe);
}
connection.destroyResponses();
connection.close();
close(false);
if (connection == mConnection) {
mConnection = null; // To prevent close() from returning the connection to the pool.
close(false);
}
return new MessagingException("IO Error", ioe);
}

112
tests/src/com/android/email/mail/store/ImapStoreUnitTests.java
View File

@ -50,6 +50,7 @@ import android.test.AndroidTestCase;
import android.test.MoreAsserts;
import android.test.suitebuilder.annotation.SmallTest;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.regex.Pattern;
@ -1612,4 +1613,115 @@ public class ImapStoreUnitTests extends AndroidTestCase {
folders[1].open(OpenMode.READ_WRITE, null);
folders[1].close(false);
}
/**
* Callback for {@link #runAndExpectMessagingException}.
*/
private interface RunAndExpectMessagingExceptionTarget {
public void run(MockTransport mockTransport) throws Exception;
}
/**
* Set up the usual mock transport, open the folder,
* run {@link RunAndExpectMessagingExceptionTarget} and make sure a {@link MessagingException}
* is thrown.
*/
private void runAndExpectMessagingException(RunAndExpectMessagingExceptionTarget target)
throws Exception {
try {
final MockTransport mockTransport = openAndInjectMockTransport();
setupOpenFolder(mockTransport);
mFolder.open(OpenMode.READ_WRITE, null);
target.run(mockTransport);
fail("MessagingException expected.");
} catch (MessagingException expected) {
}
}
/**
* Make sure that IOExceptions are always converted to MessagingException.
*/
public void testFetchIOException() throws Exception {
runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() {
public void run(MockTransport mockTransport) throws Exception {
mockTransport.expectIOException();
final Message message = mFolder.createMessage("1");
final FetchProfile fp = new FetchProfile();
fp.add(FetchProfile.Item.STRUCTURE);
mFolder.fetch(new Message[] { message }, fp, null);
}
});
}
/**
* Make sure that IOExceptions are always converted to MessagingException.
*/
public void testUnreadMessageCountIOException() throws Exception {
runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() {
public void run(MockTransport mockTransport) throws Exception {
mockTransport.expectIOException();
mFolder.getUnreadMessageCount();
}
});
}
/**
* Make sure that IOExceptions are always converted to MessagingException.
*/
public void testCopyMessagesIOException() throws Exception {
runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() {
public void run(MockTransport mockTransport) throws Exception {
mockTransport.expectIOException();
final Message message = mFolder.createMessage("1");
final Folder folder = mStore.getFolder("test");
mFolder.copyMessages(new Message[] { message }, folder, null);
}
});
}
/**
* Make sure that IOExceptions are always converted to MessagingException.
*/
public void testSearchForUidsIOException() throws Exception {
runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() {
public void run(MockTransport mockTransport) throws Exception {
mockTransport.expectIOException();
mFolder.getMessage("uid");
}
});
}
/**
* Make sure that IOExceptions are always converted to MessagingException.
*/
public void testExpungeIOException() throws Exception {
runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() {
public void run(MockTransport mockTransport) throws Exception {
mockTransport.expectIOException();
mFolder.expunge();
}
});
}
/**
* Make sure that IOExceptions are always converted to MessagingException.
*/
public void testOpenIOException() throws Exception {
runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() {
public void run(MockTransport mockTransport) throws Exception {
mockTransport.expectIOException();
final Folder folder = mStore.getFolder("test");
folder.open(OpenMode.READ_WRITE, null);
}
});
}
}

55
tests/src/com/android/email/mail/transport/MockTransport.java
View File

@ -42,6 +42,8 @@ public class MockTransport implements Transport {
private static String LOG_TAG = "MockTransport";
private static final String SPECIAL_RESPONSE_IOEXCEPTION = "!!!IOEXCEPTION!!!";
private boolean mSslAllowed = false;
private boolean mTlsAllowed = false;
@ -56,8 +58,8 @@ public class MockTransport implements Transport {
private static class Transaction {
public static final int ACTION_INJECT_TEXT = 0;
public static final int ACTION_SERVER_CLOSE = 1;
public static final int ACTION_CLIENT_CLOSE = 2;
public static final int ACTION_CLIENT_CLOSE = 1;
public static final int ACTION_IO_EXCEPTION = 2;
int mAction;
String mPattern;
@ -80,10 +82,10 @@ public class MockTransport implements Transport {
switch (mAction) {
case ACTION_INJECT_TEXT:
return mPattern + ": " + Arrays.toString(mResponses);
case ACTION_SERVER_CLOSE:
return "Close the server connection";
case ACTION_CLIENT_CLOSE:
return "Expect the client to close";
case ACTION_IO_EXCEPTION:
return "Expect IOException";
default:
return "(Hmm. Unknown action.)";
}
@ -136,9 +138,22 @@ public class MockTransport implements Transport {
mPairs.add(new Transaction(Transaction.ACTION_CLIENT_CLOSE));
}
private void sendResponse(String[] responses) {
for (String s : responses) {
mQueuedInput.add(s);
public void expectIOException() {
mPairs.add(new Transaction(Transaction.ACTION_IO_EXCEPTION));
}
private void sendResponse(Transaction pair) {
switch (pair.mAction) {
case Transaction.ACTION_INJECT_TEXT:
for (String s : pair.mResponses) {
mQueuedInput.add(s);
}
break;
case Transaction.ACTION_IO_EXCEPTION:
mQueuedInput.add(SPECIAL_RESPONSE_IOEXCEPTION);
break;
default:
Assert.fail("Invalid action for sendResponse: " + pair.mAction);
}
}
@ -250,18 +265,25 @@ public class MockTransport implements Transport {
throw new IOException("Reading from MockTransport with closed input");
}
// if there's nothing to read, see if we can find a null-pattern response
if (0 == mQueuedInput.size()) {
Transaction pair = mPairs.size() > 0 ? mPairs.get(0) : null;
if (pair != null && pair.mPattern == null) {
if ((mQueuedInput.size() == 0) && (mPairs.size() > 0)) {
Transaction pair = mPairs.get(0);
if (pair.mPattern == null) {
mPairs.remove(0);
sendResponse(pair.mResponses);
sendResponse(pair);
}
}
SmtpSenderUnitTests.assertTrue("Underflow reading from MockTransport", 0 != mQueuedInput.size());
if (mQueuedInput.size() == 0) {
// MailTransport returns "" at EOS.
Log.w(LOG_TAG, "Underflow reading from MockTransport");
return "";
}
String line = mQueuedInput.remove(0);
if (DEBUG_LOG_STREAMS) {
Log.d(LOG_TAG, "<<< " + line);
}
if (SPECIAL_RESPONSE_IOEXCEPTION.equals(line)) {
throw new IOException("Expected IOException.");
}
return line;
}
@ -292,7 +314,7 @@ public class MockTransport implements Transport {
*
* Logs the written text if DEBUG_LOG_STREAMS is true.
*/
public void writeLine(String s, String sensitiveReplacement) /* throws IOException */ {
public void writeLine(String s, String sensitiveReplacement) throws IOException {
if (DEBUG_LOG_STREAMS) {
Log.d(LOG_TAG, ">>> " + s);
}
@ -300,11 +322,14 @@ public class MockTransport implements Transport {
SmtpSenderUnitTests.assertTrue("Overflow writing to MockTransport: Getting " + s,
0 != mPairs.size());
Transaction pair = mPairs.remove(0);
if (pair.mAction == Transaction.ACTION_IO_EXCEPTION) {
throw new IOException("Expected IOException.");
}
SmtpSenderUnitTests.assertTrue("Unexpected string written to MockTransport: Actual=" + s
+ " Expected=" + pair.mPattern,
pair.mPattern != null && s.matches(pair.mPattern));
if (pair.mResponses != null) {
sendResponse(pair.mResponses);
sendResponse(pair);
}
}
@ -354,7 +379,7 @@ public class MockTransport implements Transport {
StringBuilder sb = new StringBuilder();
@Override
public void write(int oneByte) {
public void write(int oneByte) throws IOException {
// CR or CRLF will immediately dump previous line (w/o CRLF)
if (oneByte == '\r') {
writeLine(sb.toString(), null);