On Thu, 2013-12-19 at 11:02 +0200, Tomasz Bursztyka wrote:
> >> + got_address (session, host);
> >> } else {
> >
> > Again, no extra space between function name and argument list.
> >
> > When there is one call after the if(): you don't need to put { }, by
> > the way.
> >
> > But actually you might want to get a status returned from
> > handle_result_address(),
> > or then even if it has sent back a 400 or 409 http code, do_request()
> > will still return a successful code and that's not what you want in
> > such case: do_request() should return 0 then.
>
> 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. Attached is a compile-tested patch, with this approach. Let me know what you think? I've also fixed the style comments mentioned and ran it through checkpatch.. However i did leave the proxy_host struct member, as having the address field always be the IP literal except for then it's the proxy hostname just seems confusing for no real win (I was confused by it for a while when initially reading the code). -- Sjoerd Simons <sjoerd.sim...@collabora.co.uk> Collabora Ltd.
From 2730e4447e0124361239be1bcaa937cf969da772 Mon Sep 17 00:00:00 2001
From: Sjoerd Simons <sjoerd.sim...@collabora.co.uk>
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 IP 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).
Clean up the confusion by not overloading the meaning of the address
field in web_session and skipping the DNS lookup iff either the
proxy host or the real host is an ip address (as applicable).
Signed-off-by: Sjoerd Simons <sjoerd.sim...@collabora.co.uk>
---
gweb/gweb.c | 87 +++++++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 61 insertions(+), 26 deletions(-)
diff --git a/gweb/gweb.c b/gweb/gweb.c
index 8cdd5d9..1955a85 100644
--- a/gweb/gweb.c
+++ b/gweb/gweb.c
@@ -68,6 +68,7 @@ struct web_session {
char *address;
char *host;
+ char *proxy_host;
uint16_t port;
unsigned long flags;
struct addrinfo *addr;
@@ -79,6 +80,7 @@ struct web_session {
guint send_watch;
guint resolv_action;
+ guint idle_action;
char *request;
guint8 *receive_buffer;
@@ -162,6 +164,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);
@@ -190,6 +196,7 @@ static void free_session(struct web_session *session)
g_free(session->content_type);
g_free(session->host);
+ g_free(session->proxy_host);
g_free(session->address);
if (session->addr)
freeaddrinfo(session->addr);
@@ -1190,27 +1197,20 @@ static int parse_url(struct web_session *session,
}
}
- session->address = g_strdup(host);
+ session->proxy_host = g_strdup(host);
g_free(scheme);
return 0;
}
-static void resolv_result(GResolvResultStatus status,
- char **results, gpointer user_data)
+static void got_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 +1222,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 +1237,54 @@ static void resolv_result(GResolvResultStatus status,
}
}
+static gboolean idle_got_address(gpointer data)
+{
+ struct web_session *session = data;
+
+ session->idle_action = 0;
+ got_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]);
+
+ got_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 +1300,7 @@ static guint do_request(GWeb *web, const char *url,
return 0;
}
- debug(web, "address %s", session->address);
+ debug(web, "proxy host %s", session->proxy_host);
debug(web, "port %u", session->port);
debug(web, "host %s", session->host);
debug(web, "flags %lu", session->flags);
@@ -1301,20 +1341,15 @@ 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->proxy_host ? session->proxy_host : session->host;
+ if (is_ip_address(host)) {
+ g_free(session->address);
+ session->address = g_strdup(host);
+ session->idle_action = g_idle_add(idle_got_address, 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);
- if (session->resolv_action == 0) {
+ host, resolv_result, session);
+ if (session->resolv_action <= 0) {
free_session(session);
return 0;
}
--
1.8.5.2
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
