Bug 94031

I'm not sure about this... Since the unit test requires the wocky http proxy, then it probably should instantiate it directly instead of relying on priorities. But I can see that the unit test code specifically has a test that asserts whether the factory instantiated proxy is the wocky one, which may be there for a reason.

Maybe also this code was written in an era where GIO had no default http proxy and now maybe it has, so the wocky one is useless. I wonder if gabble works fine with a proxy without this patch and which proxy class is actually instantiated.

(In reply to George Kiagiadakis from comment #1)

> Maybe also this code was written in an era where GIO had no default http
> proxy and now maybe it has, so the wocky one is useless. I wonder if gabble
> works fine with a proxy without this patch and which proxy class is actually
> instantiated.

That's a really good point. I'm not even sure how the proxy is supposed to work, making it pretty hard to test to see if its working.

(In reply to George Kiagiadakis from comment #1)
> 
> Maybe also this code was written in an era where GIO had no default http
> proxy and now maybe it has, so the wocky one is useless. I wonder if gabble
> works fine with a proxy without this patch and which proxy class is actually
> instantiated.

Looks like I was right about that.

https://git.gnome.org/browse/glib/commit/gio/ghttpproxy.c?id=ed4a742946374f7ee3c46b93eb943c95f04ec4c4

The wocky proxy code obviously made its way into glib, so we no longer need it around in wocky.

George Kiagiadakis found that wocky-http-proxy was modified and became part of gio as https://git.gnome.org/browse/glib/plain/gio/ghttpproxy.c

Diffing wocky-http-proxy and ghttpproxy shows a few meaningful differences.

Without the above patch I get errors like this:

ERROR:wocky-http-proxy-test.c:229:server_thread: assertion failed (base64_cred == received_cred): ("QWxhZGRpbjpvcGVuIHNlc2FtZQ==" == "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==")

wocky-http-proxy.c (-) used a hard coded "Basic", while the ghttpproxy.c (+) includes "Basic " as part of the credential

@@ -111,9 +97,9 @@
       cred = g_strdup_printf ("%s:%s", username, password);
       base64_cred = g_base64_encode ((guchar *) cred, strlen (cred));
       g_free (cred);
       g_string_append_printf (request,
-                              "Proxy-Authorization: Basic %s\r\n",
+          "Proxy-Authorization: %s\r\n",
                               base64_cred);
       g_free (base64_cred);
     }
 
The other solution might be to be drop the wocky-http-proxy, and update the wocky-proxy tests to be compatible with giohttpproxy

Created attachment 124876 [details] [review]
[PATCH 1/2] Remove wocky-http-proxy, upstreamed in GIO now

Created attachment 124877 [details] [review]
[PATCH 2/2] Bump required glib version to 2.44, as we now depend on  GIO http proxy

Code looks good to me. I was briefly confused by removing g_type_init until I read the comment that it was deprecated.

Before applying it wocky-http-proxy-test has errors, after applying and compiling wocky-http-proxy-test passes.


Although after fixing these tests I get authentication errors in other tests, but that appears to be something else, so I opened a different bug for it.

https://bugs.freedesktop.org/show_bug.cgi?id=96796