From d3022947238957ead139094ce0450621ae5886fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Helleu?= Date: Tue, 7 Jan 2025 07:52:09 +0100 Subject: [PATCH] relay/api: always return a body with field "error" in error responses --- src/plugins/relay/api/relay-api-protocol.c | 44 +++++++---- src/plugins/relay/relay-http.h | 2 + .../relay/api/test-relay-api-protocol.cpp | 74 +++++++++++-------- 3 files changed, 75 insertions(+), 45 deletions(-) diff --git a/src/plugins/relay/api/relay-api-protocol.c b/src/plugins/relay/api/relay-api-protocol.c index 6d1252c22..faa68604d 100644 --- a/src/plugins/relay/api/relay-api-protocol.c +++ b/src/plugins/relay/api/relay-api-protocol.c @@ -1112,8 +1112,8 @@ relay_api_protocol_recv_json_request (struct t_relay_client *client, return; error: - relay_api_msg_send_json (client, RELAY_HTTP_400_BAD_REQUEST, - NULL, NULL, NULL); + relay_api_msg_send_error_json (client, RELAY_HTTP_400_BAD_REQUEST, + NULL, RELAY_HTTP_ERROR_BAD_REQUEST); } /* @@ -1165,8 +1165,11 @@ relay_api_protocol_recv_json (struct t_relay_client *client, const char *json) json_obj = cJSON_Parse (json); if (!json_obj) { - relay_api_msg_send_json (client, RELAY_HTTP_400_BAD_REQUEST, - NULL, NULL, NULL); + relay_api_msg_send_error_json ( + client, + RELAY_HTTP_400_BAD_REQUEST, + NULL, + RELAY_HTTP_ERROR_BAD_REQUEST ": invalid JSON"); return; } @@ -1193,6 +1196,7 @@ void relay_api_protocol_recv_http (struct t_relay_client *client) { int i, num_args; + char str_error[1024]; enum t_relay_api_protocol_rc return_code; struct t_relay_api_protocol_cb protocol_cb[] = { /* method, resource, auth, args, callback */ @@ -1226,15 +1230,15 @@ relay_api_protocol_recv_http (struct t_relay_client *client) client->http_req->body); } - if ((client->http_req->num_path_items < 2) || !client->http_req->path_items - || !client->http_req->path_items[0] || !client->http_req->path_items[1]) + if ((client->http_req->num_path_items < 2) + || !client->http_req->path_items + || !client->http_req->path_items[0] + || !client->http_req->path_items[1] + || (strcmp (client->http_req->path_items[0], "api") != 0)) { goto error_not_found; } - if (strcmp (client->http_req->path_items[0], "api") != 0) - goto error_not_found; - num_args = client->http_req->num_path_items - 2; for (i = 0; protocol_cb[i].resource; i++) @@ -1272,7 +1276,13 @@ relay_api_protocol_recv_http (struct t_relay_client *client) num_args, protocol_cb[i].min_args); } - goto error_not_found; + snprintf (str_error, sizeof (str_error), + "%s: not enough path parameters (min: %d)", + RELAY_HTTP_ERROR_BAD_REQUEST, + protocol_cb[i].min_args); + relay_api_msg_send_error_json (client, RELAY_HTTP_400_BAD_REQUEST, + NULL, str_error); + goto error; } if ((protocol_cb[i].max_args >= 0) @@ -1294,7 +1304,13 @@ relay_api_protocol_recv_http (struct t_relay_client *client) num_args, protocol_cb[i].max_args); } - goto error_not_found; + snprintf (str_error, sizeof (str_error), + "%s: too many path parameters (max: %d)", + RELAY_HTTP_ERROR_BAD_REQUEST, + protocol_cb[i].max_args); + relay_api_msg_send_error_json (client, RELAY_HTTP_400_BAD_REQUEST, + NULL, str_error); + goto error; } return_code = (protocol_cb[i].cmd_function) (client); @@ -1316,11 +1332,13 @@ relay_api_protocol_recv_http (struct t_relay_client *client) goto error_not_found; error_bad_request: - relay_api_msg_send_json (client, RELAY_HTTP_400_BAD_REQUEST, NULL, NULL, NULL); + relay_api_msg_send_error_json (client, RELAY_HTTP_400_BAD_REQUEST, + NULL, RELAY_HTTP_ERROR_BAD_REQUEST); goto error; error_not_found: - relay_api_msg_send_json (client, RELAY_HTTP_404_NOT_FOUND, NULL, NULL, NULL); + relay_api_msg_send_error_json (client, RELAY_HTTP_404_NOT_FOUND, + NULL, RELAY_HTTP_ERROR_NOT_FOUND); goto error; error_memory: diff --git a/src/plugins/relay/relay-http.h b/src/plugins/relay/relay-http.h index 8ec784249..b3ae89898 100644 --- a/src/plugins/relay/relay-http.h +++ b/src/plugins/relay/relay-http.h @@ -49,6 +49,8 @@ enum t_relay_client_http_status "(not found or not supported)" #define RELAY_HTTP_ERROR_INVALID_TIMESTAMP "Invalid timestamp" #define RELAY_HTTP_ERROR_INVALID_ITERATIONS "Invalid number of iterations" +#define RELAY_HTTP_ERROR_BAD_REQUEST "Bad request" +#define RELAY_HTTP_ERROR_NOT_FOUND "Resource not found" #define RELAY_HTTP_ERROR_OUT_OF_MEMORY "Out of memory" struct t_relay_http_request diff --git a/tests/unit/plugins/relay/api/test-relay-api-protocol.cpp b/tests/unit/plugins/relay/api/test-relay-api-protocol.cpp index 74fe6bada..290dbe129 100644 --- a/tests/unit/plugins/relay/api/test-relay-api-protocol.cpp +++ b/tests/unit/plugins/relay/api/test-relay-api-protocol.cpp @@ -54,14 +54,15 @@ extern int relay_api_protocol_command_delay; data_sent[0], \ strlen ("HTTP/1.1 " #__code " " __message "\r\n")); -#define WEE_CHECK_TEXT(__code, __message, __request, __body) \ +#define WEE_CHECK_TEXT(__code, __message, __request, __request_body, \ + __body) \ STRCMP_EQUAL("{\"code\":" #__code "," \ "\"message\":\"" __message "\"," \ "\"request\":\"" __request "\"," \ - "\"request_body\":" __body "," \ + "\"request_body\":" __request_body "," \ "\"request_id\":null," \ "\"body_type\":null," \ - "\"body\":null" \ + "\"body\":" __body \ "}", \ data_sent[0]); @@ -407,11 +408,12 @@ TEST(RelayApiProtocolWithClient, CbBuffers) /* error: too many parameters in path */ test_client_recv_http ("GET /api/buffers/core.weechat/too/many/parameters", NULL, NULL); - STRCMP_EQUAL("HTTP/1.1 404 Not Found\r\n" + STRCMP_EQUAL("HTTP/1.1 400 Bad Request\r\n" "Access-Control-Allow-Origin: *\r\n" "Content-Type: application/json; charset=utf-8\r\n" - "Content-Length: 0\r\n" - "\r\n", + "Content-Length: 58\r\n" + "\r\n" + "{\"error\":\"Bad request: too many path parameters (max: 3)\"}", data_sent[0]); /* get all buffers */ @@ -613,8 +615,9 @@ TEST(RelayApiProtocolWithClient, CbCompletion) STRCMP_EQUAL("HTTP/1.1 400 Bad Request\r\n" "Access-Control-Allow-Origin: *\r\n" "Content-Type: application/json; charset=utf-8\r\n" - "Content-Length: 0\r\n" - "\r\n", + "Content-Length: 23\r\n" + "\r\n" + "{\"error\":\"Bad request\"}", data_sent[0]); /* error: invalid buffer name */ @@ -693,8 +696,9 @@ TEST(RelayApiProtocolWithClient, CbInput) STRCMP_EQUAL("HTTP/1.1 400 Bad Request\r\n" "Access-Control-Allow-Origin: *\r\n" "Content-Type: application/json; charset=utf-8\r\n" - "Content-Length: 0\r\n" - "\r\n", + "Content-Length: 23\r\n" + "\r\n" + "{\"error\":\"Bad request\"}", data_sent[0]); /* error: invalid buffer name */ @@ -817,7 +821,7 @@ TEST(RelayApiProtocolWithClient, CbSyncWebsocket) data_sent[0]); test_client_recv_text ("{\"request\": \"POST /api/sync\"}"); - WEE_CHECK_TEXT(204, "No Content", "POST /api/sync", "null"); + WEE_CHECK_TEXT(204, "No Content", "POST /api/sync", "null", "null"); LONGS_EQUAL(1, RELAY_API_DATA(ptr_relay_client, sync_enabled)); LONGS_EQUAL(1, RELAY_API_DATA(ptr_relay_client, sync_nicks)); @@ -825,7 +829,7 @@ TEST(RelayApiProtocolWithClient, CbSyncWebsocket) test_client_recv_text ("{\"request\": \"POST /api/sync\", " "\"body\": {\"sync\": false}}"); - WEE_CHECK_TEXT(204, "No Content", "POST /api/sync", "{\"sync\":false}"); + WEE_CHECK_TEXT(204, "No Content", "POST /api/sync", "{\"sync\":false}", "null"); LONGS_EQUAL(0, RELAY_API_DATA(ptr_relay_client, sync_enabled)); LONGS_EQUAL(1, RELAY_API_DATA(ptr_relay_client, sync_nicks)); @@ -833,7 +837,7 @@ TEST(RelayApiProtocolWithClient, CbSyncWebsocket) test_client_recv_text ("{\"request\": \"POST /api/sync\", " "\"body\": {\"sync\": true, \"nicks\": false}}"); - WEE_CHECK_TEXT(204, "No Content", "POST /api/sync", "{\"sync\":true,\"nicks\":false}"); + WEE_CHECK_TEXT(204, "No Content", "POST /api/sync", "{\"sync\":true,\"nicks\":false}", "null"); LONGS_EQUAL(1, RELAY_API_DATA(ptr_relay_client, sync_enabled)); LONGS_EQUAL(0, RELAY_API_DATA(ptr_relay_client, sync_nicks)); @@ -841,7 +845,7 @@ TEST(RelayApiProtocolWithClient, CbSyncWebsocket) test_client_recv_text ("{\"request\": \"POST /api/sync\", " "\"body\": {\"sync\": true, \"nicks\": true, \"colors\": \"weechat\"}}"); - WEE_CHECK_TEXT(204, "No Content", "POST /api/sync", "{\"sync\":true,\"nicks\":true,\"colors\":\"weechat\"}"); + WEE_CHECK_TEXT(204, "No Content", "POST /api/sync", "{\"sync\":true,\"nicks\":true,\"colors\":\"weechat\"}", "null"); LONGS_EQUAL(1, RELAY_API_DATA(ptr_relay_client, sync_enabled)); LONGS_EQUAL(1, RELAY_API_DATA(ptr_relay_client, sync_nicks)); @@ -849,7 +853,7 @@ TEST(RelayApiProtocolWithClient, CbSyncWebsocket) test_client_recv_text ("{\"request\": \"POST /api/sync\", " "\"body\": {\"sync\": true, \"nicks\": true, \"colors\": \"strip\"}}"); - WEE_CHECK_TEXT(204, "No Content", "POST /api/sync", "{\"sync\":true,\"nicks\":true,\"colors\":\"strip\"}"); + WEE_CHECK_TEXT(204, "No Content", "POST /api/sync", "{\"sync\":true,\"nicks\":true,\"colors\":\"strip\"}", "null"); LONGS_EQUAL(1, RELAY_API_DATA(ptr_relay_client, sync_enabled)); LONGS_EQUAL(1, RELAY_API_DATA(ptr_relay_client, sync_nicks)); @@ -882,31 +886,32 @@ TEST(RelayApiProtocolWithClient, RecvJson) /* error: empty string */ test_client_recv_text (""); - WEE_CHECK_TEXT(400, "Bad Request", "", "null"); + WEE_CHECK_TEXT(400, "Bad Request", "", "null", "{\"error\":\"Bad request: invalid JSON\"}"); /* error: empty body */ test_client_recv_text ("{}"); - WEE_CHECK_TEXT(400, "Bad Request", "", "null"); + WEE_CHECK_TEXT(400, "Bad Request", "", "null", "{\"error\":\"Bad request\"}"); /* error: empty request */ test_client_recv_text ("{\"request\": \"\"}"); - WEE_CHECK_TEXT(400, "Bad Request", "", "null"); + WEE_CHECK_TEXT(400, "Bad Request", "", "null", "{\"error\":\"Bad request\"}"); /* error: invalid request (number) */ test_client_recv_text ("{\"request\": 123}"); - WEE_CHECK_TEXT(400, "Bad Request", "", "null"); + WEE_CHECK_TEXT(400, "Bad Request", "", "null", "{\"error\":\"Bad request\"}"); /* error: invalid request (string, not a valid request) */ test_client_recv_text ("{\"request\": \"abc\"}"); - WEE_CHECK_TEXT(400, "Bad Request", "", "null"); + WEE_CHECK_TEXT(400, "Bad Request", "", "null", "{\"error\":\"Bad request\"}"); /* error: invalid request (string, resource not found) */ test_client_recv_text ("{\"request\": \"GET /api/unknown\"}"); - WEE_CHECK_TEXT(404, "Not Found", "GET /api/unknown", "null"); + WEE_CHECK_TEXT(404, "Not Found", "GET /api/unknown", "null", "{\"error\":\"Resource not found\"}"); /* error: invalid request (string, resource not found) */ test_client_recv_text ("{\"request\": \"GET /api/unknown\", \"body\": {\"test\": 123}}"); - WEE_CHECK_TEXT(404, "Not Found", "GET /api/unknown", "{\"test\":123}"); + WEE_CHECK_TEXT(404, "Not Found", "GET /api/unknown", "{\"test\":123}", + "{\"error\":\"Resource not found\"}"); /* ping */ test_client_recv_text ("{\"request\": \"POST /api/ping\", \"request_id\": \"ping\"}"); @@ -961,8 +966,9 @@ TEST(RelayApiProtocolWithClient, RecvHttp404) STRCMP_EQUAL("HTTP/1.1 404 Not Found\r\n" "Access-Control-Allow-Origin: *\r\n" "Content-Type: application/json; charset=utf-8\r\n" - "Content-Length: 0\r\n" - "\r\n", + "Content-Length: 30\r\n" + "\r\n" + "{\"error\":\"Resource not found\"}", data_sent[0]); /* resource not found: error 404 */ @@ -970,8 +976,9 @@ TEST(RelayApiProtocolWithClient, RecvHttp404) STRCMP_EQUAL("HTTP/1.1 404 Not Found\r\n" "Access-Control-Allow-Origin: *\r\n" "Content-Type: application/json; charset=utf-8\r\n" - "Content-Length: 0\r\n" - "\r\n", + "Content-Length: 30\r\n" + "\r\n" + "{\"error\":\"Resource not found\"}", data_sent[0]); /* resource not found: error 404 */ @@ -979,8 +986,9 @@ TEST(RelayApiProtocolWithClient, RecvHttp404) STRCMP_EQUAL("HTTP/1.1 404 Not Found\r\n" "Access-Control-Allow-Origin: *\r\n" "Content-Type: application/json; charset=utf-8\r\n" - "Content-Length: 0\r\n" - "\r\n", + "Content-Length: 30\r\n" + "\r\n" + "{\"error\":\"Resource not found\"}", data_sent[0]); /* resource not found: error 404 */ @@ -988,8 +996,9 @@ TEST(RelayApiProtocolWithClient, RecvHttp404) STRCMP_EQUAL("HTTP/1.1 404 Not Found\r\n" "Access-Control-Allow-Origin: *\r\n" "Content-Type: application/json; charset=utf-8\r\n" - "Content-Length: 0\r\n" - "\r\n", + "Content-Length: 30\r\n" + "\r\n" + "{\"error\":\"Resource not found\"}", data_sent[0]); /* resource not found: error 404 */ @@ -997,8 +1006,9 @@ TEST(RelayApiProtocolWithClient, RecvHttp404) STRCMP_EQUAL("HTTP/1.1 404 Not Found\r\n" "Access-Control-Allow-Origin: *\r\n" "Content-Type: application/json; charset=utf-8\r\n" - "Content-Length: 0\r\n" - "\r\n", + "Content-Length: 30\r\n" + "\r\n" + "{\"error\":\"Resource not found\"}", data_sent[0]); }