dovecot-2.2: lib-lda, lmtp: Separate internal errors from remote...

dovecot at dovecot.org dovecot at dovecot.org
Fri Oct 3 13:32:04 UTC 2014


details:   http://hg.dovecot.org/dovecot-2.2/rev/f21d82a32ca8
changeset: 17866:f21d82a32ca8
user:      Timo Sirainen <tss at iki.fi>
date:      Fri Oct 03 16:31:33 2014 +0300
description:
lib-lda, lmtp: Separate internal errors from remote errors.
LMTP proxy shouldn't log remote errors with error level, because the proxy
itself didn't have any failure.

This is an API change, but I'm not aware of any plugins actually using the
lmtp-client.h directly.

diffstat:

 src/lib-lda/lmtp-client.c |  51 +++++++++++++++++++++++++++++++++++-----------
 src/lib-lda/lmtp-client.h |  15 +++++++++++-
 src/lib-lda/smtp-client.c |   8 +++---
 src/lmtp/lmtp-proxy.c     |  29 +++++++++++++++++++-------
 4 files changed, 77 insertions(+), 26 deletions(-)

diffs (277 lines):

diff -r 026cf7ee272c -r f21d82a32ca8 src/lib-lda/lmtp-client.c
--- a/src/lib-lda/lmtp-client.c	Fri Oct 03 16:04:06 2014 +0300
+++ b/src/lib-lda/lmtp-client.c	Fri Oct 03 16:31:33 2014 +0300
@@ -74,6 +74,7 @@
 	unsigned int rcpt_to_successes:1;
 	unsigned int output_finished:1;
 	unsigned int finish_called:1;
+	unsigned int global_remote_failure:1;
 };
 
 static void lmtp_client_send_rcpts(struct lmtp_client *client);
@@ -198,17 +199,22 @@
 	return "??";
 }
 
-void lmtp_client_fail(struct lmtp_client *client, const char *line)
+static void
+lmtp_client_fail_full(struct lmtp_client *client, const char *line, bool remote)
 {
+	enum lmtp_client_result result;
 	struct lmtp_rcpt *recipients;
 	unsigned int i, count;
 
 	client->global_fail_string = p_strdup(client->pool, line);
+	client->global_remote_failure = remote;
+	result = remote ? LMTP_CLIENT_RESULT_REMOTE_ERROR :
+		LMTP_CLIENT_RESULT_INTERNAL_ERROR;
 
 	lmtp_client_ref(client);
 	recipients = array_get_modifiable(&client->recipients, &count);
 	for (i = client->rcpt_next_receive_idx; i < count; i++) {
-		recipients[i].rcpt_to_callback(FALSE, line,
+		recipients[i].rcpt_to_callback(result, line,
 					       recipients[i].context);
 		recipients[i].failed = TRUE;
 	}
@@ -216,7 +222,7 @@
 
 	for (i = client->rcpt_next_data_idx; i < count; i++) {
 		if (!recipients[i].failed) {
-			recipients[i].data_callback(FALSE, line,
+			recipients[i].data_callback(result, line,
 						    recipients[i].context);
 		}
 	}
@@ -227,21 +233,33 @@
 }
 
 static void
+lmtp_client_fail_remote(struct lmtp_client *client, const char *line)
+{
+	lmtp_client_fail_full(client, line, TRUE);
+}
+
+void lmtp_client_fail(struct lmtp_client *client, const char *line)
+{
+	lmtp_client_fail_full(client, line, FALSE);
+}
+
+static void
 lmtp_client_rcpt_next(struct lmtp_client *client, const char *line)
 {
 	struct lmtp_rcpt *rcpt;
-	bool success;
+	enum lmtp_client_result result;
 
-	success = line[0] == '2';
-	if (success)
+	result = line[0] == '2' ? LMTP_CLIENT_RESULT_OK :
+		LMTP_CLIENT_RESULT_REMOTE_ERROR;
+	if (result == LMTP_CLIENT_RESULT_OK)
 		client->rcpt_to_successes = TRUE;
 
 	rcpt = array_idx_modifiable(&client->recipients,
 				    client->rcpt_next_receive_idx);
 	client->rcpt_next_receive_idx++;
 
-	rcpt->failed = !success;
-	rcpt->rcpt_to_callback(success, line, rcpt->context);
+	rcpt->failed = result != LMTP_CLIENT_RESULT_OK;
+	rcpt->rcpt_to_callback(result, line, rcpt->context);
 }
 
 static int lmtp_client_send_data_cmd(struct lmtp_client *client)
@@ -250,7 +268,8 @@
 		return 0;
 
 	if (client->global_fail_string != NULL || !client->rcpt_to_successes) {
-		lmtp_client_fail(client, client->global_fail_string);
+		lmtp_client_fail_full(client, client->global_fail_string,
+				      client->global_remote_failure);
 		return -1;
 	} else {
 		client->input_state++;
@@ -264,6 +283,7 @@
 {
 	struct lmtp_rcpt *rcpt;
 	unsigned int i, count;
+	enum lmtp_client_result result;
 
 	rcpt = array_get_modifiable(&client->recipients, &count);
 	for (i = client->rcpt_next_data_idx; i < count; i++) {
@@ -274,7 +294,9 @@
 
 		client->rcpt_next_data_idx = i + 1;
 		rcpt[i].failed = line[0] != '2';
-		rcpt[i].data_callback(!rcpt[i].failed, line, rcpt[i].context);
+		result = rcpt[i].failed ? LMTP_CLIENT_RESULT_REMOTE_ERROR :
+			LMTP_CLIENT_RESULT_OK;
+		rcpt[i].data_callback(result, line, rcpt[i].context);
 		if (client->protocol == LMTP_CLIENT_PROTOCOL_LMTP)
 			break;
 	}
@@ -529,7 +551,7 @@
 	case LMTP_INPUT_STATE_DATA_CONTINUE:
 		/* Start sending DATA */
 		if (strncmp(line, "354", 3) != 0) {
-			lmtp_client_fail(client, line);
+			lmtp_client_fail_remote(client, line);
 			return -1;
 		}
 		client->input_state++;
@@ -728,6 +750,7 @@
 			  lmtp_callback_t *data_callback, void *context)
 {
 	struct lmtp_rcpt *rcpt;
+	enum lmtp_client_result result;
 
 	rcpt = array_append_space(&client->recipients);
 	rcpt->address = p_strdup(client->pool, address);
@@ -736,12 +759,16 @@
 	rcpt->context = context;
 
 	if (client->global_fail_string != NULL) {
+		/* we've already failed */
 		client->rcpt_next_receive_idx++;
 		i_assert(client->rcpt_next_receive_idx ==
 			 array_count(&client->recipients));
 
+		result = client->global_remote_failure ?
+			LMTP_CLIENT_RESULT_REMOTE_ERROR :
+			LMTP_CLIENT_RESULT_INTERNAL_ERROR;
 		rcpt->failed = TRUE;
-		rcpt_to_callback(FALSE, client->global_fail_string, context);
+		rcpt_to_callback(result, client->global_fail_string, context);
 	} else if (client->input_state == LMTP_INPUT_STATE_RCPT_TO)
 		lmtp_client_send_rcpts(client);
 }
diff -r 026cf7ee272c -r f21d82a32ca8 src/lib-lda/lmtp-client.h
--- a/src/lib-lda/lmtp-client.h	Fri Oct 03 16:04:06 2014 +0300
+++ b/src/lib-lda/lmtp-client.h	Fri Oct 03 16:31:33 2014 +0300
@@ -12,6 +12,16 @@
 	LMTP_CLIENT_PROTOCOL_SMTP
 };
 
+enum lmtp_client_result {
+	/* Command succeeded */
+	LMTP_CLIENT_RESULT_OK = 1,
+	/* Command failed because remote server returned an error */
+	LMTP_CLIENT_RESULT_REMOTE_ERROR = 0,
+	/* Command failed because of an internal error (e.g. couldn't connect
+	   to remote) */
+	LMTP_CLIENT_RESULT_INTERNAL_ERROR = -1
+};
+
 struct lmtp_client_settings {
 	const char *my_hostname;
 	const char *mail_from;
@@ -31,7 +41,8 @@
 
 /* reply contains the reply coming from remote server, or NULL
    if it's a connection error. */
-typedef void lmtp_callback_t(bool success, const char *reply, void *context);
+typedef void lmtp_callback_t(enum lmtp_client_result result,
+			     const char *reply, void *context);
 /* called when session is finished, either because all RCPT TOs failed or
    because all DATA replies have been received. */
 typedef void lmtp_finish_callback_t(void *context);
@@ -61,7 +72,7 @@
    This is useful with non-blocking istreams and tee-istreams. */
 void lmtp_client_send_more(struct lmtp_client *client);
 /* Fail the connection with line as the reply to unfinished RCPT TO/DATA
-   replies. */
+   replies. This will be treated as an internal failure. */
 void lmtp_client_fail(struct lmtp_client *client, const char *line);
 /* Return the state (command reply) the client is currently waiting for. */
 const char *lmtp_client_state_to_string(struct lmtp_client *client);
diff -r 026cf7ee272c -r f21d82a32ca8 src/lib-lda/smtp-client.c
--- a/src/lib-lda/smtp-client.c	Fri Oct 03 16:04:06 2014 +0300
+++ b/src/lib-lda/smtp-client.c	Fri Oct 03 16:31:33 2014 +0300
@@ -217,11 +217,11 @@
 }
 
 static void
-rcpt_to_callback(bool success, const char *reply, void *context)
+rcpt_to_callback(enum lmtp_client_result result, const char *reply, void *context)
 {
 	struct smtp_client *smtp_client = context;
 
-	if (!success) {
+	if (result != LMTP_CLIENT_RESULT_OK) {
 		if (reply[0] != '5')
 			smtp_client->tempfail = TRUE;
 		smtp_client_error(smtp_client, t_strdup_printf(
@@ -231,11 +231,11 @@
 }
 
 static void
-data_callback(bool success, const char *reply, void *context)
+data_callback(enum lmtp_client_result result, const char *reply, void *context)
 {
 	struct smtp_client *smtp_client = context;
 
-	if (!success) {
+	if (result != LMTP_CLIENT_RESULT_OK) {
 		if (reply[0] != '5')
 			smtp_client->tempfail = TRUE;
 		smtp_client_error(smtp_client, t_strdup_printf(
diff -r 026cf7ee272c -r f21d82a32ca8 src/lmtp/lmtp-proxy.c
--- a/src/lmtp/lmtp-proxy.c	Fri Oct 03 16:04:06 2014 +0300
+++ b/src/lmtp/lmtp-proxy.c	Fri Oct 03 16:31:33 2014 +0300
@@ -220,7 +220,8 @@
 }
 
 static void
-lmtp_proxy_conn_rcpt_to(bool success, const char *reply, void *context)
+lmtp_proxy_conn_rcpt_to(enum lmtp_client_result result,
+			const char *reply, void *context)
 {
 	struct lmtp_proxy_recipient *rcpt = context;
 	struct lmtp_proxy_connection *conn = rcpt->conn;
@@ -228,11 +229,12 @@
 	i_assert(rcpt->reply == NULL);
 
 	rcpt->reply = p_strdup(conn->proxy->pool, reply);
-	rcpt->rcpt_to_failed = !success;
+	rcpt->rcpt_to_failed = result != LMTP_CLIENT_RESULT_OK;
 }
 
 static void
-lmtp_proxy_conn_data(bool success, const char *reply, void *context)
+lmtp_proxy_conn_data(enum lmtp_client_result result,
+		     const char *reply, void *context)
 {
 	struct lmtp_proxy_recipient *rcpt = context;
 	struct lmtp_proxy_connection *conn = rcpt->conn;
@@ -247,14 +249,25 @@
 	rcpt->reply = p_strdup(conn->proxy->pool, reply);
 	rcpt->data_reply_received = TRUE;
 
-	if (!success) {
+	switch (result) {
+	case LMTP_CLIENT_RESULT_OK:
+		i_info("%s: Sent message to <%s> at %s:%u: %s",
+		       conn->proxy->set.session_id, rcpt->address,
+		       conn->set.host, conn->set.port, reply);
+		break;
+	case LMTP_CLIENT_RESULT_REMOTE_ERROR:
+		/* the problem isn't with the proxy, it's with the remote side.
+		   so the remote side will log an error, while for us this is
+		   just an info event */
+		i_info("%s: Failed to send message to <%s> at %s:%u: %s",
+		       conn->proxy->set.session_id, rcpt->address,
+		       conn->set.host, conn->set.port, reply);
+		break;
+	case LMTP_CLIENT_RESULT_INTERNAL_ERROR:
 		i_error("%s: Failed to send message to <%s> at %s:%u: %s",
 			conn->proxy->set.session_id, rcpt->address,
 			conn->set.host, conn->set.port, reply);
-	} else {
-		i_info("%s: Sent message to <%s> at %s:%u: %s",
-		       conn->proxy->set.session_id, rcpt->address,
-		       conn->set.host, conn->set.port, reply);
+		break;
 	}
 
 	lmtp_proxy_try_finish(conn->proxy);


More information about the dovecot-cvs mailing list