GNOME.org
 

[libsoup/gnome-2-26] Fix hostname resolution behavior

commit 785dfb66d89ebeb76d65f56b2ad95f04958eccdc
Author: Dan Winship <danw gnome org>
Date:   Sat Jun 6 18:52:30 2009 -0400

    Fix hostname resolution behavior
    
    Previously we went to some effort to resolve the message URI hostname
    to an IP address before figuring out proxies/connections, but this
    turns out to be wrong for multiple reasons:
    
      1. Some hosts that send all requests via proxy don't even have a
         working DNS config.
         (http://bugzilla.gnome.org/show_bug.cgi?id=577532)
    
      2. Apparently no one expects hostnames in requests to be matched
         against IP addresses in proxy ignore lists anyway.
    
      3. The big web browsers all implement connection limits on a
         per-hostname basis, not a per-IP basis, and some web pages take
         advantage of this by using multiple aliases for the same host
         to get around the connection limit.
    
    Also update tests/proxy-test to verify that the hostname is not
    resolved when passing a request to a proxy.

 libsoup/soup-auth-manager.c  |   16 ++++----
 libsoup/soup-message-queue.c |    2 -
 libsoup/soup-message-queue.h |    5 +--
 libsoup/soup-session-async.c |   50 ----------------------------
 libsoup/soup-session-sync.c  |   12 -------
 libsoup/soup-session.c       |   61 ++++++++++------------------------
 libsoup/soup-uri.c           |   73 ++++++++++++++++++++++++++++++++++++++++++
 libsoup/soup-uri.h           |    5 +++
 tests/proxy-test.c           |    2 +-
 9 files changed, 107 insertions(+), 119 deletions(-)
---
diff --git a/libsoup/soup-auth-manager.c b/libsoup/soup-auth-manager.c
index 19c475f..c49cf3b 100644
--- a/libsoup/soup-auth-manager.c
+++ b/libsoup/soup-auth-manager.c
@@ -53,7 +53,7 @@ typedef struct {
 #define SOUP_AUTH_MANAGER_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), SOUP_TYPE_AUTH_MANAGER, SoupAuthManagerPrivate))
 
 typedef struct {
-	SoupAddress *addr;
+	SoupURI     *uri;
 	SoupPathMap *auth_realms;      /* path -> scheme:realm */
 	GHashTable  *auths;            /* scheme:realm -> SoupAuth */
 } SoupAuthHost;
@@ -64,8 +64,8 @@ soup_auth_manager_init (SoupAuthManager *manager)
 	SoupAuthManagerPrivate *priv = SOUP_AUTH_MANAGER_GET_PRIVATE (manager);
 
 	priv->auth_types = g_ptr_array_new ();
-	priv->auth_hosts = g_hash_table_new (soup_address_hash_by_name,
-					     soup_address_equal_by_name);
+	priv->auth_hosts = g_hash_table_new (soup_uri_host_hash,
+					     soup_uri_host_equal);
 }
 
 static gboolean
@@ -78,7 +78,7 @@ foreach_free_host (gpointer key, gpointer value, gpointer data)
 	if (host->auths)
 		g_hash_table_destroy (host->auths);
 
-	g_object_unref (host->addr);
+	soup_uri_free (host->uri);
 	g_slice_free (SoupAuthHost, host);
 
 	return TRUE;
@@ -318,15 +318,15 @@ static SoupAuthHost *
 get_auth_host_for_message (SoupAuthManagerPrivate *priv, SoupMessage *msg)
 {
 	SoupAuthHost *host;
-	SoupAddress *addr = soup_message_get_address (msg);
+	SoupURI *uri = soup_message_get_uri (msg);
 
-	host = g_hash_table_lookup (priv->auth_hosts, addr);
+	host = g_hash_table_lookup (priv->auth_hosts, uri);
 	if (host)
 		return host;
 
 	host = g_slice_new0 (SoupAuthHost);
-	host->addr = g_object_ref (addr);
-	g_hash_table_insert (priv->auth_hosts, host->addr, host);
+	host->uri = soup_uri_copy_host (uri);
+	g_hash_table_insert (priv->auth_hosts, host->uri, host);
 
 	return host;
 }
diff --git a/libsoup/soup-message-queue.c b/libsoup/soup-message-queue.c
index 7e73c80..8ee2fff 100644
--- a/libsoup/soup-message-queue.c
+++ b/libsoup/soup-message-queue.c
@@ -147,8 +147,6 @@ soup_message_queue_item_unref (SoupMessageQueueItem *item)
 	/* And free it */
 	g_object_unref (item->msg);
 	g_object_unref (item->cancellable);
-	if (item->msg_addr)
-		g_object_unref (item->msg_addr);
 	if (item->proxy_addr)
 		g_object_unref (item->proxy_addr);
 	g_slice_free (SoupMessageQueueItem, item);
diff --git a/libsoup/soup-message-queue.h b/libsoup/soup-message-queue.h
index 4a9ae8f..1618f47 100644
--- a/libsoup/soup-message-queue.h
+++ b/libsoup/soup-message-queue.h
@@ -26,15 +26,14 @@ struct SoupMessageQueueItem {
 	gpointer callback_data;
 
 	GCancellable *cancellable;
-	SoupAddress *msg_addr, *proxy_addr;
+	SoupAddress *proxy_addr;
 
-	guint resolving_msg_addr   : 1;
 	guint resolving_proxy_addr : 1;
 	guint resolved_proxy_addr  : 1;
 
 	/*< private >*/
 	guint removed              : 1;
-	guint ref_count            : 28;
+	guint ref_count            : 29;
 	SoupMessageQueueItem *prev, *next;
 };
 
diff --git a/libsoup/soup-session-async.c b/libsoup/soup-session-async.c
index a492ee5..f6d7a2c 100644
--- a/libsoup/soup-session-async.c
+++ b/libsoup/soup-session-async.c
@@ -110,47 +110,6 @@ soup_session_async_new_with_options (const char *optname1, ...)
 
 
 static void
-resolved_msg_addr (SoupAddress *addr, guint status, gpointer user_data)
-{
-	SoupMessageQueueItem *item = user_data;
-	SoupSession *session = item->session;
-
-	if (item->removed) {
-		/* Message was cancelled before its address resolved */
-		soup_message_queue_item_unref (item);
-		return;
-	}
-
-	if (!SOUP_STATUS_IS_SUCCESSFUL (status)) {
-		soup_session_cancel_message (session, item->msg, status);
-		soup_message_queue_item_unref (item);
-		return;
-	}
-
-	item->msg_addr = g_object_ref (addr);
-	item->resolving_msg_addr = FALSE;
-
-	soup_message_queue_item_unref (item);
-
-	/* If we got here we know session still exists */
-	run_queue ((SoupSessionAsync *)session);
-}
-
-static void
-resolve_msg_addr (SoupMessageQueueItem *item)
-{
-	if (item->resolving_msg_addr)
-		return;
-	item->resolving_msg_addr = TRUE;
-
-	soup_message_queue_item_ref (item);
-	soup_address_resolve_async (soup_message_get_address (item->msg),
-				    soup_session_get_async_context (item->session),
-				    item->cancellable,
-				    resolved_msg_addr, item);
-}
-
-static void
 resolved_proxy_addr (SoupProxyResolver *proxy_resolver, SoupMessage *msg,
 		     guint status, SoupAddress *proxy_addr, gpointer user_data)
 {
@@ -257,10 +216,6 @@ run_queue (SoupSessionAsync *sa)
 		    soup_message_io_in_progress (msg))
 			continue;
 
-		if (!item->msg_addr) {
-			resolve_msg_addr (item);
-			continue;
-		}
 		if (proxy_resolver && !item->resolved_proxy_addr) {
 			resolve_proxy_addr (item, proxy_resolver);
 			continue;
@@ -303,11 +258,6 @@ request_restarted (SoupMessage *req, gpointer user_data)
 {
 	SoupMessageQueueItem *item = user_data;
 
-	if (item->msg_addr &&
-	    item->msg_addr != soup_message_get_address (item->msg)) {
-		g_object_unref (item->msg_addr);
-		item->msg_addr = NULL;
-	}
 	if (item->proxy_addr) {
 		g_object_unref (item->proxy_addr);
 		item->proxy_addr = NULL;
diff --git a/libsoup/soup-session-sync.c b/libsoup/soup-session-sync.c
index bb8e336..624bfef 100644
--- a/libsoup/soup-session-sync.c
+++ b/libsoup/soup-session-sync.c
@@ -202,20 +202,8 @@ process_queue_item (SoupMessageQueueItem *item)
 	SoupSessionSyncPrivate *priv = SOUP_SESSION_SYNC_GET_PRIVATE (item->session);
 	SoupMessage *msg = item->msg;
 	SoupConnection *conn;
-	SoupAddress *addr;
-	guint status;
 
 	do {
-		/* Resolve address */
-		addr = soup_message_get_address (msg);
-		status = soup_address_resolve_sync (addr, item->cancellable);
-		if (!SOUP_STATUS_IS_SUCCESSFUL (status)) {
-			if (status != SOUP_STATUS_CANCELLED)
-				soup_session_cancel_message (item->session, msg, status);
-			break;
-		}
-
-		/* Get a connection */
 		conn = wait_for_connection (item->session, msg);
 		if (!conn)
 			break;
diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index cbaa42d..1bfd7a2 100644
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -22,6 +22,7 @@
 #include "soup-marshal.h"
 #include "soup-message-private.h"
 #include "soup-message-queue.h"
+#include "soup-misc.h"
 #include "soup-proxy-resolver-static.h"
 #include "soup-session.h"
 #include "soup-session-feature.h"
@@ -57,6 +58,7 @@
  **/
 
 typedef struct {
+	SoupURI *uri;
 	SoupAddress *addr;
 
 	GSList      *connections;      /* CONTAINS: SoupConnection */
@@ -76,7 +78,7 @@ typedef struct {
 	GSList *features;
 	SoupAuthManager *auth_manager;
 
-	GHashTable *hosts; /* SoupAddress -> SoupSessionHost */
+	GHashTable *hosts; /* char* -> SoupSessionHost */
 	GHashTable *conns; /* SoupConnection -> SoupSessionHost */
 	guint num_conns;
 	guint max_conns, max_conns_per_host;
@@ -153,8 +155,9 @@ soup_session_init (SoupSession *session)
 	priv->queue = soup_message_queue_new (session);
 
 	priv->host_lock = g_mutex_new ();
-	priv->hosts = g_hash_table_new (soup_address_hash_by_ip,
-					soup_address_equal_by_ip);
+	priv->hosts = g_hash_table_new_full (soup_uri_host_hash,
+					     soup_uri_host_equal,
+					     NULL, (GDestroyNotify)free_host);
 	priv->conns = g_hash_table_new (NULL, NULL);
 
 	priv->max_conns = SOUP_SESSION_MAX_CONNS_DEFAULT;
@@ -170,28 +173,6 @@ soup_session_init (SoupSession *session)
 	soup_session_add_feature (session, SOUP_SESSION_FEATURE (priv->auth_manager));
 }
 
-static gboolean
-foreach_free_host (gpointer key, gpointer host, gpointer data)
-{
-	free_host (host);
-	return TRUE;
-}
-
-static void
-cleanup_hosts (SoupSessionPrivate *priv)
-{
-	GHashTable *old_hosts;
-
-	g_mutex_lock (priv->host_lock);
-	old_hosts = priv->hosts;
-	priv->hosts = g_hash_table_new (soup_address_hash_by_ip,
-					soup_address_equal_by_ip);
-	g_mutex_unlock (priv->host_lock);
-
-	g_hash_table_foreach_remove (old_hosts, foreach_free_host, NULL);
-	g_hash_table_destroy (old_hosts);
-}
-
 static void
 dispose (GObject *object)
 {
@@ -199,7 +180,6 @@ dispose (GObject *object)
 	SoupSessionPrivate *priv = SOUP_SESSION_GET_PRIVATE (session);
 
 	soup_session_abort (session);
-	cleanup_hosts (priv);
 
 	while (priv->features)
 		soup_session_remove_feature (session, priv->features->data);
@@ -650,7 +630,6 @@ set_property (GObject *object, guint prop_id,
 			soup_session_remove_feature_by_type (session, SOUP_TYPE_PROXY_RESOLVER);
 
 		soup_session_abort (session);
-		cleanup_hosts (priv);
 		break;
 	case PROP_MAX_CONNS:
 		priv->max_conns = g_value_get_int (value);
@@ -672,13 +651,9 @@ set_property (GObject *object, guint prop_id,
 		g_free (priv->ssl_ca_file);
 		priv->ssl_ca_file = g_strdup (new_ca_file);
 
-		if (ca_file_changed) {
-			if (priv->ssl_creds) {
-				soup_ssl_free_client_credentials (priv->ssl_creds);
-				priv->ssl_creds = NULL;
-			}
-
-			cleanup_hosts (priv);
+		if (ca_file_changed && priv->ssl_creds) {
+			soup_ssl_free_client_credentials (priv->ssl_creds);
+			priv->ssl_creds = NULL;
 		}
 
 		break;
@@ -796,12 +771,14 @@ soup_session_get_async_context (SoupSession *session)
 /* Hosts */
 
 static SoupSessionHost *
-soup_session_host_new (SoupSession *session, SoupAddress *addr)
+soup_session_host_new (SoupSession *session, SoupURI *uri)
 {
 	SoupSessionHost *host;
 
 	host = g_slice_new0 (SoupSessionHost);
-	host->addr = g_object_ref (addr);
+	host->uri = soup_uri_copy_host (uri);
+	host->addr = soup_address_new (host->uri->host, host->uri->port);
+
 	return host;
 }
 
@@ -814,17 +791,14 @@ get_host_for_message (SoupSession *session, SoupMessage *msg)
 {
 	SoupSessionPrivate *priv = SOUP_SESSION_GET_PRIVATE (session);
 	SoupSessionHost *host;
-	SoupAddress *addr = soup_message_get_address (msg);
-
-	if (!soup_address_is_resolved (addr))
-		return NULL;
+	SoupURI *uri = soup_message_get_uri (msg);
 
-	host = g_hash_table_lookup (priv->hosts, addr);
+	host = g_hash_table_lookup (priv->hosts, uri);
 	if (host)
 		return host;
 
-	host = soup_session_host_new (session, addr);
-	g_hash_table_insert (priv->hosts, host->addr, host);
+	host = soup_session_host_new (session, uri);
+	g_hash_table_insert (priv->hosts, host->uri, host);
 
 	return host;
 }
@@ -839,6 +813,7 @@ free_host (SoupSessionHost *host)
 		soup_connection_disconnect (conn);
 	}
 
+	soup_uri_free (host->uri);
 	g_object_unref (host->addr);
 	g_slice_free (SoupSessionHost, host);
 }	
diff --git a/libsoup/soup-uri.c b/libsoup/soup-uri.c
index 2743576..819e5cc 100644
--- a/libsoup/soup-uri.c
+++ b/libsoup/soup-uri.c
@@ -921,6 +921,79 @@ soup_uri_set_fragment (SoupURI *uri, const char *fragment)
 	uri->fragment = g_strdup (fragment);
 }
 
+/**
+ * soup_uri_copy_host:
+ * @uri: a #SoupUri
+ *
+ * Makes a copy of @uri, considering only the protocol, host, and port
+ *
+ * Return value: the new #SoupUri
+ *
+ * Since: 2.26.3
+ **/
+SoupURI *
+soup_uri_copy_host (SoupURI *uri)
+{
+	SoupURI *dup;
+
+	g_return_val_if_fail (uri != NULL, NULL);
+
+	dup = soup_uri_new (NULL);
+	dup->scheme = uri->scheme;
+	dup->host   = g_strdup (uri->host);
+	dup->port   = uri->port;
+	if (dup->scheme == SOUP_URI_SCHEME_HTTP ||
+	    dup->scheme == SOUP_URI_SCHEME_HTTPS)
+		dup->path = g_strdup ("");
+
+	return dup;
+}
+
+/**
+ * soup_uri_host_hash:
+ * @key: a #SoupURI
+ *
+ * Hashes @key, considering only the scheme, host, and port.
+ *
+ * Return value: a hash
+ *
+ * Since: 2.26.3
+ **/
+guint
+soup_uri_host_hash (gconstpointer key)
+{
+	const SoupURI *uri = key;
+
+	return GPOINTER_TO_UINT (uri->scheme) + uri->port +
+		soup_str_case_hash (uri->host);
+}
+
+/**
+ * soup_uri_host_equal:
+ * @v1: a #SoupURI
+ * @v2: a #SoupURI
+ *
+ * Compares @v1 and @v2, considering only the scheme, host, and port.
+ *
+ * Return value: whether or not the URIs are equal in scheme, host,
+ * and port.
+ *
+ * Since: 2.26.3
+ **/
+gboolean
+soup_uri_host_equal (gconstpointer v1, gconstpointer v2)
+{
+	const SoupURI *one = v1;
+	const SoupURI *two = v2;
+
+	if (one->scheme != two->scheme)
+		return FALSE;
+	if (one->port != two->port)
+		return FALSE;
+
+	return g_ascii_strcasecmp (one->host, two->host) == 0;
+}
+
 
 GType
 soup_uri_get_type (void)
diff --git a/libsoup/soup-uri.h b/libsoup/soup-uri.h
index 9a7eef1..32de7a3 100644
--- a/libsoup/soup-uri.h
+++ b/libsoup/soup-uri.h
@@ -78,6 +78,11 @@ void      soup_uri_set_query_from_fields (SoupURI    *uri,
 void      soup_uri_set_fragment          (SoupURI    *uri,
 					  const char *fragment);
 
+SoupURI  *soup_uri_copy_host             (SoupURI    *uri);
+guint     soup_uri_host_hash             (gconstpointer key);
+gboolean  soup_uri_host_equal            (gconstpointer v1,
+					  gconstpointer v2);
+
 #define   SOUP_URI_VALID_FOR_HTTP(uri) ((uri) && ((uri)->scheme == SOUP_URI_SCHEME_HTTP || (uri)->scheme == SOUP_URI_SCHEME_HTTPS) && (uri)->host)
 
 G_END_DECLS
diff --git a/tests/proxy-test.c b/tests/proxy-test.c
index ebdd2cc..8f57c1f 100644
--- a/tests/proxy-test.c
+++ b/tests/proxy-test.c
@@ -21,7 +21,7 @@ static SoupProxyTest tests[] = {
 	{ "GET -> 404", "/not-found", SOUP_STATUS_NOT_FOUND },
 	{ "GET -> 401 -> 200", "/Basic/realm1/", SOUP_STATUS_OK },
 	{ "GET -> 401 -> 401", "/Basic/realm2/", SOUP_STATUS_UNAUTHORIZED },
-	{ "GET -> 403", "http://www.example.com/";, SOUP_STATUS_FORBIDDEN },
+	{ "GET -> 403", "http://no-such-hostname.xx/";, SOUP_STATUS_FORBIDDEN },
 };
 static int ntests = sizeof (tests) / sizeof (tests[0]);
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]