On Thu, 2014-01-09 at 14:25 +0200, Tomasz Bursztyka wrote:
> Hi,
> 
> >> There is finally a bigger issue here: in case handle_result_address()
> >> >(aka you got_address() function) is called at this place,
> >> >it should not call call_result_func() at all.
> >> >
> >> >You might handle this case via an extra parameter to
> >> >handle_result_address()  like bool from_resolv  (or a more relevant name
> >> >if you have one)
> > I've solved this slightly differently by always bouncing through back
> > through the mainloop such that the callback is always called after
> > do_request has returned. This makes the behaviour, form the API users
> > perspective, the same for both cases.
> 
> Makes sense, it's a much better approach indeed.

Attached patch should address your review comments. This time round i've
also actually gave it a testrun with proxies setup both by ip literal
and host name.

-- 
Sjoerd Simons <[email protected]>
Collabora Ltd.
From 0170b4e08b769b53ee1e82f60dcb8cbb8e98892b Mon Sep 17 00:00:00 2001
From: Sjoerd Simons <[email protected]>
Date: Mon, 6 Jan 2014 13:51:06 +0100
Subject: [PATCH] gweb: Handle proxies as addresses and hostnames

It seems the proxy handling code was initially written to only handle
proxies in the form of IPv4 addresses. 38b75abddb5b changed that
implicitly by always doing a hostname lookup for the proxy address,
which fixes proxies given by hostname but breaks ip based proxy
configuration (as sending an A query to most DNS server for an ip
address gets you a result with no answers).

Fix this issue by short-circuiting the resolving step in case the proxy
address is in the form of either an IPv4 or an IPv6 ip literal.

Signed-off-by: Sjoerd Simons <[email protected]>
---
 gweb/gweb.c | 82 ++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 59 insertions(+), 23 deletions(-)

diff --git a/gweb/gweb.c b/gweb/gweb.c
index 8cdd5d9..d9316c3 100644
--- a/gweb/gweb.c
+++ b/gweb/gweb.c
@@ -79,6 +79,7 @@ struct web_session {
 	guint send_watch;
 
 	guint resolv_action;
+	guint idle_action;
 	char *request;
 
 	guint8 *receive_buffer;
@@ -162,6 +163,10 @@ static void free_session(struct web_session *session)
 	g_free(session->request);
 
 	web = session->web;
+
+	if (session->idle_action > 0)
+		g_source_remove(session->idle_action);
+
 	if (session->resolv_action > 0)
 		g_resolv_cancel_lookup(web->resolv, session->resolv_action);
 
@@ -1197,20 +1202,13 @@ static int parse_url(struct web_session *session,
 	return 0;
 }
 
-static void resolv_result(GResolvResultStatus status,
-					char **results, gpointer user_data)
+static void handle_resolved_address(struct web_session *session)
 {
-	struct web_session *session = user_data;
 	struct addrinfo hints;
 	char *port;
 	int ret;
 
-	if (!results || !results[0]) {
-		call_result_func(session, 404);
-		return;
-	}
-
-	debug(session->web, "address %s", results[0]);
+	debug(session->web, "address %s", session->address);
 
 	memset(&hints, 0, sizeof(struct addrinfo));
 	hints.ai_flags = AI_NUMERICHOST;
@@ -1222,15 +1220,13 @@ static void resolv_result(GResolvResultStatus status,
 	}
 
 	port = g_strdup_printf("%u", session->port);
-	ret = getaddrinfo(results[0], port, &hints, &session->addr);
+	ret = getaddrinfo(session->address, port, &hints, &session->addr);
 	g_free(port);
 	if (ret != 0 || !session->addr) {
 		call_result_func(session, 400);
 		return;
 	}
 
-	g_free(session->address);
-	session->address = g_strdup(results[0]);
 	call_route_func(session);
 
 	if (create_transport(session) < 0) {
@@ -1239,12 +1235,54 @@ static void resolv_result(GResolvResultStatus status,
 	}
 }
 
+static gboolean already_resolved(gpointer data)
+{
+	struct web_session *session = data;
+
+	session->idle_action = 0;
+	handle_resolved_address(session);
+	return FALSE;
+}
+
+static void resolv_result(GResolvResultStatus status,
+					char **results, gpointer user_data)
+{
+	struct web_session *session = user_data;
+
+	if (!results || !results[0]) {
+		call_result_func(session, 404);
+		return;
+	}
+
+	g_free(session->address);
+	session->address = g_strdup(results[0]);
+
+	handle_resolved_address(session);
+}
+
+static bool is_ip_address(const char *host)
+{
+	struct addrinfo hints;
+	struct addrinfo *addr;
+	int result;
+
+	memset(&hints, 0, sizeof(struct addrinfo));
+	hints.ai_flags = AI_NUMERICHOST;
+	addr = NULL;
+
+	result = getaddrinfo(host, NULL, &hints, &addr);
+	freeaddrinfo(addr);
+
+	return result == 0;
+}
+
 static guint do_request(GWeb *web, const char *url,
 				const char *type, GWebInputFunc input,
 				int fd, gsize length, GWebResultFunc func,
 				GWebRouteFunc route, gpointer user_data)
 {
 	struct web_session *session;
+	const gchar *host;
 
 	if (!web || !url)
 		return 0;
@@ -1260,7 +1298,7 @@ static guint do_request(GWeb *web, const char *url,
 		return 0;
 	}
 
-	debug(web, "address %s", session->address);
+	debug(web, "proxy host %s", session->address);
 	debug(web, "port %u", session->port);
 	debug(web, "host %s", session->host);
 	debug(web, "flags %lu", session->flags);
@@ -1301,19 +1339,17 @@ static guint do_request(GWeb *web, const char *url,
 	session->header_done = false;
 	session->body_done = false;
 
-	if (!session->address && inet_aton(session->host, NULL) == 0) {
-		session->resolv_action = g_resolv_lookup_hostname(web->resolv,
-					session->host, resolv_result, session);
-		if (session->resolv_action == 0) {
-			free_session(session);
-			return 0;
+	host = session->address ? session->address : session->host;
+	if (is_ip_address(host)) {
+		if (session->address != host) {
+			g_free(session->address);
+			session->address = g_strdup(host);
 		}
+		session->idle_action = g_timeout_add(0, already_resolved,
+							session);
 	} else {
-		if (!session->address)
-			session->address = g_strdup(session->host);
-
 		session->resolv_action = g_resolv_lookup_hostname(web->resolv,
-					session->address, resolv_result, session);
+					host, resolv_result, session);
 		if (session->resolv_action == 0) {
 			free_session(session);
 			return 0;
-- 
1.8.5.2

Attachment: smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________
connman mailing list
[email protected]
https://lists.connman.net/mailman/listinfo/connman

Reply via email to