[nlrcomcast]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 91.13128% with 127 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.14%. Comparing base (456eb81) to head (aaca8ed).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
source/broadband/webpa_method.c	74.25%	65 Missing and 48 partials ⚠️
tests/test_webpa_method.c	98.80%	0 Missing and 9 partials ⚠️
source/broadband/webpa_adapter.c	0.00%	2 Missing and 1 partial ⚠️
source/broadband/webpa_rbus.c	90.00%	0 Missing and 1 partial ⚠️
tests/test_webpa_set.c	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #320      +/-   ##
==========================================
+ Coverage   86.68%   88.14%   +1.45%     
==========================================
  Files          25       27       +2     
  Lines        9427    10855    +1428     
  Branches      918     1028     +110     
==========================================
+ Hits         8172     9568    +1396     
+ Misses        846      821      -25     
- Partials      409      466      +57     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[nlrcomcast]

this can be error

[Copilot]

Pull request overview

Adds support for invoking RPC-style RBUS methods over WebPA by routing reserved RDK.Operate SET requests into a new method-invocation handler, along with new unit tests and build/dep updates.

Changes:

• Introduces webpa_method module (webpa_method.c/.h) to parse Base64 JSON operate payloads and synchronously invoke RBUS methods.
• Updates request processing to detect RDK.Operate SETs and bypass normal SET handling in favor of method invocation.
• Expands test suite and build definitions to include method-invocation tests and mocks.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/test_webpa_test_set.c	Adds method-invoke function stubs/mocks for test compilation.
tests/test_webpa_table.c	Adds method-invoke function stubs/mocks for test compilation.
tests/test_webpa_set.c	Adds method-invoke stubs and adjusts one SET error test setup/expectations.
tests/test_webpa_set_attr.c	Adds method-invoke function stubs/mocks for test compilation.
tests/test_webpa_replace.c	Adds method-invoke function stubs/mocks for test compilation.
tests/test_webpa_notification.c	Adds method-invoke function stubs/mocks for test compilation.
tests/test_webpa_notification_cunit.c	Adds method-invoke stubs for test compilation.
tests/test_webpa_method.c	New CUnit test suite exercising method-invocation parsing/conversion and RBUS invoke flow.
tests/test_webpa_internal.c	Adds method-invoke stubs for test compilation.
tests/test_webpa_get.c	Adds method-invoke function stubs/mocks for test compilation.
tests/test_webpa_get_attr.c	Adds method-invoke function stubs/mocks for test compilation.
tests/test_webpa_adapter.c	Adds method-invoke function stubs/mocks for test compilation.
tests/CMakeLists.txt	Adds test_webpa_method target and coverage capture for it.
source/include/webpa_rbus.h	Exposes RBUS method-invoke wrapper and rbus handle getter.
source/include/webpa_method.h	New public API for detecting and handling RDK.Operate method invocation.
source/CMakeLists.txt	Adds broadband/webpa_method.c to production build sources.
source/broadband/webpa_rbus.c	Implements webpaRbusMethodInvoke() and get_rbus_handle().
source/broadband/webpa_method.c	Implements method invocation parsing, RBUS invocation, and WDMP response population.
source/broadband/webpa_adapter.c	Routes RDK.Operate SET requests to handleMethodInvoke().
CMakeLists.txt	Switches wdmp-c external dependency repo+commit to a fork.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (7)

tests/test_webpa_set.c:652

• err_set_with_empty_value() no longer asserts anything about the returned error payload (statusCode/message/parameters). This weakens the test and would allow regressions in SET error handling to pass unnoticed; please assert the expected failure response for this scenario.

    processRequest(reqPayload, NULL, &resPayload, req_headers, res_headers);
    WalInfo("resPayload : %s\n",resPayload);

    assert_non_null(resPayload);
    response = cJSON_Parse(resPayload);
    assert_non_null(response);
    cJSON_Delete(response);

CMakeLists.txt:146

• The wdmp-c dependency is switched from the upstream Comcast repo to a fork (nlrcomcast). This changes the provenance of a core protocol library; please document/justify the fork choice (e.g., required METHOD/operate support) and ensure the chosen commit is vetted/maintained.

ExternalProject_Add(wdmp-c
    PREFIX ${CMAKE_CURRENT_BINARY_DIR}/_prefix/wdmp-c
    GIT_REPOSITORY https://github.com/nlrcomcast/wdmp-c.git
    GIT_TAG "c422db79de1bc57ea91c3ce5c6edbcd5d874726b"
    CMAKE_ARGS += -DCMAKE_INSTALL_PREFIX=${INSTALL_DIR}

source/broadband/webpa_method.c:357

• parseOperatePayload() calls strdup() and then immediately logs the result with "%s". If strdup() fails, this will dereference NULL and crash; it also leaves the caller without a clear error object.

        *methodName = strdup(methodItem->valuestring);
        WalPrint("Method invocation target: %s\n", *methodName);

source/broadband/webpa_method.c:681

• jsonLeafToRbusValue() leaks the rbusValue_t when rbusValue_SetFromString() fails (it returns -1 without releasing *out). This leak is triggered by malformed typed params in the operate payload.

        rbusValue_Init(out);
        if(!rbusValue_SetFromString(*out, rt, str))
        {
                /* rbusValue_SetFromString failed to parse the string into the
                 * requested rbus type; return -1 for error. */

source/broadband/webpa_method.c:845

• rbusObjectToJson() defines unused locals (i/value/str_value). With the tests enabling -Wall, this introduces avoidable compiler warnings in new code.

        int i = 0;
        rbusValue_t value = NULL;
        char *str_value = NULL;
        if(obj == NULL || jsonOut == NULL)

tests/test_webpa_replace.c:127

• test_mock_coverage() calls sleep(1), which adds a real delay (or at least couples the test to the custom sleep() shim) without asserting behavior. Since this test is only invoking mocks for coverage, prefer sleep(0) or remove the sleep call entirely.

    sleep(1);

tests/test_webpa_set_attr.c:519

• test_mock_coverage() calls sleep(1), which adds a delay without asserting behavior. Since this test is only invoking mocks for coverage, prefer sleep(0) or remove the sleep call entirely.

    sleep(1);

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

tests/test_webpa_set.c:652

• err_set_with_empty_value() no longer asserts anything about the response beyond “it parses as JSON”, so the test will pass even if the statusCode/message are wrong. Add assertions for the expected failure (e.g., the mapped WDMP error/status code for an empty string value).

    processRequest(reqPayload, NULL, &resPayload, req_headers, res_headers);
    WalInfo("resPayload : %s\n",resPayload);

    assert_non_null(resPayload);
    response = cJSON_Parse(resPayload);
    assert_non_null(response);
    cJSON_Delete(response);

tests/test_webpa_method.c:165

• test_isMethodInvokeRequest_valid() never sets req.paramCnt to 1, so it currently passes even if isMethodInvokeRequest() incorrectly ignores paramCnt. Once isMethodInvokeRequest() is fixed to require exactly one parameter (per webpa_method.h), this test will fail unless paramCnt is initialized appropriately.

    memset(&req, 0, sizeof(req));
    memset(&p, 0, sizeof(p));
    req.param = &p;
    p.name = "RDK.Operate";
    CU_ASSERT_TRUE(isMethodInvokeRequest(&req));

source/broadband/webpa_method.c:855

• rbusObjectToJson() declares unused locals (i, value) which will trigger compiler warnings under common warning levels and add noise to the implementation.

static int rbusObjectToJson(rbusObject_t obj, cJSON *jsonOut)
{
        int i = 0;
        rbusValue_t value = NULL;
        if(obj == NULL || jsonOut == NULL)

source/broadband/webpa_method.c:63

• isMethodInvokeRequest() does not enforce the documented/required shape (exactly one parameter). As written it will return true whenever param[0].name is "RDK.Operate", even if paramCnt != 1, which can incorrectly route multi-parameter SET requests into handleMethodInvoke() and skip normal SET processing.

    if(setReq == NULL || setReq->param == NULL)
        return false;

    return (setReq->param[0].name != NULL &&
            strcmp(setReq->param[0].name, RDK_OPERATE_PARAM) == 0);

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

tests/test_webpa_set.c:652

• This negative test no longer asserts anything about the resulting error response (statusCode/message/parameters). As written, it will pass even if processRequest returns the wrong payload, which weakens coverage for empty-value handling.

    processRequest(reqPayload, NULL, &resPayload, req_headers, res_headers);
    WalInfo("resPayload : %s\n",resPayload);

    assert_non_null(resPayload);
    response = cJSON_Parse(resPayload);
    assert_non_null(response);
    cJSON_Delete(response);

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

tests/test_webpa_set.c:653

• err_set_with_empty_value no longer asserts anything about the error response (message/statusCode). This makes the test effectively only a JSON-parse smoke test and can let regressions slip through unnoticed. Add assertions that the response indicates an error and returns a non-empty message.

    assert_non_null(resPayload);
    response = cJSON_Parse(resPayload);
    assert_non_null(response);
    cJSON_Delete(response);
}

source/broadband/webpa_method.c:332

• The decoded operate payload is logged verbatim. This can leak sensitive RPC parameters into logs (and can be very large). Prefer logging only metadata (e.g., byte length / method name) or guard full payload logging behind a very low-verbosity debug flag.

        WalPrint("Base64-decoded operate payload (%zu bytes): %s\n", decodedLen, decoded);

source/broadband/webpa_method.c:168

• The full method response JSON is logged at info level. Since method results may contain sensitive data, logging the entire payload can create data exposure and operational noise. Log only the size / status at info, and keep full payload logging behind debug-only gates if needed.

This issue also appears on line 332 of the same file.

        WalInfo("Method response: %s\n", resultStr);

source/broadband/webpa_method.c:854

• Unused local variables i and value will trigger -Wall warnings and add noise. Remove them.

        int i = 0;
        rbusValue_t value = NULL;
        if(obj == NULL || jsonOut == NULL)

source/broadband/webpa_adapter.c:272

• This comment is grammatically confusing/redundant ("Route ... through the Received ..."). Clarify it so it describes the routing behavior without duplicating wording.

                                /* Route RDK.Operate method requests through the
                                 * Received RDK.Operate method invocation request */

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (4)

source/broadband/webpa_method.c:63

• isMethodInvokeRequest can read setReq->param[0] even when paramCnt is 0, which can lead to out-of-bounds access (and mis-routing) if an empty SET request is parsed with a non-NULL param pointer. Gate the check on paramCnt >= 1 (and ideally keep it consistent with the single-parameter requirement).

bool isMethodInvokeRequest(set_req_t *setReq)
{
    if(setReq == NULL || setReq->param == NULL)
        return false;

    return (setReq->param[0].name != NULL &&
            strcmp(setReq->param[0].name, RDK_OPERATE_PARAM) == 0);
}

tests/test_webpa_set.c:652

• err_set_with_empty_value no longer asserts anything about the returned statusCode/message. This makes the test pass even if the behavior regresses (or returns success unexpectedly). Please add assertions for the expected error code and message (updated to the new behavior if it changed).

    processRequest(reqPayload, NULL, &resPayload, req_headers, res_headers);
    WalInfo("resPayload : %s\n",resPayload);

    assert_non_null(resPayload);
    response = cJSON_Parse(resPayload);
    assert_non_null(response);
    cJSON_Delete(response);

source/broadband/webpa_rbus.c:50

• webpaRbus_Uninit unconditionally calls rbus_close(rbus_handle). If webpaRbusInit was never called or failed, rbus_handle may be NULL and rbus_close(NULL) may crash depending on the RBUS implementation. Guard the close and only NULL the handle when there is something to close.

void webpaRbus_Uninit()
{
    rbus_close(rbus_handle);
    rbus_handle = NULL;
}

source/broadband/webpa_method.c:811

• rbusValueToJson converts RBUS_INT64/RBUS_UINT64 to JSON numbers by casting to double, which can silently lose precision for values outside the IEEE-754 safe integer range (2^53-1). Consider emitting a JSON string for large 64-bit values (while keeping numbers for safe-range values) to avoid returning inaccurate results.

                case RBUS_INT64:
                        leaf = cJSON_CreateNumber((double) rbusValue_GetInt64(val));
                        break;
                case RBUS_UINT64:
                        leaf = cJSON_CreateNumber((double) rbusValue_GetUInt64(val));
                        break;