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

Don't send IMAP ID unless server supports it

Browse Source 
Only send IMAP ID to servers that include ID in the CAPABILITY response.
Always sending IMAP ID was found to cause problems with some servers.

Better compliance with RFC 2971.
Thanks to Samsung for debugging & reporting this.

Change-Id: I495f80949f9f811470853a1f2f8e506d8236d8cf
This commit is contained in:
Andy Stadler 2011-01-31 15:10:49 -08:00
parent ecffd551cc
commit 927dbc7c20
2 changed files with 62 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1429,29 +1429,31 @@ public class ImapStore extends Store {
} }
// ID // ID
// Assign user-agent string (for RFC2971 ID command) if (capabilityResponse.contains(ImapConstants.ID)) {
String mUserAgent = getImapId(mContext, mUsername, mRootTransport.getHost(), // Assign user-agent string (for RFC2971 ID command)
capabilityResponse); String mUserAgent = getImapId(mContext, mUsername, mRootTransport.getHost(),
if (mUserAgent != null) { capabilityResponse);
mIdPhrase = ImapConstants.ID + " (" + mUserAgent + ")"; if (mUserAgent != null) {
} else if (DEBUG_FORCE_SEND_ID) { mIdPhrase = ImapConstants.ID + " (" + mUserAgent + ")";
mIdPhrase = ImapConstants.ID + " " + ImapConstants.NIL; } else if (DEBUG_FORCE_SEND_ID) {
} mIdPhrase = ImapConstants.ID + " " + ImapConstants.NIL;
// else: mIdPhrase = null, no ID will be emitted }
// else: mIdPhrase = null, no ID will be emitted
// Send user-agent in an RFC2971 ID command // Send user-agent in an RFC2971 ID command
if (mIdPhrase != null) { if (mIdPhrase != null) {
try { try {
executeSimpleCommand(mIdPhrase); executeSimpleCommand(mIdPhrase);
} catch (ImapException ie) { } catch (ImapException ie) {
// Log for debugging, but this is not a fatal problem. // Log for debugging, but this is not a fatal problem.
if (Config.LOGD && Email.DEBUG) { if (Config.LOGD && Email.DEBUG) {
Log.d(Email.LOG_TAG, ie.toString()); Log.d(Email.LOG_TAG, ie.toString());
}
} catch (IOException ioe) {
// Special case to handle malformed OK responses and ignore them.
// A true IOException will recur on the following login steps
// This can go away after the parser is fixed - see bug 2138981
} }
} catch (IOException ioe) {
// Special case to handle malformed OK responses and ignore them.
// A true IOException will recur on the following login steps
// This can go away after the parser is fixed - see bug 2138981 for details
} }
} }

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

@ -133,7 +133,7 @@ public class ImapStoreUnitTests extends AndroidTestCase {
*/ */
public void testLoginFailure() throws Exception { public void testLoginFailure() throws Exception {
MockTransport mockTransport = openAndInjectMockTransport(); MockTransport mockTransport = openAndInjectMockTransport();
expectLogin(mockTransport, false, new String[] {"* iD nIL", "oK"}, expectLogin(mockTransport, false, false, new String[] {"* iD nIL", "oK"},
"nO authentication failed"); "nO authentication failed");
try { try {
@ -152,7 +152,7 @@ public class ImapStoreUnitTests extends AndroidTestCase {
false); false);
// try to open it, with STARTTLS // try to open it, with STARTTLS
expectLogin(mockTransport, true, expectLogin(mockTransport, true, false,
new String[] {"* iD nIL", "oK"}, "oK user authenticated (Success)"); new String[] {"* iD nIL", "oK"}, "oK user authenticated (Success)");
mockTransport.expect( mockTransport.expect(
getNextTag(false) + " SELECT \"" + FOLDER_ENCODED + "\"", new String[] { getNextTag(false) + " SELECT \"" + FOLDER_ENCODED + "\"", new String[] {
@ -366,6 +366,19 @@ public class ImapStoreUnitTests extends AndroidTestCase {
mFolder.open(OpenMode.READ_WRITE, null); mFolder.open(OpenMode.READ_WRITE, null);
} }
/**
* Confirm that when IMAP ID is not in capability, it is not sent/received.
* This supports RFC 2971 section 3, and is important because certain servers
* (e.g. imap.vodafone.net.nz) do not process the unexpected ID command properly.
*/
public void testImapIdNotSupported() throws MessagingException {
MockTransport mockTransport = openAndInjectMockTransport();
// try to open it
setupOpenFolder(mockTransport, null, "rEAD-wRITE");
mFolder.open(OpenMode.READ_WRITE, null);
}
/** /**
* Test small Folder functions that don't really do anything in Imap * Test small Folder functions that don't really do anything in Imap
*/ */
@ -462,7 +475,7 @@ public class ImapStoreUnitTests extends AndroidTestCase {
* @param mockTransport the mock transport we're using * @param mockTransport the mock transport we're using
* @param imapIdResponse the expected series of responses to the IMAP ID command. Non-final * @param imapIdResponse the expected series of responses to the IMAP ID command. Non-final
* lines should be tagged with *. The final response should be untagged (the correct * lines should be tagged with *. The final response should be untagged (the correct
* tag will be added at runtime). * tag will be added at runtime). Pass "null" to test w/o IMAP ID.
* @param readWriteMode "READ-WRITE" or "READ-ONLY" * @param readWriteMode "READ-WRITE" or "READ-ONLY"
*/ */
private void setupOpenFolder(MockTransport mockTransport, String[] imapIdResponse, private void setupOpenFolder(MockTransport mockTransport, String[] imapIdResponse,
@ -485,15 +498,16 @@ public class ImapStoreUnitTests extends AndroidTestCase {
} }
private void expectLogin(MockTransport mockTransport, String[] imapIdResponse) { private void expectLogin(MockTransport mockTransport, String[] imapIdResponse) {
expectLogin(mockTransport, false, imapIdResponse, "oK user authenticated (Success)"); expectLogin(mockTransport, false, (imapIdResponse != null), imapIdResponse,
"oK user authenticated (Success)");
} }
private void expectLogin(MockTransport mockTransport, boolean startTls, String[] imapIdResponse, private void expectLogin(MockTransport mockTransport, boolean startTls, boolean withId,
String loginResponse) { String[] imapIdResponse, String loginResponse) {
// inject boilerplate commands that match our typical login // inject boilerplate commands that match our typical login
mockTransport.expect(null, "* oK Imap 2000 Ready To Assist You"); mockTransport.expect(null, "* oK Imap 2000 Ready To Assist You");
expectCapability(mockTransport); expectCapability(mockTransport, withId);
// TLS (if expected) // TLS (if expected)
if (startTls) { if (startTls) {
@ -501,27 +515,32 @@ public class ImapStoreUnitTests extends AndroidTestCase {
getNextTag(true) + " Ok starting TLS"); getNextTag(true) + " Ok starting TLS");
mockTransport.expectStartTls(); mockTransport.expectStartTls();
// After switching to TLS the client must re-query for capability // After switching to TLS the client must re-query for capability
expectCapability(mockTransport); expectCapability(mockTransport, withId);
} }
// ID // ID
String expectedNextTag = getNextTag(false); if (withId) {
// Fix the tag # of the ID response String expectedNextTag = getNextTag(false);
String last = imapIdResponse[imapIdResponse.length-1]; // Fix the tag # of the ID response
last = expectedNextTag + " " + last; String last = imapIdResponse[imapIdResponse.length-1];
imapIdResponse[imapIdResponse.length-1] = last; last = expectedNextTag + " " + last;
mockTransport.expect(getNextTag(false) + " ID \\(.*\\)", imapIdResponse); imapIdResponse[imapIdResponse.length-1] = last;
mockTransport.expect(getNextTag(false) + " ID \\(.*\\)", imapIdResponse);
getNextTag(true); // Advance the tag for ID response. getNextTag(true); // Advance the tag for ID response.
}
// LOGIN // LOGIN
mockTransport.expect(getNextTag(false) + " LOGIN user \"password\"", mockTransport.expect(getNextTag(false) + " LOGIN user \"password\"",
getNextTag(true) + " " + loginResponse); getNextTag(true) + " " + loginResponse);
} }
private void expectCapability(MockTransport mockTransport) { private void expectCapability(MockTransport mockTransport, boolean withId) {
String capabilityList = withId
? "* cAPABILITY iMAP4rev1 sTARTTLS aUTH=gSSAPI lOGINDISABLED iD"
: "* cAPABILITY iMAP4rev1 sTARTTLS aUTH=gSSAPI lOGINDISABLED";
mockTransport.expect(getNextTag(false) + " CAPABILITY", new String[] { mockTransport.expect(getNextTag(false) + " CAPABILITY", new String[] {
"* cAPABILITY iMAP4rev1 sTARTTLS aUTH=gSSAPI lOGINDISABLED", capabilityList,
getNextTag(true) + " oK CAPABILITY completed"}); getNextTag(true) + " oK CAPABILITY completed"});
} }
@ -1538,7 +1557,8 @@ public class ImapStoreUnitTests extends AndroidTestCase {
expectLogin(mock); expectLogin(mock);
mStore.checkSettings(); mStore.checkSettings();
expectLogin(mock, false, new String[] {"* iD nIL", "oK"}, "nO authentication failed"); expectLogin(mock, false, false,
new String[] {"* iD nIL", "oK"}, "nO authentication failed");
try { try {
mStore.checkSettings(); mStore.checkSettings();
fail(); fail();