Skip to content

RDKB-62352 : Observed telemetry2_0 crash #247

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.

Sign up for GitHub

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

Open
adrojaankur wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

RDKB-62352 : Observed telemetry2_0 crash #247

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3a0a7a2
Telemetry crashe due to Open SSL engine
Dec 19, 2025
13c9f3d
Merge pull request #246 from rdkcentral/develop
adrojaankur Jan 27, 2026
fa28348
Update curlinterface.c
adrojaankur Jan 28, 2026
a5e4202
Update curlinterface.c
adrojaankur Jan 28, 2026
6657cfe
Update curlinterface.c
adrojaankur Jan 28, 2026
26176b5
Merge pull request #252 from rdkcentral/develop
adrojaankur Feb 3, 2026
9d1e4ed
RDKB-62352 : Observed telemetry2_0 crash
yogeswaransky Feb 3, 2026
6573670
Update curlinterface.c
adrojaankur Feb 3, 2026
d9e649e
RDKB-62352 : Observed telemetry2_0 crash
yogeswaransky Feb 3, 2026
af9d118
Merge pull request #255 from rdkcentral/develop
adrojaankur Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion source/protocol/http/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ libhttp_la_SOURCES = curlinterface.c
libhttp_la_LDFLAGS = -shared -fPIC -lcurl
if IS_LIBRDKCERTSEL_ENABLED
libhttp_la_CFLAGS = $(LIBRDKCERTSEL_FLAG)
libhttp_la_LDFLAGS += -lRdkCertSelector
libhttp_la_LDFLAGS += -lRdkCertSelector -lcrypto
endif
libhttp_la_CPPFLAGS = -fPIC -I${PKG_CONFIG_SYSROOT_DIR}$(includedir)/dbus-1.0 \
-I${PKG_CONFIG_SYSROOT_DIR}$(libdir)/dbus-1.0/include \
Expand Down
30 changes: 30 additions & 0 deletions source/protocol/http/curlinterface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
#include <sys/wait.h>
#include <curl/curl.h>
#include <signal.h>
#ifdef LIBRDKCERTSEL_BUILD
#include <openssl/crypto.h>
Copy link
Contributor

@shibu-kv shibu-kv Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't be using openssl api's directly. The libcurl apis should be the only interface.

Copy link

@pdmooneypub pdmooneypub Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shibu, at one point we were hashing the passcode so we could see if it changed (without exposing it). We may want to keep this in here until we have resolved the problem with the passcode.

#endif

#include "curlinterface.h"
#include "reportprofiles.h"
Expand Down Expand Up @@ -277,6 +280,10 @@ static void curlCertSelectorInit()
#endif
if (state_red_enable && curlRcvryCertSelector == NULL )
{
if(curlCertSelector != NULL){
rdkcertselector_free(&curlCertSelector);
curlCertSelector = NULL;
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new block freeing curlCertSelector inside curlCertSelectorInit uses tab indentation, whereas the rest of this function and file use space-based indentation. For consistency with the surrounding code and established style in this file (e.g., the curlRcvryCertSelector initialization and logging below), please switch this block to use the same space-based indentation.

Copilot uses AI. Check for mistakes.
}
curlRcvryCertSelector = rdkcertselector_new( NULL, NULL, "RCVRY" );
if (curlRcvryCertSelector == NULL)
{
Expand Down Expand Up @@ -319,6 +326,7 @@ T2ERROR sendReportOverHTTP(char *httpUrl, char *payload, pid_t* outForkedPid)
rdkcertselectorStatus_t curlGetCertStatus;
char *pCertURI = NULL;
bool state_red_enable = false;
char *pEngine = NULL;
#endif
char *pCertFile = NULL;
char *pCertPC = NULL;
Expand Down Expand Up @@ -440,6 +448,11 @@ T2ERROR sendReportOverHTTP(char *httpUrl, char *payload, pid_t* outForkedPid)
*/
if(childPid == 0)
{
#ifdef LIBRDKCERTSEL_BUILD
if(OPENSSL_init_crypto(OPENSSL_INIT_NO_LOAD_CONFIG, NULL) != 1){
T2Info("%s, T2:OPENSSL_init_crypto failed.\n", __func__);
}
#endif
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newly added #ifdef LIBRDKCERTSEL_BUILD block before curl_easy_init() is indented with tabs rather than spaces, which is inconsistent with the surrounding code in this function (e.g., the curl = curl_easy_init(); line and subsequent blocks). To follow the file's established indentation style, please re-indent this preprocessor block to use spaces like the rest of the function.

Copilot uses AI. Check for mistakes.
curl = curl_easy_init();
if(curl)
{
Expand All @@ -455,6 +468,23 @@ T2ERROR sendReportOverHTTP(char *httpUrl, char *payload, pid_t* outForkedPid)
goto child_cleanReturn;
}
#ifdef LIBRDKCERTSEL_BUILD
pEngine = rdkcertselector_getEngine(thisCertSel);
if(pEngine != NULL ) {
code = curl_easy_setopt(curl, CURLOPT_SSLENGINE, pEngine);
if(code != CURLE_OK) {
childCurlResponse.lineNumber = __LINE__;
curl_easy_cleanup(curl);
goto child_cleanReturn;
}
} else {
code = curl_easy_setopt(curl, CURLOPT_SSLENGINE_DEFAULT, 1L);
if(code != CURLE_OK ) {
childCurlResponse.lineNumber = __LINE__;
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this new SSL engine handling block, on CURLOPT_SSLENGINE / CURLOPT_SSLENGINE_DEFAULT failure you only set childCurlResponse.lineNumber before cleaning up and jumping to child_cleanReturn, but you never populate childCurlResponse.curlSetopCode with the failing code. In the parent, the log at lines 633–635 prints curl_easy_strerror(childCurlResponse.curlSetopCode), so these failures will misleadingly appear as CURLE_OK, making debugging much harder. Please set childCurlResponse.curlSetopCode = code in both error branches before the goto child_cleanReturn so the actual failure reason is propagated.

Copilot uses AI. Check for mistakes.
curl_easy_cleanup(curl);
goto child_cleanReturn;
}
}
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new SSL engine selection logic under LIBRDKCERTSEL_BUILD (calling rdkcertselector_getEngine and configuring CURLOPT_SSLENGINE / CURLOPT_SSLENGINE_DEFAULT) does not appear to be covered by existing unit tests, even though this file already has GTest-based coverage for its static helpers (e.g. SetHeader in source/test/protocol/ProtocolTest.cpp). Since this path is specific and error-prone (different behavior depending on whether rdkcertselector_getEngine returns a non-NULL engine, plus the two curl_easy_setopt branches), please consider adding targeted tests that exercise both the engine and default-engine paths and validate the behavior when curl_easy_setopt succeeds or fails.

Copilot uses AI. Check for mistakes.

Copy link

@pdmooneypub pdmooneypub Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copilot noticed that there were no tests added for the new code paths. can we add unit tests?

Copy link
Contributor Author

@adrojaankur adrojaankur Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the OpenSSL engine logic is not part of the core telemetry functionality, the telemetry unit test NOT handles the curl request. Based on my understanding, this scenario should be covered under L2/L3 testing rather than as a unit or core telemetry test

do
{
pCertFile = NULL;
Expand Down
Loading