2 files changed, 194 insertions, 83 deletions

diff --git a/configure.ac b/configure.ac
index bc504ab1..ce1e9097 100644
--- a/configure.ac
+++ b/configure.ac
@@ -173,13 +173,24 @@ This is required for GNOME proxy support.
 Pass "--without-gnome" to configure if you want to build libsoup
 without GNOME support.])])
 
+	AC_MSG_CHECKING(libproxy version)
+	libproxy_version=`$PKG_CONFIG --modversion libproxy-1.0`
+	case $libproxy_version in
+	0.2.*)
+		AC_MSG_RESULT([$libproxy_version, using workarounds...])
+		AC_DEFINE(HAVE_LIBPROXY_WITH_NON_THREAD_SAFE_GNOME_MODULE, 1, [Defined if libproxy is old and has a non-thread-safe gnome module])
+		;;
+	*)
+		AC_MSG_RESULT([$libproxy_version, ok])
+		;;
+	esac
+
 	PKG_CHECK_MODULES(GCONF, gconf-2.0, , [AC_MSG_ERROR(dnl
 [Could not find GConf:
 
 $GCONF_PKG_ERRORS
 
-Due to bugs in libproxy as of the GNOME 2.26 freeze date, GConf is
-*also* required for GNOME proxy support.
+This is required for GNOME proxy support.
 
 Pass "--without-gnome" to configure if you want to build libsoup
 without GNOME support.])])
diff --git a/libsoup/soup-proxy-resolver-gnome.c b/libsoup/soup-proxy-resolver-gnome.c
index 1873fbbb..186ca4e0 100644
--- a/libsoup/soup-proxy-resolver-gnome.c
+++ b/libsoup/soup-proxy-resolver-gnome.c
@@ -22,24 +22,55 @@
 #include <gconf/gconf-client.h>
 #include <proxy.h>
 
-typedef enum {
-	SOUP_PROXY_RESOLVER_GNOME_MODE_NONE,
-	SOUP_PROXY_RESOLVER_GNOME_MODE_MANUAL,
-	SOUP_PROXY_RESOLVER_GNOME_MODE_AUTO
-} SoupProxyResolverGNOMEMode;
-
-/* Since GConf is not thread-safe, making it annoying for us to deal
- * with, we make all of these static variables rather than instance
- * variables, so that we only have to deal with it once, rather than
- * per-resolver.
+/* libproxy/GConf-based proxy resolution, working around multiple
+ * problems:
+ *
+ *   1. GConf is not thread-safe, but a SoupProxyResolver may be used
+ *      from multiple threads. We need to take steps to ensure that if
+ *      there is a thread running the default main loop, then GConf
+ *      calls are only made from that thread.
+ *
+ *   2. libproxy 0.2 was unaware of GConf's non-thread-safety, so it
+ *      is unsafe to call into libproxy other than from the main
+ *      thread as well, unless we can guarantee that it won't use the
+ *      "gnome" plugin.
+ *
+ *   3. libproxy 0.3 fixes the thread-safety issues by just calling
+ *      out to gconftool-2 on every call. That fixes the crashes but
+ *      makes things a lot slower, so ideally we still don't want to
+ *      call it if it's going to use the "gnome" plugin.
+ *
+ *   4. libproxy is not cancellable, and may block (eg, doing WPAD or
+ *      just DNS, or running a PAC script), so if we want to be able
+ *      to cancel calls to it, we need to do them in another thread
+ *      even if we're making a sync call.
+ *
+ * The only good way to ensure that libproxy won't use the gnome
+ * plugin is to ensure that it will use environment variables instead.
+ * But if we just copy the GConf settings to the environment, then
+ * we'll leak them to child processes. For libproxy 0.2, we *can't*
+ * let it use the gnome plugin, so if we need to use libproxy (to get
+ * PAC/WPAD or ignore_host support) then we just suffer through
+ * environment leakage. For libproxy 0.3, the gconftool-calling
+ * slowness is less bad than environment variable leakage would be, so
+ * we let it use the gnome plugin in that case.
+ */
+
+/* We make these variables static rather than per-resolver, since
+ * they're awkward to deal with because of GConf's non-thread-safety,
+ * and because there's only one GConf database regardless of how many
+ * SoupProxyResolverGNOMEs there are anyway.
  */
 G_LOCK_DEFINE_STATIC (resolver_gnome);
-static SoupProxyResolverGNOMEMode proxy_mode;
 static GConfClient *gconf_client;
 static char *proxy_user, *proxy_password;
+static char *http_proxy, *https_proxy;
 
 static pxProxyFactory *libproxy_factory;
 static GThreadPool *libproxy_threadpool;
+#ifdef HAVE_LIBPROXY_WITH_NON_THREAD_SAFE_GNOME_MODULE
+static gboolean overrode_environment;
+#endif
 
 static void soup_proxy_resolver_gnome_interface_init (SoupProxyURIResolverInterface *proxy_resolver_interface);
 
@@ -167,21 +198,39 @@ soup_proxy_resolver_gnome_interface_init (SoupProxyURIResolverInterface *proxy_u
 #define SOUP_GCONF_USE_SAME_PROXY       "/system/http_proxy/use_same_proxy"
 #define SOUP_GCONF_PROXY_IGNORE_HOSTS   "/system/http_proxy/ignore_hosts"
 
+typedef enum {
+	SOUP_PROXY_RESOLVER_GNOME_GCONF_MODE_NONE,
+	SOUP_PROXY_RESOLVER_GNOME_GCONF_MODE_MANUAL,
+	SOUP_PROXY_RESOLVER_GNOME_GCONF_MODE_AUTO
+} SoupProxyResolverGNOMEGConfMode;
+
 static GConfEnumStringPair proxy_mode_map [] = {
-	{ SOUP_PROXY_RESOLVER_GNOME_MODE_NONE,   "none"   },
-	{ SOUP_PROXY_RESOLVER_GNOME_MODE_MANUAL, "manual" },
-	{ SOUP_PROXY_RESOLVER_GNOME_MODE_AUTO,   "auto"   },
+	{ SOUP_PROXY_RESOLVER_GNOME_GCONF_MODE_NONE,   "none"   },
+	{ SOUP_PROXY_RESOLVER_GNOME_GCONF_MODE_MANUAL, "manual" },
+	{ SOUP_PROXY_RESOLVER_GNOME_GCONF_MODE_AUTO,   "auto"   },
 	{ 0, NULL }
 };
 
 static void
 update_proxy_settings (void)
 {
-	char *mode, *http_proxy, *https_proxy = NULL, *no_proxy = NULL;
+	SoupProxyResolverGNOMEGConfMode proxy_mode;
+	char *mode, *no_proxy = NULL;
+	char *autoconfig_url, *host;
+	guint port;
 	GSList *ignore;
+	gboolean need_libproxy;
 
 	/* resolver_gnome is locked */
 
+	if (http_proxy) {
+		g_free (http_proxy);
+		http_proxy = NULL;
+	}
+	if (https_proxy) {
+		g_free (https_proxy);
+		https_proxy = NULL;
+	}
 	if (proxy_user) {
 		g_free (proxy_user);
 		proxy_user = NULL;
@@ -192,42 +241,51 @@ update_proxy_settings (void)
 		proxy_password = NULL;
 	}
 
+#ifdef HAVE_LIBPROXY_WITH_NON_THREAD_SAFE_GNOME_MODULE
+	if (overrode_environment) {
+		g_unsetenv ("PX_CONFIG_ORDER");
+		g_unsetenv ("http_proxy");
+		g_unsetenv ("https_proxy");
+		g_unsetenv ("no_proxy");
+		overrode_environment = FALSE;
+	}
+#endif
+
 	/* Get new settings */
 	mode = gconf_client_get_string (
 		gconf_client, SOUP_GCONF_PROXY_MODE, NULL);
 	if (!mode || !gconf_string_to_enum (proxy_mode_map, mode,
 					    (int *)&proxy_mode))
-		proxy_mode = SOUP_PROXY_RESOLVER_GNOME_MODE_NONE;
+		proxy_mode = SOUP_PROXY_RESOLVER_GNOME_GCONF_MODE_NONE;
 	g_free (mode);
 
-	if (proxy_mode == SOUP_PROXY_RESOLVER_GNOME_MODE_NONE) {
-		if (libproxy_factory) {
-			/* Unset anything we previously set */
-			g_unsetenv ("PX_CONFIG_ORDER");
-			g_unsetenv ("http_proxy");
-			g_unsetenv ("https_proxy");
-			g_unsetenv ("no_proxy");
-		}
-		return;
-	} else if (proxy_mode == SOUP_PROXY_RESOLVER_GNOME_MODE_AUTO) {
-		char *autoconfig_url;
+	switch (proxy_mode) {
+	case SOUP_PROXY_RESOLVER_GNOME_GCONF_MODE_NONE:
+		/* If GConf says "no", but the environment variables are
+		 * set, then use the environment variables
+		 */
+		http_proxy = g_strdup (g_getenv ("http_proxy"));
+		https_proxy = g_strdup (g_getenv ("https_proxy"));
+		no_proxy = g_strdup (g_getenv ("no_proxy"));
+		break;
 
+	case SOUP_PROXY_RESOLVER_GNOME_GCONF_MODE_AUTO:
 		autoconfig_url = gconf_client_get_string (
 			gconf_client, SOUP_GCONF_PROXY_AUTOCONFIG_URL, NULL);
 		if (autoconfig_url && !strncmp (autoconfig_url, "http", 4))
 			http_proxy = g_strconcat ("pac+", autoconfig_url, NULL);
 		else
 			http_proxy = g_strdup ("wpad://");
+		https_proxy = g_strdup (http_proxy);
 		g_free (autoconfig_url);
-	} else /* SOUP_PROXY_RESOLVER_GNOME_MODE_MANUAL */ {
-		char *host;
-		guint port;
+		goto do_ignore_list;
 
+	case SOUP_PROXY_RESOLVER_GNOME_GCONF_MODE_MANUAL:
 		host = gconf_client_get_string (
 			gconf_client, SOUP_GCONF_HTTP_PROXY_HOST, NULL);
 		if (!host || !*host) {
 			g_free (host);
-			proxy_mode = SOUP_PROXY_RESOLVER_GNOME_MODE_NONE;
+			proxy_mode = SOUP_PROXY_RESOLVER_GNOME_GCONF_MODE_NONE;
 			return;
 		}
 		port = gconf_client_get_int (
@@ -264,50 +322,87 @@ update_proxy_settings (void)
 			proxy_password = gconf_client_get_string (
 				gconf_client, SOUP_GCONF_HTTP_PROXY_PASSWORD, NULL);
 		}
+
+		/* fall through */
+
+	do_ignore_list:
+		ignore = gconf_client_get_list (
+			gconf_client, SOUP_GCONF_PROXY_IGNORE_HOSTS,
+			GCONF_VALUE_STRING, NULL);
+		if (ignore) {
+			GString *ignore_list;
+			GSList *i;
+
+			ignore_list = g_string_new (NULL);
+			for (i = ignore; i; i = i->next) {
+				if (ignore_list->len)
+					g_string_append_c (ignore_list, ',');
+				g_string_append (ignore_list, i->data);
+				g_free (i->data);
+			}
+			g_slist_free (ignore);
+			no_proxy = g_string_free (ignore_list, FALSE);
+		}
+		break;
 	}
 
-	ignore = gconf_client_get_list (
-		gconf_client, SOUP_GCONF_PROXY_IGNORE_HOSTS,
-		GCONF_VALUE_STRING, NULL);
-	if (ignore) {
-		GString *ignore_list;
-		GSList *i;
-
-		ignore_list = g_string_new (NULL);
-		for (i = ignore; i; i = i->next) {
-			if (ignore_list->len)
-				g_string_append_c (ignore_list, ',');
-			g_string_append (ignore_list, i->data);
-			g_free (i->data);
+	/* We need to use libproxy if (a) there is an ignore list, or
+	 * (b) we're using PAC or WPAD.
+	 */
+	if (no_proxy)
+		need_libproxy = TRUE;
+	else if (http_proxy &&
+		 (g_str_has_prefix (http_proxy, "pac+") ||
+		  g_str_has_prefix (http_proxy, "wpad:")))
+		need_libproxy = TRUE;
+	else if (https_proxy &&
+		 (g_str_has_prefix (https_proxy, "pac+") ||
+		  g_str_has_prefix (https_proxy, "wpad:")))
+		need_libproxy = TRUE;
+	else
+		need_libproxy = FALSE;
+
+	if (!need_libproxy) {
+		if (libproxy_factory) {
+			px_proxy_factory_free (libproxy_factory);
+			libproxy_factory = NULL;
 		}
-		g_slist_free (ignore);
-		no_proxy = g_string_free (ignore_list, FALSE);
+		if (libproxy_threadpool) {
+			g_thread_pool_free (libproxy_threadpool, FALSE, FALSE);
+			libproxy_threadpool = NULL;
+		}
+		return;
 	}
 
+	/* If we have a "bad" libproxy, set environment variables
+	 * and force it to use them.
+	 */
+#ifdef HAVE_LIBPROXY_WITH_NON_THREAD_SAFE_GNOME_MODULE
 	g_setenv ("PX_CONFIG_ORDER", "envvar", TRUE);
-	g_setenv ("http_proxy", http_proxy, TRUE);
-	g_free (http_proxy);
-	if (https_proxy) {
+	if (http_proxy)
+		g_setenv ("http_proxy", http_proxy, TRUE);
+	else
+		g_unsetenv ("http_proxy");
+	if (https_proxy)
 		g_setenv ("https_proxy", https_proxy, TRUE);
-		g_free (https_proxy);
-	} else
+	else
 		g_unsetenv ("https_proxy");
-	if (no_proxy) {
+	if (no_proxy)
 		g_setenv ("no_proxy", no_proxy, TRUE);
-		g_free (no_proxy);
-	} else
+	else
 		g_unsetenv ("no_proxy");
+	overrode_environment = TRUE;
+#endif
 
 	/* If we haven't created a proxy factory or thread pool yet,
 	 * do so. If we already have one, we don't need to update
-	 * anything, because it rechecks the environment variables
+	 * anything, because it rechecks the environment variables, etc
 	 * every time.
 	 */
 	if (!libproxy_factory)
 		libproxy_factory = px_proxy_factory_new ();
 
-	if (proxy_mode == SOUP_PROXY_RESOLVER_GNOME_MODE_AUTO &&
-	    !libproxy_threadpool) {
+	if (!libproxy_threadpool) {
 		libproxy_threadpool =
 			g_thread_pool_new (libproxy_threadpool_func,
 					   NULL, -1, FALSE, NULL);
@@ -324,7 +419,27 @@ gconf_value_changed (GConfClient *client, const char *key,
 }
 
 static guint
-get_proxy_for_uri (SoupURI *uri, SoupURI **proxy_uri)
+get_proxy_for_uri_direct (SoupURI *uri, SoupURI **proxy_uri)
+{
+	/* resolver_gnome is locked */
+
+	if (uri->scheme == SOUP_URI_SCHEME_HTTP && http_proxy)
+		*proxy_uri = soup_uri_new (http_proxy);
+	else if (uri->scheme == SOUP_URI_SCHEME_HTTPS && https_proxy)
+		*proxy_uri = soup_uri_new (https_proxy);
+	else
+		*proxy_uri = NULL;
+
+	if (*proxy_uri && proxy_user) {
+		soup_uri_set_user (*proxy_uri, proxy_user);
+		soup_uri_set_password (*proxy_uri, proxy_password);
+	}
+
+	return SOUP_STATUS_OK;
+}
+
+static guint
+get_proxy_for_uri_via_libproxy (SoupURI *uri, SoupURI **proxy_uri)
 {
 	char *uristr, **proxies;
 	gboolean got_proxy;
@@ -430,30 +545,16 @@ get_proxy_uri_async (SoupProxyURIResolver  *proxy_uri_resolver,
 	sgad->user_data = user_data;
 
 	G_LOCK (resolver_gnome);
-	switch (proxy_mode) {
-	case SOUP_PROXY_RESOLVER_GNOME_MODE_NONE:
-		sgad->proxy_uri = NULL;
-		sgad->status = SOUP_STATUS_OK;
-		break;
-
-	case SOUP_PROXY_RESOLVER_GNOME_MODE_MANUAL:
-		/* We know libproxy won't do PAC or WPAD in this case,
-		 * so we can make a "blocking" call to it.
-		 */
-		sgad->status = get_proxy_for_uri (uri, &sgad->proxy_uri);
-		break;
-
-	case SOUP_PROXY_RESOLVER_GNOME_MODE_AUTO:
+	if (libproxy_threadpool) {
 		/* FIXME: cancellable */
 		sgad->uri = soup_uri_copy (uri);
 		sgad->async_context = async_context ? g_main_context_ref (async_context) : NULL;
 		g_thread_pool_push (libproxy_threadpool, sgad, NULL);
-		G_UNLOCK (resolver_gnome);
-		return;
+	} else {
+		sgad->status = get_proxy_for_uri_direct (uri, &sgad->proxy_uri);
+		soup_add_completion (async_context, resolved_proxy, sgad);
 	}
 	G_UNLOCK (resolver_gnome);
-
-	soup_add_completion (async_context, resolved_proxy, sgad);
 }
 
 static guint
@@ -465,11 +566,10 @@ get_proxy_uri_sync (SoupProxyURIResolver  *proxy_uri_resolver,
 	guint status;
 
 	G_LOCK (resolver_gnome);
-	if (proxy_mode == SOUP_PROXY_RESOLVER_GNOME_MODE_NONE) {
-		*proxy_uri = NULL;
-		status = SOUP_STATUS_OK;
-	} else
-		status = get_proxy_for_uri (uri, proxy_uri);
+	if (libproxy_factory)
+		status = get_proxy_for_uri_via_libproxy (uri, proxy_uri);
+	else
+		status = get_proxy_for_uri_direct (uri, proxy_uri);
 	G_UNLOCK (resolver_gnome);
 
 	return status;