diff --git a/lib/jsonrpc/jsonrpc_internal.h b/lib/jsonrpc/jsonrpc_internal.h index 05e2ac630..d070c02e0 100644 --- a/lib/jsonrpc/jsonrpc_internal.h +++ b/lib/jsonrpc/jsonrpc_internal.h @@ -42,12 +42,16 @@ #define SPDK_JSONRPC_RECV_BUF_SIZE (32 * 1024) #define SPDK_JSONRPC_SEND_BUF_SIZE (32 * 1024) +#define SPDK_JSONRPC_ID_MAX_LEN 128 #define SPDK_JSONRPC_MAX_CONNS 64 #define SPDK_JSONRPC_MAX_VALUES 1024 struct spdk_jsonrpc_request { struct spdk_jsonrpc_server_conn *conn; - const struct spdk_json_val *id; + + /* Copy of request id value */ + struct spdk_json_val id; + uint8_t id_data[SPDK_JSONRPC_ID_MAX_LEN]; }; struct spdk_jsonrpc_server_conn { diff --git a/lib/jsonrpc/jsonrpc_server.c b/lib/jsonrpc/jsonrpc_server.c index 9e3e11680..796a7141f 100644 --- a/lib/jsonrpc/jsonrpc_server.c +++ b/lib/jsonrpc/jsonrpc_server.c @@ -59,14 +59,10 @@ static const struct spdk_json_object_decoder jsonrpc_request_decoders[] = { }; static void -parse_single_request(struct spdk_jsonrpc_server_conn *conn, struct spdk_json_val *values) +parse_single_request(struct spdk_jsonrpc_request *request, struct spdk_json_val *values) { bool invalid = false; struct jsonrpc_request req = {}; - struct spdk_jsonrpc_request request; - - request.conn = conn; - request.id = NULL; if (spdk_json_decode_object(values, jsonrpc_request_decoders, SPDK_COUNTOF(jsonrpc_request_decoders), @@ -86,13 +82,22 @@ parse_single_request(struct spdk_jsonrpc_server_conn *conn, struct spdk_json_val } if (req.id) { - if (req.id->type != SPDK_JSON_VAL_STRING && - req.id->type != SPDK_JSON_VAL_NUMBER && - req.id->type != SPDK_JSON_VAL_NULL) { - req.id = NULL; + if (req.id->type == SPDK_JSON_VAL_STRING || + req.id->type == SPDK_JSON_VAL_NUMBER) { + /* Copy value into request */ + if (req.id->len <= SPDK_JSONRPC_ID_MAX_LEN) { + request->id.type = req.id->type; + request->id.len = req.id->len; + memcpy(request->id.start, req.id->start, req.id->len); + } else { + SPDK_TRACELOG(SPDK_TRACE_RPC, "JSON-RPC request id too long (%u)\n", + req.id->len); + invalid = true; + } + } else if (req.id->type == SPDK_JSON_VAL_NULL) { + request->id.type = SPDK_JSON_VAL_NULL; + } else { invalid = true; - } else { - request.id = req.id; } } @@ -106,30 +111,39 @@ parse_single_request(struct spdk_jsonrpc_server_conn *conn, struct spdk_json_val done: if (invalid) { - spdk_jsonrpc_server_handle_error(&request, SPDK_JSONRPC_ERROR_INVALID_REQUEST); + spdk_jsonrpc_server_handle_error(request, SPDK_JSONRPC_ERROR_INVALID_REQUEST); } else { - spdk_jsonrpc_server_handle_request(&request, req.method, req.params); + spdk_jsonrpc_server_handle_request(request, req.method, req.params); } } int spdk_jsonrpc_parse_request(struct spdk_jsonrpc_server_conn *conn, void *json, size_t size) { - struct spdk_jsonrpc_request error_request; + struct spdk_jsonrpc_request *request; ssize_t rc; void *end = NULL; - /* Build a request with id = NULL since we don't have a valid request ID */ - error_request.conn = conn; - error_request.id = NULL; - /* Check to see if we have received a full JSON value. */ rc = spdk_json_parse(json, size, NULL, 0, &end, 0); if (rc == SPDK_JSON_PARSE_INCOMPLETE) { return 0; - } else if (rc < 0 || rc > SPDK_JSONRPC_MAX_VALUES) { + } + + request = calloc(1, sizeof(*request)); + if (request == NULL) { + SPDK_TRACELOG(SPDK_TRACE_RPC, "Out of memory allocating request\n"); + return -1; + } + + request->conn = conn; + request->id.start = request->id_data; + request->id.len = 0; + request->id.type = SPDK_JSON_VAL_INVALID; + + if (rc < 0 || rc > SPDK_JSONRPC_MAX_VALUES) { SPDK_TRACELOG(SPDK_TRACE_RPC, "JSON parse error\n"); - spdk_jsonrpc_server_handle_error(&error_request, SPDK_JSONRPC_ERROR_PARSE_ERROR); + spdk_jsonrpc_server_handle_error(request, SPDK_JSONRPC_ERROR_PARSE_ERROR); /* * Can't recover from parse error (no guaranteed resync point in streaming JSON). @@ -143,20 +157,20 @@ spdk_jsonrpc_parse_request(struct spdk_jsonrpc_server_conn *conn, void *json, si SPDK_JSON_PARSE_FLAG_DECODE_IN_PLACE); if (rc < 0 || rc > SPDK_JSONRPC_MAX_VALUES) { SPDK_TRACELOG(SPDK_TRACE_RPC, "JSON parse error on second pass\n"); - spdk_jsonrpc_server_handle_error(&error_request, SPDK_JSONRPC_ERROR_PARSE_ERROR); + spdk_jsonrpc_server_handle_error(request, SPDK_JSONRPC_ERROR_PARSE_ERROR); return -1; } assert(end != NULL); if (conn->values[0].type == SPDK_JSON_VAL_OBJECT_BEGIN) { - parse_single_request(conn, conn->values); + parse_single_request(request, conn->values); } else if (conn->values[0].type == SPDK_JSON_VAL_ARRAY_BEGIN) { SPDK_TRACELOG(SPDK_TRACE_RPC, "Got batch array (not currently supported)\n"); - spdk_jsonrpc_server_handle_error(&error_request, SPDK_JSONRPC_ERROR_INVALID_REQUEST); + spdk_jsonrpc_server_handle_error(request, SPDK_JSONRPC_ERROR_INVALID_REQUEST); } else { SPDK_TRACELOG(SPDK_TRACE_RPC, "top-level JSON value was not array or object\n"); - spdk_jsonrpc_server_handle_error(&error_request, SPDK_JSONRPC_ERROR_INVALID_REQUEST); + spdk_jsonrpc_server_handle_error(request, SPDK_JSONRPC_ERROR_INVALID_REQUEST); } return end - json; @@ -176,10 +190,8 @@ begin_response(struct spdk_jsonrpc_server_conn *conn, const struct spdk_json_val spdk_json_write_name(w, "jsonrpc"); spdk_json_write_string(w, "2.0"); - if (id) { - spdk_json_write_name(w, "id"); - spdk_json_write_val(w, id); - } + spdk_json_write_name(w, "id"); + spdk_json_write_val(w, id); return w; } @@ -197,8 +209,15 @@ spdk_jsonrpc_begin_result(struct spdk_jsonrpc_request *request) { struct spdk_json_write_ctx *w; - w = begin_response(request->conn, request->id); + if (request->id.type == SPDK_JSON_VAL_INVALID) { + /* Notification - no response required */ + free(request); + return NULL; + } + + w = begin_response(request->conn, &request->id); if (w == NULL) { + free(request); return NULL; } @@ -213,6 +232,7 @@ spdk_jsonrpc_end_result(struct spdk_jsonrpc_request *request, struct spdk_json_w assert(w != NULL); end_response(request->conn, w); + free(request); } void @@ -220,18 +240,15 @@ spdk_jsonrpc_send_error_response(struct spdk_jsonrpc_request *request, int error_code, const char *msg) { struct spdk_json_write_ctx *w; - struct spdk_json_val v_null; - const struct spdk_json_val *id; - id = request->id; - if (id == NULL) { + if (request->id.type == SPDK_JSON_VAL_INVALID) { /* For error responses, if id is missing, explicitly respond with "id": null. */ - v_null.type = SPDK_JSON_VAL_NULL; - id = &v_null; + request->id.type = SPDK_JSON_VAL_NULL; } - w = begin_response(request->conn, id); + w = begin_response(request->conn, &request->id); if (w == NULL) { + free(request); return; } @@ -244,6 +261,7 @@ spdk_jsonrpc_send_error_response(struct spdk_jsonrpc_request *request, spdk_json_write_object_end(w); end_response(request->conn, w); + free(request); } SPDK_LOG_REGISTER_TRACE_FLAG("rpc", SPDK_TRACE_RPC) diff --git a/test/unit/lib/jsonrpc/jsonrpc_server.c/jsonrpc_server_ut.c b/test/unit/lib/jsonrpc/jsonrpc_server.c/jsonrpc_server_ut.c index 2ff6a19f3..4e3d9f691 100644 --- a/test/unit/lib/jsonrpc/jsonrpc_server.c/jsonrpc_server_ut.c +++ b/test/unit/lib/jsonrpc/jsonrpc_server.c/jsonrpc_server_ut.c @@ -45,6 +45,7 @@ struct req { bool got_method; bool got_id; bool got_params; + struct spdk_jsonrpc_request *request; struct spdk_json_val method; struct spdk_json_val id; struct spdk_json_val params[MAX_PARAMS]; @@ -98,6 +99,10 @@ static size_t g_num_reqs; CU_ASSERT(g_cur_req->id.type == SPDK_JSON_VAL_STRING); \ CU_ASSERT(memcmp(g_cur_req->id.start, str, sizeof(str) - 1) == 0) +#define REQ_ID_NULL() \ + CU_ASSERT(g_cur_req->got_id); \ + CU_ASSERT(g_cur_req->id.type == SPDK_JSON_VAL_NULL) + #define REQ_ID_MISSING() \ CU_ASSERT(g_cur_req->got_id == false) @@ -142,16 +147,21 @@ static size_t g_num_reqs; CU_ASSERT(memcmp(g_params->start, str, g_params->len) == 0); \ g_params++ +#define FREE_REQUEST() \ + free(g_reqs->request); \ + g_reqs->request = NULL + static void ut_handle(struct spdk_jsonrpc_request *request, int error, const struct spdk_json_val *method, const struct spdk_json_val *params) { - const struct spdk_json_val *id = request->id; + const struct spdk_json_val *id = &request->id; struct req *r; SPDK_CU_ASSERT_FATAL(g_num_reqs != MAX_REQS); r = &g_reqs[g_num_reqs++]; + r->request = request; r->error = error; if (method) { @@ -169,7 +179,7 @@ ut_handle(struct spdk_jsonrpc_request *request, int error, const struct spdk_jso r->got_params = false; } - if (id) { + if (id && id->type != SPDK_JSON_VAL_INVALID) { r->got_id = true; r->id = *id; } else { @@ -180,6 +190,14 @@ ut_handle(struct spdk_jsonrpc_request *request, int error, const struct spdk_jso void spdk_jsonrpc_server_handle_error(struct spdk_jsonrpc_request *request, int error) { + /* + * Map missing id to Null - this mirrors the behavior in the real + * spdk_jsonrpc_server_handle_error() function. + */ + if (request->id.type == SPDK_JSON_VAL_INVALID) { + request->id.type = SPDK_JSON_VAL_NULL; + } + ut_handle(request, error, NULL, NULL); } @@ -221,6 +239,7 @@ test_parse_request(void) PARAM_NUM("42"); PARAM_NUM("23"); PARAM_ARRAY_END(); + FREE_REQUEST(); /* rpc call with named parameters */ PARSE_PASS("{\"jsonrpc\": \"2.0\", \"method\": \"subtract\", \"params\": {\"subtrahend\": 23, \"minuend\": 42}, \"id\": 3}", @@ -235,6 +254,7 @@ test_parse_request(void) PARAM_NAME("minuend"); PARAM_NUM("42"); PARAM_OBJECT_END(); + FREE_REQUEST(); /* notification */ PARSE_PASS("{\"jsonrpc\": \"2.0\", \"method\": \"update\", \"params\": [1,2,3,4,5]}", ""); @@ -249,20 +269,23 @@ test_parse_request(void) PARAM_NUM("4"); PARAM_NUM("5"); PARAM_ARRAY_END(); + FREE_REQUEST(); /* invalid JSON */ PARSE_FAIL("{\"jsonrpc\": \"2.0\", \"method\": \"foobar, \"params\": \"bar\", \"baz]"); REQ_BEGIN_INVALID(SPDK_JSONRPC_ERROR_PARSE_ERROR); REQ_METHOD_MISSING(); - REQ_ID_MISSING(); + REQ_ID_NULL(); REQ_PARAMS_MISSING(); + FREE_REQUEST(); /* invalid request (method must be a string; params must be array or object) */ PARSE_PASS("{\"jsonrpc\": \"2.0\", \"method\": 1, \"params\": \"bar\"}", ""); REQ_BEGIN_INVALID(SPDK_JSONRPC_ERROR_INVALID_REQUEST); REQ_METHOD_MISSING(); - REQ_ID_MISSING(); + REQ_ID_NULL(); REQ_PARAMS_MISSING(); + FREE_REQUEST(); /* batch, invalid JSON */ PARSE_FAIL( @@ -272,15 +295,17 @@ test_parse_request(void) "]"); REQ_BEGIN_INVALID(SPDK_JSONRPC_ERROR_PARSE_ERROR); REQ_METHOD_MISSING(); - REQ_ID_MISSING(); + REQ_ID_NULL(); REQ_PARAMS_MISSING(); + FREE_REQUEST(); /* empty array */ PARSE_PASS("[]", ""); REQ_BEGIN_INVALID(SPDK_JSONRPC_ERROR_INVALID_REQUEST); REQ_METHOD_MISSING(); - REQ_ID_MISSING(); + REQ_ID_NULL(); REQ_PARAMS_MISSING(); + FREE_REQUEST(); /* batch - not supported */ PARSE_PASS( @@ -295,8 +320,9 @@ test_parse_request(void) REQ_BEGIN_INVALID(SPDK_JSONRPC_ERROR_INVALID_REQUEST); REQ_METHOD_MISSING(); - REQ_ID_MISSING(); + REQ_ID_NULL(); REQ_PARAMS_MISSING(); + FREE_REQUEST(); free(conn); free(server); @@ -332,6 +358,7 @@ test_parse_request_streaming(void) PARAM_ARRAY_BEGIN(); PARAM_NUM("1"); PARAM_ARRAY_END(); + FREE_REQUEST(); /* Partial (but not invalid) requests - parse should not consume anything. */ strcpy(g_buf, "{\"jsonrpc\":\"2.0\",\"method\":\"b\",\"params\":[2],\"id\":2}"); @@ -342,10 +369,12 @@ test_parse_request_streaming(void) int rc = spdk_jsonrpc_parse_request(conn, g_buf, i); /* Partial request - no data consumed */ CU_ASSERT(rc == 0); + FREE_REQUEST(); } /* Verify that full request can be parsed successfully */ CU_ASSERT(spdk_jsonrpc_parse_request(conn, g_buf, len) == (ssize_t)len); + FREE_REQUEST(); free(conn); free(server);