changeset 776:de31a9e15c5b

Improved and more consistent closing of SMTP sessions
author Ben Schmidt
date Fri, 12 Nov 2010 02:30:00 +1100
parents d0bf2135ab34
children 5f038f36f66f
files ChangeLog src/mlmmj-send.c
diffstat 2 files changed, 39 insertions(+), 28 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Fri Nov 12 02:27:50 2010 +1100
+++ b/ChangeLog	Fri Nov 12 02:30:00 2010 +1100
@@ -1,3 +1,4 @@
+ o Improved and more consistent closing of SMTP sessions in error cases
  o Check the relayhost gives a reply before reading it to avoid a crash
  o Avoid checking addresses multiple times for notmetoo and make it work even
    when delivering messages individually
--- a/src/mlmmj-send.c	Fri Nov 12 02:27:50 2010 +1100
+++ b/src/mlmmj-send.c	Fri Nov 12 02:30:00 2010 +1100
@@ -420,7 +420,6 @@
 	}
 
 	close(*sockfd);
-
 	*sockfd = -1;
 
 	return retval;
@@ -1008,11 +1007,11 @@
 	case '1': /* A single mail is to be sent */
 	case '6':
 		initsmtp(&sockfd, relay, smtpport);
-		sendres = send_mail(sockfd, bounceaddr, to_addr, replyto,
+		if(send_mail(sockfd, bounceaddr, to_addr, replyto,
 				mailmap, st.st_size, listdir, NULL,
-				hdrs, hdrslen, body, bodylen);
-		endsmtp(&sockfd);
-		if(sendres) {
+				hdrs, hdrslen, body, bodylen)) {
+			close(sockfd);
+			sockfd = -1;
 			/* error, so keep it in the queue */
 			deletewhensent = 0;
 			/* dump data we want when resending first check
@@ -1060,6 +1059,8 @@
 				}
 				close(tmpfd);
 			}
+		} else {
+			endsmtp(&sockfd);
 		}
 		break;
 	case '2': /* Moderators */
@@ -1067,20 +1068,24 @@
 		if(send_mail_many_fd(sockfd, bounceaddr, NULL, mailmap,
 				     st.st_size, subfd, NULL, NULL, NULL,
 				     listdir, NULL, hdrs, hdrslen,
-				     body, bodylen))
+				     body, bodylen)) {
 			close(sockfd);
-		else
+			sockfd = -1;
+		} else {
 			endsmtp(&sockfd);
+		}
 		break;
 	case '3': /* resending earlier failed mails */
 		initsmtp(&sockfd, relay, smtpport);
 		if(send_mail_many_fd(sockfd, NULL, NULL, mailmap, st.st_size,
 				subfd, listaddr, listdelim, mailfilename,
 				listdir, mlmmjbounce, hdrs, hdrslen,
-				body, bodylen))
+				body, bodylen)) {
 			close(sockfd);
-		else
+			sockfd = -1;
+		} else {
 			endsmtp(&sockfd);
+		}
 		unlink(subfilename);
 		break;
 	case '4': /* send mails to owner */
@@ -1088,18 +1093,20 @@
 		if(send_mail_many_fd(sockfd, bounceaddr, NULL, mailmap,
 				st.st_size, subfd, listaddr, listdelim,
 				mailfilename, listdir, mlmmjbounce,
-				hdrs, hdrslen, body, bodylen))
+				hdrs, hdrslen, body, bodylen)) {
 			close(sockfd);
-		else
+			sockfd = -1;
+		} else {
 			endsmtp(&sockfd);
+		}
 		break;
 	case '5': /* bounceprobe - handle relayhost local users bouncing*/
 		initsmtp(&sockfd, relay, smtpport);
-		sendres = send_mail(sockfd, bounceaddr, to_addr, replyto,
+		if(send_mail(sockfd, bounceaddr, to_addr, replyto,
 				mailmap, st.st_size, listdir, NULL,
-				hdrs, hdrslen, body, bodylen);
-		endsmtp(&sockfd);
-		if(sendres) {
+				hdrs, hdrslen, body, bodylen)) {
+			close(sockfd);
+			sockfd = -1;
 			/* error, so remove the probefile */
 			tmpstr = mystrdup(to_addr);
 			a = strchr(tmpstr, '@');
@@ -1110,6 +1117,8 @@
 			unlink(probefile);
 			myfree(probefile);
 			myfree(tmpstr);
+		} else {
+			endsmtp(&sockfd);
 		}
 		break;
 	case '7':
@@ -1173,6 +1182,8 @@
 					verp = NULL;
 				}
 			}
+			/* We can't be in SMTP DATA state or anything like
+			 * that, so should be able to safely QUIT. */
 			endsmtp(&sockfd);
 		}
 
@@ -1238,7 +1249,12 @@
 								hdrs, hdrslen,
 								body, bodylen);
 					}
+				    	if (sendres) {
+					    	close(sockfd);
+					    	sockfd = -1;
+				    	} else {
 					endsmtp(&sockfd);
+				    	}
 					for(i = 0; i < stl.count; i++)
 						myfree(stl.strs[i]);
 					stl.count = 0;
@@ -1266,21 +1282,16 @@
 						mlmmjbounce, hdrs, hdrslen,
 						body, bodylen);
 			}
+			if (sendres) {
+				close(sockfd);
+				sockfd = -1;
+			} else {
 			endsmtp(&sockfd);
+			}
 			for(i = 0; i < stl.count; i++)
 				myfree(stl.strs[i]);
 			stl.count = 0;
 		}
-
-		if (sendres) {
-			/* If send_mail_many() failed we close the
-			 * connection to the mail server in a brutal
-			 * manner, because we could be in any state
-			 * (DATA for instance). */
-			close(sockfd);
-		} else {
-			endsmtp(&sockfd);
-		}
 		myfree(stl.strs);
 		myfree(verpfrom);
 		closedir(subddir);
@@ -1296,7 +1307,6 @@
 	myfree(hdrs);
 	myfree(body);
 	myfree(mlmmjbounce);
-	close(sockfd);
 	munmap(mailmap, st.st_size);
 	close(mailfd);
 	myfree(verp);