am 275b89d9: am 927dbc7c: Don\'t send IMAP ID unless server supports it

* commit '275b89d96949de6544c7bca999e241f659cf18b3':
  Don't send IMAP ID unless server supports it
This commit is contained in:
Andy Stadler 2011-01-31 15:20:01 -08:00 committed by Android Git Automerger
commit 11070a5397
2 changed files with 62 additions and 40 deletions

View File

@ -1484,29 +1484,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
} }
} }

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();