Hi,
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).
Good catch!
And you are actually fixing another issue with is_ip_address(), see below.
So you could tell it in your commit message.
Some comments:
char *address;
char *host;
+ char *proxy_host;
No need of that extra field, keep address for proxy as usual.
uint16_t port;
unsigned long flags;
struct addrinfo *addr;
@@ -190,6 +191,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 +1192,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, const char *address)
Remove the extra space between function name and parameter list.
Change the name to: handle_result_address()
{
- 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", address);
memset(&hints, 0, sizeof(struct addrinfo));
hints.ai_flags = AI_NUMERICHOST;
@@ -1222,7 +1217,7 @@ static void resolv_result(GResolvResultStatus status,
}
port = g_strdup_printf("%u", session->port);
- ret = getaddrinfo(results[0], port, &hints, &session->addr);
+ ret = getaddrinfo(address, port, &hints, &session->addr);
g_free(port);
if (ret != 0 || !session->addr) {
call_result_func(session, 400);
@@ -1230,7 +1225,8 @@ static void resolv_result(GResolvResultStatus status,
}
g_free(session->address);
- session->address = g_strdup(results[0]);
+ session->address = g_strdup(address);
+
call_route_func(session);
if (create_transport(session) < 0) {
@@ -1239,12 +1235,42 @@ static void resolv_result(GResolvResultStatus status,
}
}
+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;
+ }
+
+ got_address (session, results[0]);
Remove the extra space between function name and argument list.
+}
+
+bool is_ip_address(const char *host)
Make this function static.
+{
+ 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 +1286,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);
The message content is more relevant here indeed. Just use
session->address as usual.
debug(web, "port %u", session->port);
debug(web, "host %s", session->host);
debug(web, "flags %lu", session->flags);
@@ -1301,19 +1327,12 @@ 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) {
So here you are fixing another issue while removing inet_aton():
this was tight to ipv4, even though gweb can specifically work against
ipv6 or unspecified family.
- 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)) {
+ 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.
- 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;
About the style issues, you can use checkpatch.pl script found in linux
source tree, in scripts/.
Just do a ./checkpatch.pl --no-tree your_patch.patch
Tomasz
_______________________________________________
connman mailing list
[email protected]
https://lists.connman.net/mailman/listinfo/connman
