After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 729613 - Add support to add vendor specific headers to messages
Add support to add vendor specific headers to messages
Status: RESOLVED FIXED
Product: gssdp
Classification: Other
Component: General
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: GUPnP Maintainers
GUPnP Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-05-06 03:45 UTC by Louis-Francis Ratté-Boulianne
Modified: 2019-02-22 09:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add support for additionnal vendor specific headers in messages (8.15 KB, patch)
2014-05-06 03:45 UTC, Louis-Francis Ratté-Boulianne
none Details | Review
Check each existing attachment made obsolete by your new attachment. (8.15 KB, patch)
2014-05-09 15:19 UTC, Louis-Francis Ratté-Boulianne
none Details | Review
Add support for additionnal vendor specific headers in messages (9.94 KB, patch)
2014-05-24 14:00 UTC, Louis-Francis Ratté-Boulianne
needs-work Details | Review
Add support for additionnal vendor specific headers in messages (9.58 KB, patch)
2014-05-24 14:28 UTC, Louis-Francis Ratté-Boulianne
committed Details | Review

Description Louis-Francis Ratté-Boulianne 2014-05-06 03:45:59 UTC
Created attachment 275932 [details] [review]
Add support for additionnal vendor specific headers in messages

I'm developping a library to control a Sonos system. The controller discover the components using SSDP, however, the messages add a custom a custom field to avoir discovery response from other Sonos systems.

X-RINCON-HOUSEHOLD: Sonos_somelongidentifier

The ideal solution would be to be have an api directly in gssdp to do that. Here's my take on this. New methods are:

 void gssdp_client_append_header (GSSDPClient *client, const char *name, const char *value);
 void gssdp_client_remove_header (GSSDPClient *client, const gchar *name);
 void gssdp_client_clear_headers (GSSDPClient *client);

Of course, some other methods could easily be added if needed, for example:

 void gssdp_client_append_headers (GSSDPClient *client, ...);
 const char* gssdp_client_lookup_header (GSSDPClient *client, const char *name);
 etc.

I realize this is out of the protocol definition, but I couldn't find a better way to realize that without re-implementating the whole of GSSDP in my library.
Comment 1 Louis-Francis Ratté-Boulianne 2014-05-09 15:19:09 UTC
Created attachment 276251 [details] [review]
Check each existing attachment made obsolete by your new attachment.

Re-upload the diff so it appears as a patch
Comment 2 Jens Georg 2014-05-24 12:29:28 UTC
This patch breaks the functional tests:

/functional/resource-group/discovery/ssdp:all: 
** (/home/jens/Source/gssdp/tests/gtest/.libs/lt-test-functional:20515): WARNING **: Received packet lacks "\r\n\r\n" sequence. Packed dropped.
Comment 3 Louis-Francis Ratté-Boulianne 2014-05-24 14:00:12 UTC
Created attachment 277112 [details] [review]
Add support for additionnal vendor specific headers in messages

Fix the tests
Comment 4 Jens Georg 2014-05-24 14:13:45 UTC
Review of attachment 277112 [details] [review]:

::: libgssdp/gssdp-client.c
@@ +852,3 @@
+                      const gchar *message)
+{
+        GList *iter;

Somehow implementing this using GString would look easier to me than dupping, mallocing joining and freeing

@@ +1002,3 @@
         inet_address = g_inet_address_new_from_string (dest_ip);
         address = g_inet_socket_address_new (inet_address, dest_port);
+        extended_message = append_header_fields (client, message);

Are you sure you want to apply the headers to all kind of messages?
Comment 5 Louis-Francis Ratté-Boulianne 2014-05-24 14:28:03 UTC
Created attachment 277114 [details] [review]
Add support for additionnal vendor specific headers in messages

Use GString instread of joining substrings
Comment 6 Louis-Francis Ratté-Boulianne 2014-05-24 14:31:02 UTC
(In reply to comment #4)
> Are you sure you want to apply the headers to all kind of messages?

For my use case, I only need the extensions on the discovery requests but I'm not quite sure what other scenarios are possible.

Also, ideally, we could read the vendor-specific headers of received messages. Unfortunately, I don't see how that's possible without breaking the API...
Comment 7 Jens Georg 2014-05-24 19:00:03 UTC
(In reply to comment #6)
> (In reply to comment #4)

> Also, ideally, we could read the vendor-specific headers of received messages.
> Unfortunately, I don't see how that's possible without breaking the API...

True. Maybe something to keep in mind for when that's the case.
Comment 8 Jens Georg 2014-05-24 19:03:38 UTC
Attachment 277114 [details] pushed as 9f02229 - Add support for additionnal vendor specific headers in messages
Comment 9 Jens Georg 2014-06-10 13:29:32 UTC
Have to re-open. This patch breaks sending of SSDP packets to my WDTV live.
Comment 10 Jens Georg 2014-06-10 13:46:58 UTC
Sorry, fooled by bisect