Implement a couple of the TODO items as patches:

- actually display the error messages from the remote server;
- treat a QUIT failure as non-fatal, since the message transaction should
  already have been completed;
- bounce messages immediately upon receiving a permanent delivery error;
- let "dma -bp" list all messages, including those that are locked because
  they are currently being delivered.

Also, fix two typos in crypto.c messages.

diff --git a/changelog b/changelog
index 3b5a9e43900ffd24fa6a1dfc2a0b44466d0e98b7..e6ab5ba5cac8f6d4d4cbb91e8c7fcd8b1dc57218 100644 (file)
--- a/changelog
+++ b/changelog
@@ -2,12 +2,8 @@ dma (0.0.2009.02.11-1~1) unstable; urgency=low
 
   * Initial release (Closes: #511410)
   * TODO:
-    - recognize permanent delivery errors;
-    - only warn on a QUIT error, treat the message as sent;
-    - find a way to list all queued mails;
     - use liblockfile in deliver_local() for /var/mail locking;
-    - RCPT TO failed: Success? :)
     - crontab an invocation of "dma -bp"
     - use debconf.
 
- -- Peter Pentchev <roam@ringlet.net>  Fri, 06 Mar 2009 15:28:33 +0200
+ -- Peter Pentchev <roam@ringlet.net>  Mon, 09 Mar 2009 16:14:23 +0200

diff --git a/patches/05-remote-error-messages.patch b/patches/05-remote-error-messages.patch
new file mode 100644 (file)
index 0000000..fc094fd
--- /dev/null
+++ b/patches/05-remote-error-messages.patch
@@ -0,0 +1,197 @@
+Store the last error or status message received from the remote server in
+the neterr[] buffer and display it instead of the meaningless %m in
+remote delivery syslog messages.
+
+--- a/crypto.c
++++ b/crypto.c
+@@ -122,7 +122,8 @@
+                       send_remote_command(fd, "STARTTLS");
+                       if (read_remote(fd, 0, NULL) != 2) {
+                               syslog(LOG_ERR, "%s: remote delivery failed:"
+-                                " STARTTLS not available: %m", it->queueid);
++                                " STARTTLS not available: %s", it->queueid,
++                                neterr);
+                               config->features &= ~NOSSL;
+                               return (-1);
+                       }
+@@ -267,7 +268,8 @@
+       send_remote_command(fd, "AUTH CRAM-MD5");
+       if (read_remote(fd, sizeof(buffer), buffer) != 3) {
+               syslog(LOG_ERR, "%s: smarthost authentification:"
+-                     " AUTH cram-md5 not available: %m", it->queueid);
++                     " AUTH cram-md5 not available: %s", it->queueid,
++                     neterr);
+               /* if cram-md5 is not available */
+               return (-1);
+       }
+@@ -296,7 +298,8 @@
+       send_remote_command(fd, "%s", temp);
+       if (read_remote(fd, 0, NULL) != 2) {
+               syslog(LOG_ERR, "%s: remote delivery deferred:"
+-                              " AUTH cram-md5 failed: %m", it->queueid);
++                              " AUTH cram-md5 failed: %s", it->queueid,
++                              neterr);
+               return (-2);
+       }
+ 
+--- a/dma.h
++++ b/dma.h
+@@ -139,6 +139,8 @@
+ 
+ extern struct aliases aliases;
+ 
++extern char neterr[BUF_SIZE];
++
+ /* aliases_parse.y */
+ extern int yyparse(void);
+ extern FILE *yyin;
+--- a/net.c
++++ b/net.c
+@@ -48,6 +48,7 @@
+ #endif /* HAVE_CRYPTO */
+ 
+ #include <err.h>
++#include <errno.h>
+ #include <netdb.h>
+ #include <setjmp.h>
+ #include <signal.h>
+@@ -59,6 +60,7 @@
+ extern struct config *config;
+ extern struct authusers authusers;
+ static jmp_buf timeout_alarm;
++char neterr[BUF_SIZE];
+ 
+ static void
+ sig_alarm(int signo __unused)
+@@ -99,10 +101,12 @@
+       int done = 0, status = 0, extbufpos = 0;
+ 
+       if (signal(SIGALRM, sig_alarm) == SIG_ERR) {
+-              syslog(LOG_ERR, "SIGALRM error: %m");
++              snprintf(neterr, sizeof(neterr), "SIGALRM error: %s",
++                  strerror(errno));
++              return (1);
+       }
+       if (setjmp(timeout_alarm) != 0) {
+-              syslog(LOG_ERR, "Timeout reached");
++              snprintf(neterr, sizeof(neterr), "Timeout reached");
+               return (1);
+       }
+       alarm(CON_TIMEOUT);
+@@ -163,6 +167,10 @@
+       }
+       alarm(0);
+ 
++      buff[len] = '\0';
++      while (len > 0 && (buff[len - 1] == '\r' || buff[len - 1] == '\n'))
++              buff[--len] = '\0';
++      snprintf(neterr, sizeof(neterr), "%s", buff);
+       status = atoi(buff);
+       return (status/100);
+ }
+@@ -194,7 +202,8 @@
+               send_remote_command(fd, "AUTH LOGIN");
+               if (read_remote(fd, 0, NULL) != 3) {
+                       syslog(LOG_ERR, "%s: remote delivery deferred:"
+-                                      " AUTH login not available: %m", it->queueid);
++                                      " AUTH login not available: %s",
++                                      it->queueid, neterr);
+                       return (1);
+               }
+ 
+@@ -205,7 +214,8 @@
+               send_remote_command(fd, "%s", temp);
+               if (read_remote(fd, 0, NULL) != 3) {
+                       syslog(LOG_ERR, "%s: remote delivery deferred:"
+-                                      " AUTH login failed: %m", it->queueid);
++                                      " AUTH login failed: %s", it->queueid,
++                                      neterr);
+                       return (-1);
+               }
+ 
+@@ -217,11 +227,13 @@
+               res = read_remote(fd, 0, NULL);
+               if (res == 5) {
+                       syslog(LOG_ERR, "%s: remote delivery failed:"
+-                                      " Authentication failed: %m", it->queueid);
++                                      " Authentication failed: %s",
++                                      it->queueid, neterr);
+                       return (-1);
+               } else if (res != 2) {
+                       syslog(LOG_ERR, "%s: remote delivery failed:"
+-                                      " AUTH password failed: %m", it->queueid);
++                                      " AUTH password failed: %s",
++                                      it->queueid, neterr);
+                       return (-1);
+               }
+       } else {
+@@ -341,7 +353,7 @@
+               send_remote_command(fd, "EHLO %s", hostname());
+               if (read_remote(fd, 0, NULL) != 2) {
+                       syslog(LOG_ERR, "%s: remote delivery deferred: "
+-                             " EHLO failed: %m", it->queueid);
++                             " EHLO failed: %s", it->queueid, neterr);
+                       return (-1);
+               }
+       }
+@@ -350,7 +362,7 @@
+               send_remote_command(fd, "EHLO %s", hostname());
+               if (read_remote(fd, 0, NULL) != 2) {
+                       syslog(LOG_ERR, "%s: remote delivery deferred: "
+-                             " EHLO failed: %m", it->queueid);
++                             " EHLO failed: %s", it->queueid, neterr);
+                       return (-1);
+               }
+       }
+@@ -388,27 +400,27 @@
+       send_remote_command(fd, "MAIL FROM:<%s>", it->sender);
+       if (read_remote(fd, 0, NULL) != 2) {
+               syslog(LOG_ERR, "%s: remote delivery deferred:"
+-                     " MAIL FROM failed: %m", it->queueid);
++                     " MAIL FROM failed: %s", it->queueid, neterr);
+               return (1);
+       }
+ 
+       send_remote_command(fd, "RCPT TO:<%s>", it->addr);
+       if (read_remote(fd, 0, NULL) != 2) {
+               syslog(LOG_ERR, "%s: remote delivery deferred:"
+-                              " RCPT TO failed: %m", it->queueid);
++                              " RCPT TO failed: %s", it->queueid, neterr);
+               return (1);
+       }
+ 
+       send_remote_command(fd, "DATA");
+       if (read_remote(fd, 0, NULL) != 3) {
+               syslog(LOG_ERR, "%s: remote delivery deferred:"
+-                     " DATA failed: %m", it->queueid);
++                     " DATA failed: %s", it->queueid, neterr);
+               return (1);
+       }
+ 
+       if (fseek(it->queuef, it->hdrlen, SEEK_SET) != 0) {
+-              syslog(LOG_ERR, "%s: remote delivery deferred: cannot seek: %m",
+-                     it->queueid);
++              syslog(LOG_ERR, "%s: remote delivery deferred: cannot seek: %s",
++                     it->queueid, neterr);
+               return (1);
+       }
+ 
+@@ -444,15 +456,15 @@
+ 
+       send_remote_command(fd, ".");
+       if (read_remote(fd, 0, NULL) != 2) {
+-              syslog(LOG_ERR, "%s: remote delivery deferred: %m",
+-                     it->queueid);
++              syslog(LOG_ERR, "%s: remote delivery deferred: %s",
++                     it->queueid, neterr);
+               return (1);
+       }
+ 
+       send_remote_command(fd, "QUIT");
+       if (read_remote(fd, 0, NULL) != 2) {
+               syslog(LOG_ERR, "%s: remote delivery deferred: "
+-                     "QUIT failed: %m", it->queueid);
++                     "QUIT failed: %s", it->queueid, neterr);
+               return (1);
+       }
+ out:

diff --git a/patches/06-quit-nonfatal.patch b/patches/06-quit-nonfatal.patch
new file mode 100644 (file)
index 0000000..b36c3a2
--- /dev/null
+++ b/patches/06-quit-nonfatal.patch
@@ -0,0 +1,21 @@
+Treat a QUIT error as merely a warning: RFC 2821 only mandates that
+a QUIT error should abort an unfinished transaction, and since we've
+reached this point, the DATA command has succeeded and the message has
+been accepted for delivery by the remote end.  Thus, just warn about it.
+
+--- a/net.c
++++ b/net.c
+@@ -462,11 +462,9 @@
+       }
+ 
+       send_remote_command(fd, "QUIT");
+-      if (read_remote(fd, 0, NULL) != 2) {
+-              syslog(LOG_ERR, "%s: remote delivery deferred: "
++      if (read_remote(fd, 0, NULL) != 2)
++              syslog(LOG_WARNING, "%s: remote delivery succeeded but "
+                      "QUIT failed: %s", it->queueid, neterr);
+-              return (1);
+-      }
+ out:
+ 
+       close(fd);

diff --git a/patches/07-permanent-errors.patch b/patches/07-permanent-errors.patch
new file mode 100644 (file)
index 0000000..384cf33
--- /dev/null
+++ b/patches/07-permanent-errors.patch
@@ -0,0 +1,60 @@
+Recognize permanent delivery errors on remote deliveries and
+bounce the message immediately.
+
+--- a/net.c
++++ b/net.c
+@@ -397,26 +397,26 @@
+                                       " Try without", it->queueid);
+       }
+ 
+-      send_remote_command(fd, "MAIL FROM:<%s>", it->sender);
+-      if (read_remote(fd, 0, NULL) != 2) {
+-              syslog(LOG_ERR, "%s: remote delivery deferred:"
+-                     " MAIL FROM failed: %s", it->queueid, neterr);
+-              return (1);
++#define READ_REMOTE_CHECK(c, exp)     \
++      res = read_remote(fd, 0, NULL); \
++      if (res == 5) { \
++              syslog(LOG_ERR, "%s: remote delivery failed: " \
++                     c " failed: %s", it->queueid, neterr); \
++              return (-1); \
++      } else if (res != exp) { \
++              syslog(LOG_ERR, "%s: remote delivery deferred: " \
++                     c " failed: %s", it->queueid, neterr); \
++              return (1); \
+       }
+ 
++      send_remote_command(fd, "MAIL FROM:<%s>", it->sender);
++      READ_REMOTE_CHECK("MAIL FROM", 2);
++
+       send_remote_command(fd, "RCPT TO:<%s>", it->addr);
+-      if (read_remote(fd, 0, NULL) != 2) {
+-              syslog(LOG_ERR, "%s: remote delivery deferred:"
+-                              " RCPT TO failed: %s", it->queueid, neterr);
+-              return (1);
+-      }
++      READ_REMOTE_CHECK("RCPT TO", 2);
+ 
+       send_remote_command(fd, "DATA");
+-      if (read_remote(fd, 0, NULL) != 3) {
+-              syslog(LOG_ERR, "%s: remote delivery deferred:"
+-                     " DATA failed: %s", it->queueid, neterr);
+-              return (1);
+-      }
++      READ_REMOTE_CHECK("DATA", 3);
+ 
+       if (fseek(it->queuef, it->hdrlen, SEEK_SET) != 0) {
+               syslog(LOG_ERR, "%s: remote delivery deferred: cannot seek: %s",
+@@ -455,11 +455,7 @@
+       }
+ 
+       send_remote_command(fd, ".");
+-      if (read_remote(fd, 0, NULL) != 2) {
+-              syslog(LOG_ERR, "%s: remote delivery deferred: %s",
+-                     it->queueid, neterr);
+-              return (1);
+-      }
++      READ_REMOTE_CHECK("final DATA", 2);
+ 
+       send_remote_command(fd, "QUIT");
+       if (read_remote(fd, 0, NULL) != 2)

diff --git a/patches/08-queue-list-all.patch b/patches/08-queue-list-all.patch
new file mode 100644 (file)
index 0000000..8440a4c
--- /dev/null
+++ b/patches/08-queue-list-all.patch
@@ -0,0 +1,93 @@
+When listing the queue, show all messages, even those that are
+currently locked for delivery.  Indicate the lock with a '*' after
+the queue ID.
+
+--- a/dma.c
++++ b/dma.c
+@@ -667,7 +667,7 @@
+ }
+ 
+ static void
+-load_queue(struct queue *queue)
++load_queue(struct queue *queue, int nolock)
+ {
+       struct stat st;
+       struct qitem *it;
+@@ -683,7 +683,7 @@
+       char *queueid;
+       char *queuefn;
+       off_t hdrlen;
+-      int fd;
++      int fd, locked;
+ 
+       LIST_INIT(&queue->queue);
+ 
+@@ -705,12 +705,18 @@
+                       continue;
+               if (asprintf(&queuefn, "%s/%s", config->spooldir, de->d_name) < 0)
+                       goto fail;
++              locked = 0;
+               fd = open_locked(queuefn, O_RDONLY|O_NONBLOCK);
+               if (fd < 0) {
+                       /* Ignore locked files */
+-                      if (errno == EWOULDBLOCK)
++                      if (errno != EWOULDBLOCK)
++                              goto skip_item;
++                      if (!nolock)
+                               continue;
+-                      goto skip_item;
++                      fd = open(queuefn, O_RDONLY);
++                      if (fd < 0)
++                              goto skip_item;
++                      locked = 1;
+               }
+ 
+               queuef = fdopen(fd, "r");
+@@ -751,6 +757,7 @@
+                       it->queuef = queuef;
+                       it->queueid = queueid;
+                       it->queuefn = fn;
++                      it->locked = locked;
+                       fn = NULL;
+               }
+               if (LIST_EMPTY(&itmqueue.queue)) {
+@@ -810,9 +817,9 @@
+ 
+       LIST_FOREACH(it, &queue->queue, next) {
+               printf("\
+-ID\t: %s\n\
++ID\t: %s%s\n\
+ From\t: %s\n\
+-To\t: %s\n--\n", it->queueid, it->sender, it->addr);
++To\t: %s\n--\n", it->queueid, it->locked? "*": "", it->sender, it->addr);
+       }
+ }
+ 
+@@ -914,7 +921,7 @@
+               if (argc != 0)
+                       errx(1, "sending mail and displaying queue is"
+                               " mutually exclusive");
+-              load_queue(&lqueue);
++              load_queue(&lqueue, 1);
+               show_queue(&lqueue);
+               return (0);
+       }
+@@ -922,7 +929,7 @@
+       if (doqueue) {
+               if (argc != 0)
+                       errx(1, "sending mail and queue pickup is mutually exclusive");
+-              load_queue(&lqueue);
++              load_queue(&lqueue, 0);
+               run_queue(&lqueue);
+               return (0);
+       }
+--- a/dma.h
++++ b/dma.h
+@@ -97,6 +97,7 @@
+       FILE *queuef;
+       off_t hdrlen;
+       int remote;
++      int locked;
+ };
+ LIST_HEAD(queueh, qitem);
+ 

diff --git a/patches/09-typos.patch b/patches/09-typos.patch
new file mode 100644 (file)
index 0000000..132d1fb
--- /dev/null
+++ b/patches/09-typos.patch
@@ -0,0 +1,22 @@
+Fix two typos and a word order problem.
+
+--- a/crypto.c
++++ b/crypto.c
+@@ -155,7 +155,7 @@
+       error = SSL_connect(config->ssl);
+       if (error < 0) {
+               syslog(LOG_ERR, "%s: remote delivery failed:"
+-                     " SSL handshake fataly failed: %m", it->queueid);
++                     " SSL handshake failed fatally: %m", it->queueid);
+               return (-1);
+       }
+ 
+@@ -163,7 +163,7 @@
+       cert = SSL_get_peer_certificate(config->ssl);
+       if (cert == NULL) {
+               syslog(LOG_ERR, "%s: remote delivery deferred:"
+-                     " Peer did not provied certificate: %m", it->queueid);
++                     " Peer did not provide certificate: %m", it->queueid);
+       }
+       X509_free(cert);
+ 

diff --git a/patches/series b/patches/series
index 1c54b77b3819f1f64259d9dfeb294f2d135c2afc..ad7b01ef33f66988dfbbde1b4bd89c35a23cd40d 100644 (file)
--- a/patches/series
+++ b/patches/series
@@ -2,3 +2,8 @@
 02-man-dragonflybsd.patch
 03-debian-locations.patch
 04-debian-setgid.patch
+05-remote-error-messages.patch
+06-quit-nonfatal.patch
+07-permanent-errors.patch
+08-queue-list-all.patch
+09-typos.patch