Always destroy ImapResponses.
Unfortunately it's hard to write tests for this change, but at least all tests pass with Idc7b88c4. Change-Id: If0335a848dfcc23aecea22c21b2cce73dac7ff6f
This commit is contained in:
parent
4a82cd7720
commit
977a7d206a
@ -414,18 +414,21 @@ public class ImapStore extends Store {
|
||||
connection.close();
|
||||
throw new MessagingException("Unable to get folder list.", ioe);
|
||||
} finally {
|
||||
connection.destroyResponses();
|
||||
poolConnection(connection);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void checkSettings() throws MessagingException {
|
||||
ImapConnection connection = new ImapConnection();
|
||||
try {
|
||||
ImapConnection connection = new ImapConnection();
|
||||
connection.open();
|
||||
connection.close();
|
||||
} catch (IOException ioe) {
|
||||
throw new MessagingException(MessagingException.IOERROR, ioe.toString());
|
||||
} finally {
|
||||
connection.destroyResponses();
|
||||
}
|
||||
}
|
||||
|
||||
@ -442,6 +445,8 @@ public class ImapStore extends Store {
|
||||
// Fall through
|
||||
} catch (IOException e) {
|
||||
// Fall through
|
||||
} finally {
|
||||
connection.destroyResponses();
|
||||
}
|
||||
connection.close();
|
||||
connection = null;
|
||||
@ -522,6 +527,8 @@ public class ImapStore extends Store {
|
||||
|
||||
} catch (IOException ioe) {
|
||||
ioExceptionHandler(mConnection, ioe);
|
||||
} finally {
|
||||
mConnection.destroyResponses();
|
||||
}
|
||||
} else {
|
||||
// Return the connection to the pool, if exists.
|
||||
@ -576,6 +583,8 @@ public class ImapStore extends Store {
|
||||
|
||||
} catch (IOException ioe) {
|
||||
throw ioExceptionHandler(mConnection, ioe);
|
||||
} finally {
|
||||
mConnection.destroyResponses();
|
||||
}
|
||||
} catch (MessagingException e) {
|
||||
mExists = false;
|
||||
@ -641,6 +650,7 @@ public class ImapStore extends Store {
|
||||
throw ioExceptionHandler(connection, ioe);
|
||||
|
||||
} finally {
|
||||
connection.destroyResponses();
|
||||
if (mConnection == null) {
|
||||
mStore.poolConnection(connection);
|
||||
}
|
||||
@ -680,6 +690,7 @@ public class ImapStore extends Store {
|
||||
throw ioExceptionHandler(connection, ioe);
|
||||
|
||||
} finally {
|
||||
connection.destroyResponses();
|
||||
if (mConnection == null) {
|
||||
mStore.poolConnection(connection);
|
||||
}
|
||||
@ -697,6 +708,8 @@ public class ImapStore extends Store {
|
||||
encodeFolderName(folder.getName())));
|
||||
} catch (IOException ioe) {
|
||||
throw ioExceptionHandler(mConnection, ioe);
|
||||
} finally {
|
||||
mConnection.destroyResponses();
|
||||
}
|
||||
}
|
||||
|
||||
@ -723,6 +736,8 @@ public class ImapStore extends Store {
|
||||
return unreadMessageCount;
|
||||
} catch (IOException ioe) {
|
||||
throw ioExceptionHandler(mConnection, ioe);
|
||||
} finally {
|
||||
mConnection.destroyResponses();
|
||||
}
|
||||
}
|
||||
|
||||
@ -736,32 +751,36 @@ public class ImapStore extends Store {
|
||||
checkOpen();
|
||||
List<ImapResponse> responses;
|
||||
try {
|
||||
responses = mConnection.executeSimpleCommand(
|
||||
ImapConstants.UID_SEARCH + " " + searchCriteria);
|
||||
} catch (ImapException e) {
|
||||
return Utility.EMPTY_STRINGS; // not found;
|
||||
} catch (IOException ioe) {
|
||||
throw ioExceptionHandler(mConnection, ioe);
|
||||
}
|
||||
// S: * SEARCH 2 3 6
|
||||
for (ImapResponse response : responses) {
|
||||
if (!response.isDataResponse(0, ImapConstants.SEARCH)) {
|
||||
continue;
|
||||
try {
|
||||
responses = mConnection.executeSimpleCommand(
|
||||
ImapConstants.UID_SEARCH + " " + searchCriteria);
|
||||
} catch (ImapException e) {
|
||||
return Utility.EMPTY_STRINGS; // not found;
|
||||
} catch (IOException ioe) {
|
||||
throw ioExceptionHandler(mConnection, ioe);
|
||||
}
|
||||
// Found SEARCH response data
|
||||
final int count = response.size() - 1;
|
||||
if (count <= 0) {
|
||||
return Utility.EMPTY_STRINGS; // ... but no UIDs in it! Return empty array.
|
||||
}
|
||||
|
||||
ArrayList<String> ret = new ArrayList<String>(count);
|
||||
for (int i = 1; i < response.size(); i++) {
|
||||
ImapString s = response.getStringOrEmpty(i);
|
||||
if (s.isString()) {
|
||||
ret.add(s.getString());
|
||||
// S: * SEARCH 2 3 6
|
||||
for (ImapResponse response : responses) {
|
||||
if (!response.isDataResponse(0, ImapConstants.SEARCH)) {
|
||||
continue;
|
||||
}
|
||||
// Found SEARCH response data
|
||||
final int count = response.size() - 1;
|
||||
if (count <= 0) {
|
||||
return Utility.EMPTY_STRINGS; // ... but no UIDs in it! Return empty array.
|
||||
}
|
||||
|
||||
ArrayList<String> ret = new ArrayList<String>(count);
|
||||
for (int i = 1; i < response.size(); i++) {
|
||||
ImapString s = response.getStringOrEmpty(i);
|
||||
if (s.isString()) {
|
||||
ret.add(s.getString());
|
||||
}
|
||||
}
|
||||
return ret.toArray(Utility.EMPTY_STRINGS);
|
||||
}
|
||||
return ret.toArray(Utility.EMPTY_STRINGS);
|
||||
} finally {
|
||||
mConnection.destroyResponses();
|
||||
}
|
||||
return Utility.EMPTY_STRINGS;
|
||||
}
|
||||
@ -826,6 +845,8 @@ public class ImapStore extends Store {
|
||||
Log.w(Email.LOG_TAG, "Exception detected: " + e.getMessage());
|
||||
mConnection.logLastDiscourse();
|
||||
throw e;
|
||||
} finally {
|
||||
mConnection.destroyResponses();
|
||||
}
|
||||
}
|
||||
|
||||
@ -978,9 +999,7 @@ public class ImapStore extends Store {
|
||||
listener.messageRetrieved(message);
|
||||
}
|
||||
} finally {
|
||||
if (response != null) {
|
||||
response.destroy();
|
||||
}
|
||||
mConnection.destroyResponses();
|
||||
}
|
||||
} while (!response.isTagged());
|
||||
} catch (IOException ioe) {
|
||||
@ -1273,6 +1292,8 @@ public class ImapStore extends Store {
|
||||
}
|
||||
} catch (IOException ioe) {
|
||||
throw ioExceptionHandler(mConnection, ioe);
|
||||
} finally {
|
||||
mConnection.destroyResponses();
|
||||
}
|
||||
}
|
||||
|
||||
@ -1283,6 +1304,8 @@ public class ImapStore extends Store {
|
||||
handleUntaggedResponses(mConnection.executeSimpleCommand(ImapConstants.EXPUNGE));
|
||||
} catch (IOException ioe) {
|
||||
throw ioExceptionHandler(mConnection, ioe);
|
||||
} finally {
|
||||
mConnection.destroyResponses();
|
||||
}
|
||||
return null;
|
||||
}
|
||||
@ -1316,6 +1339,8 @@ public class ImapStore extends Store {
|
||||
|
||||
} catch (IOException ioe) {
|
||||
throw ioExceptionHandler(mConnection, ioe);
|
||||
} finally {
|
||||
mConnection.destroyResponses();
|
||||
}
|
||||
}
|
||||
|
||||
@ -1371,7 +1396,7 @@ public class ImapStore extends Store {
|
||||
mTransport.open();
|
||||
mTransport.setSoTimeout(MailTransport.SOCKET_READ_TIMEOUT);
|
||||
|
||||
mParser = new ImapResponseParser(mTransport.getInputStream(), mDiscourse);
|
||||
createParser();
|
||||
|
||||
// BANNER
|
||||
mParser.readResponse();
|
||||
@ -1395,8 +1420,7 @@ public class ImapStore extends Store {
|
||||
|
||||
mTransport.reopenTls();
|
||||
mTransport.setSoTimeout(MailTransport.SOCKET_READ_TIMEOUT);
|
||||
mParser = new ImapResponseParser(mTransport.getInputStream(),
|
||||
mDiscourse);
|
||||
createParser();
|
||||
} else {
|
||||
if (Config.LOGD && Email.DEBUG) {
|
||||
Log.d(Email.LOG_TAG, "TLS not supported but required");
|
||||
@ -1457,6 +1481,8 @@ public class ImapStore extends Store {
|
||||
Log.d(Email.LOG_TAG, ioe.toString());
|
||||
}
|
||||
throw ioe;
|
||||
} finally {
|
||||
destroyResponses();
|
||||
}
|
||||
}
|
||||
|
||||
@ -1467,6 +1493,24 @@ public class ImapStore extends Store {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create an {@link ImapResponseParser} from {@code mTransport.getInputStream()} and
|
||||
* set it to {@link #mParser}.
|
||||
*
|
||||
* If we already have an {@link ImapResponseParser}, we
|
||||
* {@link #destroyResponses()} and throw it away.
|
||||
*/
|
||||
private void createParser() {
|
||||
destroyResponses();
|
||||
mParser = new ImapResponseParser(mTransport.getInputStream(), mDiscourse);
|
||||
}
|
||||
|
||||
public void destroyResponses() {
|
||||
if (mParser != null) {
|
||||
mParser.destroyResponses();
|
||||
}
|
||||
}
|
||||
|
||||
/* package */ boolean isTransportOpenForTest() {
|
||||
return mTransport != null ? mTransport.isOpen() : false;
|
||||
}
|
||||
@ -1508,8 +1552,10 @@ public class ImapStore extends Store {
|
||||
responses.add(response);
|
||||
} while (!response.isTagged());
|
||||
if (!response.isOk()) {
|
||||
throw new ImapException(response.toString(),
|
||||
response.getAlertTextOrEmpty().getString());
|
||||
final String toString = response.toString();
|
||||
final String alert = response.getAlertTextOrEmpty().getString();
|
||||
destroyResponses();
|
||||
throw new ImapException(toString, alert);
|
||||
}
|
||||
return responses;
|
||||
}
|
||||
|
@ -29,6 +29,7 @@ import android.util.Log;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.util.ArrayList;
|
||||
|
||||
/**
|
||||
* IMAP response parser.
|
||||
@ -59,6 +60,12 @@ public class ImapResponseParser {
|
||||
/** StringBuilder used by parseBareString() */
|
||||
private final StringBuilder mParseBareString = new StringBuilder();
|
||||
|
||||
/**
|
||||
* We store all {@link ImapResponse} in it. {@link #destroyResponses()} must be called from
|
||||
* time to time to destroy them and clear it.
|
||||
*/
|
||||
private final ArrayList<ImapResponse> mResponsesToDestroy = new ArrayList<ImapResponse>();
|
||||
|
||||
/**
|
||||
* Exception thrown when we receive BYE. It derives from IOException, so it'll be treated
|
||||
* in the same way EOF does.
|
||||
@ -127,15 +134,31 @@ public class ImapResponseParser {
|
||||
return next;
|
||||
}
|
||||
|
||||
/**
|
||||
* Destroy all the {@link ImapResponse}s stored in the internal storage and clear it.
|
||||
*
|
||||
* @see #readResponse()
|
||||
*/
|
||||
public void destroyResponses() {
|
||||
for (ImapResponse r : mResponsesToDestroy) {
|
||||
r.destroy();
|
||||
}
|
||||
mResponsesToDestroy.clear();
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads the next response available on the stream and returns an
|
||||
* {@link ImapResponse} object that represents it.
|
||||
*
|
||||
* <p>When this method successfully returns an {@link ImapResponse}, the {@link ImapResponse}
|
||||
* is stored in the internal storage. When the {@link ImapResponse} is no longer used
|
||||
* {@link #destroyResponses} should be called to destroy all the responses in the array.
|
||||
*
|
||||
* @return the parsed {@link ImapResponse} object.
|
||||
* @exception ByeException when detects BYE.
|
||||
*/
|
||||
public ImapResponse readResponse() throws IOException, MessagingException {
|
||||
final ImapResponse response;
|
||||
ImapResponse response = null;
|
||||
try {
|
||||
response = parseResponse();
|
||||
if (Config.LOGD && Email.DEBUG) {
|
||||
@ -155,8 +178,10 @@ public class ImapResponseParser {
|
||||
// Handle this outside of try-catch. We don't have to dump protocol log when getting BYE.
|
||||
if (response.is(0, ImapConstants.BYE)) {
|
||||
Log.w(Email.LOG_TAG, ByeException.MESSAGE);
|
||||
response.destroy();
|
||||
throw new ByeException();
|
||||
}
|
||||
mResponsesToDestroy.add(response);
|
||||
return response;
|
||||
}
|
||||
|
||||
@ -221,61 +246,85 @@ public class ImapResponseParser {
|
||||
* Parse and return the response line.
|
||||
*/
|
||||
private ImapResponse parseResponse() throws IOException, MessagingException {
|
||||
final int ch = peek();
|
||||
if (ch == '+') { // Continuation request
|
||||
readByte(); // skip +
|
||||
expect(' ');
|
||||
ImapResponse response = new ImapResponse(null, true);
|
||||
// We need to destroy the response if we get an exception.
|
||||
// So, we first store the response that's being built in responseToDestroy, until it's
|
||||
// completely built, at which point we copy it into responseToReturn and null out
|
||||
// responseToDestroyt.
|
||||
// If responseToDestroy is not null in finally, we destroy it because that means
|
||||
// we got an exception somewhere.
|
||||
ImapResponse responseToDestroy = null;
|
||||
final ImapResponse responseToReturn;
|
||||
|
||||
// If it's continuation request, we don't really care what's in it.
|
||||
response.add(new ImapSimpleString(readUntilEol()));
|
||||
return response;
|
||||
}
|
||||
try {
|
||||
final int ch = peek();
|
||||
if (ch == '+') { // Continuation request
|
||||
readByte(); // skip +
|
||||
expect(' ');
|
||||
responseToDestroy = new ImapResponse(null, true);
|
||||
|
||||
// Status response or response data
|
||||
final String tag;
|
||||
if (ch == '*') {
|
||||
tag = null;
|
||||
readByte(); // skip *
|
||||
expect(' ');
|
||||
} else {
|
||||
tag = readUntil(' ');
|
||||
}
|
||||
final ImapResponse response = new ImapResponse(tag, false);
|
||||
// If it's continuation request, we don't really care what's in it.
|
||||
responseToDestroy.add(new ImapSimpleString(readUntilEol()));
|
||||
|
||||
final ImapString firstString = parseBareString();
|
||||
response.add(firstString);
|
||||
// Response has successfully been built. Let's return it.
|
||||
responseToReturn = responseToDestroy;
|
||||
responseToDestroy = null;
|
||||
} else {
|
||||
// Status response or response data
|
||||
final String tag;
|
||||
if (ch == '*') {
|
||||
tag = null;
|
||||
readByte(); // skip *
|
||||
expect(' ');
|
||||
} else {
|
||||
tag = readUntil(' ');
|
||||
}
|
||||
responseToDestroy = new ImapResponse(tag, false);
|
||||
|
||||
// parseBareString won't eat a space after the string, so we need to skip it, if exists.
|
||||
// If the next char is not ' ', it should be EOL.
|
||||
if (peek() == ' ') {
|
||||
readByte(); // skip ' '
|
||||
final ImapString firstString = parseBareString();
|
||||
responseToDestroy.add(firstString);
|
||||
|
||||
if (response.isStatusResponse()) { // It's a status response
|
||||
// parseBareString won't eat a space after the string, so we need to skip it,
|
||||
// if exists.
|
||||
// If the next char is not ' ', it should be EOL.
|
||||
if (peek() == ' ') {
|
||||
readByte(); // skip ' '
|
||||
|
||||
// Is there a response code?
|
||||
final int next = peek();
|
||||
if (next == '[') {
|
||||
response.add(parseList('[', ']'));
|
||||
if (peek() == ' ') { // Skip following space
|
||||
readByte();
|
||||
if (responseToDestroy.isStatusResponse()) { // It's a status response
|
||||
|
||||
// Is there a response code?
|
||||
final int next = peek();
|
||||
if (next == '[') {
|
||||
responseToDestroy.add(parseList('[', ']'));
|
||||
if (peek() == ' ') { // Skip following space
|
||||
readByte();
|
||||
}
|
||||
}
|
||||
|
||||
String rest = readUntilEol();
|
||||
if (!TextUtils.isEmpty(rest)) {
|
||||
// The rest is free-form text.
|
||||
responseToDestroy.add(new ImapSimpleString(rest));
|
||||
}
|
||||
} else { // It's a response data.
|
||||
parseElements(responseToDestroy, '\0');
|
||||
}
|
||||
} else {
|
||||
expect('\r');
|
||||
expect('\n');
|
||||
}
|
||||
|
||||
String rest = readUntilEol();
|
||||
if (!TextUtils.isEmpty(rest)) {
|
||||
// The rest is free-form text.
|
||||
response.add(new ImapSimpleString(rest));
|
||||
}
|
||||
} else { // It's a response data.
|
||||
parseElements(response, '\0');
|
||||
// Response has successfully been built. Let's return it.
|
||||
responseToReturn = responseToDestroy;
|
||||
responseToDestroy = null;
|
||||
}
|
||||
} finally {
|
||||
if (responseToDestroy != null) {
|
||||
// We get an exception.
|
||||
responseToDestroy.destroy();
|
||||
}
|
||||
} else {
|
||||
expect('\r');
|
||||
expect('\n');
|
||||
}
|
||||
|
||||
return response;
|
||||
return responseToReturn;
|
||||
}
|
||||
|
||||
private ImapElement parseElement() throws IOException, MessagingException {
|
||||
|
@ -58,9 +58,9 @@ public class ImapTempFileLiteral extends ImapString {
|
||||
}
|
||||
|
||||
/**
|
||||
* Because we can't use File.deleteOnExit(), let finalizer clean up the temp files....
|
||||
* Make sure we delete the temp file.
|
||||
*
|
||||
* TODO: Don't rely on this. Always explicitly call destroy().
|
||||
* We should always be calling {@link ImapResponse#destroy()}, but it's here as a last resort.
|
||||
*/
|
||||
@Override
|
||||
protected void finalize() throws Throwable {
|
||||
|
Loading…
Reference in New Issue
Block a user