RDKEMW-22210: Fix coverity issues on xdial

[balav08]

Reason for change: Fixing coverity reported issues.
Test Procedure: Refer ticket.
Risks: Low
Priority: P1
version: minor

[Copilot]

Pull request overview

This PR targets Coverity-reported issues in the xdial server implementation, primarily by adding NULL checks and tightening memory/resource handling in the REST and SSDP paths.

Changes:

• Add NULL-guarding around request payload logging and additional-data URL escaping in the libsoup 3.0 REST POST handler.
• Free an allocated handler path string in the libsoup 2.4 SSDP initialization path to address a resource leak.
• Add a NULL check around gdial_app_state_to_string() usage when building the app state XML response.

Reviewed changes

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

File	Description
server/libsoup/3.0/gdial-rest.c	Adds Coverity-driven NULL checks and adjusts string initialization in response builder logic.
server/libsoup/2.4/gdial-ssdp.c	Frees dail_ssdp_handler after handler registration to address a reported leak.
server/gdial-app.c	Guards against NULL state string when creating the <state> XML node.

---

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

[Copilot]

Pull request overview

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

Suppressed comments (3)

server/libsoup/3.0/gdial-rest.c:438

• payload_str is guarded, but the log still uses request_body->length even when request_body->data is NULL. With %.*s, a non-zero length would cause an out-of-bounds read from the empty string (and potentially log unrelated memory). Compute the length only when request_body and request_body->data are present (otherwise use 0).

  // coverity fix : FORWARD_NULL - check for NULL request_body->data before using in log
  const char *payload_str = (request_body && request_body->data) ? request_body->data : "";
  GDIAL_LOGERROR("Starting the app with payload %.*s", (int)request_body->length, payload_str);

server/libsoup/3.0/gdial-rest.c:503

• additional_data_url_safe comes from g_uri_escape_string(), which returns GLib-allocated memory intended to be released with g_free(). Using free() can be incorrect on builds where GLib's allocator differs from libc.

    if (additional_data_url_safe) free(additional_data_url_safe);

server/libsoup/3.0/gdial-rest.c:469

• request_body is treated as nullable in this function (e.g., the earlier if (request_body && request_body->data && request_body->length)), but later request_body->data is accessed unconditionally when setting payload. If request_body is NULL, this will crash before the subsequent if (payload && strlen(payload)) guard runs.

    // coverity fix : FORWARD_NULL - check for NULL before calling g_uri_escape_string
    gchar *additional_data_url_safe = additional_data_url ? g_uri_escape_string(additional_data_url, NULL, FALSE) : NULL;
    GDIAL_LOGINFO("additionalDataUrl = %s, %s", additional_data_url ? additional_data_url : "(null)", additional_data_url_safe ? additional_data_url_safe : "(null)");

[Copilot]

Pull request overview

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

Suppressed comments (3)

server/libsoup/3.0/gdial-rest.c:439

• payload_len is derived from request_body->length even when request_body->data is NULL. In that case payload_str becomes "" but %.*s will still read payload_len bytes, which can read past the empty string and trigger undefined behavior. Make payload_len conditional on request_body->data as well.

  const char *payload_str = (request_body && request_body->data) ? request_body->data : "";
  int payload_len = (request_body) ? (int)request_body->length : 0;
  GDIAL_LOGERROR("Starting the app with payload %.*s", payload_len, payload_str);	

server/libsoup/3.0/gdial-rest.c:505

• additional_data_url_safe is allocated by g_uri_escape_string(), which should be released with g_free() (GLib allocator), not free(). Using free() can break if GLib is built with a non-system allocator.

    if (additional_data_url_safe) free(additional_data_url_safe);

server/gdial-app.c:536

• This change makes the <state> element disappear entirely when gdial_app_state_to_string() returns NULL (eg, GDIAL_APP_STATE_MAX). Previously the XML always contained a <state> element (possibly empty). To avoid a response-shape change, always emit the element and fall back to an empty string when the state is unknown.

  const gchar *state_str = gdial_app_state_to_string(state);
  if (state_str) {
    xmlNewChild(nservice, NULL, BAD_CAST "state", BAD_CAST state_str);
  }

[Copilot]

Pull request overview

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

Suppressed comments (2)

server/libsoup/3.0/gdial-rest.c:505

• additional_data_url_safe is allocated by g_uri_escape_string(), which should be released with g_free() rather than free() to match GLib's allocation API.

    if (additional_data_url_safe) free(additional_data_url_safe);

server/libsoup/3.0/gdial-rest.c:438

• payload_len is computed from request_body->length even when request_body->data is NULL, but payload_str falls back to an empty string in that case. If length is non-zero while data is NULL, %.*s will read past the empty string buffer. Compute the length only when data is non-NULL as well.

  // coverity fix : FORWARD_NULL - check for NULL request_body before accessing length and data
  const char *payload_str = (request_body && request_body->data) ? request_body->data : "";
  int payload_len = (request_body) ? (int)request_body->length : 0;
  GDIAL_LOGERROR("Starting the app with payload %.*s", payload_len, payload_str);	

[Copilot]

Pull request overview

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

Suppressed comments (2)

server/libsoup/3.0/gdial-rest.c:506

• g_uri_escape_string() returns memory that should be released with g_free(), not free(). Using free() here can be an allocator mismatch if GLib is built with a non-default allocator.

    start_error = gdial_app_start(app, payload_safe, query_str_safe, additional_data_url_safe, gdial_rest_server);
    if (query_str_safe) g_free(query_str_safe);
    if (payload_safe) g_free(payload_safe);
    if (additional_data_url_safe) free(additional_data_url_safe);
    if (additional_data_url) g_free(additional_data_url);

server/gdial-app.c:536

• The new comment refers to a "USE_AFTER_FREE" issue, but the code change is a NULL-check on the return value of gdial_app_state_to_string(). This is misleading for future readers and for tracking Coverity findings.

  // coverity fix : USE_AFTER_FREE - check for NULL before using state_str
  const gchar *state_str = gdial_app_state_to_string(state);
  if (state_str) {
    xmlNewChild(nservice, NULL, BAD_CAST "state", BAD_CAST state_str);
  }

[Copilot]

Pull request overview

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

Suppressed comments (2)

server/libsoup/3.0/gdial-rest.c:506

• additional_data_url_safe is created by g_uri_escape_string() (GLib allocation) but is freed with free(). This can cause an allocator mismatch; use g_free() for GLib-allocated strings.

    start_error = gdial_app_start(app, payload_safe, query_str_safe, additional_data_url_safe, gdial_rest_server);
    if (query_str_safe) g_free(query_str_safe);
    if (payload_safe) g_free(payload_safe);
    if (additional_data_url_safe) free(additional_data_url_safe);
    if (additional_data_url) g_free(additional_data_url);

server/libsoup/3.0/gdial-rest.c:551

• In the error path, app may refer to an existing singleton instance returned by gdial_app_find_instance_by_name() (no ref added). Unref'ing it here can destroy a running app instance unexpectedly. Only unref if this function created the instance.

    // coverity fix : DEADCODE - check app for NULL before calling g_object_unref
    if (app) {
      g_object_unref(app);
      app = NULL;
    }

[Copilot]

Pull request overview

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

Suppressed comments (3)

server/libsoup/3.0/gdial-rest.c:506

• g_uri_escape_string() returns GLib-allocated memory and should be released with g_free(), not free(). Mixing allocators can break when GLib is configured with a different allocator/vtable.

    if (additional_data_url) g_free(additional_data_url);
  }

  /*

server/libsoup/3.0/gdial-rest.c:439

• payload_len is derived from request_body->length even when request_body->data is NULL, which can make %.*s read past payload_str (""), leading to out-of-bounds reads if libsoup ever provides a body with NULL data. Tie the length to the same condition used for payload_str.

  const char *payload_str = (request_body && request_body->data) ? request_body->data : "";
  int payload_len = (request_body) ? (int)request_body->length : 0;
  GDIAL_LOGERROR("Starting the app with payload %.*s", payload_len, payload_str);	
  GDialApp *app = gdial_app_find_instance_by_name(app_registry->name);

build_dependencies.sh:9

• Unquoted variable expansion can break if $GITHUB_WORKSPACE ever contains spaces or glob characters (word splitting / pathname expansion). Quote the variable when passing it to commands.

ls -la ${GITHUB_WORKSPACE}
cd ${GITHUB_WORKSPACE}

[Copilot]

Pull request overview

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

Suppressed comments (2)

server/libsoup/3.0/gdial-rest.c:438

• payload_len is derived from request_body->length even when request_body->data is NULL; in that case payload_str becomes "" and %.*s will read past the end of the empty string (undefined/out-of-bounds read). Tie the length to the same condition as the pointer.

  const char *payload_str = (request_body && request_body->data) ? request_body->data : "";
  int payload_len = (request_body) ? (int)request_body->length : 0;
  GDIAL_LOGERROR("Starting the app with payload %.*s", payload_len, payload_str);	

server/libsoup/3.0/gdial-rest.c:502

• additional_data_url_safe is returned by g_uri_escape_string(), which is documented to be freed with g_free(). Using free() here can cause alloc/free mismatches if GLib uses a different allocator.

    if (additional_data_url_safe) free(additional_data_url_safe);

[Copilot]

Pull request overview

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

Suppressed comments (3)

server/libsoup/3.0/gdial-rest.c:502

• additional_data_url_safe is allocated by g_uri_escape_string() (GLib allocator) but freed with free(). Mixing alloc/free families can cause invalid frees depending on the GLib build/allocator. Use g_free() for memory returned by GLib APIs.

    if (additional_data_url_safe) free(additional_data_url_safe);

server/libsoup/3.0/gdial-rest.c:438

• payload_len is derived from request_body->length even when request_body->data is NULL, while payload_str falls back to "". If length > 0 with NULL data (possible when no body is loaded/parsed), %.*s will read past the end of the empty string buffer and can crash or log garbage. Tie the length to the presence of data (or set both to 0/NULL together).

  const char *payload_str = (request_body && request_body->data) ? request_body->data : "";
  int payload_len = (request_body) ? (int)request_body->length : 0;
  GDIAL_LOGERROR("Starting the app with payload %.*s", payload_len, payload_str);	

server/gdial-app.c:535

• When state is GDIAL_APP_STATE_MAX (or any unexpected value), gdial_app_state_to_string() returns NULL. The new code omits the <state> element entirely in that case, which can break clients expecting the element to always exist. Prefer emitting <state> with an empty/fallback value rather than skipping the node.

  const gchar *state_str = gdial_app_state_to_string(state);
  if (state_str) {
    xmlNewChild(nservice, NULL, BAD_CAST "state", BAD_CAST state_str);
  }