From 20bf1f632f47c1ba408d23d87b7d9dc3fc5ba5ec Mon Sep 17 00:00:00 2001 From: Marc Blank Date: Wed, 17 Aug 2011 13:50:28 -0700 Subject: [PATCH] Support IMAP search in UTF-8, in addition to US-ASCII * The existing IMAP search code is well-known to be primitive and largely broken. In particular, it fails with any non-ASCII character and with a variety of symbols (e.g. quotes, slashes, etc.) Basically, it's an accident waiting to happen, returning empty data sets even when the query might reasonably be expected (or known) to return valid data. * The current CL uses the IMAP literal string format to represent the query text; this string can be sent either in ascii or in UTF-8, and since it is sent as an octet (byte) count followed by 8-bit data, it also solves any quoting issues that might come up. So, we kill two birds with one stone. * The bug in question was punted to a subsequent MR; however, I think it would be a mistake to ship the code without this CL, which has been tested against the three common IMAP servers that we aim to support. Bug: 4690713 Change-Id: Iaa542bfc56737871f3cbc9c83f0e768415a7f2b6 --- .../email/mail/store/ImapConnection.java | 83 ++++++++++++-- .../android/email/mail/store/ImapFolder.java | 102 ++++++++++++------ 2 files changed, 144 insertions(+), 41 deletions(-) diff --git a/src/com/android/email/mail/store/ImapConnection.java b/src/com/android/email/mail/store/ImapConnection.java index 4d4d16f75..0fbf6036f 100644 --- a/src/com/android/email/mail/store/ImapConnection.java +++ b/src/com/android/email/mail/store/ImapConnection.java @@ -16,6 +16,9 @@ package com.android.email.mail.store; +import android.text.TextUtils; +import android.util.Log; + import com.android.email.Email; import com.android.email.mail.Transport; import com.android.email.mail.store.ImapStore.ImapException; @@ -31,9 +34,6 @@ import com.android.emailcommon.mail.AuthenticationFailedException; import com.android.emailcommon.mail.CertificateValidationException; import com.android.emailcommon.mail.MessagingException; -import android.text.TextUtils; -import android.util.Log; - import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -248,14 +248,53 @@ class ImapConnection { return tag; } + + /** + * Send a single, complex command to the server. The command will be preceded by an IMAP + * command tag and followed by \r\n (caller need not supply them). After each piece of the + * command, a response will be read which MUST be a continuation request. + * + * @param commands An array of Strings comprising the command to be sent to the server + * @return Returns the command tag that was sent + */ + String sendComplexCommand(List commands, boolean sensitive) throws MessagingException, + IOException { + open(); + String tag = Integer.toString(mNextCommandTag.incrementAndGet()); + int len = commands.size(); + for (int i = 0; i < len; i++) { + String commandToSend = commands.get(i); + // The first part of the command gets the tag + if (i == 0) { + commandToSend = tag + " " + commandToSend; + } else { + // Otherwise, read the response from the previous part of the command + ImapResponse response = readResponse(); + // If it isn't a continuation request, that's an error + if (!response.isContinuationRequest()) { + throw new MessagingException("Expected continuation request"); + } + } + // Send the command + mTransport.writeLine(commandToSend, null); + mDiscourse.addSentCommand(sensitive ? IMAP_REDACTED_LOG : commandToSend); + } + return tag; + } + List executeSimpleCommand(String command) throws IOException, MessagingException { return executeSimpleCommand(command, false); } - List executeSimpleCommand(String command, boolean sensitive) - throws IOException, MessagingException { - String tag = sendCommand(command, sensitive); + /** + * Read and return all of the responses from the most recent command sent to the server + * + * @return a list of ImapResponses + * @throws IOException + * @throws MessagingException + */ + List getCommandResponses() throws IOException, MessagingException { ArrayList responses = new ArrayList(); ImapResponse response; do { @@ -271,6 +310,38 @@ class ImapConnection { return responses; } + /** + * Execute a simple command at the server, a simple command being one that is sent in a single + * line of text + * + * @param command the command to send to the server + * @param sensitive whether the command should be redacted in logs (used for login) + * @return a list of ImapResponses + * @throws IOException + * @throws MessagingException + */ + List executeSimpleCommand(String command, boolean sensitive) + throws IOException, MessagingException { + sendCommand(command, sensitive); + return getCommandResponses(); + } + + /** + * Execute a complex command at the server, a complex command being one that must be sent in + * multiple lines due to the use of string literals + * + * @param commands a list of strings that comprise the command to be sent to the server + * @param sensitive whether the command should be redacted in logs (used for login) + * @return a list of ImapResponses + * @throws IOException + * @throws MessagingException + */ + List executeComplexCommand(List commands, boolean sensitive) + throws IOException, MessagingException { + sendComplexCommand(commands, sensitive); + return getCommandResponses(); + } + /** * Query server for capabilities. */ diff --git a/src/com/android/email/mail/store/ImapFolder.java b/src/com/android/email/mail/store/ImapFolder.java index aa6722e39..98d25c99a 100644 --- a/src/com/android/email/mail/store/ImapFolder.java +++ b/src/com/android/email/mail/store/ImapFolder.java @@ -366,34 +366,37 @@ class ImapFolder extends Folder { throw new Error("ImapStore.delete() not yet implemented"); } - /* package */ String[] searchForUids(String searchCriteria) - throws MessagingException { + String[] getSearchUids(List responses) { + // S: * SEARCH 2 3 6 + final ArrayList uids = new ArrayList(); + for (ImapResponse response : responses) { + if (!response.isDataResponse(0, ImapConstants.SEARCH)) { + continue; + } + // Found SEARCH response data + for (int i = 1; i < response.size(); i++) { + ImapString s = response.getStringOrEmpty(i); + if (s.isString()) { + uids.add(s.getString()); + } + } + } + return uids.toArray(Utility.EMPTY_STRINGS); + } + + @VisibleForTesting + String[] searchForUids(String searchCriteria) throws MessagingException { checkOpen(); - List responses; try { try { - responses = mConnection.executeSimpleCommand( - ImapConstants.UID_SEARCH + " " + searchCriteria); + String command = ImapConstants.UID_SEARCH + " " + searchCriteria; + return getSearchUids(mConnection.executeSimpleCommand(command)); } catch (ImapException e) { + Log.d(Logging.LOG_TAG, "ImapException in search: " + searchCriteria); return Utility.EMPTY_STRINGS; // not found; } catch (IOException ioe) { throw ioExceptionHandler(mConnection, ioe); } - // S: * SEARCH 2 3 6 - final ArrayList uids = new ArrayList(); - for (ImapResponse response : responses) { - if (!response.isDataResponse(0, ImapConstants.SEARCH)) { - continue; - } - // Found SEARCH response data - for (int i = 1; i < response.size(); i++) { - ImapString s = response.getStringOrEmpty(i); - if (s.isString()) { - uids.add(s.getString()); - } - } - } - return uids.toArray(Utility.EMPTY_STRINGS); } finally { destroyResponses(); } @@ -413,29 +416,58 @@ class ImapFolder extends Folder { return null; } + @VisibleForTesting + protected static boolean isAsciiString(String str) { + int len = str.length(); + for (int i = 0; i < len; i++) { + char c = str.charAt(i); + if (c >= 128) return false; + } + return true; + } + /** * Retrieve messages based on search parameters. We search FROM, TO, CC, SUBJECT, and BODY - * We send: SEARCH OR FROM "foo" (OR TO "foo" (OR CC "foo" (OR SUBJECT "foo" BODY "foo"))) - * TODO: Properly quote the filter + * We send: SEARCH OR FROM "foo" (OR TO "foo" (OR CC "foo" (OR SUBJECT "foo" BODY "foo"))), but + * with the additional CHARSET argument and sending "foo" as a literal (e.g. {3}foo} */ @Override @VisibleForTesting public Message[] getMessages(SearchParams params, MessageRetrievalListener listener) throws MessagingException { + List commands = new ArrayList(); String filter = params.mFilter; - StringBuilder sb = new StringBuilder(); - sb.append("OR FROM \""); - sb.append(filter); - sb.append("\" (OR TO \""); - sb.append(filter); - sb.append("\" (OR CC \""); - sb.append(filter); - sb.append("\" (OR SUBJECT \""); - sb.append(filter); - sb.append("\" BODY \""); - sb.append(filter); - sb.append("\")))"); - return getMessagesInternal(searchForUids(sb.toString()), listener); + // All servers MUST accept US-ASCII, so we'll send this as the CHARSET unless we're really + // dealing with a string that contains non-ascii characters + String charset = "US-ASCII"; + if (!isAsciiString(filter)) { + charset = "UTF-8"; + } + // This is the length of the string in octets (bytes), formatted as a string literal {n} + String octetLength = "{" + filter.getBytes().length + "}"; + // Break the command up into pieces ending with the string literal length + commands.add(ImapConstants.UID_SEARCH + " CHARSET " + charset + " OR FROM " + octetLength); + commands.add(filter + " (OR TO " + octetLength); + commands.add(filter + " (OR CC " + octetLength); + commands.add(filter + " (OR SUBJECT " + octetLength); + commands.add(filter + " BODY " + octetLength); + commands.add(filter + ")))"); + return getMessagesInternal(complexSearchForUids(commands), listener); + } + + /* package */ String[] complexSearchForUids(List commands) throws MessagingException { + checkOpen(); + try { + try { + return getSearchUids(mConnection.executeComplexCommand(commands, false)); + } catch (ImapException e) { + return Utility.EMPTY_STRINGS; // not found; + } catch (IOException ioe) { + throw ioExceptionHandler(mConnection, ioe); + } + } finally { + destroyResponses(); + } } @Override