Bug 94031 - Unit test failure: FAIL: wocky http-proxy-test
Summary: Unit test failure: FAIL: wocky http-proxy-test
Status: RESOLVED FIXED
Alias: None
Product: Wocky
Classification: Unclassified
Component: General (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-07 04:24 UTC by diane
Modified: 2016-07-12 17:29 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Up g_io_extension_point_implement priority. (868 bytes, text/plain)
2016-02-07 04:24 UTC, diane
Details
[PATCH 1/2] Remove wocky-http-proxy, upstreamed in GIO now (20.70 KB, patch)
2016-07-03 20:28 UTC, George Kiagiadakis
Details | Splinter Review
[PATCH 2/2] Bump required glib version to 2.44, as we now depend on GIO http proxy (4.11 KB, patch)
2016-07-03 20:28 UTC, George Kiagiadakis
Details | Splinter Review

Description diane 2016-02-07 04:24:35 UTC
Created attachment 121563 [details]
Up g_io_extension_point_implement priority.

The wocky http proxy tests fail because the GIO ends up instantiating a normal HTTP proxy and not the Wocky proxy. Upping the priority of the Wocky HTTP proxy fixes it.

ERROR:wocky-http-proxy-test.c:167:test_http_proxy_instantiation: assertion failed: (WOCKY_IS_HTTP_PROXY (proxy))
FAIL
GTester: last random seed: R02Sd3180f6a88f85b0fd8d3499d4434cbe2
(pid=16669)
  /http-proxy/close-by-peer:                                           OK

...[ trim passing tests ]...

  /http-proxy/authentication-failed:                                   **
ERROR:wocky-http-proxy-test.c:229:server_thread: assertion failed (base64_cred == received_cred): ("dXNlcm5hbWU6YmFkLXBhc3N3b3Jk" == "Basic dXNlcm5hbWU6YmFkLXBhc3N3b3Jk")
FAIL
GTester: last random seed: R02S8f4b3df1d3ccc6131f82afb8364b2308
(pid=16687)
  /http-proxy/authentication-failed-async:                             **
ERROR:wocky-http-proxy-test.c:229:server_thread: assertion failed (base64_cred == received_cred): ("dXNlcm5hbWU6YmFkLXBhc3N3b3Jk" == "Basic dXNlcm5hbWU6YmFkLXBhc3N3b3Jk")
FAIL
GTester: last random seed: R02S9be7d25767c927a76f2ba09883dd41d8
(pid=16691)
  /http-proxy/authenticated:                                           **
ERROR:wocky-http-proxy-test.c:229:server_thread: assertion failed (base64_cred == received_cred): ("QWxhZGRpbjpvcGVuIHNlc2FtZQ==" == "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==")
FAIL
GTester: last random seed: R02S1c8b4ea53aab34cd114ab8d1fdf2130e
(pid=16693)
  /http-proxy/authenticated-async:                                     **
ERROR:wocky-http-proxy-test.c:229:server_thread: assertion failed (base64_cred == received_cred): ("QWxhZGRpbjpvcGVuIHNlc2FtZQ==" == "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==")
FAIL
GTester: last random seed: R02S435d6d07e7f932d735f2ecb817329f3a
(pid=16697)
FAIL: wocky-http-proxy-test
Comment 1 George Kiagiadakis 2016-06-12 09:47:33 UTC
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.
Comment 2 diane 2016-06-12 17:54:34 UTC
(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.
Comment 3 George Kiagiadakis 2016-07-03 17:47:29 UTC
(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.
Comment 4 diane 2016-07-03 18:09:31 UTC
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
Comment 5 George Kiagiadakis 2016-07-03 20:28:13 UTC
Created attachment 124876 [details] [review]
[PATCH 1/2] Remove wocky-http-proxy, upstreamed in GIO now
Comment 6 George Kiagiadakis 2016-07-03 20:28:43 UTC
Created attachment 124877 [details] [review]
[PATCH 2/2] Bump required glib version to 2.44, as we now depend on  GIO http proxy
Comment 7 diane 2016-07-04 05:46:39 UTC
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


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.