From 0215c727b1e1a7992276fae5cd5fb95f0436877f Mon Sep 17 00:00:00 2001 From: Pepijn Noltes Date: Sun, 27 Sep 2026 22:48:09 +0200 Subject: [PATCH 1/2] Add type checking to JSON serializer --- libs/dfi/gtest/src/json_serializer_tests.cpp | 37 +++++++++ libs/dfi/src/json_rpc.c | 7 +- libs/dfi/src/json_serializer.c | 84 +++++++++++++++++--- libs/etcdlib/src/etcd.c | 2 +- 4 files changed, 116 insertions(+), 14 deletions(-) diff --git a/libs/dfi/gtest/src/json_serializer_tests.cpp b/libs/dfi/gtest/src/json_serializer_tests.cpp index a5cbcd6e5..ca98523f1 100644 --- a/libs/dfi/gtest/src/json_serializer_tests.cpp +++ b/libs/dfi/gtest/src/json_serializer_tests.cpp @@ -556,6 +556,43 @@ static void parseTests() { celix_err_printErrors(stderr, nullptr, nullptr); dynType_destroy(type); + // integer type mismatch - string instead of integer + rc = dynType_parseWithStr("{I a}", nullptr, nullptr, &type); + ASSERT_EQ(0, rc); + inputStr = R"({"a":"not_an_integer"})"; + rc = jsonSerializer_deserialize(type, inputStr, strlen(inputStr), &inst); + ASSERT_EQ(1, rc); + celix_err_printErrors(stderr, nullptr, nullptr); + dynType_destroy(type); + + // double type mismatch - string instead of double + rc = dynType_parseWithStr("{D a}", nullptr, nullptr, &type); + ASSERT_EQ(0, rc); + inputStr = R"({"a":"not_a_double"})"; + rc = jsonSerializer_deserialize(type, inputStr, strlen(inputStr), &inst); + ASSERT_EQ(1, rc); + celix_err_printErrors(stderr, nullptr, nullptr); + dynType_destroy(type); + + // boolean type mismatch - integer instead of boolean + rc = dynType_parseWithStr("{Z a}", nullptr, nullptr, &type); + ASSERT_EQ(0, rc); + inputStr = R"({"a":1})"; + rc = jsonSerializer_deserialize(type, inputStr, strlen(inputStr), &inst); + ASSERT_EQ(1, rc); + celix_err_printErrors(stderr, nullptr, nullptr); + dynType_destroy(type); + + // float type mismatch - string instead of float + rc = dynType_parseWithStr("{F a}", nullptr, nullptr, &type); + ASSERT_EQ(0, rc); + inputStr = R"({"a":"not_a_float"})"; + rc = jsonSerializer_deserialize(type, inputStr, strlen(inputStr), &inst); + ASSERT_EQ(1, rc); + celix_err_printErrors(stderr, nullptr, nullptr); + dynType_destroy(type); + + // extra member ("b") is allowed rc = dynType_parseWithStr("{t a}", nullptr, nullptr, &type); ASSERT_EQ(0, rc); diff --git a/libs/dfi/src/json_rpc.c b/libs/dfi/src/json_rpc.c index d7e9eb780..faae2beb7 100644 --- a/libs/dfi/src/json_rpc.c +++ b/libs/dfi/src/json_rpc.c @@ -270,7 +270,12 @@ int jsonRpc_handleReply(const dyn_function_type* func, const char* reply, void* rsError = json_object_get(replyJson, "e"); if (rsError != NULL) { //get the invocation error of remote service function - *rsErrno = (int) json_integer_value(rsError); + if (json_is_integer(rsError)) { + *rsErrno = (int) json_integer_value(rsError); + } else { + celix_err_pushf("Expected json integer for error code but got %i", json_typeof(rsError)); + return ERROR; + } return OK; } if (meta != DYN_FUNCTION_ARGUMENT_META__PRE_ALLOCATED_OUTPUT && meta != DYN_FUNCTION_ARGUMENT_META__OUTPUT) { diff --git a/libs/dfi/src/json_serializer.c b/libs/dfi/src/json_serializer.c index 453d6128f..58aec80ee 100644 --- a/libs/dfi/src/json_serializer.c +++ b/libs/dfi/src/json_serializer.c @@ -129,40 +129,100 @@ static int jsonSerializer_parseAny(const dyn_type* type, void* loc, json_t* val) switch (c) { case 'Z' : - *(bool*)loc = (bool) json_is_true(val); + if (json_is_boolean(val)) { + *(bool*)loc = (bool) json_is_true(val); + } else { + status = ERROR; + celix_err_pushf("Expected json boolean type but got %i", json_typeof(val)); + } break; case 'F' : - *(float*)loc = (float) json_real_value(val); + if (json_is_real(val)) { + *(float*)loc = (float) json_real_value(val); + } else { + status = ERROR; + celix_err_pushf("Expected json real type but got %i", json_typeof(val)); + } break; case 'D' : - *(double*)loc = json_real_value(val); + if (json_is_real(val)) { + *(double*)loc = json_real_value(val); + } else { + status = ERROR; + celix_err_pushf("Expected json real type but got %i", json_typeof(val)); + } break; case 'N' : - *(int*)loc = (int) json_integer_value(val); + if (json_is_integer(val)) { + *(int*)loc = (int) json_integer_value(val); + } else { + status = ERROR; + celix_err_pushf("Expected json integer type but got %i", json_typeof(val)); + } break; case 'B' : - *(char*)loc = (char) json_integer_value(val); + if (json_is_integer(val)) { + *(char*)loc = (char) json_integer_value(val); + } else { + status = ERROR; + celix_err_pushf("Expected json integer type but got %i", json_typeof(val)); + } break; case 'S' : - *(int16_t*)loc = (int16_t) json_integer_value(val); + if (json_is_integer(val)) { + *(int16_t*)loc = (int16_t) json_integer_value(val); + } else { + status = ERROR; + celix_err_pushf("Expected json integer type but got %i", json_typeof(val)); + } break; case 'I' : - *(int32_t*)loc = (int32_t) json_integer_value(val); + if (json_is_integer(val)) { + *(int32_t*)loc = (int32_t) json_integer_value(val); + } else { + status = ERROR; + celix_err_pushf("Expected json integer type but got %i", json_typeof(val)); + } break; case 'J' : - *(int64_t*)loc = (int64_t) json_integer_value(val); + if (json_is_integer(val)) { + *(int64_t*)loc = (int64_t) json_integer_value(val); + } else { + status = ERROR; + celix_err_pushf("Expected json integer type but got %i", json_typeof(val)); + } break; case 'b' : - *(uint8_t*)loc = (uint8_t) json_integer_value(val); + if (json_is_integer(val)) { + *(uint8_t*)loc = (uint8_t) json_integer_value(val); + } else { + status = ERROR; + celix_err_pushf("Expected json integer type but got %i", json_typeof(val)); + } break; case 's' : - *(uint16_t*)loc = (uint16_t) json_integer_value(val); + if (json_is_integer(val)) { + *(uint16_t*)loc = (uint16_t) json_integer_value(val); + } else { + status = ERROR; + celix_err_pushf("Expected json integer type but got %i", json_typeof(val)); + } break; case 'i' : - *(uint32_t*)loc = (uint32_t) json_integer_value(val); + if (json_is_integer(val)) { + *(uint32_t*)loc = (uint32_t) json_integer_value(val); + } else { + status = ERROR; + celix_err_pushf("Expected json integer type but got %i", json_typeof(val)); + } break; case 'j' : - *(uint64_t*)loc = (uint64_t) json_integer_value(val); + if (json_is_integer(val)) { + *(uint64_t*)loc = (uint64_t) json_integer_value(val); + } else { + status = ERROR; + celix_err_pushf("Expected json integer type but got %i", json_typeof(val)); + } break; case 'E' : if (json_is_string(val)){ diff --git a/libs/etcdlib/src/etcd.c b/libs/etcdlib/src/etcd.c index 37d1e4295..a4b806f65 100644 --- a/libs/etcdlib/src/etcd.c +++ b/libs/etcdlib/src/etcd.c @@ -252,7 +252,7 @@ etcd_get_recursive_values(json_t *js_root, etcdlib_key_value_callback callback, static long long etcd_get_current_index(const char *headerData) { long long index = -1; - char *indexStr = strstr(headerData, ETCD_HEADER_INDEX); + const char *indexStr = strstr(headerData, ETCD_HEADER_INDEX); indexStr += strlen(ETCD_HEADER_INDEX); if (sscanf(indexStr, "%lld\n", &index) == 1) { From 26adf3510d9154d61b83a9e09dfe09f70a6b1c45 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Mon, 28 Sep 2026 11:49:10 +0800 Subject: [PATCH 2/2] Add tests for handling invalid JSON replies and type mismatches in JSON serializer. --- libs/dfi/gtest/src/json_rpc_tests.cpp | 38 ++++++++++++++++++++ libs/dfi/gtest/src/json_serializer_tests.cpp | 16 +++++++++ 2 files changed, 54 insertions(+) diff --git a/libs/dfi/gtest/src/json_rpc_tests.cpp b/libs/dfi/gtest/src/json_rpc_tests.cpp index b01470999..644965c52 100644 --- a/libs/dfi/gtest/src/json_rpc_tests.cpp +++ b/libs/dfi/gtest/src/json_rpc_tests.cpp @@ -417,6 +417,40 @@ extern "C" { dynInterface_destroy(intf); } + void handleTestInvalidReplyError(void) { + dyn_interface_type *intf = nullptr; + FILE *desc = fopen("descriptors/example1.descriptor", "r"); + ASSERT_TRUE(desc != nullptr); + int rc = dynInterface_parse(desc, &intf); + ASSERT_EQ(0, rc); + fclose(desc); + + dyn_function_type *func = dynInterface_findMethod(intf, "add(DD)D")->dynFunc; + assert(func != nullptr); + + const char *reply = R"({"e":"not_an_error_code"})"; + + void *args[4]; + args[0] = nullptr; + args[1] = nullptr; + args[2] = nullptr; + args[3] = nullptr; + + double result = 0; + void *out = &result; + args[3] = &out; + + int rsErrno = 0; + rc = jsonRpc_handleReply(func, reply, args, &rsErrno); + ASSERT_NE(0, rc); + ASSERT_EQ(0, rsErrno); + char expectedErr[64]; + snprintf(expectedErr, sizeof(expectedErr), "Expected json integer for error code but got %i", JSON_STRING); + EXPECT_STREQ(expectedErr, celix_err_popLastError()); + + dynInterface_destroy(intf); + } + void callTestUnknownRequest(void) { dyn_interface_type *intf = nullptr; FILE *desc = fopen("descriptors/example1.descriptor", "r"); @@ -1119,6 +1153,10 @@ TEST_F(JsonRpcTests, handleReplyError) { handleTestReplyError(); } +TEST_F(JsonRpcTests, handleInvalidReplyError) { + handleTestInvalidReplyError(); +} + TEST_F(JsonRpcTests, handleTestOutWithEmptyReply) { handleTestOutWithEmptyReply(); } diff --git a/libs/dfi/gtest/src/json_serializer_tests.cpp b/libs/dfi/gtest/src/json_serializer_tests.cpp index ca98523f1..57aa34ecc 100644 --- a/libs/dfi/gtest/src/json_serializer_tests.cpp +++ b/libs/dfi/gtest/src/json_serializer_tests.cpp @@ -592,6 +592,22 @@ static void parseTests() { celix_err_printErrors(stderr, nullptr, nullptr); dynType_destroy(type); + // integer type mismatches - string instead of integer for all remaining integer descriptors + const char *integerDescriptors[] = {"N", "B", "S", "J", "b", "s", "i", "j"}; + for (const char *descriptor : integerDescriptors) { + char descriptorStr[8]; + snprintf(descriptorStr, sizeof(descriptorStr), "{%s a}", descriptor); + rc = dynType_parseWithStr(descriptorStr, nullptr, nullptr, &type); + ASSERT_EQ(0, rc); + inputStr = R"({"a":"not_an_integer"})"; + rc = jsonSerializer_deserialize(type, inputStr, strlen(inputStr), &inst); + ASSERT_EQ(1, rc); + char expectedErr[64]; + snprintf(expectedErr, sizeof(expectedErr), "Expected json integer type but got %i", JSON_STRING); + EXPECT_STREQ("Error cannot deserialize json. Input is '{\"a\":\"not_an_integer\"}'", celix_err_popLastError()); + EXPECT_STREQ(expectedErr, celix_err_popLastError()); + dynType_destroy(type); + } // extra member ("b") is allowed rc = dynType_parseWithStr("{t a}", nullptr, nullptr, &type);