RDKEMW-6898: gdialserver not compatible with Libsoup3 library#200
Closed
balav08 wants to merge 2 commits into
Closed
RDKEMW-6898: gdialserver not compatible with Libsoup3 library#200balav08 wants to merge 2 commits into
balav08 wants to merge 2 commits into
Conversation
Reason for change: gdialserver not compatible with Libsoup3 library Updating gssdp to 1.6.3 provides libsoup3 compatibility . Test Procedure: Compiled and Verified Risks: Low Priority: P1 version: minor Signed-off-by: balaji velmurugan <balaji_velmurugan@comcast.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds libsoup-3 / gssdp-1.6 compatibility to gdialserver. The legacy libsoup-2.4 sources are forked into server/libsoup/2.4/ (preserving existing behavior) and new libsoup-3 ports are added under server/libsoup/3.0/. CMake selects the appropriate set based on libsoup-3.0 package availability. gdialservice.cpp is updated with version-guarded code paths and adopts std::move for payload strings.
Changes:
- Vendored libsoup-2.4 and libsoup-3.0 ports of
gdial-rest.c,gdial-ssdp.c, andgdial-shield.c. - CMake updated to pick gssdp-1.6/libsoup-3 when present, else fall back to gssdp-1.2/1.0 + libsoup-2.4.
gdialservice.cppaddsHAVE_LIBSOUP_VERSION_3ifdefs, removes theonStopped()observer call, and usesstd::movein observer dispatches.
Reviewed changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| server/CMakeLists.txt | Select libsoup3/gssdp-1.6 vs legacy and pick source set accordingly. |
| server/gdialservice.cpp | Version-guarded SoupServer message API; moved strings; removed onStopped(). |
| server/libsoup/3.0/gdial-ssdp.c | New libsoup-3/gssdp-1.6 port of SSDP handling. |
| server/libsoup/3.0/gdial-shield.c | New libsoup-3 port of connection-shield logic. |
| server/libsoup/3.0/gdial-rest.c | New libsoup-3 port of REST DIAL endpoints. |
| server/libsoup/2.4/gdial-ssdp.c | Legacy libsoup-2.4 SSDP source moved into version subdir. |
| server/libsoup/2.4/gdial-shield.c | Legacy libsoup-2.4 shield source moved into version subdir. |
| server/libsoup/2.4/gdial-rest.c | Legacy libsoup-2.4 REST source moved into version subdir. |
Comments suppressed due to low confidence (2)
server/libsoup/3.0/gdial-rest.c:466
- If
app_registry->use_additional_datais FALSE,additional_data_urlremains NULL, and the subsequentg_uri_escape_string(additional_data_url, NULL, FALSE)on line 465 is called with a NULLunescapedargument.g_uri_escape_stringrequires a non-NULL string and will crash/abort. The call to escape should be guarded so thatadditional_data_url_safestays NULL whenadditional_data_urlis NULL. Same issue exists in the libsoup 2.4 variant (line 461).
gchar *additional_data_url = NULL;
if (app_registry->use_additional_data) {
additional_data_url = gdial_rest_server_new_additional_data_url(listening_port, app_registry->name, FALSE, app_registry->app_uri );
}
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);
server/libsoup/3.0/gdial-rest.c:1336
g_string_new_len('\0', 128)passes the character literal'\0'(integer 0) as theconst gchar *initargument. Aside from being misleading, the second argument128then asks GString to copy 128 bytes from the NULL pointer — depending on glib internals this is either a no-op (because init == NULL) or undefined behavior. The function carried over from the legacy version; it should beg_string_sized_new(128)(org_string_new(NULL)). Same pattern in the libsoup 2.4 file.
GString *rbuf = g_string_new_len('\0', 128);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+206
to
+207
| GDIAL_LOGERROR("%s", error->message); | ||
| g_error_free(error); |
Comment on lines
+177
to
+181
| if (0 != pthread_mutex_init(&ssdpServerEventSync, NULL)) | ||
| { | ||
| GDIAL_LOGERROR("Failed to initializing mutex"); | ||
| return EXIT_FAILURE; | ||
| } |
Comment on lines
+251
to
+263
| gchar *dail_ssdp_handler = g_strdup_printf("/%s/%s", random_uuid,"dd.xml"); | ||
| soup_server_add_handler(ssdp_http_server_, dail_ssdp_handler, ssdp_http_server_callback, NULL, NULL); | ||
| ssdp_client_ = ssdp_client; | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| int gdial_ssdp_destroy() { | ||
| GDIAL_LOGTRACE("Entering ..."); | ||
| pthread_mutex_lock(&ssdpServerEventSync); | ||
| if (ssdp_http_server_) | ||
| { | ||
| soup_server_remove_handler(ssdp_http_server_, "/dd.xml"); |
Comment on lines
+427
to
+436
| gdial_rest_server_http_return_if_fail(app_registry, msg, SOUP_STATUS_NOT_FOUND); | ||
| if (request_body && request_body->data && request_body->length) { | ||
| gdial_rest_server_http_return_if_fail(request_body->length <= GDIAL_REST_HTTP_MAX_PAYLOAD, msg, SOUP_STATUS_REQUEST_ENTITY_TOO_LARGE); | ||
| gdial_rest_server_http_return_if_fail(!gdial_rest_server_is_bad_payload(request_body->data, request_body->length), msg, SOUP_STATUS_BAD_REQUEST); | ||
| } | ||
| GSocketAddress *socket_addr = soup_server_message_get_local_address(msg); | ||
| guint listening_port = g_inet_socket_address_get_port(G_INET_SOCKET_ADDRESS(socket_addr)); | ||
| gdial_rest_server_http_return_if_fail(listening_port != 0, msg, SOUP_STATUS_INTERNAL_SERVER_ERROR); | ||
|
|
||
| GDIAL_LOGERROR("Starting the app with payload %.*s", (int)request_body->length, request_body->data); |
| GDIAL_STATIC_INLINE void *GET_APP_response_builder_new(const gchar *app_name) { | ||
| GDialServerResponseBuilderGetApp *rbuilder = (GDialServerResponseBuilderGetApp *) malloc(sizeof(*rbuilder)); | ||
| memset(rbuilder, 0, sizeof(*rbuilder)); | ||
| memset(rbuilder, 0, sizeof(*rbuilder)); |
Comment on lines
29
to
47
| pkg_search_module (LIBSOUP3 libsoup-3.0) | ||
| if (GSSDP12_FOUND) | ||
| pkg_search_module (GSSDP REQUIRED gssdp-1.2) | ||
| add_definitions(-DHAVE_GSSDP_VERSION_1_2_OR_NEWER) | ||
| message("Using gssdp-1.2") | ||
| if (LIBSOUP3_FOUND) | ||
| add_definitions(-DHAVE_LIBSOUP_VERSION_3) | ||
| 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() | ||
| else() | ||
| pkg_search_module (GSSDP REQUIRED gssdp-1.0) | ||
| pkg_search_module (GSSDP12 gssdp-1.2) | ||
| if (GSSDP12_FOUND) | ||
| pkg_search_module (GSSDP REQUIRED gssdp-1.2) | ||
| add_definitions(-DHAVE_GSSDP_VERSION_1_2_OR_NEWER) | ||
| message("Using gssdp-1.2") | ||
| else() | ||
| pkg_search_module (GSSDP REQUIRED gssdp-1.0) | ||
| endif() | ||
| endif() |
Comment on lines
848
to
851
| } | ||
| } | ||
| _instance->m_gdialserver_response_handler_thread = 0; | ||
| _instance->m_observer->onStopped(); | ||
| pthread_exit(nullptr); |
| gdial_rest_server_http_return_if_fail(request_body->length <= GDIAL_REST_HTTP_MAX_PAYLOAD, msg, SOUP_STATUS_REQUEST_ENTITY_TOO_LARGE); | ||
| gdial_rest_server_http_return_if_fail(!gdial_rest_server_is_bad_payload(request_body->data, request_body->length), msg, SOUP_STATUS_BAD_REQUEST); | ||
| } | ||
| GSocketAddress *socket_addr = soup_server_message_get_local_address(msg); |
Comment on lines
+82
to
+91
| static void ssdp_http_server_callback(SoupServer *server, SoupServerMessage *msg, const char *path, GHashTable *query, gpointer user_data) { | ||
| /* | ||
| * /dd.xml only supports GET | ||
| */ | ||
| if (!msg || !soup_server_message_get_method(msg) || soup_server_message_get_method(msg) != SOUP_METHOD_GET) { | ||
| soup_server_message_set_status(msg, SOUP_STATUS_BAD_REQUEST, NULL); | ||
| GDIAL_CHECK("GET_method_only"); | ||
| GDIAL_DEBUG("warning: SSDP HTTP Method is not GET"); | ||
| return; | ||
| } |
| ssdp_client_ = NULL; | ||
| } | ||
| pthread_mutex_unlock(&ssdpServerEventSync); | ||
| pthread_mutex_destroy(&ssdpServerEventSync); |
Contributor
Comment on lines
+462
to
+465
| if (app_registry->use_additional_data) { | ||
| additional_data_url = gdial_rest_server_new_additional_data_url(listening_port, app_registry->name, FALSE, app_registry->app_uri ); | ||
| } | ||
| gchar *additional_data_url_safe = g_uri_escape_string(additional_data_url, NULL, FALSE); |
| 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); |
Comment on lines
+206
to
+207
| GDIAL_LOGERROR("%s", error->message); | ||
| g_error_free(error); |
Comment on lines
+261
to
+264
| if (ssdp_http_server_) | ||
| { | ||
| soup_server_remove_handler(ssdp_http_server_, "/dd.xml"); | ||
| } |
Comment on lines
+177
to
+182
| if (0 != pthread_mutex_init(&ssdpServerEventSync, NULL)) | ||
| { | ||
| GDIAL_LOGERROR("Failed to initializing mutex"); | ||
| return EXIT_FAILURE; | ||
| } | ||
|
|
| guint listening_port = g_inet_socket_address_get_port(G_INET_SOCKET_ADDRESS(socket_addr)); | ||
| gdial_rest_server_http_return_if_fail(listening_port != 0, msg, SOUP_STATUS_INTERNAL_SERVER_ERROR); | ||
|
|
||
| GDIAL_LOGERROR("Starting the app with payload %.*s", (int)request_body->length, request_body->data); |
Comment on lines
+227
to
+237
| g_return_val_if_fail(self != NULL && app_name != NULL, FALSE); | ||
| GDialRestServerPrivate *priv = gdial_rest_server_get_instance_private(self); | ||
| GList *found = g_list_find_custom(priv->registered_apps, app_name, GCompareFunc_match_registry_app_name); | ||
| if (found) { | ||
| return (GDialAppRegistry *)found->data; | ||
| } | ||
| return NULL; | ||
| } | ||
|
|
||
| GDIAL_STATIC GDialAppRegistry *gdial_rest_server_find_app_registry_by_uuid(GDialRestServer *self, const gchar *app_uuid) { | ||
| g_return_val_if_fail(self != NULL && app_uuid != NULL, FALSE); |
| g_socket_close(conn_context->read_gsocket, NULL);//this will trigger abort callback | ||
| } | ||
| GDIAL_LOGTRACE("Exiting ..."); | ||
| return G_SOURCE_REMOVE;; |
Comment on lines
+251
to
+252
| gchar *dail_ssdp_handler = g_strdup_printf("/%s/%s", random_uuid,"dd.xml"); | ||
| soup_server_add_handler(ssdp_http_server_, dail_ssdp_handler, ssdp_http_server_callback, NULL, NULL); |
| return EXIT_FAILURE; | ||
| } | ||
|
|
||
| gdail_plat_dev_register_nwstandbymode_cb(gdial_ssdp_networkstandbymode_handler); |
Contributor
Contributor
|
b'## WARNING: A Blackduck scan failure has been waived A prior failure has been upvoted
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason for change: gdialserver not compatible with Libsoup3 library Updating gssdp to 1.6.3 provides libsoup3 compatibility .
Test Procedure: Compiled and Verified
Risks: Low
Priority: P1
version: minor