-
Notifications
You must be signed in to change notification settings - Fork 4
Topic/crashtesting #372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Topic/crashtesting #372
Changes from all commits
cb10860
273404e
2095512
d7e7f07
d97d649
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -326,6 +326,7 @@ int hostIf_WiFi_EndPoint::refreshCache() | |||||||||||||||||||||||||||||||||||||
| int hostIf_WiFi_EndPoint::refreshCache() | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| LOG_ENTRY_EXIT; | ||||||||||||||||||||||||||||||||||||||
| RDK_LOG (RDK_LOG_INFO, LOG_TR69HOSTIF, "[%s][STEP 1] refreshCache start\n", __FUNCTION__); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| static time_t time_of_last_successful_query = 0; | ||||||||||||||||||||||||||||||||||||||
| static int last_call_status = NOK; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -336,39 +337,73 @@ int hostIf_WiFi_EndPoint::refreshCache() | |||||||||||||||||||||||||||||||||||||
| // Using a 1-second cache. | ||||||||||||||||||||||||||||||||||||||
| if ((last_call_status == OK ) && (time (0) <= time_of_last_successful_query + 1)) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| RDK_LOG (RDK_LOG_INFO, LOG_TR69HOSTIF, "[%s][STEP 2] Cache still valid, returning cached values\n", __FUNCTION__); | ||||||||||||||||||||||||||||||||||||||
| RDK_LOG (RDK_LOG_DEBUG, LOG_TR69HOSTIF, "[%s] Cache not stale. last call status is SUCCESS, Refresh not required.\n", __FUNCTION__); | ||||||||||||||||||||||||||||||||||||||
| return OK; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| RDK_LOG (RDK_LOG_INFO, LOG_TR69HOSTIF, "[%s][STEP 3] Fetching WifiState from NetworkManager\n", __FUNCTION__); | ||||||||||||||||||||||||||||||||||||||
| std::string postData = "{\"jsonrpc\":\"2.0\",\"id\":\"42\",\"method\": \"org.rdk.NetworkManager.GetWifiState\"}"; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| string response = getJsonRPCData(std::move(postData)); | ||||||||||||||||||||||||||||||||||||||
| RDK_LOG (RDK_LOG_INFO, LOG_TR69HOSTIF, "[%s][STEP 4] WifiState RPC returned\n", __FUNCTION__); | ||||||||||||||||||||||||||||||||||||||
| if(response.c_str()) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| RDK_LOG (RDK_LOG_INFO, LOG_TR69HOSTIF, "%s: curl response string = %s\n", __FUNCTION__, response.c_str()); | ||||||||||||||||||||||||||||||||||||||
| cJSON* root = cJSON_Parse(response.c_str()); | ||||||||||||||||||||||||||||||||||||||
| RDK_LOG (RDK_LOG_INFO, LOG_TR69HOSTIF, "[%s][STEP 5] WifiState JSON parse attempted\n", __FUNCTION__); | ||||||||||||||||||||||||||||||||||||||
| if(root) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| cJSON* jsonObj = cJSON_GetObjectItem(root, "result"); | ||||||||||||||||||||||||||||||||||||||
| RDK_LOG (RDK_LOG_INFO, LOG_TR69HOSTIF, "[%s][STEP 6] WifiState result object lookup done\n", __FUNCTION__); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (jsonObj) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| cJSON *interfaces = cJSON_GetObjectItem(jsonObj, "interfaces"); | ||||||||||||||||||||||||||||||||||||||
| cJSON *interface = nullptr, *interfaceType; | ||||||||||||||||||||||||||||||||||||||
| for (int i = 0; i < cJSON_GetArraySize(interfaces); i++) { | ||||||||||||||||||||||||||||||||||||||
| interface = cJSON_GetArrayItem(interfaces, i); | ||||||||||||||||||||||||||||||||||||||
| interfaceType = cJSON_GetObjectItem(interface, "type"); | ||||||||||||||||||||||||||||||||||||||
| if (strcmp(interfaceType->valuestring, "WIFI") == 0) | ||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| RDK_LOG (RDK_LOG_INFO, LOG_TR69HOSTIF, "[%s][STEP 7] Iterating interfaces to find WIFI\n", __FUNCTION__); | ||||||||||||||||||||||||||||||||||||||
| if (cJSON_IsArray(interfaces)) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| for (int i = 0; i < cJSON_GetArraySize(interfaces); i++) { | ||||||||||||||||||||||||||||||||||||||
| interface = cJSON_GetArrayItem(interfaces, i); | ||||||||||||||||||||||||||||||||||||||
| interfaceType = cJSON_GetObjectItem(interface, "type"); | ||||||||||||||||||||||||||||||||||||||
| if (cJSON_IsString(interfaceType) && interfaceType->valuestring && (strcmp(interfaceType->valuestring, "WIFI") == 0)) | ||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+367
to
+372
|
||||||||||||||||||||||||||||||||||||||
| for (int i = 0; i < cJSON_GetArraySize(interfaces); i++) { | |
| interface = cJSON_GetArrayItem(interfaces, i); | |
| interfaceType = cJSON_GetObjectItem(interface, "type"); | |
| if (cJSON_IsString(interfaceType) && interfaceType->valuestring && (strcmp(interfaceType->valuestring, "WIFI") == 0)) | |
| break; | |
| } | |
| bool foundWifi = false; | |
| for (int i = 0; i < cJSON_GetArraySize(interfaces); i++) { | |
| interface = cJSON_GetArrayItem(interfaces, i); | |
| interfaceType = cJSON_GetObjectItem(interface, "type"); | |
| if (cJSON_IsString(interfaceType) && interfaceType->valuestring && (strcmp(interfaceType->valuestring, "WIFI") == 0)) { | |
| foundWifi = true; | |
| break; | |
| } | |
| } | |
| if (!foundWifi) { | |
| interface = nullptr; | |
| } |
Copilot
AI
Mar 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because interface can remain non-null even when a WIFI interface wasn't found, this if (interface) block can incorrectly update Enable based on a non-WIFI interface. Gate this block on an explicit "found WIFI" condition instead of pointer non-nullness.
Copilot
AI
Mar 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (response.c_str()) is always true for a std::string, so this check won't detect empty/failed ConnectedSSID responses and the else path is unreachable. Switch to !response.empty() (or propagate an explicit status) and adjust the error message accordingly.
| if(response.c_str()) | |
| if (!response.empty()) |
Copilot
AI
Mar 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logging the connected SSID at INFO (SSIDReference parsed: %s) can leak potentially sensitive user/network information into standard logs. Consider lowering this to DEBUG and/or redacting/hashing the SSID value before logging.
| RDK_LOG (RDK_LOG_INFO, LOG_TR69HOSTIF, "[%s][STEP 15] SSIDReference parsed: %s\n", __FUNCTION__, SSIDReference); | |
| RDK_LOG (RDK_LOG_DEBUG, LOG_TR69HOSTIF, "[%s][STEP 15] SSIDReference parsed: %s\n", __FUNCTION__, SSIDReference); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -49,7 +49,7 @@ extern "C" | |||||||||||||||||||||
| using namespace std; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| #ifdef GTEST_ENABLE | ||||||||||||||||||||||
| extern size_t (*getWriteCurlResponse(void))(void *ptr, size_t size, size_t nmemb, std::string stream); | ||||||||||||||||||||||
| extern size_t (*getWriteCurlResponse(void))(void *ptr, size_t size, size_t nmemb, void *userdata); | ||||||||||||||||||||||
| #endif | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| XRFCStore* m_rfcStore; | ||||||||||||||||||||||
|
|
@@ -306,7 +306,7 @@ TEST(srcTest, writeCurlResponse) { | |||||||||||||||||||||
| size_t size = 1; | ||||||||||||||||||||||
| size_t nmemb = strlen(input); | ||||||||||||||||||||||
| std::string response; | ||||||||||||||||||||||
| size_t written = getWriteCurlResponse()((void*)input, size, nmemb, response); | ||||||||||||||||||||||
| size_t written = getWriteCurlResponse()((void*)input, size, nmemb, &response); | ||||||||||||||||||||||
| EXPECT_EQ(written, nmemb); | ||||||||||||||||||||||
|
||||||||||||||||||||||
| EXPECT_EQ(written, nmemb); | |
| EXPECT_EQ(written, nmemb); | |
| EXPECT_EQ(response, std::string(input)); | |
| // Verify that multiple invocations append to the response buffer rather than overwriting it | |
| const char* secondInput = "MoreData"; | |
| size_t nmemb2 = strlen(secondInput); | |
| size_t written2 = getWriteCurlResponse()((void*)secondInput, size, nmemb2, &response); | |
| EXPECT_EQ(written2, nmemb2); | |
| EXPECT_EQ(response, std::string(input) + std::string(secondInput)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (response.c_str())is always true for astd::string(even when empty), so the failure branch is unreachable. Use!response.empty()(or return a status fromgetJsonRPCData) and log the correct failure reason (this isn't a curl init failure here).