Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Re: [PATCH] [Pacrunner]: Domains are looked up to match host
of URL. (Patrik Flykt)
----------------------------------------------------------------------
Message: 1
Date: Tue, 14 Jun 2016 16:28:44 +0300
From: Patrik Flykt <[email protected]>
To: Atul Anand <[email protected]>, [email protected]
Subject: Re: [PATCH] [Pacrunner]: Domains are looked up to match host
of URL.
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
Hi,
Please split this patch into three, documentation, implementation and
testing.
On Mon, 2016-06-13 at 16:48 +0530, Atul Anand wrote:
> Pacrunner now scan stored domains to match the host of URL.
> In this way the most appropriate proxy config is selected
> to answer the proxy query.
> ---
> ?doc/manager-api.txt????????????|???3 +-
> ?src/manager.c??????????????????|???7 +-
> ?src/pacrunner.h????????????????|???2 +
> ?src/proxy.c????????????????????| 207
> +++++++++++++++++++++++++++++++++++++++--
> ?unit/suite/manual_basic.test???|???2 +
> ?unit/suite/manual_exclude.test |???2 +
> ?unit/suite/pac_basic.test??????|???2 +
> ?unit/suite/pac_direct.test?????|???2 +
> ?unit/suite/proxy_domain.test???|??37 ++++++++
> ?unit/suite/stub.test???????????|???3 +
> ?unit/test-pacrunner.c??????????|??86 ++++++++++++++++-
> ?11 files changed, 336 insertions(+), 17 deletions(-)
> ?create mode 100644 unit/suite/proxy_domain.test
>
> diff --git a/doc/manager-api.txt b/doc/manager-api.txt
> index ab2f6b9..622fffa 100644
> --- a/doc/manager-api.txt
> +++ b/doc/manager-api.txt
> @@ -62,7 +62,8 @@ Methods object CreateProxyConfiguration(dict
> settings)
> ?
> ? array{string} Domains [optional]
> ?
> - Domain names for which the URL is valid.
> + Domain names and IP ranges for which this proxy
> + configuration shall be valid.
> ?
> ? array{string} Nameservers [optional]
The above should be the first patch updating documentation. IP range
formats should also be documented so that one does not have to guess
what the string should look like.
?
> diff --git a/src/manager.c b/src/manager.c
> index 1676466..5a8b4fd 100644
> --- a/src/manager.c
> +++ b/src/manager.c
> @@ -35,7 +35,6 @@ struct proxy_config {
> ? DBusConnection *conn;
> ? guint watch;
> ?
> - char **domains;
> ? char **nameservers;
> ?
> ? struct pacrunner_proxy *proxy;
> @@ -58,7 +57,6 @@ static void destroy_config(gpointer data)
> ? if (config->watch > 0)
> ? g_dbus_remove_watch(config->conn, config->watch);
> ?
> - g_strfreev(config->domains);
> ? g_strfreev(config->nameservers);
> ?
> ? g_free(config->sender);
> @@ -224,12 +222,13 @@ static DBusMessage *create_proxy_config(DBusConnection
> *conn,
> ? goto done;
> ? }
> ?
> - config->domains = domains;
> ? config->nameservers = nameservers;
> ?
> - domains = NULL;
> ? nameservers = NULL;
> ?
> + if (pacrunner_proxy_set_domains(config->proxy, domains) < 0)
> + pacrunner_error("Failed to set proxy domains");
> +
> ? if (g_str_equal(method, "direct")) {
> ? if (pacrunner_proxy_set_direct(config->proxy) < 0)
> ? pacrunner_error("Failed to set direct proxy");
> diff --git a/src/pacrunner.h b/src/pacrunner.h
> index 6731d7c..db534cf 100644
> --- a/src/pacrunner.h
> +++ b/src/pacrunner.h
> @@ -63,6 +63,8 @@ void pacrunner_proxy_unref(struct pacrunner_proxy *proxy);
> ?const char *pacrunner_proxy_get_interface(struct pacrunner_proxy *proxy);
> ?const char *pacrunner_proxy_get_script(struct pacrunner_proxy *proxy);
> ?
> +int pacrunner_proxy_set_domains(struct pacrunner_proxy *proxy,
> + char **domains);
> ?int pacrunner_proxy_set_direct(struct pacrunner_proxy *proxy);
> ?int pacrunner_proxy_set_manual(struct pacrunner_proxy *proxy,
> ? char **servers, char **excludes);
> diff --git a/src/proxy.c b/src/proxy.c
> index 8bb03af..2aeae0b 100644
> --- a/src/proxy.c
> +++ b/src/proxy.c
> @@ -23,6 +23,9 @@
> ?#include
> ?#endif
> ?
> +#include
> +#include
> +#include
> ?#include
> ?#include
> ?
> @@ -37,6 +40,17 @@ struct pacrunner_proxy {
> ? char *script;
> ? GList **servers;
> ? GList **excludes;
> + GList *domains;
> +};
> +
> +struct proxy_domain {
> + char *domain;
> + int proto;
> + union {
> + struct in_addr ip4;
> + struct in6_addr ip6;
> + } addr;
> + int mask;
> ?};
> ?
> ?static GList *proxy_list = NULL;
> @@ -77,6 +91,14 @@ struct pacrunner_proxy *pacrunner_proxy_ref(struct
> pacrunner_proxy *proxy)
> ? return proxy;
> ?}
> ?
> +static void proxy_domain_destroy(struct proxy_domain *domain)
> +{
> + g_return_if_fail(domain != NULL);
> +
> + g_free(domain->domain);
> + g_free(domain);
> +}
> +
> ?static void reset_proxy(struct pacrunner_proxy *proxy)
> ?{
> ? DBG("proxy %p", proxy);
> @@ -92,6 +114,12 @@ static void reset_proxy(struct pacrunner_proxy *proxy)
> ?
> ? __pacrunner_manual_destroy_excludes(proxy->excludes);
> ? proxy->excludes = NULL;
> +
> + if (proxy->domains) {
> + g_list_free_full(proxy->domains,
> + (GDestroyNotify) proxy_domain_destroy);
Please don't cast here, if needed do necessary casts in
proxy_domain_destroy() instead.
> + }
> + proxy->domains = NULL;
> ?}
> ?
> ?void pacrunner_proxy_unref(struct pacrunner_proxy *proxy)
> @@ -130,6 +158,69 @@ const char *pacrunner_proxy_get_script(struct
> pacrunner_proxy *proxy)
> ? return proxy->script;
> ?}
> ?
> +int pacrunner_proxy_set_domains(struct pacrunner_proxy *proxy, char
> **domains)
> +{
> + int len;
> + char *slash, **domain;
> + char ip[INET6_ADDRSTRLEN + 1];
> +
> + DBG("proxy %p domains %p", proxy, domains);
> +
> + if (!proxy)
> + return -EINVAL;
> +
> + if (!domains)
> + return -EINVAL;
> +
> + for (domain = (char **)domains; *domain; domain++) {
The cast to (char **) does not seem necessary here?
> + struct proxy_domain *data;
> +
> + data = g_malloc0(sizeof(struct proxy_domain));
> + g_return_val_if_fail(data != NULL, -EINVAL);
> +
> + slash = strchr(*domain, '/');
> + if (!slash) {
> + data->domain = g_strdup(*domain);
> + data->proto = 0;
> +
> + proxy->domains = g_list_append(proxy->domains, data);
> + continue;
> + }
> +
> + len = slash - *domain;
> + if (len > INET6_ADDRSTRLEN)
> + return -EINVAL;
> +
> + strncpy(ip, *domain, len);
> + ip[len] = '\0';
> +
> + if (inet_pton(AF_INET, ip, &(data->addr.ip4)) == 1) {
> + data->domain = NULL;
> + data->proto = 4;
> +
> + errno = 0;
> + data->mask = strtol(slash + 1, NULL, 10);
Hmm, it would look a bit cleaner if the return value for strtol() were
examined for INT_MIN and 32 with errno returned in that case.
> + if (errno || data->mask > 32)
> + return -EINVAL;
If this return is taken, 'data' leaks memory? It's probably easier with
a goto error; to the very end of this function?
> +
> + proxy->domains = g_list_append(proxy->domains, data);
> + } else if (inet_pton(AF_INET6, ip, &(data->addr.ip6)) == 1) {
> + data->domain = NULL;
> + data->proto = 6;
> +
> + errno = 0;
Same as above.
> + data->mask = strtol(slash + 1, NULL, 10);
> + if (errno || data->mask > 128)
> + return -EINVAL;
> +
> + proxy->domains = g_list_append(proxy->domains, data);
> + } else
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> ?static int set_method(struct pacrunner_proxy *proxy,
> ? enum pacrunner_proxy_method method)
> ?{
> @@ -324,10 +415,60 @@ int pacrunner_proxy_disable(struct pacrunner_proxy
> *proxy)
> ? return 0;
> ?}
> ?
> +static int compare_legacy_ip_in_net(struct in_addr *host,
> + struct proxy_domain *match)
> +{
> + if (ntohl(host->s_addr ^ match->addr.ip4.s_addr) >> (32 - match->mask))
> + return -1;
> +
> + return 0;
> +}
> +
> +static int compare_ipv6_in_net(struct in6_addr *host,
> + struct proxy_domain *match)
> +{
> + int i, shift;
> +
> + for (i = 0; i < (match->mask)/8; i++) {
> + if (host->s6_addr[i] != match->addr.ip6.s6_addr[i])
> + return -1;
> + }
> +
> + if ((match->mask) % 8) {
> + /**
> + ?* If mask bits are not multiple of 8 , 1-7 bits are left
> + ?* to be compared.
> + ?*/
> + shift = 8 - (match->mask - (i*8));
> +
> + if ((host->s6_addr[i] >> shift) !=
> + (match->addr.ip6.s6_addr[i] >> shift))
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int compare_host_in_domain(const char *host, struct proxy_domain
> *match)
> +{
> + if (g_str_has_suffix(host, match->domain)) {
g_str_has_suffix() appears starting from glib 2.2. Currently only 2.16
is required, so either do a string comparison for the last bytes or
update glib requirement to >= 2.2. It is possible some embedded
developers might still be upset with the latter, though.
> + size_t hlen = strlen(host);
> + size_t dlen = strlen(match->domain);
> +
> + if (hlen == dlen || host[hlen - dlen - 1] == '.')
Can the following happen: hlen("foo"), dlen("myverylongdomain") ->
host[3 - 16 - 1] == host[-14] which is indexed outside the array?
> + return 0;
> + }
> +
> + return -1;
> +}
> +
> ?char *pacrunner_proxy_lookup(const char *url, const char *host)
> ?{
> - GList *list;
> - struct pacrunner_proxy *selected_proxy = NULL;
> + GList *l, *list;
> + struct in_addr ip4_addr;
> + struct in6_addr ip6_addr;
> + struct pacrunner_proxy *selected_proxy = NULL, *default_proxy = NULL;
> + int protocol = 0;
> ?
> ? DBG("url %s host %s", url, host);
> ?
> @@ -340,17 +481,67 @@ char *pacrunner_proxy_lookup(const char *url, const
> char *host)
> ? return NULL;
> ? }
> ?
> + if (inet_pton(AF_INET, host, &ip4_addr) == 1) {
> + protocol = 4;
> + } else if (inet_pton(AF_INET6, host, &ip6_addr) == 1) {
> + protocol = 6;
> + } else if (host[0] == '[') {
proxy_set_domains() above seems not to be able to handle an IPv6
address written as [2002:dead:beef::1]. Should the host and the domains
allow for the same formatting? AF_INET and AF_INET6 may look nicer to
some eyes (but that may introduce unnecessary glitches in the code, so
it can stay as is).
> + char ip[INET6_ADDRSTRLEN + 1];
> + int len = strlen(host);
> +
> + if (len < INET6_ADDRSTRLEN + 2 && host[len - 1] == ']') {
> + strncpy(ip, host + 1, len - 2);
> + ip[len - 2] = '\0';
> +
> + if (inet_pton(AF_INET6, ip, &ip6_addr) == 1)
> + protocol = 6;
> + }
> + }
> +
> ? for (list = g_list_first(proxy_list); list; list = g_list_next(list)) {
> ? struct pacrunner_proxy *proxy = list->data;
> ?
> - if (proxy->method == PACRUNNER_PROXY_METHOD_MANUAL ||
> - proxy->method == PACRUNNER_PROXY_METHOD_AUTO) {
> - selected_proxy = proxy;
> - break;
> - } else if (proxy->method == PACRUNNER_PROXY_METHOD_DIRECT)
> - selected_proxy = proxy;
> + if (!proxy->domains) {
> + if (!default_proxy)
> + default_proxy = proxy;
> + continue;
> + }
> +
> + for (l = g_list_first(proxy->domains); l; l = g_list_next(l)) {
> + struct proxy_domain *data = l->data;
> +
> + if (data->proto != protocol)
> + continue;
> +
> + switch (protocol) {
> + case 4:
> + if (compare_legacy_ip_in_net(&ip4_addr,
> + data) == 0) {
> + selected_proxy = proxy;
> + goto found;
> + }
> + break;
> + case 6:
> + if (compare_ipv6_in_net(&ip6_addr,
> + data) == 0) {
> + selected_proxy = proxy;
> + goto found;
> + }
> + break;
> + default:
> + if (compare_host_in_domain(host, data) == 0) {
> + selected_proxy = proxy;
> + goto found;
> + }
> + break;
> + }
> + }
> ? }
> ?
> + if (!selected_proxy)
> + selected_proxy = default_proxy;
> +
> +found:
> ? pthread_mutex_unlock(&proxy_mutex);
> ?
> ? if (!selected_proxy)
The above part should be patch #2, with the below patch #3. Good to see
there are test cases added for this!
> diff --git a/unit/suite/manual_basic.test b/unit/suite/manual_basic.test
> index a5ec3a1..4406d9c 100644
> --- a/unit/suite/manual_basic.test
> +++ b/unit/suite/manual_basic.test
> @@ -10,6 +10,8 @@ socks4://sockproxy.internal.com
> ?
> ?[excludes]
> ?
> +[domains]
> +
> ?[config]
> ?VALID
> ?
> diff --git a/unit/suite/manual_exclude.test b/unit/suite/manual_exclude.test
> index c155743..211ae16 100644
> --- a/unit/suite/manual_exclude.test
> +++ b/unit/suite/manual_exclude.test
> @@ -15,6 +15,8 @@ ftp://
> ?*net
> ?tri*
> ?
> +[domains]
> +
> ?[config]
> ?VALID
> ?
> diff --git a/unit/suite/pac_basic.test b/unit/suite/pac_basic.test
> index 58af200..c63757e 100644
> --- a/unit/suite/pac_basic.test
> +++ b/unit/suite/pac_basic.test
> @@ -17,6 +17,8 @@ function FindProxyForURL(url, host)
> ?
> ?[excludes]
> ?
> +[domains]
> +
> ?[config]
> ?VALID
> ?
> diff --git a/unit/suite/pac_direct.test b/unit/suite/pac_direct.test
> index 3164872..b820abc 100644
> --- a/unit/suite/pac_direct.test
> +++ b/unit/suite/pac_direct.test
> @@ -11,6 +11,8 @@ function FindProxyForURL(url, host)
> ?
> ?[excludes]
> ?
> +[domains]
> +
> ?[config]
> ?VALID
> ?
> diff --git a/unit/suite/proxy_domain.test b/unit/suite/proxy_domain.test
> new file mode 100644
> index 0000000..8c2c5e4
> --- /dev/null
> +++ b/unit/suite/proxy_domain.test
> @@ -0,0 +1,37 @@
> +[title]
> +Proxy Domain lookup
> +
> +[pac]
> +
> +[servers]
> +http://proxy.suite.com
> +
> +[excludes]
> +
> +[domains]
> +suite.com
> +test.suite.com
> +172.132.231.6/24
> +
> +[config]
> +VALID
> +
> +[tests]
> +http://foo.suite.com foo.suite.com
> +PROXY proxy.suite.com
> +http://172.132.231.101/search=?true 172.132.231.101
> +PROXY proxy.suite.com
> +http://111.121.131.141/page1 111.121.131.141
> +DIRECT
> +http://notintel.com notintel.com
> +DIRECT
> +http://intel.com intel.com
> +PROXY proxy2.com; PROXY secproxy2.com
> +https://bar.domain2.com bar.domain2.com
> +PROXY secproxy2.com; PROXY proxy2.com
> +http://192.168.4.4/index.html 192.168.4.4
> +PROXY proxy2.com; PROXY secproxy2.com
> +socks4://baz.domain3.com/xyz baz.domain3.com
> +SOCKS4 sockproxy3.com; PROXY proxy3.com
> +http://[fe80:96db:12ce::43ef]/ip6.mp4 [fe80:96db:12ce::43ef]
> +PROXY proxy3.com; SOCKS4 sockproxy3.com
> diff --git a/unit/suite/stub.test b/unit/suite/stub.test
> index 12a0426..cde0aeb 100644
> --- a/unit/suite/stub.test
> +++ b/unit/suite/stub.test
> @@ -11,6 +11,9 @@ Stub suite file
> ?[excludes]
> ?#?If so, optional exlusion rules can be written here
> ?
> +[domains]
> +# List of domains are here
> +
> ?[config]
> ?#?Result of the configuration: VALID or INVALID
> ?
> diff --git a/unit/test-pacrunner.c b/unit/test-pacrunner.c
> index f234a35..0c4ac69 100644
> --- a/unit/test-pacrunner.c
> +++ b/unit/test-pacrunner.c
> @@ -42,9 +42,10 @@ enum test_suite_part {
> ? SUITE_PAC??????= 1,
> ? SUITE_SERVERS??= 2,
> ? SUITE_EXCLUDES = 3,
> - SUITE_CONFIG???= 4,
> - SUITE_TESTS????= 5,
> - SUITE_NOTHING??= 6,
> + SUITE_DOMAINS??= 4,
> + SUITE_CONFIG???= 5,
> + SUITE_TESTS????= 6,
> + SUITE_NOTHING??= 7,
> ?};
> ?
> ?enum cu_test_mode {
> @@ -58,6 +59,7 @@ struct pacrunner_test_suite {
> ? gchar *pac;
> ? gchar **servers;
> ? gchar **excludes;
> + gchar **domains;
> ?
> ? bool config_result;
> ?
> @@ -67,7 +69,7 @@ struct pacrunner_test_suite {
> ?static struct pacrunner_test_suite *test_suite;
> ?static bool verbose = false;
> ?
> -static struct pacrunner_proxy *proxy;
> +static struct pacrunner_proxy *proxy, *proxy2 = NULL, *proxy3 = NULL;
> ?static bool test_config;
> ?
> ?static void free_pacrunner_test_suite(struct pacrunner_test_suite *suite)
> @@ -79,6 +81,7 @@ static void free_pacrunner_test_suite(struct
> pacrunner_test_suite *suite)
> ? g_free(suite->pac);
> ? g_strfreev(suite->servers);
> ? g_strfreev(suite->excludes);
> + g_strfreev(suite->domains);
> ? g_strfreev(suite->tests);
> ?
> ? g_free(suite);
> @@ -142,6 +145,13 @@ static void print_test_suite(struct pacrunner_test_suite
> *suite)
> ? } else
> ? printf("(none)\n");
> ?
> + printf("\nDomains:\n");
> + if (suite->domains) {
> + for (line = suite->domains; *line; line++)
> + printf("%s\n", *line);
> + } else
> + printf("(none)\n");
> +
> ? printf("\nConfig result: %s\n",
> ? suite->config_result ? "Valid" : "Invalid");
> ?
> @@ -240,6 +250,15 @@ static struct pacrunner_test_suite
> *read_test_suite(const char *path)
> ? suite->excludes = array;
> ?
> ? break;
> + case SUITE_DOMAINS:
> + array = _g_strappendv(suite->domains, *line);
> + if (!array)
> + goto error;
> +
> + g_free(suite->domains);
> + suite->domains = array;
> +
> + break;
> ? case SUITE_CONFIG:
> ? if (strncmp(*line, "VALID", 5) == 0)
> ? suite->config_result = true;
> @@ -272,6 +291,8 @@ static struct pacrunner_test_suite *read_test_suite(const
> char *path)
> ? part = SUITE_SERVERS;
> ? else if (strncmp(*line, "[excludes]", 10) == 0)
> ? part = SUITE_EXCLUDES;
> + else if (strncmp(*line, "[domains]", 9) == 0)
> + part = SUITE_DOMAINS;
> ? else if (strncmp(*line, "[config]", 8) == 0)
> ? part = SUITE_CONFIG;
> ? else if (strncmp(*line, "[tests]", 7) == 0)
> @@ -338,6 +359,54 @@ static void test_manual_config(void)
> ? CU_ASSERT_TRUE(test_suite->config_result == test_config);
> ?}
> ?
> +static void test_proxy_domain(void)
> +{
> + int val = 0;
> +
> + if (pacrunner_proxy_set_domains(proxy, test_suite->domains) != 0)
> + val = -1;
> +
> + proxy2 = pacrunner_proxy_create("eth1");
> + if (proxy2) {
> + char *servers[] = {
> + "http://proxy2.com";,
> + "https://secproxy2.com";,
> + NULL};
> + char *domains[] = {
> + "intel.com",
> + "domain2.com",
> + "192.168.4.0/16",
> + NULL};
> +
> + if (pacrunner_proxy_set_manual(proxy2, servers, NULL) != 0)
> + val = -1;
> +
> + if (pacrunner_proxy_set_domains(proxy2, domains) != 0)
> + val = -1;
> + }
> +
> + proxy3 = pacrunner_proxy_create("wl0");
> + if (proxy3) {
> + char *servers[] = {
> + "http://proxy3.com";,
> + "socks4://sockproxy3.com",
> + NULL};
> + char *domains[] = {
> + "redhat.com",
> + "domain3.com",
> + "fe80:96db::/32",
> + NULL};
> +
> + if (pacrunner_proxy_set_manual(proxy3, servers, NULL) != 0)
> + val = -1;
> +
> + if (pacrunner_proxy_set_domains(proxy3, domains) != 0)
> + val = -1;
> + }
> +
> + CU_ASSERT_TRUE(val == 0);
> +}
> +
> ?static void test_proxy_requests(void)
> ?{
> ? gchar **test_strings;
> @@ -430,12 +499,21 @@ static void run_test_suite(const char *test_file_path,
> enum cu_test_mode mode)
> ? CU_add_test(cu_suite, "Manual config test",
> ? test_manual_config);
> ?
> + if (test_suite->domains)
> + CU_add_test(cu_suite, "Proxy domain test",
> + test_proxy_domain);
> +
> ? if (test_suite->config_result && test_suite->tests)
> ? CU_add_test(cu_suite, "Proxy requests test",
> ? test_proxy_requests);
> ?
> ? test_config = false;
> ?
> + if (test_suite->domains) {
> + pacrunner_proxy_unref(proxy2);
> + pacrunner_proxy_unref(proxy3);
> + }
> +
> ? switch (mode) {
> ? case CU_MODE_BASIC:
> ? CU_basic_set_mode(CU_BRM_VERBOSE);
Cheers,
Patrik
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 8, Issue 17
**************************************
