Created attachment 242974 [details] [review]
Patch to also perform translation on g_libproxy_resolver_lookup_async

I am not a maintainer but the patch looks good to me.

Review of attachment 242974 [details] [review]:

I am not a maintainer either, but I think this needs a bit of adjustment:

::: proxy/libproxy/glibproxyresolver.c
@@ +156,2 @@
 static gchar **
+_fixup_proxies (gchar **proxies)

This is a sufficiently strange calling convention (consumes its argument) that it deserves a pseudo-doc-comment; you could also move the "We always copy" comment up here too, in fact.

Here's what I'd use:

/*
 * Convert the array of proxies into a GStrv, expanding any "socks" entries
 * into separate entries for "socks5", "socks4a", "socks4".
 *
 * The @proxies argument is consumed.
 *
 * @proxies: (array zero-terminated=1) (transfer full): proxies from
 *   libproxy, freed by this function using free_libproxy_proxies()
 * Returns: (array zero-terminated=1) (transfer full): new set of proxies,
 *   to be freed with g_strfreev()
 */

@@ +210,3 @@
+      if (proxies)
+        proxies = _fixup_proxies (proxies);
+      g_task_return_pointer (task, proxies, (GDestroyNotify)free_libproxy_proxies);

The result of _fixup_proxies() should be freed with g_strfreev(), not free_libproxy_proxies().

This does matter if the underlying allocator used by g_malloc()/g_free() is not the same as the one used by malloc()/free() (although it's usually the same). libproxy always uses malloc()/free() (I think?) whereas GLib can be configured to use an alternative.

Created attachment 242989 [details] [review]
libproxy: fix handling of SOCKS in async API, add tests

Fix the handling of SOCKS proxies with g_proxy_resolver_lookup_async()
and the libproxy resolver. (Also, make the sync proxy resolver
properly cancellable.)

Add some tests of the libproxy resolver, based on the existing gnome
proxy resolver tests. Unfortunately, libproxy doesn't implement ignore
hosts in exactly the same way GProxyResolverGnome does, so we need to
keep track of which URIs they give different results for...

can you try this patch instead?

Thanks Dan, I can verify this patch works well for my use.

fixed in master. unfortunately it just missed 2.37.1

Attachment 242989 [details] pushed as 1b9f7e0 - libproxy: fix handling of SOCKS in async API, add tests

As a follow-up, does this patch solve connecting through a SOCKS proxy that wants to use HTTP CONNECT?  A user is reporting an error when Geary attempts to make an IMAP connection with a SOCKS proxy: "Proxy protocol 'http' is not supported".  Geary uses async calls with GSocketClient.  I can't quite tell from this patch if it solves this problem.

(In reply to comment #8)
> A user is reporting an error when Geary attempts to
> make an IMAP connection with a SOCKS proxy: "Proxy protocol 'http' is not
> supported".

I think that error means that the user misconfigured their proxy settings, and claimed to have an http proxy, rather than a SOCKS proxy. (ie, in the control center, they filled in the "HTTP proxy" field rather than the "SOCKS proxy" field).

The user sent us the contents of their /etc/environment file:

PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games"
http_proxy="http://10.0.0.1:80/"
https_proxy="https://10.0.0.1:80/"
ftp_proxy="ftp://10.0.0.1:80/"
socks_proxy="socks://10.0.0.1:80/"
no_proxy="127.0.0.1,localhost,*10.*"
HTTP_PROXY="http://10.0.0.1:80/"
HTTPS_PROXY="https://10.0.0.1:80/"
FTP_PROXY="ftp://10.0.0.1:80/"
SOCKS_PROXY="socks://10.0.0.1:80/"
NO_PROXY="127.0.0.1,localhost,*10.*"

Is it possible the problem is that they have both an HTTP and a SOCKS proxy?

oh. no, the problem is that "socks_proxy" is not a standard environment variable, and libproxy doesn't look at it. (But it returns $http_proxy as a fallback for any non-http, non-ftp request.) It looks like there is no way to get libproxy's environment-variable plugin to return a socks proxy.