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 680436 - net: Add possibility to set custom HTTP headers on requests
net: Add possibility to set custom HTTP headers on requests
Status: RESOLVED FIXED
Product: grilo
Classification: Other
Component: core
git master
Other Linux
: Normal enhancement
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks: 672934
 
 
Reported: 2012-07-23 10:14 UTC by Jens Georg
Modified: 2012-09-18 09:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
suggested fix (8.36 KB, patch)
2012-07-23 10:14 UTC, Jens Georg
needs-work Details | Review
Updated patch which includes a varargs version which delegates to the (11.29 KB, patch)
2012-09-11 07:06 UTC, Jens Georg
none Details | Review
Patch v3 - Introspection magic (11.34 KB, patch)
2012-09-11 07:10 UTC, Jens Georg
needs-work Details | Review
v4: Fix leak and address other comments (11.39 KB, patch)
2012-09-17 11:06 UTC, Jens Georg
committed Details | Review

Description Jens Georg 2012-07-23 10:14:39 UTC
Created attachment 219475 [details] [review]
suggested fix

Add a grl_net_wc_request_async_full function that takes a string,string hash with header → value mapping
Comment 1 Juan A. Suarez Romero 2012-09-07 17:09:08 UTC
Review of attachment 219475 [details] [review]:

Usually the developer that needs to set custom headers will need to set just one header, 2 at most.

The point is that to do it, basically, developer first need to create a hash table, then set the header (as saying most of cases will just add one entry), invoke the function and free the hash table. IMHO, too many steps for something that should be simpler, and specially easy to read.

I was thinking that something similar to g_object_set() would be more appropriate and easy to read: in the same function you set up each header with its value.

The API I have in mind is:

void grl_net_wc_request_with_headers_async (GrlNet *self, const gchar *uri, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data, const gchar *header, ...);

An example of calling this function would be:

grl_net_wc_request_with_headers_async (wc, "http://myuri", cancellable, on_callback, myuser_data, "header1", "value1", "header2", "value2", NULL);
Comment 2 Jens Georg 2012-09-10 13:49:08 UTC
Still, vaargs are bad for bindings/introspection
Comment 3 Juan A. Suarez Romero 2012-09-10 14:11:10 UTC
Yes, not very good for introspection. But that problem is also in other parts of Grilo that, I think, must be addressed in the future.

Anyway, IIRC (and correct me if I'm wrong), it is possible to provide an alternative method for the binding case that works like you have proposed (using a hashtable), and that would be used instead of varargs version when making the bindings.


Still, if above doesn't work, I still prefer the varargs version. And check in future how to solve all the introspection/bindings problem in Grilo in one shot.
Comment 4 Jens Georg 2012-09-11 07:06:47 UTC
Created attachment 223988 [details] [review]
Updated patch which includes a varargs version which delegates to the

(renamed) hash version from the previous patch.
Comment 5 Jens Georg 2012-09-11 07:10:25 UTC
Created attachment 223990 [details] [review]
Patch v3 - Introspection magic

Same as version 2, but with added introspection magic to make the hash version
shadow the varargs version in GI.
Comment 6 Juan A. Suarez Romero 2012-09-13 10:25:16 UTC
Review of attachment 223990 [details] [review]:

::: libs/net/grl-net-wc.c
@@ +433,3 @@
+                                              cancellable,
+                                              callback,
+                                              user_data);

Shouldn't free the hashtable?

::: libs/net/grl-net-wc.h
@@ +122,3 @@
 			       gpointer user_data);
 
+void grl_net_wc_request_async_with_headers_hash (GrlNetWc *self,

Let's keep the standard use "_async" as the suffix.

So rename it to grl_net_wc_request_with_headers_hash_async()

@@ +129,3 @@
+                                                 gpointer user_data);
+
+void grl_net_wc_request_async_with_headers (GrlNetWc *self,

Same here: rename it to grl_net_wc_request_with_headers_async()
Comment 7 Jens Georg 2012-09-17 11:06:52 UTC
Created attachment 224491 [details] [review]
v4: Fix leak and address other comments

net: Add possibility to add arbitrary HTTP headers

Some webservices need custom HTTP headers for requests; this patch adds
a function to schedule requests with those special headers.

Under contract for Canonical Ltd.
Comment 8 Juan A. Suarez Romero 2012-09-18 09:15:46 UTC
commit dd619934612a98493ee8ab6b76eb883acc52cd9c
Author: Jens Georg <jensg@openismus.com>
Date:   Thu Jul 12 22:44:47 2012 +0200

    net: Add possibility to add arbitrary HTTP headers
    
    Some webservices need custom HTTP headers for requests; this patch adds
    a function to schedule requests with those special headers.
    
    Under contract for Canonical Ltd.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=680436

 libs/net/grl-net-private.h       |  1 +
 libs/net/grl-net-soup-stable.c   | 13 +++++++++++++
 libs/net/grl-net-soup-unstable.c | 19 +++++++++++++++++++
Comment 9 Juan A. Suarez Romero 2012-09-18 09:18:05 UTC
(In reply to comment #8)
> commit dd619934612a98493ee8ab6b76eb883acc52cd9c

Wrong commit



commit bf186d0626220b8515cbf2ce2809e404ce6f0d28
Author: Jens Georg <jensg@openismus.com>
Date:   Thu Jul 12 22:44:47 2012 +0200

    net: Add possibility to add arbitrary HTTP headers
    
    Some webservices need custom HTTP headers for requests; this patch adds
    a function to schedule requests with those special headers.
    
    Under contract for Canonical Ltd.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=680436
    
    Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>

 libs/net/grl-net-private.h       |  1 +
 libs/net/grl-net-soup-stable.c   | 13 +++++++++++++
 libs/net/grl-net-soup-unstable.c | 19 +++++++++++++++++++
 libs/net/grl-net-wc.c            | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 libs/net/grl-net-wc.h            | 16 ++++++++++++++++
 5 files changed, 143 insertions(+), 3 deletions(-)