diff --git a/source/bulkdata/profile.c b/source/bulkdata/profile.c index 67a2e554..2c3d3aae 100644 --- a/source/bulkdata/profile.c +++ b/source/bulkdata/profile.c @@ -24,6 +24,7 @@ #include #include #include +#include #include "profile.h" #include "reportprofiles.h" @@ -73,11 +74,15 @@ static bool isEmptySubscriberValue(const char *value) { return true; } - if(strlen(value) < 1 || value[0] == ' ' || strncmp(value, "NULL", 4) == 0) + if(strncmp(value, "NULL", sizeof("NULL")) == 0) { return true; } - return false; + while(*value != '\0' && isspace((unsigned char) * value)) + { + value++; + } + return (*value == '\0'); } static void freeRequestURIparam(void *data) @@ -424,6 +429,11 @@ static void* CollectAndReport(void* data) pthread_mutex_unlock(&profile->reportInProgressMutex); pthread_mutex_unlock(&profile->reuseThreadMutex); + /* xconfProfileLock (static in profilexconf.c) guards only the XConf singleProfile's + * GrepSeekProfile. This multi-profile's grepSeekProfile is a separate instance that is + * read/replaced exclusively by this report thread and freed only after the thread is + * joined, so no cross-thread access exists here. */ + /* coverity[missing_lock] */ int count = profile->grepSeekProfile->execCounter; Vector *profileParamVals = NULL; diff --git a/source/bulkdata/profilexconf.c b/source/bulkdata/profilexconf.c index 41abe597..5213a35e 100644 --- a/source/bulkdata/profilexconf.c +++ b/source/bulkdata/profilexconf.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "profilexconf.h" @@ -63,11 +64,15 @@ static bool isEmptySubscriberValue(const char *value) { return true; } - if(strlen(value) < 1 || value[0] == ' ' || strncmp(value, "NULL", 4) == 0) + if(strncmp(value, "NULL", sizeof("NULL")) == 0) { return true; } - return false; + while(*value != '\0' && isspace((unsigned char) * value)) + { + value++; + } + return (*value == '\0'); } #ifdef GTEST_ENABLE @@ -384,10 +389,10 @@ static void* CollectAndReportXconf(void* data) { encodeEventMarkersInJSON(valArray, profile->eMarkerList); } - pthread_mutex_unlock(&xconfProfileLock); profile->grepSeekProfile->execCounter += 1; T2Info("Xconf Profile Execution Count = %d\n", profile->grepSeekProfile->execCounter); + pthread_mutex_unlock(&xconfProfileLock); ret = prepareJSONReport(profile->jsonReportObj, &jsonReport); destroyJSONReport(profile->jsonReportObj); diff --git a/source/reportgen/reportgen.c b/source/reportgen/reportgen.c index 98834b1f..90525a0c 100644 --- a/source/reportgen/reportgen.c +++ b/source/reportgen/reportgen.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -55,6 +56,20 @@ static bool checkForEmptyString( char* valueString ) return isEmpty; } +// Unlike checkForEmptyString, a value with leading/trailing spaces but real content is not empty. +static bool isEmptyMarkerValue(const char *value) +{ + if(value == NULL || strncmp(value, "NULL", sizeof("NULL")) == 0) + { + return true; + } + while(*value != '\0' && isspace((unsigned char) * value)) + { + value++; + } + return (*value == '\0'); +} + void freeParamValueSt(tr181ValStruct_t **valStructs, int valSize) { int i; @@ -141,6 +156,7 @@ void trimLeadingAndTrailingws(char* string) string[j++] = string[i]; } string[j] = '\0'; + j = -1; for(i = 0; string[i] != '\0'; i++) { @@ -1567,7 +1583,7 @@ T2ERROR encodeEventMarkersInJSON(cJSON *valArray, Vector *eventMarkerList) default: if(eventMarker->u.markerValue != NULL) { - if(!eventMarker->reportEmptyParam && checkForEmptyString(eventMarker->u.markerValue)) + if(!eventMarker->reportEmptyParam && isEmptyMarkerValue(eventMarker->u.markerValue)) { T2Debug("Skipping empty/\"NULL\" marker value for : %s\n", eventMarker->markerName); free(eventMarker->u.markerValue); diff --git a/source/test/bulkdata/profileTest.cpp b/source/test/bulkdata/profileTest.cpp index f76e377c..3ec06a54 100644 --- a/source/test/bulkdata/profileTest.cpp +++ b/source/test/bulkdata/profileTest.cpp @@ -18,6 +18,7 @@ #include "test/mocks/VectorMock.h" #include "test/bulkdata/SchedulerMock.h" #include "test/bulkdata/profileMock.h" +#include using namespace std; @@ -371,6 +372,86 @@ TEST_F(ProfileTest, TriggerReportOnCondition_SchedulerFailureKeepsActiveMutexOwn test_reset_profile_list(); } +class ProfileMarkerStorageTest : public ProfileTest, + public ::testing::WithParamInterface> { +protected: + Profile profile = {}; + EventMarker marker = {}; + Vector *profiles = nullptr; + + void SetUp() override { + ProfileTest::SetUp(); + delete g_vectorMock; + g_vectorMock = nullptr; + ASSERT_EQ(Vector_Create(&profiles), T2ERROR_SUCCESS); + ASSERT_EQ(Vector_Create(&profile.eMarkerList), T2ERROR_SUCCESS); + profile.name = strdup("StorageProfile"); + profile.enable = true; + ASSERT_EQ(pthread_mutex_init(&profile.eventMutex, nullptr), 0); + marker.markerName = strdup("StorageMarker"); + marker.mType = std::get<0>(GetParam()); + marker.reportEmptyParam = std::get<1>(GetParam()); + if(marker.mType == MTYPE_ACCUMULATE) { + ASSERT_EQ(Vector_Create(&marker.u.accumulatedValues), T2ERROR_SUCCESS); + } + ASSERT_EQ(Vector_PushBack(profile.eMarkerList, &marker), T2ERROR_SUCCESS); + ASSERT_EQ(Vector_PushBack(profiles, &profile), T2ERROR_SUCCESS); + test_set_profile_list(profiles); + } + + void TearDown() override { + test_reset_profile_list(); + if(marker.mType == MTYPE_ACCUMULATE) { + Vector_Destroy(marker.u.accumulatedValues, free); + } else { + free(marker.u.markerValue); + } + free(marker.markerName); + Vector_Destroy(profile.eMarkerList, nullptr); + Vector_Destroy(profiles, nullptr); + pthread_mutex_destroy(&profile.eventMutex); + free(profile.name); + ProfileTest::TearDown(); + } +}; + +TEST_P(ProfileMarkerStorageTest, StoresClassifiedValuesWithoutTrimming) { + const struct { + const char *value; + bool empty; + } cases[] = { + {" test log ", false}, {" xyz ", false}, {"NULLify", false}, + {"", true}, {" ", true}, {"\t\r\n", true}, {"NULL", true} + }; + for(const auto &testCase : cases) { + SCOPED_TRACE(testCase.value); + const bool retained = marker.reportEmptyParam || !testCase.empty; + if(marker.mType == MTYPE_ACCUMULATE) { + Vector_Clear(marker.u.accumulatedValues, free); + ASSERT_EQ(Vector_PushBack(marker.u.accumulatedValues, strdup("previous")), T2ERROR_SUCCESS); + } else { + free(marker.u.markerValue); + marker.u.markerValue = strdup("previous"); + } + T2Event event = {}; + event.name = marker.markerName; + event.value = const_cast(testCase.value); + EXPECT_EQ(Profile_storeMarkerEvent(profile.name, &event), T2ERROR_SUCCESS); + if(marker.mType == MTYPE_ACCUMULATE) { + ASSERT_EQ(Vector_Size(marker.u.accumulatedValues), retained ? 2u : 1u); + EXPECT_STREQ(static_cast(Vector_At(marker.u.accumulatedValues, 0)), "previous"); + if(retained) { + EXPECT_STREQ(static_cast(Vector_At(marker.u.accumulatedValues, 1)), testCase.value); + } + } else { + EXPECT_STREQ(marker.u.markerValue, retained ? testCase.value : "previous"); + } + } +} + +INSTANTIATE_TEST_SUITE_P(SubscriberValues, ProfileMarkerStorageTest, + ::testing::Combine(::testing::Values(MTYPE_ABSOLUTE, MTYPE_ACCUMULATE), ::testing::Bool())); + TEST_F(ProfileTest, getMinThresholdDuration_Failure) { EXPECT_CALL(*g_vectorMock, Vector_Size(_)).Times(::testing::AtMost(1)).WillRepeatedly(Return(0)); EXPECT_EQ(getMinThresholdDuration("profile1"), 0); diff --git a/source/test/bulkdata/profilexconfTest.cpp b/source/test/bulkdata/profilexconfTest.cpp index e2ad1068..d84184f2 100644 --- a/source/test/bulkdata/profilexconfTest.cpp +++ b/source/test/bulkdata/profilexconfTest.cpp @@ -551,6 +551,80 @@ TEST_F(profileXconfTestFixture, ReportProfiles_storeMarkerEvent_success_2) free(eventInfo); } +class XconfMarkerStorageTest : public profileXconfTestFixture, + public ::testing::WithParamInterface> { +protected: + ProfileXConf *profile = nullptr; + EventMarker *marker = nullptr; + + void SetUp() override { + profileXconfTestFixture::SetUp(); + delete g_fileIOMock; + g_fileIOMock = new ::testing::NiceMock(); + EXPECT_CALL(*g_fileIOMock, opendir(_)) + .WillRepeatedly(Return(nullptr)); + ASSERT_EQ(ProfileXConf_init(false), T2ERROR_SUCCESS); + profile = static_cast(calloc(1, sizeof(ProfileXConf))); + ASSERT_NE(profile, nullptr); + profile->name = strdup("StorageProfile"); + ASSERT_EQ(Vector_Create(&profile->eMarkerList), T2ERROR_SUCCESS); + EXPECT_CALL(*g_schedulerMock, registerProfileWithScheduler(_, _, _, _, _, _, _, _)) + .WillOnce(Return(T2ERROR_SUCCESS)); + ASSERT_EQ(ProfileXConf_set(profile), T2ERROR_SUCCESS); + marker = static_cast(calloc(1, sizeof(EventMarker))); + ASSERT_NE(marker, nullptr); + marker->markerName = strdup("StorageMarker"); + marker->mType = std::get<0>(GetParam()); + marker->reportEmptyParam = std::get<1>(GetParam()); + if(marker->mType == MTYPE_XCONF_ACCUMULATE) { + ASSERT_EQ(Vector_Create(&marker->u.accumulatedValues), T2ERROR_SUCCESS); + } + ASSERT_EQ(Vector_PushBack(profile->eMarkerList, marker), T2ERROR_SUCCESS); + } + + void TearDown() override { + EXPECT_EQ(ProfileXConf_uninit(), T2ERROR_SUCCESS); + profileXconfTestFixture::TearDown(); + } +}; + +TEST_P(XconfMarkerStorageTest, StoresClassifiedValuesWithoutTrimming) { + const struct { + const char *value; + bool empty; + } cases[] = { + {" test log ", false}, {" xyz ", false}, {"NULLify", false}, + {"", true}, {" ", true}, {"\t\r\n", true}, {"NULL", true} + }; + for(const auto &testCase : cases) { + SCOPED_TRACE(testCase.value); + const bool retained = marker->reportEmptyParam || !testCase.empty; + if(marker->mType == MTYPE_XCONF_ACCUMULATE) { + Vector_Clear(marker->u.accumulatedValues, free); + ASSERT_EQ(Vector_PushBack(marker->u.accumulatedValues, strdup("previous")), T2ERROR_SUCCESS); + } else { + free(marker->u.markerValue); + marker->u.markerValue = strdup("previous"); + } + T2Event event = {}; + event.name = marker->markerName; + event.value = const_cast(testCase.value); + EXPECT_EQ(ProfileXConf_storeMarkerEvent(&event), T2ERROR_SUCCESS); + if(marker->mType == MTYPE_XCONF_ACCUMULATE) { + ASSERT_EQ(Vector_Size(marker->u.accumulatedValues), retained ? 2u : 1u); + EXPECT_STREQ(static_cast(Vector_At(marker->u.accumulatedValues, 0)), "previous"); + if(retained) { + EXPECT_STREQ(static_cast(Vector_At(marker->u.accumulatedValues, 1)), testCase.value); + } + } else { + EXPECT_STREQ(marker->u.markerValue, retained ? testCase.value : "previous"); + } + } +} + +INSTANTIATE_TEST_SUITE_P(SubscriberValues, XconfMarkerStorageTest, + ::testing::Combine(::testing::Values(MTYPE_XCONF_ABSOLUTE, MTYPE_XCONF_ACCUMULATE), ::testing::Bool())); + TEST_F(profileXconfTestFixture, ProfileXConf_OnDemandStatusSnapshot) { testing::InSequence sequence; diff --git a/source/test/reportgen/reportgenTest.cpp b/source/test/reportgen/reportgenTest.cpp index a8b00bc9..15b81389 100644 --- a/source/test/reportgen/reportgenTest.cpp +++ b/source/test/reportgen/reportgenTest.cpp @@ -39,6 +39,8 @@ sigset_t blocking_signal; #include "../mocks/rdklogMock.h" #include #include +#include +#include using namespace std; using ::testing::_; @@ -1526,6 +1528,101 @@ TEST_F(reportgenTestFixture, encodeEventMarkersInJSON7) Vector_Destroy(eventMarkerList, freeEMarker); } +class MarkerReportFilterTest : public reportgenTestFixture, + public ::testing::WithParamInterface> { +protected: + EventMarker marker = {}; + Vector *markers = nullptr; + cJSON *report = nullptr; + void *jsonLibrary = nullptr; + cJSON *(*createArray)() = nullptr; + void (*deleteJson)(cJSON *) = nullptr; + + void SetUp() override { + jsonLibrary = dlopen("libcjson.so.1", RTLD_NOW | RTLD_LOCAL); + ASSERT_NE(jsonLibrary, nullptr) << dlerror(); + createArray = reinterpret_cast(dlsym(jsonLibrary, "cJSON_CreateArray")); + auto createObject = reinterpret_cast(dlsym(jsonLibrary, "cJSON_CreateObject")); + auto createString = reinterpret_cast( + dlsym(jsonLibrary, "cJSON_CreateString")); + auto addString = reinterpret_cast( + dlsym(jsonLibrary, "cJSON_AddStringToObject")); + auto addItem = reinterpret_cast( + dlsym(jsonLibrary, "cJSON_AddItemToArray")); + deleteJson = reinterpret_cast(dlsym(jsonLibrary, "cJSON_Delete")); + ASSERT_NE(createArray, nullptr); + ASSERT_NE(createObject, nullptr); + ASSERT_NE(createString, nullptr); + ASSERT_NE(addString, nullptr); + ASSERT_NE(addItem, nullptr); + ASSERT_NE(deleteJson, nullptr); + EXPECT_CALL(mock_IO, cJSON_CreateObject()).Times(::testing::AnyNumber()) + .WillRepeatedly(::testing::Invoke(createObject)); + EXPECT_CALL(mock_IO, cJSON_CreateString(_)).Times(::testing::AnyNumber()) + .WillRepeatedly(::testing::Invoke(createString)); + EXPECT_CALL(mock_IO, cJSON_AddStringToObject(_, _, _)).Times(::testing::AnyNumber()) + .WillRepeatedly(::testing::Invoke(addString)); + EXPECT_CALL(mock_IO, cJSON_AddItemToArray(_, _)).Times(::testing::AnyNumber()) + .WillRepeatedly(::testing::Invoke(addItem)); + EXPECT_CALL(mock_IO, cJSON_Delete(_)).Times(::testing::AnyNumber()) + .WillRepeatedly(::testing::Invoke(deleteJson)); + ASSERT_EQ(Vector_Create(&markers), T2ERROR_SUCCESS); + marker.markerName = strdup("StorageMarker"); + marker.mType = MTYPE_ABSOLUTE; + marker.reportEmptyParam = std::get<0>(GetParam()); + marker.trimParam = std::get<1>(GetParam()); + ASSERT_EQ(Vector_PushBack(markers, &marker), T2ERROR_SUCCESS); + } + + void TearDown() override { + if(deleteJson) { + deleteJson(report); + } + free(marker.u.markerValue); + free(marker.markerName); + Vector_Destroy(markers, nullptr); + if(jsonLibrary) { + dlclose(jsonLibrary); + } + } +}; + +TEST_P(MarkerReportFilterTest, SerializesRetainedValuesWithOptionalTrimming) { + const struct { + const char *value; + const char *trimmed; + bool empty; + } cases[] = { + {" test log ", "test log", false}, {" xyz ", "xyz", false}, + {"NULLsuffix", "NULLsuffix", false}, {"", "", true}, + {" ", "", true}, {"\t\r\n", "\r\n", true}, {"NULL", "NULL", true} + }; + for(const auto &testCase : cases) { + SCOPED_TRACE(testCase.value); + report = createArray(); + ASSERT_NE(report, nullptr); + marker.u.markerValue = strdup(testCase.value); + ASSERT_NE(marker.u.markerValue, nullptr); + EXPECT_EQ(encodeEventMarkersInJSON(report, markers), T2ERROR_SUCCESS); + EXPECT_EQ(marker.u.markerValue, nullptr); + if(marker.reportEmptyParam || !testCase.empty) { + ASSERT_NE(report->child, nullptr); + EXPECT_EQ(report->child->next, nullptr); + ASSERT_NE(report->child->child, nullptr); + EXPECT_STREQ(report->child->child->string, marker.markerName); + EXPECT_STREQ(report->child->child->valuestring, + marker.trimParam ? testCase.trimmed : testCase.value); + } else { + EXPECT_EQ(report->child, nullptr); + } + deleteJson(report); + report = nullptr; + } +} + +INSTANTIATE_TEST_SUITE_P(SubscriberValues, MarkerReportFilterTest, + ::testing::Combine(::testing::Bool(), ::testing::Bool())); + TEST_F(reportgenTestFixture, encodeEventMarkersInJSON8) { cJSON *valArray = (cJSON *)malloc(sizeof(cJSON)); diff --git a/test/functional-tests/tests/test_multiprofile_msgpacket.py b/test/functional-tests/tests/test_multiprofile_msgpacket.py index 2dd6c24c..eb1fcf2e 100644 --- a/test/functional-tests/tests/test_multiprofile_msgpacket.py +++ b/test/functional-tests/tests/test_multiprofile_msgpacket.py @@ -272,9 +272,9 @@ def test_for_invalid_activation_timeout(): assert "TR_AC88" in grep_T2logs(ERROR_PROFILE_TIMEOUT) # Verify profile not set if activation timeout is less than reporting interval assert "MODEL_NAME\":\"NULL" in grep_T2logs("cJSON Report ") # 239 - Support for reportEmpty of profiles assert "TR_AC6919" in grep_T2logs("firstreporting interval is given") # 240 - Support for First Reporting Interval -1 - assert "5 sec" in grep_T2logs("firstreporting interval is given") # 240 - Support for First Reporting Interval - 2 - assert "NEW TEST PROFILE" in grep_T2logs(LOG_PROFILE_SET) # 101 - Report fetch and parse via HTTP - assert "60 sec" in grep_T2logs("reporting interval is taken - NEW TEST PROFILE") # 107 - Configurable reporting interval + assert "5 sec" in grep_T2logs("firstreporting interval is given") # 240 - Support for First Reporting Interval - 2/ + assert "NEW_TEST_PROFILE" in grep_T2logs(LOG_PROFILE_SET) # 101 - Report fetch and parse via HTTP + assert "60 sec" in grep_T2logs("reporting interval is taken - NEW_TEST_PROFILE") # 107 - Configurable reporting interval # 241 - Support for DCM profile and multiprofile parallel execution assert "AccountId\":\"Platform_Container_Test_DEVICE" in grep_T2logs("cJSON Report ") # 102 - Include data from data source as TR181 Parameter assert "SYS_GREP_TEST" in grep_T2logs("cJSON Report ") # 110, 111 - Data harvesting from previous logs folder for DCA profiles with log file search markers. diff --git a/test/functional-tests/tests/test_xconf_communications.py b/test/functional-tests/tests/test_xconf_communications.py index 99a10522..3b706edd 100644 --- a/test/functional-tests/tests/test_xconf_communications.py +++ b/test/functional-tests/tests/test_xconf_communications.py @@ -96,7 +96,7 @@ def test_change_profile(): kill_telemetry(12) sleep(10) assert os.path.exists(XCONF_PERSISTANT_FILE) - assert "NEW TEST PROFILE" in grep_T2logs("NEW TEST PROFILE") + assert "NEW_TEST_PROFILE" in grep_T2logs("NEW_TEST_PROFILE") @pytest.mark.run(order=6) def test_verify_urlencoding(): @@ -118,7 +118,7 @@ def test_verify_schedule(): command = ["echo 'This is Test Log' > /opt/logs/test.log"] run_shell_command(command) sleep(60) - ERROR_MSG = "Waiting for 60 sec for next TIMEOUT for profile as reporting interval is taken - NEW TEST PROFILE" + ERROR_MSG = "Waiting for 60 sec for next TIMEOUT for profile as reporting interval is taken - NEW_TEST_PROFILE" assert ERROR_MSG in grep_T2logs(ERROR_MSG) @pytest.mark.run(order=8) @@ -131,14 +131,14 @@ def test_verify_markers(): @pytest.mark.run(order=9) def test_verify_report(): - ERROR_MSG1 = "CollectAndReportXconf ++in profileName : NEW TEST PROFILE" + ERROR_MSG1 = "CollectAndReportXconf ++in profileName : NEW_TEST_PROFILE" ERROR_MSG2 = "cJSON Report =" assert ERROR_MSG1 in grep_T2logs(ERROR_MSG1) assert ERROR_MSG2 in grep_T2logs(ERROR_MSG2) @pytest.mark.run(order=10) def test_log_upload(): - LOG_MSG1 = "CollectAndReportXconf ++in profileName : NEW TEST PROFILE" + LOG_MSG1 = "CollectAndReportXconf ++in profileName : NEW_TEST_PROFILE" LOG_MSG2 = "cJSON Report =" clear_T2logs() kill_telemetry(10) @@ -148,7 +148,7 @@ def test_log_upload(): @pytest.mark.run(order=11) def test_log_upload_on_demand(): - LOG_MSG1 = "CollectAndReportXconf ++in profileName : NEW TEST PROFILE" + LOG_MSG1 = "CollectAndReportXconf ++in profileName : NEW_TEST_PROFILE" LOG_MSG2 = "cJSON Report =" clear_T2logs() kill_telemetry(29)