From bd88f5a0c1cfde856edfbecac9a3249daacf074a Mon Sep 17 00:00:00 2001
From: op <op@openbsd.org>
Date: Thu, 23 May 2024 17:10:00 +0000
Subject: [PATCH] require an error message on table_proc failure replies

The error message is not really used, it just gets translated to a
TEMPFAIL, but it allows to have mandatory logging of errors instead
of relying on tables to hopefully log something.

To ease the transition, don't make it mandatory in smtpd(8) yet,
but document it as such.

ok millert@, gilles@
---
 usr.sbin/smtpd/smtpd-tables.7 | 16 ++++++++--------
 usr.sbin/smtpd/table_proc.c   | 30 +++++++++++++++++++++++++-----
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/usr.sbin/smtpd/smtpd-tables.7 b/usr.sbin/smtpd/smtpd-tables.7
index 2ecf72f23fb..c5cd954e232 100644
--- a/usr.sbin/smtpd/smtpd-tables.7
+++ b/usr.sbin/smtpd/smtpd-tables.7
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: smtpd-tables.7,v 1.2 2024/05/22 08:39:50 op Exp $
+.\"	$OpenBSD: smtpd-tables.7,v 1.3 2024/05/23 17:10:00 op Exp $
 .\"
 .\" Copyright (c) 2008 Janne Johansson <jj@openbsd.org>
 .\" Copyright (c) 2009 Jacek Masiulaniec <jacekm@dobremiasto.net>
@@ -18,7 +18,7 @@
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
 .\"
-.Dd $Mdocdate: May 22 2024 $
+.Dd $Mdocdate: May 23 2024 $
 .Dt SMTPD-TABLES 7
 .Os
 .Sh NAME
@@ -144,7 +144,7 @@ The result is either
 .Sq ok
 on success or
 .Sq error
-upon a failure to do so.
+and a message upon a failure to do so.
 .It Cm check Ar service id query
 Check whether
 .Ar query
@@ -155,7 +155,7 @@ if found,
 .Sq not-found
 if not, or
 .Sq error
-upon an error.
+and a message upon an error.
 .It Cm lookup Ar service id query
 Look up a value in the table for given the
 .Ar query .
@@ -165,7 +165,7 @@ and the value if found,
 .Sq not-found
 if not found, or
 .Sq error
-upon an error.
+and a message upon an error.
 .It Cm fetch Ar service id
 Fetch the next item from the table, eventually wrapping around.
 It is only supported for the
@@ -179,7 +179,7 @@ and the value if found,
 .Sq not-found
 if the table is empty, or
 .Sq error
-upon an error.
+and a message upon an error.
 .El
 .Pp
 Each service has a specific format for the result.
@@ -221,12 +221,12 @@ Used to map IP addresses to hostnames.
 .Sh EXAMPLES
 Assuming the table is called
 .Dq devs ,
-here's an example of a successful
+here's an example of a failed
 .Cm update
 transaction:
 .Bd -literal -offset indent
 table|0.1|1713795097.394049|devs|update|478ff0d2
-update-result|478ff0d2|ok
+update-result|478ff0d2|error|failed to connect to the database
 .Ed
 .Pp
 A
diff --git a/usr.sbin/smtpd/table_proc.c b/usr.sbin/smtpd/table_proc.c
index f21fd2b5095..e7f2a736bbd 100644
--- a/usr.sbin/smtpd/table_proc.c
+++ b/usr.sbin/smtpd/table_proc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: table_proc.c,v 1.21 2024/05/23 17:05:45 op Exp $	*/
+/*	$OpenBSD: table_proc.c,v 1.22 2024/05/23 17:10:00 op Exp $	*/
 
 /*
  * Copyright (c) 2024 Omar Polo <op@openbsd.org>
@@ -186,8 +186,15 @@ table_proc_update(struct table *table)
 	r = table_proc_recv(table, "update-result");
 	if (!strcmp(r, "ok"))
 		return (1);
-	if (!strcmp(r, "error"))
+
+	if (!strncmp(r, "error", 5)) {
+		if (r[5] == '|') {
+			r += 6;
+			log_warnx("warn: table-proc: %s update failed: %s",
+			    table->t_name, r);
+		}
 		return (0);
+	}
 
 	log_warnx("warn: table-proc: failed parse reply");
 	fatalx("table-proc: exiting");
@@ -222,11 +229,17 @@ table_proc_lookup(struct table *table, enum table_service s, const char *k, char
 	table_proc_send(table, req, s, k);
 	r = table_proc_recv(table, res);
 
-	/* common replies */
 	if (!strcmp(r, "not-found"))
 		return (0);
-	if (!strcmp(r, "error"))
+
+	if (!strncmp(r, "error", 5)) {
+		if (r[5] == '|') {
+			r += 6;
+			log_warnx("warn: table-proc: %s %s failed: %s",
+			    table->t_name, req, r);
+		}
 		return (-1);
+	}
 
 	if (dst == NULL) {
 		/* check op */
@@ -261,8 +274,15 @@ table_proc_fetch(struct table *table, enum table_service s, char **dst)
 
 	if (!strcmp(r, "not-found"))
 		return (0);
-	if (!strcmp(r, "error"))
+
+	if (!strncmp(r, "error", 5)) {
+		if (r[5] == '|') {
+			r += 6;
+			log_warnx("warn: table-proc: %s fetch failed: %s",
+			    table->t_name, r);
+		}
 		return (-1);
+	}
 
 	if (strncmp(r, "found|", 6) != 0) {
 		log_warnx("warn: table-proc: failed to parse reply");
-- 
2.20.1