[Fix] Another bunch of fixes towards protocol mess

4 files changed, 68 insertions, 55 deletions

diff --git a/src/libserver/protocol.c b/src/libserver/protocol.c
index 30ab766ce..12bef1dce 100644
--- a/src/libserver/protocol.c
+++ b/src/libserver/protocol.c
@@ -96,7 +96,7 @@ rspamd_protocol_escape_braces (struct rspamd_task *task, rspamd_ftok_t *in)
 	return rspamd_mempool_ftokdup (task->task_pool, &tok);
 }
 
-#define CMD_CHECK(str, cmd, len) (sizeof(cmd) - 1 == (len) && rspamd_lc_cmp((str), (cmd), (len)) == 0)
+#define COMPARE_CMD(str, cmd, len) (sizeof(cmd) - 1 == (len) && rspamd_lc_cmp((str), (cmd), (len)) == 0)
 
 static gboolean
 rspamd_protocol_handle_url (struct rspamd_task *task,
@@ -140,11 +140,13 @@ rspamd_protocol_handle_url (struct rspamd_task *task,
 	case 'c':
 	case 'C':
 		/* check */
-		if (CMD_CHECK (p, MSG_CMD_CHECK_V2, pathlen)) {
+		if (COMPARE_CMD (p, MSG_CMD_CHECK_V2, pathlen)) {
 			task->cmd = CMD_CHECK_V2;
+			msg_debug_protocol ("got checkv2 command");
 		}
-		else if (CMD_CHECK (p, MSG_CMD_CHECK, pathlen)) {
+		else if (COMPARE_CMD (p, MSG_CMD_CHECK, pathlen)) {
 			task->cmd = CMD_CHECK;
+			msg_debug_protocol ("got check command");
 		}
 		else {
 			goto err;
@@ -153,13 +155,16 @@ rspamd_protocol_handle_url (struct rspamd_task *task,
 	case 's':
 	case 'S':
 		/* symbols, skip */
-		if (CMD_CHECK (p, MSG_CMD_SYMBOLS, pathlen)) {
-			task->cmd = CMD_SYMBOLS;
+		if (COMPARE_CMD (p, MSG_CMD_SYMBOLS, pathlen)) {
+			task->cmd = CMD_CHECK;
+			msg_debug_protocol ("got symbols -> old check command");
 		}
-		else if (CMD_CHECK (p, MSG_CMD_SCAN, pathlen)) {
-			task->cmd = CMD_CHECK_V2;
+		else if (COMPARE_CMD (p, MSG_CMD_SCAN, pathlen)) {
+			task->cmd = CMD_CHECK;
+			msg_debug_protocol ("got scan -> old check command");
 		}
-		else if (CMD_CHECK (p, MSG_CMD_SKIP, pathlen)) {
+		else if (COMPARE_CMD (p, MSG_CMD_SKIP, pathlen)) {
+			msg_debug_protocol ("got skip command");
 			task->cmd = CMD_SKIP;
 		}
 		else {
@@ -169,11 +174,13 @@ rspamd_protocol_handle_url (struct rspamd_task *task,
 	case 'p':
 	case 'P':
 		/* ping, process */
-		if (CMD_CHECK (p, MSG_CMD_PING, pathlen)) {
+		if (COMPARE_CMD (p, MSG_CMD_PING, pathlen)) {
+			msg_debug_protocol ("got ping command");
 			task->cmd = CMD_PING;
 		}
-		else if (CMD_CHECK (p, MSG_CMD_PROCESS, pathlen)) {
-			task->cmd = CMD_PROCESS;
+		else if (COMPARE_CMD (p, MSG_CMD_PROCESS, pathlen)) {
+			msg_debug_protocol ("got process -> old check command");
+			task->cmd = CMD_CHECK;
 		}
 		else {
 			goto err;
@@ -182,11 +189,13 @@ rspamd_protocol_handle_url (struct rspamd_task *task,
 	case 'r':
 	case 'R':
 		/* report, report_ifspam */
-		if (CMD_CHECK (p, MSG_CMD_REPORT, pathlen)) {
-			task->cmd = CMD_REPORT;
+		if (COMPARE_CMD (p, MSG_CMD_REPORT, pathlen)) {
+			msg_debug_protocol ("got report -> old check command");
+			task->cmd = CMD_CHECK;
 		}
-		else if (CMD_CHECK (p, MSG_CMD_REPORT_IFSPAM, pathlen)) {
-			task->cmd = CMD_REPORT_IFSPAM;
+		else if (COMPARE_CMD (p, MSG_CMD_REPORT_IFSPAM, pathlen)) {
+			msg_debug_protocol ("got reportifspam -> old check command");
+			task->cmd = CMD_CHECK;
 		}
 		else {
 			goto err;
@@ -813,17 +822,20 @@ rspamd_protocol_handle_request (struct rspamd_task *task,
 	gboolean ret = TRUE;
 
 	if (msg->method == HTTP_SYMBOLS) {
-		task->cmd = CMD_SYMBOLS;
+		msg_debug_protocol ("got legacy SYMBOLS method, enable rspamc protocol workaround");
+		task->cmd = CMD_CHECK_RSPAMC;
 	}
 	else if (msg->method == HTTP_CHECK) {
-		task->cmd = CMD_SYMBOLS;
+		msg_debug_protocol ("got legacy CHECK method, enable rspamc protocol workaround");
+		task->cmd = CMD_CHECK_RSPAMC;
 	}
 	else {
 		ret = rspamd_protocol_handle_url (task, msg);
 	}
 
 	if (msg->flags & RSPAMD_HTTP_FLAG_SPAMC) {
-		task->protocol_flags |= RSPAMD_TASK_PROTOCOL_FLAG_SPAMC;
+		msg_debug_protocol ("got legacy SA input, enable spamc protocol workaround");
+		task->cmd = CMD_CHECK_SPAMC;
 	}
 
 	return ret;
@@ -1126,7 +1138,7 @@ rspamd_metric_result_ucl (struct rspamd_task *task,
 	action = rspamd_check_action_metric (task);
 	is_spam = !(action->flags & RSPAMD_ACTION_HAM);
 
-	if (task->cmd != CMD_CHECK_V2) {
+	if (task->cmd == CMD_CHECK) {
 		obj = ucl_object_typed_new (UCL_OBJECT);
 		ucl_object_insert_key (obj,
 				ucl_object_frombool (is_spam),
@@ -1176,7 +1188,7 @@ rspamd_metric_result_ucl (struct rspamd_task *task,
 	}
 
 	/* Now handle symbols */
-	if (task->cmd == CMD_CHECK_V2) {
+	if (task->cmd != CMD_CHECK) {
 		obj = ucl_object_typed_new (UCL_OBJECT);
 	}
 
@@ -1187,7 +1199,7 @@ rspamd_metric_result_ucl (struct rspamd_task *task,
 		}
 	})
 
-	if (task->cmd == CMD_CHECK_V2) {
+	if (task->cmd != CMD_CHECK) {
 		ucl_object_insert_key (top, obj, "symbols", 0, false);
 	}
 	else {
@@ -1458,7 +1470,7 @@ rspamd_protocol_write_ucl (struct rspamd_task *task,
 				RSPAMD_MEMPOOL_MILTER_REPLY);
 
 		if (milter_reply) {
-			if (task->cmd == CMD_CHECK_V2) {
+			if (task->cmd != CMD_CHECK) {
 				ucl_object_insert_key (top, ucl_object_ref (milter_reply),
 						"milter", 0, false);
 			}
@@ -1508,6 +1520,9 @@ rspamd_protocol_http_reply (struct rspamd_http_message *msg,
 	if (!(task->flags & RSPAMD_TASK_FLAG_NO_LOG)) {
 		rspamd_roll_history_update (task->worker->srv->history, task);
 	}
+	else {
+		msg_debug_protocol ("skip history update due to no log flag");
+	}
 
 	rspamd_task_write_log (task);
 
@@ -1529,13 +1544,16 @@ rspamd_protocol_http_reply (struct rspamd_http_message *msg,
 	reply = rspamd_fstring_sized_new (1000);
 
 	if (msg->method < HTTP_SYMBOLS && !RSPAMD_TASK_IS_SPAMC (task)) {
+		msg_debug_protocol ("writing json reply");
 		rspamd_ucl_emit_fstring (top, UCL_EMIT_JSON_COMPACT, &reply);
 	}
 	else {
 		if (RSPAMD_TASK_IS_SPAMC (task)) {
+			msg_debug_protocol ("writing spamc legacy reply to client");
 			rspamd_ucl_tospamc_output (top, &reply);
 		}
 		else {
+			msg_debug_protocol ("writing rspamc legacy reply to client");
 			rspamd_ucl_torspamc_output (top, &reply);
 		}
 	}
@@ -1604,6 +1622,7 @@ rspamd_protocol_http_reply (struct rspamd_http_message *msg,
 end:
 	if (!(task->flags & RSPAMD_TASK_FLAG_NO_STAT)) {
 		/* Update stat for default metric */
+		msg_debug_protocol ("skip stats update due to no_stat flag");
 		metric_res = task->result;
 
 		if (metric_res != NULL) {
@@ -1871,20 +1890,20 @@ rspamd_protocol_write_reply (struct rspamd_task *task, ev_tstamp timeout)
 				MESSAGE_FIELD_CHECK (task, message_id));
 	}
 
-	if (task->cmd == CMD_SYMBOLS) {
-		/* Turn compatibility on */
+	/* Compatibility */
+	if (task->cmd == CMD_CHECK_RSPAMC) {
 		msg->method = HTTP_SYMBOLS;
 	}
-
-	if (RSPAMD_TASK_IS_SPAMC (task)) {
+	else if (task->cmd == CMD_CHECK_SPAMC)  {
+		msg->method = HTTP_SYMBOLS;
 		msg->flags |= RSPAMD_HTTP_FLAG_SPAMC;
 	}
 
 	ev_now_update (task->event_loop);
 	msg->date = ev_time ();
 
-	msg_debug_protocol ("writing reply to client");
 	if (task->err != NULL) {
+		msg_debug_protocol ("writing error reply to client");
 		ucl_object_t *top = NULL;
 
 		top = ucl_object_typed_new (UCL_OBJECT);
@@ -1905,21 +1924,20 @@ rspamd_protocol_write_reply (struct rspamd_task *task, ev_tstamp timeout)
 		msg->status = rspamd_fstring_new_init ("OK", 2);
 
 		switch (task->cmd) {
-		case CMD_REPORT_IFSPAM:
-		case CMD_REPORT:
 		case CMD_CHECK:
-		case CMD_SYMBOLS:
-		case CMD_PROCESS:
+		case CMD_CHECK_RSPAMC:
+		case CMD_CHECK_SPAMC:
 		case CMD_SKIP:
 		case CMD_CHECK_V2:
 			rspamd_protocol_http_reply (msg, task, NULL);
 			rspamd_protocol_write_log_pipe (task);
 			break;
 		case CMD_PING:
+			msg_debug_protocol ("writing pong to client");
 			rspamd_http_message_set_body (msg, "pong" CRLF, 6);
 			ctype = "text/plain";
 			break;
-		case CMD_OTHER:
+		default:
 			msg_err_protocol ("BROKEN");
 			break;
 		}
diff --git a/src/libserver/task.c b/src/libserver/task.c
index ba9843963..402f39987 100644
--- a/src/libserver/task.c
+++ b/src/libserver/task.c
@@ -1497,6 +1497,7 @@ rspamd_task_write_log (struct rspamd_task *task)
 
 	if (task->cfg->log_format == NULL ||
 			(task->flags & RSPAMD_TASK_FLAG_NO_LOG)) {
+		msg_debug_task ("skip logging due to no log flag");
 		return;
 	}
 
diff --git a/src/libserver/task.h b/src/libserver/task.h
index ec66febd4..e6805eb57 100644
--- a/src/libserver/task.h
+++ b/src/libserver/task.h
@@ -30,15 +30,12 @@ extern "C" {
 #endif
 
 enum rspamd_command {
-	CMD_CHECK,
-	CMD_SYMBOLS,
-	CMD_REPORT,
-	CMD_REPORT_IFSPAM,
-	CMD_SKIP,
+	CMD_SKIP = 0,
 	CMD_PING,
-	CMD_PROCESS,
-	CMD_CHECK_V2,
-	CMD_OTHER
+	CMD_CHECK_SPAMC, /* Legacy spamasassin format */
+	CMD_CHECK_RSPAMC, /* Legacy rspamc format (like SA one) */
+	CMD_CHECK, /* Legacy check - metric json reply */
+	CMD_CHECK_V2, /* Modern check - symbols in json reply  */
 };
 
 enum rspamd_task_stage {
@@ -118,24 +115,23 @@ enum rspamd_task_stage {
 #define RSPAMD_TASK_FLAG_MESSAGE_REWRITE (1u << 24u)
 #define RSPAMD_TASK_FLAG_MAX_SHIFT (24u)
 
-/* Spamc message */
-#define RSPAMD_TASK_PROTOCOL_FLAG_SPAMC (1u << 0u)
+
 /* Request has a JSON control block */
-#define RSPAMD_TASK_PROTOCOL_FLAG_HAS_CONTROL (1u << 1u)
+#define RSPAMD_TASK_PROTOCOL_FLAG_HAS_CONTROL (1u << 0u)
 /* Request has been done by a local client */
-#define RSPAMD_TASK_PROTOCOL_FLAG_LOCAL_CLIENT (1u << 2u)
+#define RSPAMD_TASK_PROTOCOL_FLAG_LOCAL_CLIENT (1u << 1u)
 /* Request has been sent via milter */
-#define RSPAMD_TASK_PROTOCOL_FLAG_MILTER (1u << 3u)
+#define RSPAMD_TASK_PROTOCOL_FLAG_MILTER (1u << 2u)
 /* Compress protocol reply */
-#define RSPAMD_TASK_PROTOCOL_FLAG_COMPRESSED (1u << 4u)
+#define RSPAMD_TASK_PROTOCOL_FLAG_COMPRESSED (1u << 3u)
 /* Include all URLs */
-#define RSPAMD_TASK_PROTOCOL_FLAG_EXT_URLS (1u << 5u)
+#define RSPAMD_TASK_PROTOCOL_FLAG_EXT_URLS (1u << 4u)
 /* Client allows body block (including headers in no FLAG_MILTER) */
-#define RSPAMD_TASK_PROTOCOL_FLAG_BODY_BLOCK (1u << 6u)
-#define RSPAMD_TASK_PROTOCOL_FLAG_MAX_SHIFT (6u)
+#define RSPAMD_TASK_PROTOCOL_FLAG_BODY_BLOCK (1u << 5u)
+#define RSPAMD_TASK_PROTOCOL_FLAG_MAX_SHIFT (5u)
 
 #define RSPAMD_TASK_IS_SKIPPED(task) (((task)->flags & RSPAMD_TASK_FLAG_SKIP))
-#define RSPAMD_TASK_IS_SPAMC(task) (((task)->protocol_flags & RSPAMD_TASK_PROTOCOL_FLAG_SPAMC))
+#define RSPAMD_TASK_IS_SPAMC(task) (((task)->cmd == CMD_CHECK_SPAMC))
 #define RSPAMD_TASK_IS_PROCESSED(task) (((task)->processed_stages & RSPAMD_TASK_STAGE_DONE))
 #define RSPAMD_TASK_IS_CLASSIFIED(task) (((task)->processed_stages & RSPAMD_TASK_STAGE_CLASSIFIERS))
 #define RSPAMD_TASK_IS_EMPTY(task) (((task)->flags & RSPAMD_TASK_FLAG_EMPTY))
diff --git a/src/rspamd_proxy.c b/src/rspamd_proxy.c
index 9122df514..8d1f7c116 100644
--- a/src/rspamd_proxy.c
+++ b/src/rspamd_proxy.c
@@ -1576,12 +1576,10 @@ rspamd_proxy_scan_self_reply (struct rspamd_task *task)
 	msg->code = 200;
 
 	switch (task->cmd) {
-	case CMD_REPORT_IFSPAM:
-	case CMD_REPORT:
 	case CMD_CHECK:
-	case CMD_SYMBOLS:
-	case CMD_PROCESS:
 	case CMD_SKIP:
+	case CMD_CHECK_RSPAMC:
+	case CMD_CHECK_SPAMC:
 	case CMD_CHECK_V2:
 		rspamd_task_set_finish_time (task);
 		rspamd_protocol_http_reply (msg, task, &rep);
@@ -1591,7 +1589,7 @@ rspamd_proxy_scan_self_reply (struct rspamd_task *task)
 		rspamd_http_message_set_body (msg, "pong" CRLF, 6);
 		ctype = "text/plain";
 		break;
-	case CMD_OTHER:
+	default:
 		msg_err_task ("BROKEN");
 		break;
 	}