From dee7dc77b3ec662f60044f25abe6b5193b87f086 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 6 Jun 2009 18:52:30 -0400 Subject: 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 +++ 8 files changed, 106 insertions(+), 118 deletions(-) (limited to 'libsoup') diff --git a/libsoup/soup-auth-manager.c b/libsoup/soup-auth-manager.c index 19c475f5..c49cf3b6 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 7e73c807..8ee2fff8 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 4a9ae8f0..1618f47e 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 a492ee54..f6d7a2c3 100644 --- a/libsoup/soup-session-async.c +++ b/libsoup/soup-session-async.c @@ -109,47 +109,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 bb8e336b..624bfefd 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 cbaa42d6..1bfd7a2a 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 2743576c..819e5cce 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 9a7eef1c..32de7a3b 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 -- cgit v1.2.1