Conversation
RDKECMF-213 Add CODEOWNERS file
Signed-off-by: apatel859 <amit_patel5@comcast.com>
RDKEMW-254: libsoup3 support
DELIA-67407: Code syncup main to develop
Rebase with Develop Branch
…p_set_friendlyname Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
RDKTV-35185: Add sync between ssdp_http_server_callback and gdial_ssdp_set_friendlyname
Reason for change: Fix issues identified within xcast Test Procedure: Risks: low Priority: P1 Signed-off-by:Hayden Gfeller <Hayden_Gfeller@comcast.com>
…ialserver into feature/RDKEMW-2033
Reason for change: Implemented setManufacturerName and setModelName APIs for DIAL Server name configuration maintained the additional data url to specific app Test Procedure: DIAL should work Risks: None Priority: P1 Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…4_Dial_Args_1 RDK-55044: Implement DIAL requirement to use on EU product
RDKEMW-2033 : Coverity
Reason for change: Added DISABLE_SECURITY_TOKEN Flag to disable the WPEFrameworkSecurity Token generation changes Test Procedure: please referred from the ticket Risks: Medium Signed-off-by: Thamim Razith <ThamimRazith_AbbasAli@comcast.com>
RDKEMW-2278: Removal of WPEFrameworkSecurity Agent Utility
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…129_Coverity RDKEMW-4129: Prepare native build environment
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…129_CoverityTest RDKEMW-4129: Test
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…alserver into topic/RDKEMW-4129_CoverityTest Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…129_CoverityTest RDKEMW-4129: Prepare native build environment for Coverity
Signed-off-by: apatel859 <amit_patel5@comcast.com>
RDKEMW-4129: Prepare native build environment for Coverity
RDKEMW-2854 : Fix the double free issue on call to onApplicationStateChanged api
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
* RDKEMW-12059: Fix Coverity identified issues
* RDKEMW-12555 : Fix coveirty workflow scan in xdialserver repo Reason for Change: Fix coveirty scan workflow failure in xdialserver repo Test Procedure: Verify coveirty workflow Risks: Low Priority: P1 version: minor Signed-off-by:AkshayKumar_Gampa AkshayKumar_Gampa@comcast.com * RDKEMW-12555 : Fix coveirty workflow scan in xdialserver repo Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
b'## Copyright scan failure |
There was a problem hiding this comment.
Pull request overview
This PR updates the project’s network discovery / REST stack to support newer GNOME components (libsoup 3 + gssdp 1.6), while also extending the GDial platform/service APIs (manufacturer/model updates) and introducing UUID-based per-app URIs plus build/CI scaffolding.
Changes:
- Add conditional build support for libsoup 3 / gssdp 1.6 (new
gdial1p6-*.csources) while keeping libsoup 2.4 compatibility. - Introduce UUID-based app URIs persisted under
/tmpand update REST/local REST routing accordingly. - Add manufacturer/model update callbacks end-to-end; add optional security-token disable flag and related build changes.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| stubs/securityagent/SecurityTokenUtil.h | Adds SecurityAgent token utility stub header. |
| stubs/securityagent/SecurityTokenUtil.cpp | Adds stub implementation of token retrieval. |
| stubs/iarm_stubs.cpp | Adds IARM bus stub implementations for native builds. |
| server/plat/gdialappcache.hpp | Initializes observer pointer to nullptr. |
| server/plat/gdialappcache.cpp | Uses moves for cache IDs, improves logging, adds null check. |
| server/plat/gdial_app_registry.c | Persists/loads per-app UUID and sets app_uri. |
| server/plat/gdial.hpp | Adds manufacturer/model callback typedefs + registration APIs. |
| server/plat/gdial.cpp | Adds manufacturer/model propagation; refactors logging; adds DISABLE_SECURITY_TOKEN logic; more std::move usage. |
| server/plat/gdial-plat-util.c | Fixes thread-id formatting in logs. |
| server/plat/gdial-plat-dev.c | Removes manufacturer/model getters from plat-dev. |
| server/plat/gdial-plat-app.c | Wires new manufacturer/model update calls through platform layer. |
| server/plat/gdial-os-app.h | Adds OS API declarations for manufacturer/model updates. |
| server/plat/CMakeLists.txt | Adds DISABLE_SECURITY_TOKEN option and adjusts link libraries (+uuid). |
| server/include/gdialserviceimpl.h | Adds new request payload fields; initializes observer; removes onStopped override. |
| server/include/gdialservicecommon.h | Removes onStopped() from GDialNotifier interface. |
| server/include/gdialservice.h | Adds setManufacturerName / setModelName to service API. |
| server/include/gdial_app_registry.h | Adds app_uri field and UUID buffer size define. |
| server/include/gdial-plat-dev.h | Removes manufacturer/model getter declarations. |
| server/include/gdial-plat-app.h | Adds manufacturer/model callbacks and update APIs. |
| server/include/gdial-config.h | Renames include guard; removes old GDialError enum. |
| server/gdialservice.cpp | Adds libsoup3 handler compatibility; wires manufacturer/model callbacks; minor type-cast tweaks. |
| server/gdialserver_ut.cpp | Adjusts to interface changes; minor move/cleanup changes. |
| server/gdial1p6-ssdp.c | New libsoup3/gssdp1.6-compatible SSDP implementation with manufacturer/model support. |
| server/gdial1p6-shield.c | New libsoup3-compatible shield/throttle implementation. |
| server/gdial1p6-rest.c | New libsoup3/gssdp1.6-compatible REST implementation with UUID local routing. |
| server/gdial-ssdp.h | Exposes manufacturer/model setters. |
| server/gdial-ssdp.c | Adds manufacturer/model fields + locking; improves null handling and dd.xml generation. |
| server/gdial-rest.h | Updates additionalDataUrl builder signature; adds registry lookup by UUID. |
| server/gdial-rest.c | Updates additionalDataUrl generation and local handler routing by UUID; removes now-per-app handler registration from constructor. |
| server/CMakeLists.txt | Adds libsoup3 detection and selects gdial1p6-* sources when available; updates gssdp probing. |
| cov_build.sh | Adds native build wrapper script. |
| build_dependencies.sh | Adds dependency installation + builds external deps and stubs for CI. |
| Makefile | Passes DISABLE_SECURITY_TOKEN variable into cmake invocation. |
| .github/workflows/native_full_build.yml | Adds GitHub Actions workflow for native/container build. |
| .github/CODEOWNERS | Adds default code owners for PR review routing. |
Comments suppressed due to low confidence (1)
server/plat/gdial.cpp:599
controllerRemoteObjectis allocated unconditionally on everyGetCurrentState()call (line 598), even though token retrieval is guarded byif (NULL == controllerRemoteObject). This leaks the previouscontrollerRemoteObjectand defeats the caching logic; it can also change behavior across calls. Only create theLinkTypewhencontrollerRemoteObjectis null (or delete/reset the previous instance before reassigning).
if (NULL == controllerRemoteObject) {
int ret = GetSecurityToken(MAX_LENGTH,buffer);
if(ret>0)
{
sToken = (char*)buffer;
query = "token=" + sToken;
GDIAL_LOGINFO("Security token[%s] ",query.c_str());
}
}
#endif
controllerRemoteObject = new JSONRPC::LinkType<Core::JSON::IElement>(std::string(), false, query);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pkg_search_module (GSSDP16 gssdp-1.6) | ||
| if (GSSDP16_FOUND) | ||
| pkg_search_module (GSSDP REQUIRED gssdp-1.6) | ||
| add_definitions(-DHAVE_GSSDP_VERSION_1_6_OR_NEWER) | ||
| message("Using gssdp-1.6") | ||
| endif() |
There was a problem hiding this comment.
When LIBSOUP3_FOUND is true, GSSDP is only set if gssdp-1.6 is found, but there is no fallback or hard failure if it isn’t. This can leave GSSDP_LIBRARIES/INCLUDE_DIRS unset and cause confusing build/link errors later. Consider making gssdp-1.6 REQUIRED in the libsoup3 path (or explicitly error/fallback to libsoup2/gssdp1.2).
| pkg_search_module (GSSDP16 gssdp-1.6) | |
| if (GSSDP16_FOUND) | |
| pkg_search_module (GSSDP REQUIRED gssdp-1.6) | |
| add_definitions(-DHAVE_GSSDP_VERSION_1_6_OR_NEWER) | |
| message("Using gssdp-1.6") | |
| endif() | |
| pkg_search_module (GSSDP REQUIRED gssdp-1.6) | |
| add_definitions(-DHAVE_GSSDP_VERSION_1_6_OR_NEWER) | |
| message("Using gssdp-1.6") |
| #include <stdio.h> | ||
| #include <glib.h> | ||
| #include <assert.h> | ||
| #include <libsoup/soup.h> | ||
|
|
||
| #include "gdial-config.h" | ||
| #include "gdial-debug.h" | ||
|
|
||
| typedef struct DialShieldConnectionContext { | ||
| GSocket *read_gsocket; | ||
| guint read_timeout_source; | ||
| } DialShieldConnectionContext; | ||
|
|
||
| static GHashTable *active_conns_ = NULL; | ||
|
|
||
| static void soup_message_weak_ref_callback(gpointer user_data, GObject *obj); | ||
| static void server_request_remove_callback (SoupMessage *msg) { | ||
| GDIAL_LOGTRACE("Entering ..."); | ||
| DialShieldConnectionContext * conn_context = (DialShieldConnectionContext *) g_hash_table_lookup(active_conns_, msg); | ||
| if (conn_context) { | ||
| if (conn_context->read_timeout_source != 0) { | ||
| g_print_with_timestamp("server_request_remove_callback tid=[%lx] msg=%p timeout source %d removed", | ||
| pthread_self(), msg, conn_context->read_timeout_source); | ||
| g_source_remove(conn_context->read_timeout_source); |
There was a problem hiding this comment.
This file uses pthread_self() and usleep() but doesn’t include <pthread.h> / <unistd.h>, which can fail to compile depending on toolchain and warnings. Also server_request_remove_callback (and related code) uses SoupMessage* while the server callbacks provide SoupServerMessage*; the mismatch can cause type warnings and is semantically incorrect for libsoup server APIs. Include the missing headers and consistently use SoupServerMessage* for server message objects throughout this file.
| gchar *additional_data_url_safe = g_uri_escape_string(additional_data_url, NULL, FALSE); | ||
| GDIAL_LOGINFO("additionalDataUrl = %s, %s", additional_data_url, additional_data_url_safe); | ||
| g_signal_connect_object(app, "state-changed", G_CALLBACK(gdial_rest_app_state_changed_cb), gdial_rest_server, 0); | ||
| const gchar *query_str = g_uri_get_query(soup_server_message_get_uri(msg)); | ||
| gchar *query_str_safe = NULL; | ||
| const int use_query_directly_from_soup = 1; | ||
| if(query_str && strlen(query_str)) { | ||
| GDIAL_LOGINFO("query = %s", query_str); | ||
| if (!use_query_directly_from_soup) { | ||
| char *tmp = g_uri_escape_string(query_str, NULL, FALSE); | ||
| // note that we later g_free(query_str_safe) which doesn't necessarily work with malloc'ed memory (seems to depend on glib version) | ||
| query_str_safe = g_strdup(tmp); | ||
| free(tmp); | ||
| } | ||
| else { | ||
| query_str_safe = g_strdup(query_str); | ||
| } | ||
| } | ||
| const gchar *payload = request_body->data; | ||
| gchar *payload_safe = NULL; | ||
| if (payload && strlen(payload)) { | ||
| if (g_str_has_prefix(app->name, "YouTube")) { | ||
| /* temporary disabling encoding payload for YouTube till cloud side changed*/ | ||
| payload_safe = g_strdup(payload); | ||
| } | ||
| else { | ||
| char *tmp = g_uri_escape_string(payload, "=&", FALSE); | ||
| // note that we later g_free(payload_safe) which doesn't necessarily work with malloc'ed memory (seems to depend on glib version) | ||
| payload_safe = g_strdup(tmp); | ||
| free(tmp); | ||
| } | ||
| } | ||
| 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); | ||
| free(additional_data_url_safe); | ||
| g_free(additional_data_url); |
There was a problem hiding this comment.
Memory management is inconsistent: values returned by g_uri_escape_string() must be released with g_free(), but the code frees additional_data_url_safe (and other tmp escape buffers) with free(). Mixing allocators can cause crashes on some platforms/libc configurations. Use g_free() for all g_uri_escape_string() results (and keep free() only for allocations that truly come from malloc).
|
b'## Copyright scan failure |
|
b'## Copyright scan failure |
Coverity Issue - Destroy held lock"pthread_mutex_destroy" destroys "ssdpServerEventSync" while it is locked. Medium Impact, CWE-667 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Variable copied when it could be moved"model" is copied in call to copy assignment for class "std::string", when it could be moved instead. Low Impact, CWE-none How to fixUse "std::move(""model"")" instead of "model". Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Uninitialized scalar variableUsing uninitialized value "payload". Field "payload.data_param" is uninitialized when calling "sendRequest". High Impact, CWE-457 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Uninitialized scalar variableUsing uninitialized value "payload". Field "payload.data_param" is uninitialized when calling "sendRequest". High Impact, CWE-457 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Variable copied when it could be moved"manufacturer" is copied in call to copy assignment for class "std::string", when it could be moved instead. Low Impact, CWE-none How to fixUse "std::move(""manufacturer"")" instead of "manufacturer". Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
|
b'## WARNING: A Blackduck scan failure has been waived A prior failure has been upvoted
|
No description provided.