GNOME Bugzilla – Bug 730945
Allow adding custom headers to requests to host paths on GUPnPContext
Last modified: 2021-05-17 16:30:13 UTC
Created attachment 277469 [details] [review] Patch While implementing a DIAL (http://dial-multiscreen.org/) service using gupnp I stumbled upon an issue where the DIAL protocol would require adding a custom header to requests to the device location, but gupnp wouldn't allow it. The attached patch adds a method to allow setting a custom callback for host path requests, which can then be used to add custom headers to responses. Relevant code: static void _get_device_description_cb (GUPnPContext *context, SoupMessage *msg, const gchar *server_path, gpointer user_data) { SdkDialServiceDevice *self = (SdkDialServiceDevice *) user_data; soup_message_headers_append (msg->response_headers, "Application-URL", self->app_url); } { ... self->location = (SoupURI *) gupnp_device_info_get_url_base (GUPNP_DEVICE_INFO (self->dev)); /* Setup custom callback to be invoked on requests to the device description * location url, required to add the DIAL specific Application-URL header to * the response */ gupnp_context_host_path_set_callback (self->context, soup_uri_get_path ((SoupURI *) self->location), _get_device_description_cb, self); ... }
Review of attachment 277469 [details] [review]: Please hook it up to the docs as well and if there were a test or two it would be awsome. ::: libgupnp/gupnp-context.c @@ +1131,3 @@ + if (host_path_data->callback) + host_path_data->callback (host_path_data->context, Wrong indent @@ +1253,3 @@ + * @server_path: Web server path already being hosted + * @callback: (scope async): The callback to be called when handling requests to @server_path + * @user_data: User data for @callback Are you sure async is correct? According to introspection docs, async means the callback is only called once, but this would be more "notified" and would require a GDestroyNotify parameter to be nice to bindings (we had this discussion with gupnp_service_proxy_add_notify in bug 701446 Also I don't like the name, but that's a minor issue.
Actually, why not make it a signal? The signal could only be emitted when there are subscriptions
(In reply to comment #2) > Actually, why not make it a signal? The signal could only be emitted when there > are subscriptions Any suggestion for the signal name?
Please let me know if you prefer to make it a signal or only update the current patch with the suggestions from comment #1. Thanks.
Please go ahead with the original approach. Also I can't think of a better name, they all sound stupid.
Is there a reason not to add this to gupnp_context_host_path()? Something like gupnp_context_host_path_full (GUPnPContext *context, const char *local_path, const char *server_path, GUPnPContextHostPathHandler callback, gpointer user_data) and same for the other host_path() method. Have I missed something?
Hm, good point. Is there a use-case that needs the ability to remove that callback at some point?
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gupnp/-/issues/39.