commit 232f06d: [Fix] Another bunch of fixes towards protocol mess
Vsevolod Stakhov
vsevolod at highsecure.ru
Wed Jul 17 12:42:05 UTC 2019
Author: Vsevolod Stakhov
Date: 2019-07-17 13:04:54 +0100
URL: https://github.com/rspamd/rspamd/commit/232f06de6252554901159a49864fac6ebd9dc8a8
[Fix] Another bunch of fixes towards protocol mess
---
src/libserver/protocol.c | 82 +++++++++++++++++++++++++++++-------------------
src/libserver/task.c | 1 +
src/libserver/task.h | 32 +++++++++----------
src/rspamd_proxy.c | 8 ++---
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;
}
More information about the Commits
mailing list