client: handling SMTP errors from server

In case server has some settings, which don't let some
types of SMTP request to be handled, SMTP client may receive
5xx, 4xx, 3xx codes. This patch fixes the problem, when
on the client side this SMTP codes are not handled normally.
Instead of runtime error and unusual message that there is
some connection error.

Fixes #13

Author: vr009

.github/workflows/testing.yml

@@ -194,7 +194,7 @@ jobs:
           #
           # v3 is to re-verify all Mac OS builds after fix for the
           # gh-6076 problem (see below).
-          key: ${{ matrix.runs-on }}-${{ matrix.tarantool }}-v3
+          key: ${{ matrix.runs-on }}-${{ matrix.tarantool }}-v3-
         if: matrix.tarantool != 'brew' && matrix.tarantool != 'master'
 
       - name: Install tarantool build dependencies

smtp/smtpc.c

@@ -241,6 +241,14 @@ smtpc_request_new(struct smtpc_env *env, const char *url, const char *from)
 		return NULL;
 	}
 	memset(req, 0, sizeof(*req));
+
+	req->error_buf = calloc(CURL_ERROR_SIZE, sizeof(char));
+	if (req->error_buf == NULL) {
+		free(req);
+		box_error_set(__FILE__, __LINE__, ER_MEMORY_ISSUE,
+					  "Can't alloc error message buffer");
+		return NULL;
+	}
 	req->env = env;
 
 	req->easy = curl_easy_init();
@@ -252,6 +260,7 @@ smtpc_request_new(struct smtpc_env *env, const char *url, const char *from)
 	}
 	curl_easy_setopt(req->easy, CURLOPT_URL, url);
 	curl_easy_setopt(req->easy, CURLOPT_MAIL_FROM, from);
+	curl_easy_setopt(req->easy, CURLOPT_ERRORBUFFER, req->error_buf);
 
 	return req;
 }
@@ -270,6 +279,7 @@ smtpc_request_delete(struct smtpc_request *req)
 	if (req->easy != NULL)
 		coio_call(smtpc_task_delete, req);
 	free(req->body);
+	free(req->error_buf);
 	if (req->recipients)
 		curl_slist_free_all(req->recipients);
 
@@ -440,10 +450,25 @@ smtpc_execute(struct smtpc_request *req, double timeout)
 		++env->stat.failed_requests;
 		smtpc_request_delete(req);
 		return -1;
+	case CURLE_SEND_ERROR:
+	case CURLE_RECV_ERROR:
+		curl_easy_getinfo(req->easy, CURLINFO_RESPONSE_CODE, &longval);
+		req->status = (int) longval;
+		/*
+		 * When client can not read bytes from another side that
+		 * leads to a message about failed response reading but
+		 * response code stays 250, that is not correct.
+		 */
+		if (strcmp(req->error_buf, "response reading failed") == 0) {
+			req->status = -1;
+		}
+		req->reason = req->error_buf;
+		++env->stat.failed_requests;
+		break;
 	default: {
 		char error_msg[256];
 		curl_easy_getinfo(req->easy, CURLINFO_OS_ERRNO, &longval);
-		snprintf(error_msg, sizeof(error_msg), "SMTP error %i (os errno %li)", req->code, longval);
+		snprintf(error_msg, sizeof(error_msg), "CURL error %i (os errno %li)", req->code, longval);
 		box_error_set(__FILE__, __LINE__, ER_UNKNOWN, error_msg);
 		++env->stat.failed_requests;
 		smtpc_request_delete(req);

smtp/smtpc.h

@@ -112,10 +112,27 @@ struct smtpc_request {
 	int body_size;
 	/** Buffer read position. */
 	char *body_rpos;
-	/** SMTP status code. */
+	/**
+	 * SMTP status code.
+	 * It takes the value of -1 if there is some problem,
+	 * which is not related to SMTP, like connection error.
+	 */
 	int status;
-	/** Error message. */
+	/**
+	 * Error message.
+	 * It is a string to report details of an error to a user.
+	 * Does not require freeing.
+	 * It is never NULL if smtpc_execute() returns zero.
+	 */
 	const char *reason;
+	/**
+	 * Error buffer for receiving messages.
+	 * It should exist during a request lifetime and
+	 * must be freed at freeing the request structure.
+	 * This field is not for reading directly, cause
+	 * reason field points to it, when appropriate.
+	 */
+	char *error_buf;
 };
 
 /**

test/smtp.test.lua

@@ -10,6 +10,22 @@ local os = require('os')
 test:plan(1)
 mails = fiber.channel(100)
 
+function write_reply_code(s, l)
+    if l:find('3xx') then
+        s:write('354 Start mail input\r\n')
+    elseif l:find('4xx') then
+        s:write('421 Service not available, closing transmission channel\r\n')
+    elseif l:find('5xx') then
+        s:write('510 Bad email address\r\n')
+    elseif l:find('breakconnect') then
+        s:write('Breaking connection\r\n')
+        return -1
+    else
+        s:write('250 OK\r\n')
+    end
+    return 1
+end
+
 local function smtp_h(s)
     s:write('220 localhost ESMTP Tarantool\r\n')
     local l
@@ -26,10 +42,14 @@ local function smtp_h(s)
             s:write('250 HELP\r\n')
         elseif l:find('MAIL FROM:') then
             mail.from = l:sub(11):sub(1, -3)
-            s:write('250 OK\r\n')
+            if write_reply_code(s, l) == -1 then
+                return
+            end
         elseif l:find('RCPT TO:') then
             mail.rcpt[#mail.rcpt + 1] = l:sub(9):sub(1, -3)
-            s:write('250 OK\r\n')
+            if write_reply_code(s, l) == -1 then
+                return
+            end
         elseif l == 'DATA\r\n' then
             s:write('354 Enter message, ending with "." on a line by itself\r\n')
             while true do
@@ -56,7 +76,7 @@ local server = socket.tcp_server('127.0.0.1', 0, smtp_h)
 local addr = 'smtp://127.0.0.1:' .. server:name().port
 
 test:test("smtp.client", function(test)
-    test:plan(10)
+    test:plan(26)
     local r
     local m
 
@@ -146,6 +166,54 @@ test:test("smtp.client", function(test)
                   m.text,
                   "Subject: =%?utf%-8%?b%?YWJjZGVmZ2hpamvRj2xtbm9wcXJzdHV2d3h5eg==%?=", ""))
     test:is(subj, 1, 'subject codes >127')
-end)
 
+    r = client:request(addr, '[email protected]',
+                       '[email protected]',
+                       'mail.body')
+    test:is(r.reason, 'MAIL failed: 354', 'errors 3xx')
+    test:is(r.status, 354, 'expected code')
+
+    r = client:request(addr, '[email protected]',
+                       '[email protected]',
+                       'mail.body')
+    test:is(r.reason, 'MAIL failed: 421', 'service unavailable')
+    test:is(r.status, 421, 'expected code')
+
+    r = client:request(addr, '[email protected]',
+                       '[email protected]',
+                       'mail.body')
+    test:is(r.reason, 'MAIL failed: 510', 'unexisting recipient')
+    test:is(r.status, 510, 'expected code')
+
+    r = client:request(addr, '[email protected]',
+                       '[email protected]',
+                       'mail.body')
+    test:is(r.reason, 'response reading failed', 'unexpected response code')
+    test:is(r.status, -1, 'expected code')
+
+    r = client:request(addr, '[email protected]',
+                       '[email protected]',
+                       'mail.body')
+    test:is(r.reason, 'RCPT failed: 354', 'errors 3xx')
+    test:is(r.status, 354, 'expected code')
+
+    r = client:request(addr, '[email protected]',
+                       '[email protected]',
+                       'mail.body')
+    test:is(r.reason, 'RCPT failed: 421', 'service unavailable')
+    test:is(r.status, 421, 'expected code')
+
+    r = client:request(addr, '[email protected]',
+                       '[email protected]',
+                       'mail.body')
+    test:is(r.reason, 'RCPT failed: 510', 'unexisting recipient')
+    test:is(r.status, 510, 'expected code')
+
+    r = client:request(addr, '[email protected]',
+                       '[email protected]',
+                       'mail.body')
+    test:is(r.reason, 'response reading failed', 'unexpected response code')
+    test:is(r.status, -1, 'expected code')
+
+end)
 os.exit(test:check() == true and 0 or -1)