From: Peter Pentchev Date: Mon, 9 Mar 2009 14:35:10 +0000 (+0000) Subject: Implement a couple of the TODO items as patches: X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2f4203a7d8714de7e7b0813c552d603d52dbef42;p=people%2Fms%2Fdma.git 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 3b5a9e4..e6ab5ba 100644 --- 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 Fri, 06 Mar 2009 15:28:33 +0200 + -- Peter Pentchev 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 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 ++#include + #include + #include + #include +@@ -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 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 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 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 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 1c54b77..ad7b01e 100644 --- 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