am 1a791e67
: Fix NPE when switching IMAP into TLS
* commit '1a791e675ba5092569125bf09a9fcc721bd4c5e1': Fix NPE when switching IMAP into TLS
This commit is contained in:
commit
42749c9105
@ -232,7 +232,7 @@ public class ImapStore extends Store {
|
||||
*
|
||||
* @param userName the username of the account
|
||||
* @param host the host (server) of the account
|
||||
* @param capability the capabilities string from the server
|
||||
* @param capabilityResponse the capabilities list from the server
|
||||
* @return a String for use in an IMAP ID message.
|
||||
*/
|
||||
/* package */ static String getImapId(Context context, String userName, String host,
|
||||
@ -1407,17 +1407,9 @@ public class ImapStore extends Store {
|
||||
mParser.readResponse();
|
||||
|
||||
// CAPABILITY
|
||||
ImapResponse capabilityResponse = null;
|
||||
for (ImapResponse r : executeSimpleCommand(ImapConstants.CAPABILITY)) {
|
||||
if (r.is(0, ImapConstants.CAPABILITY)) {
|
||||
capabilityResponse = r;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (capabilityResponse == null) {
|
||||
throw new MessagingException("Invalid CAPABILITY response received");
|
||||
}
|
||||
ImapResponse capabilityResponse = queryCapabilities();
|
||||
|
||||
// TLS
|
||||
if (mTransport.canTryTlsSecurity()) {
|
||||
if (capabilityResponse.contains(ImapConstants.STARTTLS)) {
|
||||
// STARTTLS
|
||||
@ -1426,6 +1418,8 @@ public class ImapStore extends Store {
|
||||
mTransport.reopenTls();
|
||||
mTransport.setSoTimeout(MailTransport.SOCKET_READ_TIMEOUT);
|
||||
createParser();
|
||||
// Per RFC requirement (3501-6.2.1) gather new capabilities
|
||||
capabilityResponse = queryCapabilities();
|
||||
} else {
|
||||
if (Config.LOGD && Email.DEBUG) {
|
||||
Log.d(Email.LOG_TAG, "TLS not supported but required");
|
||||
@ -1434,6 +1428,7 @@ public class ImapStore extends Store {
|
||||
}
|
||||
}
|
||||
|
||||
// ID
|
||||
// Assign user-agent string (for RFC2971 ID command)
|
||||
String mUserAgent = getImapId(mContext, mUsername, mRootTransport.getHost(),
|
||||
capabilityResponse);
|
||||
@ -1460,6 +1455,7 @@ public class ImapStore extends Store {
|
||||
}
|
||||
}
|
||||
|
||||
// LOGIN
|
||||
try {
|
||||
// TODO eventually we need to add additional authentication
|
||||
// options such as SASL
|
||||
@ -1565,6 +1561,23 @@ public class ImapStore extends Store {
|
||||
return responses;
|
||||
}
|
||||
|
||||
/**
|
||||
* Query server for capabilities.
|
||||
*/
|
||||
private ImapResponse queryCapabilities() throws IOException, MessagingException {
|
||||
ImapResponse capabilityResponse = null;
|
||||
for (ImapResponse r : executeSimpleCommand(ImapConstants.CAPABILITY)) {
|
||||
if (r.is(0, ImapConstants.CAPABILITY)) {
|
||||
capabilityResponse = r;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (capabilityResponse == null) {
|
||||
throw new MessagingException("Invalid CAPABILITY response received");
|
||||
}
|
||||
return capabilityResponse;
|
||||
}
|
||||
|
||||
/** @see ImapResponseParser#logLastDiscourse() */
|
||||
public void logLastDiscourse() {
|
||||
mDiscourse.logLastDiscourse();
|
||||
|
@ -26,13 +26,13 @@ import com.android.email.mail.Body;
|
||||
import com.android.email.mail.FetchProfile;
|
||||
import com.android.email.mail.Flag;
|
||||
import com.android.email.mail.Folder;
|
||||
import com.android.email.mail.Folder.FolderType;
|
||||
import com.android.email.mail.Folder.OpenMode;
|
||||
import com.android.email.mail.Message;
|
||||
import com.android.email.mail.Message.RecipientType;
|
||||
import com.android.email.mail.MessagingException;
|
||||
import com.android.email.mail.Part;
|
||||
import com.android.email.mail.Transport;
|
||||
import com.android.email.mail.Folder.FolderType;
|
||||
import com.android.email.mail.Folder.OpenMode;
|
||||
import com.android.email.mail.Message.RecipientType;
|
||||
import com.android.email.mail.internet.MimeBodyPart;
|
||||
import com.android.email.mail.internet.MimeMultipart;
|
||||
import com.android.email.mail.internet.MimeUtility;
|
||||
@ -50,7 +50,6 @@ 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;
|
||||
@ -80,7 +79,7 @@ public class ImapStoreUnitTests extends AndroidTestCase {
|
||||
*/
|
||||
private final static String FOLDER_ENCODED = "&ZeU-";
|
||||
|
||||
private static ImapResponse CAPABILITY_RESPONSE = ImapTestUtils.parseResponse(
|
||||
private final static ImapResponse CAPABILITY_RESPONSE = ImapTestUtils.parseResponse(
|
||||
"* CAPABILITY IMAP4rev1 STARTTLS");
|
||||
|
||||
/* These values are provided by setUp() */
|
||||
@ -129,9 +128,13 @@ public class ImapStoreUnitTests extends AndroidTestCase {
|
||||
// TODO: inject specific facts in the initial folder SELECT and check them here
|
||||
}
|
||||
|
||||
/**
|
||||
* Test simple login with failed authentication
|
||||
*/
|
||||
public void testLoginFailure() throws Exception {
|
||||
MockTransport mockTransport = openAndInjectMockTransport();
|
||||
expectLogin(mockTransport, new String[] {"* iD nIL", "oK"}, "nO authentication failed");
|
||||
expectLogin(mockTransport, false, new String[] {"* iD nIL", "oK"},
|
||||
"nO authentication failed");
|
||||
|
||||
try {
|
||||
mStore.getConnection().open();
|
||||
@ -140,10 +143,35 @@ public class ImapStoreUnitTests extends AndroidTestCase {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Test simple TLS open
|
||||
*/
|
||||
public void testTlsOpen() throws MessagingException {
|
||||
|
||||
MockTransport mockTransport = openAndInjectMockTransport(Transport.CONNECTION_SECURITY_TLS,
|
||||
false);
|
||||
|
||||
// try to open it, with STARTTLS
|
||||
expectLogin(mockTransport, true,
|
||||
new String[] {"* iD nIL", "oK"}, "oK user authenticated (Success)");
|
||||
mockTransport.expect(
|
||||
getNextTag(false) + " SELECT \"" + FOLDER_ENCODED + "\"", new String[] {
|
||||
"* fLAGS (\\Answered \\Flagged \\Draft \\Deleted \\Seen)",
|
||||
"* oK [pERMANENTFLAGS (\\Answered \\Flagged \\Draft \\Deleted \\Seen \\*)]",
|
||||
"* 0 eXISTS",
|
||||
"* 0 rECENT",
|
||||
"* OK [uNSEEN 0]",
|
||||
"* OK [uIDNEXT 1]",
|
||||
getNextTag(true) + " oK [" + "rEAD-wRITE" + "] " +
|
||||
FOLDER_ENCODED + " selected. (Success)"});
|
||||
|
||||
mFolder.open(OpenMode.READ_WRITE, null);
|
||||
assertTrue(mockTransport.isTlsStarted());
|
||||
}
|
||||
|
||||
/**
|
||||
* TODO: Test with SSL negotiation (faked)
|
||||
* TODO: Test with SSL required but not supported
|
||||
* TODO: Test with TLS negotiation (faked)
|
||||
* TODO: Test with TLS required but not supported
|
||||
*/
|
||||
|
||||
@ -390,9 +418,17 @@ public class ImapStoreUnitTests extends AndroidTestCase {
|
||||
* Set up a basic MockTransport. open it, and inject it into mStore
|
||||
*/
|
||||
private MockTransport openAndInjectMockTransport() {
|
||||
return openAndInjectMockTransport(Transport.CONNECTION_SECURITY_NONE, false);
|
||||
}
|
||||
|
||||
/**
|
||||
* Set up a MockTransport with security settings
|
||||
*/
|
||||
private MockTransport openAndInjectMockTransport(int connectionSecurity,
|
||||
boolean trustAllCertificates) {
|
||||
// Create mock transport and inject it into the ImapStore that's already set up
|
||||
MockTransport mockTransport = new MockTransport();
|
||||
mockTransport.setSecurity(Transport.CONNECTION_SECURITY_NONE, false);
|
||||
mockTransport.setSecurity(connectionSecurity, trustAllCertificates);
|
||||
mockTransport.setMockHost("mock.server.com");
|
||||
mStore.setTransport(mockTransport);
|
||||
return mockTransport;
|
||||
@ -427,8 +463,7 @@ public class ImapStoreUnitTests extends AndroidTestCase {
|
||||
* @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
|
||||
* tag will be added at runtime).
|
||||
* @param "READ-WRITE" or "READ-ONLY"
|
||||
* @return the next tag# to use
|
||||
* @param readWriteMode "READ-WRITE" or "READ-ONLY"
|
||||
*/
|
||||
private void setupOpenFolder(MockTransport mockTransport, String[] imapIdResponse,
|
||||
String readWriteMode) {
|
||||
@ -450,18 +485,26 @@ public class ImapStoreUnitTests extends AndroidTestCase {
|
||||
}
|
||||
|
||||
private void expectLogin(MockTransport mockTransport, String[] imapIdResponse) {
|
||||
expectLogin(mockTransport, imapIdResponse, "oK user authenticated (Success)");
|
||||
expectLogin(mockTransport, false, imapIdResponse, "oK user authenticated (Success)");
|
||||
}
|
||||
|
||||
private void expectLogin(MockTransport mockTransport, String[] imapIdResponse,
|
||||
private void expectLogin(MockTransport mockTransport, boolean startTls, String[] imapIdResponse,
|
||||
String loginResponse) {
|
||||
// inject boilerplate commands that match our typical login
|
||||
mockTransport.expect(null, "* oK Imap 2000 Ready To Assist You");
|
||||
|
||||
mockTransport.expect(getNextTag(false) + " CAPABILITY", new String[] {
|
||||
"* cAPABILITY iMAP4rev1 sTARTTLS aUTH=gSSAPI lOGINDISABLED",
|
||||
getNextTag(true) + " oK CAPABILITY completed"});
|
||||
expectCapability(mockTransport);
|
||||
|
||||
// TLS (if expected)
|
||||
if (startTls) {
|
||||
mockTransport.expect(getNextTag(false) + " STARTTLS",
|
||||
getNextTag(true) + " Ok starting TLS");
|
||||
mockTransport.expectStartTls();
|
||||
// After switching to TLS the client must re-query for capability
|
||||
expectCapability(mockTransport);
|
||||
}
|
||||
|
||||
// ID
|
||||
String expectedNextTag = getNextTag(false);
|
||||
// Fix the tag # of the ID response
|
||||
String last = imapIdResponse[imapIdResponse.length-1];
|
||||
@ -471,10 +514,17 @@ public class ImapStoreUnitTests extends AndroidTestCase {
|
||||
|
||||
getNextTag(true); // Advance the tag for ID response.
|
||||
|
||||
// LOGIN
|
||||
mockTransport.expect(getNextTag(false) + " LOGIN user \"password\"",
|
||||
getNextTag(true) + " " + loginResponse);
|
||||
}
|
||||
|
||||
private void expectCapability(MockTransport mockTransport) {
|
||||
mockTransport.expect(getNextTag(false) + " CAPABILITY", new String[] {
|
||||
"* cAPABILITY iMAP4rev1 sTARTTLS aUTH=gSSAPI lOGINDISABLED",
|
||||
getNextTag(true) + " oK CAPABILITY completed"});
|
||||
}
|
||||
|
||||
private void expectNoop(MockTransport mockTransport, boolean ok) {
|
||||
String response = ok ? " oK success" : " nO timeout";
|
||||
mockTransport.expect(getNextTag(false) + " NOOP",
|
||||
@ -1420,7 +1470,7 @@ public class ImapStoreUnitTests extends AndroidTestCase {
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for {@link ImapStore#getConnection} and {@link ImapStore#keepConnectionForReuse}
|
||||
* Test for {@link ImapStore#getConnection}
|
||||
*/
|
||||
public void testGetConnection() throws Exception {
|
||||
MockTransport mock = openAndInjectMockTransport();
|
||||
@ -1488,7 +1538,7 @@ public class ImapStoreUnitTests extends AndroidTestCase {
|
||||
expectLogin(mock);
|
||||
mStore.checkSettings();
|
||||
|
||||
expectLogin(mock, new String[] {"* iD nIL", "oK"}, "nO authentication failed");
|
||||
expectLogin(mock, false, new String[] {"* iD nIL", "oK"}, "nO authentication failed");
|
||||
try {
|
||||
mStore.checkSettings();
|
||||
fail();
|
||||
|
@ -44,8 +44,7 @@ public class MockTransport implements Transport {
|
||||
|
||||
private static final String SPECIAL_RESPONSE_IOEXCEPTION = "!!!IOEXCEPTION!!!";
|
||||
|
||||
private boolean mSslAllowed = false;
|
||||
private boolean mTlsAllowed = false;
|
||||
private boolean mTlsStarted = false;
|
||||
|
||||
private boolean mOpen;
|
||||
private boolean mInputOpen;
|
||||
@ -60,6 +59,7 @@ public class MockTransport implements Transport {
|
||||
public static final int ACTION_INJECT_TEXT = 0;
|
||||
public static final int ACTION_CLIENT_CLOSE = 1;
|
||||
public static final int ACTION_IO_EXCEPTION = 2;
|
||||
public static final int ACTION_START_TLS = 3;
|
||||
|
||||
int mAction;
|
||||
String mPattern;
|
||||
@ -86,6 +86,8 @@ public class MockTransport implements Transport {
|
||||
return "Expect the client to close";
|
||||
case ACTION_IO_EXCEPTION:
|
||||
return "Expect IOException";
|
||||
case ACTION_START_TLS:
|
||||
return "Expect StartTls";
|
||||
default:
|
||||
return "(Hmm. Unknown action.)";
|
||||
}
|
||||
@ -142,6 +144,10 @@ public class MockTransport implements Transport {
|
||||
mPairs.add(new Transaction(Transaction.ACTION_IO_EXCEPTION));
|
||||
}
|
||||
|
||||
public void expectStartTls() {
|
||||
mPairs.add(new Transaction(Transaction.ACTION_START_TLS));
|
||||
}
|
||||
|
||||
private void sendResponse(Transaction pair) {
|
||||
switch (pair.mAction) {
|
||||
case Transaction.ACTION_INJECT_TEXT:
|
||||
@ -169,6 +175,13 @@ public class MockTransport implements Transport {
|
||||
return mTrustCertificates;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check that TLS was started
|
||||
*/
|
||||
public boolean isTlsStarted() {
|
||||
return mTlsStarted;
|
||||
}
|
||||
|
||||
/**
|
||||
* This simulates a condition where the server has closed its side, causing
|
||||
* reads to fail.
|
||||
@ -289,8 +302,9 @@ public class MockTransport implements Transport {
|
||||
|
||||
public void reopenTls() /* throws MessagingException */ {
|
||||
SmtpSenderUnitTests.assertTrue(mOpen);
|
||||
SmtpSenderUnitTests.assertTrue(mTlsAllowed);
|
||||
SmtpSenderUnitTests.fail("reopenTls() not implemented");
|
||||
Transaction expect = mPairs.remove(0);
|
||||
SmtpSenderUnitTests.assertTrue(expect.mAction == Transaction.ACTION_START_TLS);
|
||||
mTlsStarted = true;
|
||||
}
|
||||
|
||||
public void setSecurity(int connectionSecurity, boolean trustAllCertificates) {
|
||||
|
Loading…
Reference in New Issue
Block a user