Issue 234 Issue 366 Issue 329

Hardening of POP3 and SMTP communication:

SMTP: Decrement failure counter when no possible message send
occurred.  This way, K-9 will only stop attempting to send messages
for which a possible successful send occurred, but K-9 could not
detect.  Any message that is known to have completely failed to send
will be reattempted indefinitely.

POP3: Some reworking of Exception handling.  Also, if it is not
possible to get a "message number" for the UID of the message that is
being deleted, conclude that the message has already been deleted on
the server.  Mark this as a permanent error, so that it gets removed
from the pending actions queue.

MessagingController: Look for the permanentFailure flag on the
MessagingException, and if a pending action raised a permanent
failure, remove the pending action from the queue so that it will not
be re-attempted nor block later requests.

src/com/android/email/MessagingController.java

@@ -1229,6 +1229,8 @@ s             * critical data as fast as possible, and then we'll fill in the de
 	             * most likely due to a server or IO error and it must be retried before any
 	             * other command processes. This maintains the order of the commands.
 	             */
+	        	try
+	        	{
 	            if (PENDING_COMMAND_APPEND.equals(command.command)) {
 	                processPendingAppend(command, account);
 	            }
@@ -1246,13 +1248,24 @@ s             * critical data as fast as possible, and then we'll fill in the de
 	            }
 	            localStore.removePendingCommand(command);
 	            Log.d(Email.LOG_TAG, "Done processing pending command '" + command + "'");
-
+	        	}
+	        	catch (MessagingException me)
+	        	{
+	        	  if (me.isPermanentFailure())
+	            {
+	        	    Log.e(Email.LOG_TAG, "Failure of command '" + command + "' was permanent, removing command from queue");
+	              localStore.removePendingCommand(processingCommand);
+	            }
+  	        	else
+  	        	{
+  	        	  throw me;
+  	        	}
+	        	}
 	        }
         }
         catch (MessagingException me)
         {
           addErrorMessage(account, me);
-
         	Log.e(Email.LOG_TAG, "Could not process command '" + processingCommand + "'", me);
         	throw me;
         }
@@ -2122,6 +2135,17 @@ s             * critical data as fast as possible, and then we'll fill in the de
                         processPendingCommands(account);
                     }
                     catch (Exception e) {
+                      if (e instanceof MessagingException)
+                      {
+                        MessagingException me = (MessagingException)e;
+                        if (me.isPermanentFailure() == false)
+                        {
+                          // Decrement the counter if the message could not possibly have been sent
+                          int newVal = count.decrementAndGet();
+                          Log.i(Email.LOG_TAG, "Decremented send count for message " + message.getUid() + " to " + newVal 
+                              + "; no possible send");
+                        }
+                      }
                         message.setFlag(Flag.X_SEND_FAILED, true);
                         Log.e(Email.LOG_TAG, "Failed to send message", e);
                         for (MessagingListener l : getListeners()) {

src/com/android/email/mail/MessagingException.java

@@ -4,6 +4,8 @@ package com.android.email.mail;
 public class MessagingException extends Exception {
     public static final long serialVersionUID = -1;
     
+    boolean permanentFailure = false;
+    
     public MessagingException(String message) {
         super(message);
     }
@@ -11,4 +13,16 @@ public class MessagingException extends Exception {
     public MessagingException(String message, Throwable throwable) {
         super(message, throwable);
     }
+
+    public boolean isPermanentFailure()
+    {
+      return permanentFailure;
+    }
+
+    public void setPermanentFailure(boolean permanentFailure)
+    {
+      this.permanentFailure = permanentFailure;
+    }
+    
+    
 }

src/com/android/email/mail/store/Pop3Store.java

@@ -158,16 +158,13 @@ public class Pop3Store extends Store {
              * Run an additional test to see if UIDL is supported on the server. If it's not we
              * can't service this account.
              */
-            try{
+            
-                /*
+            /*
-                 * If the server doesn't support UIDL it will return a - response, which causes
+             * If the server doesn't support UIDL it will return a - response, which causes
-                 * executeSimpleCommand to throw a MessagingException, exiting this method.
+             * executeSimpleCommand to throw a MessagingException, exiting this method.
-                 */
+             */
-                folder.executeSimpleCommand("UIDL");
+            folder.executeSimpleCommand("UIDL");
-            }
+           
-            catch (IOException ioe) {
-                throw new MessagingException(null, ioe);
-            }
         }
         folder.close(false);
     }
@@ -262,14 +259,10 @@ public class Pop3Store extends Store {
                 throw new MessagingException("Unable to open connection to POP server.", ioe);
             }
 
-            try {
+            String response = executeSimpleCommand("STAT");
-                String response = executeSimpleCommand("STAT");
+            String[] parts = response.split(" ");
-                String[] parts = response.split(" ");
+            mMessageCount = Integer.parseInt(parts[1]);
-                mMessageCount = Integer.parseInt(parts[1]);
+            
-            }
-            catch (IOException ioe) {
-                throw new MessagingException("Unable to STAT folder.", ioe);
-            }
             mUidToMsgMap.clear();
             mMsgNumToMsgMap.clear();
             mUidToMsgNumMap.clear();
@@ -448,7 +441,10 @@ public class Pop3Store extends Store {
             HashSet<String> unindexedUids = new HashSet<String>();
             for (String uid : uids) {
                 if (mUidToMsgMap.get(uid) == null) {
-                    unindexedUids.add(uid);
+                  if (Config.LOGD) {
+                    Log.d(Email.LOG_TAG, "Need to index UID " + uid);
+                  }
+                  unindexedUids.add(uid);
                 }
             }
             if (unindexedUids.size() == 0) {
@@ -461,23 +457,30 @@ public class Pop3Store extends Store {
              */
             String response = executeSimpleCommand("UIDL");
             while ((response = readLine()) != null) {
-                if (response.equals(".")) {
+              if (response.equals(".")) {
-                    break;
+                break;
+              }
+              String[] uidParts = response.split(" ");
+              Integer msgNum = Integer.valueOf(uidParts[0]);
+              String msgUid = uidParts[1];
+              if (unindexedUids.contains(msgUid)) {
+                if (Config.LOGD) {
+                  Log.d(Email.LOG_TAG, "Got msgNum " + msgNum + " for UID " + msgUid);
                 }
-                String[] uidParts = response.split(" ");
+
-                Integer msgNum = Integer.valueOf(uidParts[0]);
+                Pop3Message message = mUidToMsgMap.get(msgUid);
-                String msgUid = uidParts[1];
+                if (message == null) {
-                if (unindexedUids.contains(msgUid)) {
+                  message = new Pop3Message(msgUid, this);
-		    Pop3Message message = mUidToMsgMap.get(msgUid);
-		    if (message == null) {
-			message = new Pop3Message(msgUid, this);
-		    }
-		    indexMessage(msgNum, message);
                 }
+                indexMessage(msgNum, message);
+              }
             }
         }
 
         private void indexMessage(int msgNum, Pop3Message message) {
+          if (Config.LOGD){
+            Log.d(Email.LOG_TAG, "Adding index for UID " + message.getUid() + " to msgNum " + msgNum);
+          }
             mMsgNumToMsgMap.put(msgNum, message);
             mUidToMsgMap.put(message.getUid(), message);
             mUidToMsgNumMap.put(message.getUid(), msgNum);
@@ -721,20 +724,18 @@ public class Pop3Store extends Store {
             {
             	throw new MessagingException("Could not get message number for uid " + uids, ioe);
             }
-            try {
+            for (Message message : messages) {
-                for (Message message : messages) {
+              
-                    executeSimpleCommand(String.format("DELE %s",
+              Integer msgNum = mUidToMsgNumMap.get(message.getUid());
-                            mUidToMsgNumMap.get(message.getUid())));
+              if (msgNum == null)
-                }
+              {
+                MessagingException me = new MessagingException("Could not delete message " + message.getUid()
+                    + " because no msgNum found; permanent error");
+                me.setPermanentFailure(true);
+                throw me;
+              }
+              executeSimpleCommand(String.format("DELE %s", msgNum));
             }
-            catch (IOException ioe) {
-                throw new MessagingException("setFlags()", ioe);
-            }
-        }
-
-        @Override
-        public void copyMessages(Message[] msgs, Folder folder) throws MessagingException {
-            throw new UnsupportedOperationException("copyMessages is not supported in POP3");
         }
 
 //        private boolean isRoundTripModeSuggested() {
@@ -814,7 +815,7 @@ public class Pop3Store extends Store {
             return capabilities;
         }
 
-        private String executeSimpleCommand(String command) throws IOException, MessagingException {
+        private String executeSimpleCommand(String command) throws MessagingException {
             try {
                 open(OpenMode.READ_WRITE);
                 if (Config.LOGV)
@@ -837,9 +838,9 @@ public class Pop3Store extends Store {
     
                 return response;
             }
-            catch (IOException e) {
+            catch (Exception e) {
                 closeIO();
-                throw e;
+                throw new MessagingException("Unable to execute POP3 command", e);
             }
         }
 

src/com/android/email/mail/transport/SmtpTransport.java

@@ -228,7 +228,7 @@ public class SmtpTransport extends Transport {
         close();
         open();
         Address[] from = message.getFrom();
-
+        boolean possibleSend = false;
         try {
             executeSimpleCommand("MAIL FROM: " + "<" + from[0].getAddress() + ">");
             for (Address address : message.getRecipients(RecipientType.TO)) {
@@ -246,14 +246,21 @@ public class SmtpTransport extends Transport {
             message.writeTo(
                     new EOLConvertingOutputStream(
                             new BufferedOutputStream(mOut, 1024)));
+ 
+            possibleSend = true; // After the "\r\n." is attempted, we may have sent the message
             executeSimpleCommand("\r\n.");
-        } catch (IOException ioe) {
+        } catch (Exception e) {
-            throw new MessagingException("Unable to send message", ioe);
+          MessagingException me = new MessagingException("Unable to send message", e);
+          me.setPermanentFailure(possibleSend);
+          throw me;
         }
         finally
         {
         	close();
         }
+        
+        
+      
     }
 
     public void close() {