From 72c52962e49b3aab29a54c801dfdba25256cc02a Mon Sep 17 00:00:00 2001 From: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Date: Thu, 10 Sep 2026 17:46:21 +0100 Subject: [PATCH] Part #120, reject unmatched derived command dispatch --- cfecfs/edsmsg/CMakeLists.txt | 1 + cfecfs/edsmsg/fsw/inc/edsmsg_dispatcher.h | 4 + cfecfs/edsmsg/fsw/src/edsmsg_dispatcher.c | 30 ++- cfecfs/edsmsg/fsw/unit-test/CMakeLists.txt | 15 ++ .../fsw/unit-test/edsmsg_dispatcher_test.c | 210 ++++++++++++++++++ 5 files changed, 252 insertions(+), 8 deletions(-) create mode 100644 cfecfs/edsmsg/fsw/unit-test/CMakeLists.txt create mode 100644 cfecfs/edsmsg/fsw/unit-test/edsmsg_dispatcher_test.c diff --git a/cfecfs/edsmsg/CMakeLists.txt b/cfecfs/edsmsg/CMakeLists.txt index bd003f9..122c29d 100644 --- a/cfecfs/edsmsg/CMakeLists.txt +++ b/cfecfs/edsmsg/CMakeLists.txt @@ -29,4 +29,5 @@ target_link_libraries(edsmsg PRIVATE core_private) if (ENABLE_UNIT_TESTS) add_subdirectory(fsw/ut-stubs) + add_subdirectory(fsw/unit-test) endif() diff --git a/cfecfs/edsmsg/fsw/inc/edsmsg_dispatcher.h b/cfecfs/edsmsg/fsw/inc/edsmsg_dispatcher.h index c7c0303..9692e21 100644 --- a/cfecfs/edsmsg/fsw/inc/edsmsg_dispatcher.h +++ b/cfecfs/edsmsg/fsw/inc/edsmsg_dispatcher.h @@ -63,6 +63,10 @@ * * If the message payload is defined by the application's EDS, the correct * handler function in the dispatch table will be called. + * Types with registered derivatives must match one of those derivatives; + * a failed identification returns CFE_STATUS_VALIDATION_FAILURE without invoking + * a handler. Only types without derivatives use dispatch table position zero + * directly. Both paths validate the message size against the selected type. * * \note this function is generally not directly invoked from applications, it * should be invoked through a wrapper generated from the EDS tool. The wrapper diff --git a/cfecfs/edsmsg/fsw/src/edsmsg_dispatcher.c b/cfecfs/edsmsg/fsw/src/edsmsg_dispatcher.c index 9d8bbc7..bea9bcb 100644 --- a/cfecfs/edsmsg/fsw/src/edsmsg_dispatcher.c +++ b/cfecfs/edsmsg/fsw/src/edsmsg_dispatcher.c @@ -205,6 +205,7 @@ CFE_Status_t CFE_EDSMSG_Dispatch_CheckActualBufferType(const CFE_SB_Buffer_t *Bu { const EdsLib_DatabaseObject_t *GD; EdsLib_DataTypeDB_TypeInfo_t TypeInfo; + EdsLib_DataTypeDB_DerivedTypeInfo_t DerivedInfo; EdsLib_DataTypeDB_DerivativeObjectInfo_t DerivObjInfo; int32_t Status; CFE_MSG_Size_t MessageSize; @@ -212,21 +213,34 @@ CFE_Status_t CFE_EDSMSG_Dispatch_CheckActualBufferType(const CFE_SB_Buffer_t *Bu GD = CFE_Config_GetObjPointer(CFE_CONFIGID_MISSION_EDS_DB); - CFE_MSG_GetSize(&Buffer->Msg, &MessageSize); + ReturnCode = CFE_MSG_GetSize(&Buffer->Msg, &MessageSize); + if (ReturnCode != CFE_SUCCESS) + { + return ReturnCode; + } - /* Check if the argument type has derivatives. This is typical for CMD interfaces where there - * are many possible command codes, and in this case each command code will have its own - * entry in the dispatch table. If this fails, it does not fail the overall process, it just - * means that the argument is not derived. */ - Status = EdsLib_DataTypeDB_IdentifyBufferWithSize(GD, *EdsId, Buffer, MessageSize, &DerivObjInfo); - if (Status == EDSLIB_SUCCESS) + /* A failed identification is not evidence that a type is non-derived. + * Consult the database before allowing the sole-handler fallback. */ + Status = EdsLib_DataTypeDB_GetDerivedInfo(GD, *EdsId, &DerivedInfo); + if (Status != EDSLIB_SUCCESS) { + return CFE_SB_INTERNAL_ERR; + } + + if (DerivedInfo.NumDerivatives > 0) + { + Status = EdsLib_DataTypeDB_IdentifyBufferWithSize(GD, *EdsId, Buffer, MessageSize, &DerivObjInfo); + if (Status != EDSLIB_SUCCESS) + { + return CFE_STATUS_VALIDATION_FAILURE; + } + *EdsId = DerivObjInfo.EdsId; *DispatchTblPosition = DerivObjInfo.DerivativeTableIndex; } else { - /* Non derived, there is just one entry, it is always first */ + /* A genuinely non-derived type has exactly one handler. */ *DispatchTblPosition = 0; } diff --git a/cfecfs/edsmsg/fsw/unit-test/CMakeLists.txt b/cfecfs/edsmsg/fsw/unit-test/CMakeLists.txt new file mode 100644 index 0000000..32fb550 --- /dev/null +++ b/cfecfs/edsmsg/fsw/unit-test/CMakeLists.txt @@ -0,0 +1,15 @@ +# Exercise the real dispatcher against the existing cFE, EDS, and missionlib stubs. +add_executable(edsmsg-dispatcher-test + edsmsg_dispatcher_test.c + ../src/edsmsg_dispatcher.c +) +target_include_directories(edsmsg-dispatcher-test PRIVATE + $ +) +target_compile_options(edsmsg-dispatcher-test PRIVATE ${UT_COVERAGE_COMPILE_FLAGS}) +target_link_libraries(edsmsg-dispatcher-test + ${UT_COVERAGE_LINK_FLAGS} + ut_core_api_stubs ut_missionlib_stubs ut_edslib_stubs + cfe_missionlib_sb_dispatchdb_static cfe_edsdb_static ut_assert +) +add_test(edsmsg-dispatcher edsmsg-dispatcher-test) diff --git a/cfecfs/edsmsg/fsw/unit-test/edsmsg_dispatcher_test.c b/cfecfs/edsmsg/fsw/unit-test/edsmsg_dispatcher_test.c new file mode 100644 index 0000000..0946a02 --- /dev/null +++ b/cfecfs/edsmsg/fsw/unit-test/edsmsg_dispatcher_test.c @@ -0,0 +1,210 @@ +/* +** GSC-18128-1, "Core Flight Executive Version 6.7" +** +** Copyright (c) 2006-2019 United States Government as represented by +** the Administrator of the National Aeronautics and Space Administration. +** All Rights Reserved. +** +** Licensed under the Apache License, Version 2.0 (the "License"); +** you may not use this file except in compliance with the License. +** You may obtain a copy of the License at +** +** http://www.apache.org/licenses/LICENSE-2.0 +** +** Unless required by applicable law or agreed to in writing, software +** distributed under the License is distributed on an "AS IS" BASIS, +** WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +** See the License for the specific language governing permissions and +** limitations under the License. +*/ + +#include +#include "cfe_error.h" +#include "cfe_msg.h" +#include "edsmsg_dispatcher.h" +#include "edslib_datatypedb.h" +#include "edslib_intfdb.h" +#include "cfe_missionlib_api.h" +#include "cfe_missionlib_runtime.h" +#include "utassert.h" +#include "utstubs.h" +#include "uttest.h" + +#define TEST_COMPONENT EDSLIB_INTF_ID(1, 1) +#define TEST_BASE_TYPE EDSLIB_MAKE_ID(1, 1) +#define TEST_DERIVED_TYPE EDSLIB_MAKE_ID(1, 2) + +static CFE_SB_Buffer_t Buffer; +static CFE_MSG_Size_t MessageSize; +static EdsLib_DataTypeDB_DerivedTypeInfo_t DerivedInfo; +static EdsLib_DataTypeDB_DerivativeObjectInfo_t IdentifiedType; +static EdsLib_DataTypeDB_TypeInfo_t TypeInfo; +static EdsLib_Id_t LookedUpType; +static unsigned int HandlerCalls[2]; + +static CFE_Status_t Handler0(const CFE_SB_Buffer_t *Message) +{ + UtAssert_True(Message == &Buffer, "Original buffer passed to handler zero"); + ++HandlerCalls[0]; + return 10; +} + +static CFE_Status_t Handler1(const CFE_SB_Buffer_t *Message) +{ + UtAssert_True(Message == &Buffer, "Original buffer passed to handler one"); + ++HandlerCalls[1]; + return 11; +} + +static void GetInterface(void *UserObj, UT_EntryKey_t FuncKey, const UT_StubContext_t *Context) +{ + EdsLib_IntfDB_InterfaceInfo_t *Info = + UT_Hook_GetArgValueByName(Context, "IntfInfoBuffer", EdsLib_IntfDB_InterfaceInfo_t *); + memset(Info, 0, sizeof(*Info)); + Info->ParentCompEdsId = TEST_COMPONENT; +} + +static void GetId(void *UserObj, UT_EntryKey_t FuncKey, const UT_StubContext_t *Context) +{ + *UT_Hook_GetArgValueByName(Context, "IdBuffer", EdsLib_Id_t *) = TEST_BASE_TYPE; +} + +static void GetDerived(void *UserObj, UT_EntryKey_t FuncKey, const UT_StubContext_t *Context) +{ + *UT_Hook_GetArgValueByName(Context, "DerivInfo", EdsLib_DataTypeDB_DerivedTypeInfo_t *) = DerivedInfo; +} + +static void Identify(void *UserObj, UT_EntryKey_t FuncKey, const UT_StubContext_t *Context) +{ + *UT_Hook_GetArgValueByName(Context, "DerivObjInfo", EdsLib_DataTypeDB_DerivativeObjectInfo_t *) = IdentifiedType; +} + +static void GetType(void *UserObj, UT_EntryKey_t FuncKey, const UT_StubContext_t *Context) +{ + LookedUpType = UT_Hook_GetArgValueByName(Context, "EdsId", EdsLib_Id_t); + *UT_Hook_GetArgValueByName(Context, "TypeInfo", EdsLib_DataTypeDB_TypeInfo_t *) = TypeInfo; +} + +static void Setup(void) +{ + UT_ResetState(0); + memset(&Buffer, 0, sizeof(Buffer)); + memset(&DerivedInfo, 0, sizeof(DerivedInfo)); + memset(&IdentifiedType, 0, sizeof(IdentifiedType)); + memset(&TypeInfo, 0, sizeof(TypeInfo)); + memset(HandlerCalls, 0, sizeof(HandlerCalls)); + MessageSize = sizeof(CFE_MSG_CommandHeader_t); + TypeInfo.Size.Bytes = MessageSize; + DerivedInfo.NumDerivatives = 2; + IdentifiedType.EdsId = TEST_DERIVED_TYPE; + IdentifiedType.DerivativeTableIndex = 1; + LookedUpType = EDSLIB_ID_INVALID; + UT_SetDefaultReturnValue(UT_KEY(CFE_MissionLib_PubSub_IsListenerComponent), 1); + UT_SetDataBuffer(UT_KEY(CFE_MSG_GetSize), &MessageSize, sizeof(MessageSize), false); + UT_SetHandlerFunction(UT_KEY(EdsLib_IntfDB_GetComponentInterfaceInfo), GetInterface, NULL); + UT_SetHandlerFunction(UT_KEY(EdsLib_IntfDB_FindAllCommands), GetId, NULL); + UT_SetHandlerFunction(UT_KEY(EdsLib_IntfDB_FindAllArgumentTypes), GetId, NULL); + UT_SetHandlerFunction(UT_KEY(EdsLib_DataTypeDB_GetDerivedInfo), GetDerived, NULL); + UT_SetHandlerFunction(UT_KEY(EdsLib_DataTypeDB_IdentifyBufferWithSize), Identify, NULL); + UT_SetHandlerFunction(UT_KEY(EdsLib_DataTypeDB_GetTypeInfo), GetType, NULL); +} + +static CFE_Status_t Dispatch(void) +{ + CFE_Status_t (*const Handlers[])(const CFE_SB_Buffer_t *) = { Handler0, Handler1 }; + return CFE_EDSMSG_Dispatch(TEST_COMPONENT, TEST_COMPONENT, &Buffer, Handlers); +} + +static void AssertNoHandler(void) +{ + UtAssert_UINT32_EQ(HandlerCalls[0], 0); + UtAssert_UINT32_EQ(HandlerCalls[1], 0); +} + +static void TestMatchingDerivatives(void) +{ + IdentifiedType.DerivativeTableIndex = 0; + UtAssert_INT32_EQ(Dispatch(), 10); + UtAssert_UINT32_EQ(LookedUpType, TEST_DERIVED_TYPE); + UtAssert_UINT32_EQ(HandlerCalls[0], 1); + UtAssert_UINT32_EQ(HandlerCalls[1], 0); + Setup(); + UtAssert_INT32_EQ(Dispatch(), 11); + UtAssert_UINT32_EQ(LookedUpType, TEST_DERIVED_TYPE); + UtAssert_UINT32_EQ(HandlerCalls[0], 0); + UtAssert_UINT32_EQ(HandlerCalls[1], 1); +} + +static void TestUnmatchedDerivative(void) +{ + UT_SetDefaultReturnValue(UT_KEY(EdsLib_DataTypeDB_IdentifyBufferWithSize), EDSLIB_NO_MATCHING_VALUE); + UtAssert_INT32_EQ(Dispatch(), CFE_STATUS_VALIDATION_FAILURE); + AssertNoHandler(); + UtAssert_STUB_COUNT(EdsLib_DataTypeDB_GetTypeInfo, 0); +} + +static void TestIdentificationError(void) +{ + UT_SetDefaultReturnValue(UT_KEY(EdsLib_DataTypeDB_IdentifyBufferWithSize), EDSLIB_FAILURE); + UtAssert_INT32_EQ(Dispatch(), CFE_STATUS_VALIDATION_FAILURE); + AssertNoHandler(); +} + +static void TestNonDerivedType(void) +{ + DerivedInfo.NumDerivatives = 0; + UT_SetDefaultReturnValue(UT_KEY(EdsLib_DataTypeDB_IdentifyBufferWithSize), EDSLIB_NO_MATCHING_VALUE); + UtAssert_INT32_EQ(Dispatch(), 10); + UtAssert_UINT32_EQ(LookedUpType, TEST_BASE_TYPE); + UtAssert_STUB_COUNT(EdsLib_DataTypeDB_IdentifyBufferWithSize, 0); + UtAssert_UINT32_EQ(HandlerCalls[0], 1); + UtAssert_UINT32_EQ(HandlerCalls[1], 0); +} + +static void TestDerivedInfoError(void) +{ + UT_SetDefaultReturnValue(UT_KEY(EdsLib_DataTypeDB_GetDerivedInfo), EDSLIB_FAILURE); + UtAssert_INT32_EQ(Dispatch(), CFE_SB_INTERNAL_ERR); + AssertNoHandler(); + UtAssert_STUB_COUNT(EdsLib_DataTypeDB_IdentifyBufferWithSize, 0); +} + +static void TestTypeInfoError(void) +{ + UT_SetDefaultReturnValue(UT_KEY(EdsLib_DataTypeDB_GetTypeInfo), EDSLIB_FAILURE); + UtAssert_INT32_EQ(Dispatch(), CFE_SB_INTERNAL_ERR); + AssertNoHandler(); +} + +static void TestMessageSizeError(void) +{ + UT_SetDefaultReturnValue(UT_KEY(CFE_MSG_GetSize), CFE_STATUS_WRONG_MSG_LENGTH); + UtAssert_INT32_EQ(Dispatch(), CFE_STATUS_WRONG_MSG_LENGTH); + AssertNoHandler(); + UtAssert_STUB_COUNT(EdsLib_DataTypeDB_GetDerivedInfo, 0); + UtAssert_STUB_COUNT(EdsLib_DataTypeDB_IdentifyBufferWithSize, 0); +} + +static void TestWrongMessageLength(void) +{ + ++TypeInfo.Size.Bytes; + UtAssert_INT32_EQ(Dispatch(), CFE_STATUS_WRONG_MSG_LENGTH); + AssertNoHandler(); + Setup(); + DerivedInfo.NumDerivatives = 0; + --TypeInfo.Size.Bytes; + UtAssert_INT32_EQ(Dispatch(), CFE_STATUS_WRONG_MSG_LENGTH); + AssertNoHandler(); +} + +void UtTest_Setup(void) +{ + UtTest_Add(TestMatchingDerivatives, Setup, NULL, "Matching derivatives keep their handlers"); + UtTest_Add(TestUnmatchedDerivative, Setup, NULL, "Unknown derivative never invokes handler zero"); + UtTest_Add(TestIdentificationError, Setup, NULL, "Identification errors do not fall back"); + UtTest_Add(TestNonDerivedType, Setup, NULL, "Non-derived type retains handler zero"); + UtTest_Add(TestDerivedInfoError, Setup, NULL, "Database metadata errors are rejected"); + UtTest_Add(TestTypeInfoError, Setup, NULL, "Type information errors are rejected"); + UtTest_Add(TestMessageSizeError, Setup, NULL, "Unreadable message size is rejected"); + UtTest_Add(TestWrongMessageLength, Setup, NULL, "Derived and base size checks remain enforced"); +}