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 2/3] src/proxy.c: modify the proxy_lookup ()
      supporting non-browser schemes (Atul Anand)
   2. Re: [PATCH 3/3] unit: Add tests for BrowserOnly Key (Atul Anand)


----------------------------------------------------------------------

Message: 1
Date: Sun, 21 Aug 2016 21:44:15 +0530
From: Atul Anand <[email protected]>
To: David Woodhouse <[email protected]>, [email protected]
Subject: Re: [PATCH 2/3] src/proxy.c: modify the proxy_lookup ()
        supporting non-browser schemes
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8

As discussed, the proxy lookup for browser and non browser schemes should
be handled in an order as follows:
A request for a "browser" protocol would match the following configs
order of preference (if they exist):
 ? Matching "Domains", BrowserOnly==TRUE
 ? Matching "Domains", BrowserOnly==FALSE
 ? Domains==NULL, BrowserOnly==TRUE
 ? Domains==NULL, BrowserOnly==FALSE

A request for a non-browser protocol would match the following:
 ? Matching "Domains", BrowserOnly==FALSE
 ? Domains==NULL, BrowserOnly==FALSE (except if a config exists with
   Matching "Domains", BrowserOnly==TRUE, in which case we need to
   return NULL).
---
 src/manager.c   |  11 +++++-
 src/pacrunner.h |   2 +-
 src/proxy.c     | 121 +++++++++++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 108 insertions(+), 26 deletions(-)

diff --git a/src/manager.c b/src/manager.c
index 5a8b4fd..fa8a7b4 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -142,6 +142,7 @@ static DBusMessage *create_proxy_config(DBusConnection 
*conn,
        const char *url = NULL, *script = NULL;
        char **servers = NULL, **excludes = NULL;
        char **domains = NULL, **nameservers = NULL;
+       gboolean browser_only = FALSE;
 
        sender = dbus_message_get_sender(msg);
 
@@ -199,13 +200,18 @@ static DBusMessage *create_proxy_config(DBusConnection 
*conn,
                                nameservers = extract_string_array(&list);
                        }
                        break;
+               case DBUS_TYPE_BOOLEAN:
+                       if (g_str_equal(key, "BrowserOnly"))
+                               dbus_message_iter_get_basic(&value,
+                                                               &browser_only);
+                       break;
                }
 
                dbus_message_iter_next(&array);
        }
 
        DBG("sender %s method %s interface %s", sender, method, interface);
-       DBG("url %s script %p", url, script);
+       DBG("browser-only %u url %s script %p", browser_only, url, script);
 
        if (!method) {
                reply = g_dbus_create_error(msg,
@@ -226,7 +232,8 @@ static DBusMessage *create_proxy_config(DBusConnection 
*conn,
 
        nameservers = NULL;
 
-       if (pacrunner_proxy_set_domains(config->proxy, domains) < 0)
+       if (pacrunner_proxy_set_domains(config->proxy, domains,
+                                               browser_only) < 0)
                pacrunner_error("Failed to set proxy domains");
 
        if (g_str_equal(method, "direct")) {
diff --git a/src/pacrunner.h b/src/pacrunner.h
index 87c51da..e31d7ef 100644
--- a/src/pacrunner.h
+++ b/src/pacrunner.h
@@ -64,7 +64,7 @@ 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);
+                                       char **domains, gboolean browser_only);
 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 db49c58..7579887 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -40,6 +40,7 @@ struct pacrunner_proxy {
        char *script;
        GList **servers;
        GList **excludes;
+       gboolean browser_only;
        GList *domains;
        void *jsctx;
 };
@@ -159,13 +160,37 @@ const char *pacrunner_proxy_get_script(struct 
pacrunner_proxy *proxy)
        return proxy->script;
 }
 
-int pacrunner_proxy_set_domains(struct pacrunner_proxy *proxy, char **domains)
+static gboolean check_browser_protocol(const char *url)
+{
+       static const char *browser_schemes[] = {
+               "http://";,
+               "https://";,
+               "ftp://";,
+               "nntp://";,
+               "nntps://",
+       };
+       guint i;
+
+       for (i = 0; i < G_N_ELEMENTS(browser_schemes); i++) {
+               if (strncmp(browser_schemes[i], url,
+                               strlen(browser_schemes[i])) == 0)
+                       return TRUE;
+       }
+
+       return FALSE;
+}
+
+int pacrunner_proxy_set_domains(struct pacrunner_proxy *proxy, char **domains,
+                                       gboolean browser_only)
 {
        int len;
        char *slash, **domain;
        char ip[INET6_ADDRSTRLEN + 1];
 
-       DBG("proxy %p domains %p", proxy, domains);
+       DBG("proxy %p domains %p browser-only %u", proxy,
+                                       domains, browser_only);
+
+       proxy->browser_only = browser_only;
 
        if (!proxy)
                return -EINVAL;
@@ -476,13 +501,34 @@ static int compare_host_in_domain(const char *host, 
struct proxy_domain *match)
        return -1;
 }
 
+/**
+ * A request for a "browser" protocol would match the following configs
+ * order of preference (if they exist):
+ * ? Matching "Domains", BrowserOnly==TRUE
+ * ? Matching "Domains", BrowserOnly==FALSE
+ * ? Domains==NULL, BrowserOnly==TRUE
+ * ? Domains==NULL, BrowserOnly==FALSE
+ *
+ * A request for a non-browser protocol would match the following :
+ * ? Matching "Domains", BrowserOnly==FALSE
+ * ? Domains==NULL, BrowserOnly==FALSE (except if a config exists with
+ * Matching "Domains", BrowserOnly==TRUE, in which case we need to
+ * return NULL).
+ **/
 char *pacrunner_proxy_lookup(const char *url, const char *host)
 {
        GList *l, *list;
+       int protocol = 0;
        struct in_addr ip4_addr;
        struct in6_addr ip6_addr;
-       struct pacrunner_proxy *selected_proxy = NULL, *default_proxy = NULL;
-       int protocol = 0;
+       gboolean request_is_browser;
+       struct pacrunner_proxy *proxy = NULL;
+
+       /* Four classes of 'match' */
+       struct pacrunner_proxy *alldomains_browseronly = NULL;
+       struct pacrunner_proxy *alldomains_allprotos = NULL;
+       struct pacrunner_proxy *domainmatch_browseronly = NULL;
+       struct pacrunner_proxy *domainmatch_allprotos = NULL;
 
        DBG("url %s host %s", url, host);
 
@@ -512,12 +558,16 @@ char *pacrunner_proxy_lookup(const char *url, const char 
*host)
                }
        }
 
+       request_is_browser = check_browser_protocol(url);
+
        for (list = g_list_first(proxy_list); list; list = g_list_next(list)) {
-               struct pacrunner_proxy *proxy = list->data;
+               proxy = list->data;
 
                if (!proxy->domains) {
-                       if (!default_proxy)
-                               default_proxy = proxy;
+                       if (proxy->browser_only && !alldomains_browseronly)
+                               alldomains_browseronly = proxy;
+                       else if (!proxy->browser_only && !alldomains_allprotos)
+                               alldomains_allprotos = proxy;
                        continue;
                }
 
@@ -531,54 +581,79 @@ char *pacrunner_proxy_lookup(const char *url, const char 
*host)
                        case 4:
                                if (compare_legacy_ip_in_net(&ip4_addr,
                                                                data) == 0) {
-                                       selected_proxy = proxy;
                                        DBG("match proxy %p Legacy IP range %s",
                                            proxy, data->domain);
-                                       goto found;
+                                       goto matches;
                                }
                                break;
                        case 6:
                                if (compare_ipv6_in_net(&ip6_addr,
                                                        data) == 0) {
-                                       selected_proxy = proxy;
                                        DBG("match proxy %p IPv6 range %s",
                                            proxy, data->domain);
-                                       goto found;
+                                       goto matches;
                                }
                                break;
                        default:
                                if (compare_host_in_domain(host, data) == 0) {
-                                       selected_proxy = proxy;
                                        DBG("match proxy %p DNS domain %s",
                                            proxy, data->domain);
-                                       goto found;
+                                       goto matches;
                                }
                                break;
                        }
                }
+               /* No match */
+               continue;
+
+       matches:
+               if (proxy->browser_only == request_is_browser) {
+                       goto found;
+               } else if (proxy->browser_only) {
+                       /* A non-browser request will return DIRECT instead of
+                        * falling back to alldomains_* if this exists.
+                        */
+                       if (!domainmatch_browseronly)
+                               domainmatch_browseronly = proxy;
+               } else {
+                       /* We might fall back to this, for a browser request */
+                       if (!domainmatch_allprotos)
+                               domainmatch_allprotos = proxy;
+               }
        }
 
-       if (!selected_proxy) {
-               DBG("default proxy %p", default_proxy);
-               selected_proxy = default_proxy;
+       if (request_is_browser) {
+               /* We'll have bailed out immediately if we found a domain match
+                * with proxy->browser_only==TRUE. Fallbacks in order of prefe-
+                * rence.
+                */
+               proxy = domainmatch_allprotos;
+               if (!proxy)
+                       proxy = alldomains_browseronly;
+               if (!proxy)
+                       proxy = alldomains_allprotos;
+       } else {
+               if (!domainmatch_browseronly)
+                       proxy = alldomains_allprotos;
+               else
+                       proxy = NULL;
        }
 
-found:
+ found:
        pthread_mutex_unlock(&proxy_mutex);
 
-       if (!selected_proxy)
+       if (!proxy)
                return NULL;
 
-       switch (selected_proxy->method) {
+       switch (proxy->method) {
        case PACRUNNER_PROXY_METHOD_UNKNOWN:
        case PACRUNNER_PROXY_METHOD_DIRECT:
                break;
        case PACRUNNER_PROXY_METHOD_MANUAL:
-               return __pacrunner_manual_execute(url, host,
-                                               selected_proxy->servers,
-                                               selected_proxy->excludes);
+               return __pacrunner_manual_execute(url, host, proxy->servers,
+                                                 proxy->excludes);
        case PACRUNNER_PROXY_METHOD_AUTO:
-               return __pacrunner_js_execute(selected_proxy, url, host);
+               return __pacrunner_js_execute(proxy, url, host);
        }
 
        return NULL;
-- 
2.5.5



------------------------------

Message: 2
Date: Sun, 21 Aug 2016 21:55:02 +0530
From: Atul Anand <[email protected]>
To: David Woodhouse <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 3/3] unit: Add tests for BrowserOnly Key
Message-ID:
        <CABYWKtmo=snjg3ckgte4tev67uugvxyqephb16ny6ev7wxg...@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8

We are installing 3 configs in all the suites (2 hard coded and one
from the suite). Test can be performed on two configs (check we are
getting from the above one in the order we specified)
BrowserOnly key is not an optional case (if we don't set it , it is
set by default) so we don't need a special suite for this one. We are
testing it all along in all the suites.

We are testing our special case 3 times if you have noted above in the patch. :)

Best,
Atul

On 8/18/16, David Woodhouse <[email protected]> wrote:
> On Tue, 2016-08-16 at 20:19 +0530, Atul Anand wrote:
>> test-pacrunner and suites have been fixed to test BrowserOnly key
>> and it's function.
>
> http://www.angryflower.com/itsits.gif?:)
>
> The table in patch 2 had about 6 different cases to test, if not
> more...
>
>> A request for a "browser" protocol would match the following configs
>> order of preference (if they exist):
>>??? Matching "Domains", BrowserOnly==TRUE
>>??? Matching "Domains", BrowserOnly==FALSE
>>??? Domains==NULL, BrowserOnly==TRUE
>>??? Domains==NULL, BrowserOnly==FALSE
>
> So here you want tests which install a subset of those *four* different
> proxy configs (let's call them A, B, C and D) and then check the
> results.
>
> So one test would install just B, C and D... and check that the result
> it gets is actually from B, not from one of the others.
>
> Another test would install only C and D, and check it gets C.
>
>> A request for a non-browser protocol would match the following:
>>??? Matching "Domains", BrowserOnly==FALSE
>>??? Domains==NULL, BrowserOnly==FALSE (except if a config exists with
>>????Matching "Domains", BrowserOnly==TRUE, in which case we need to
>>????return NULL).
>
> Likewise we need tests to exercise these and especially the special
> case.
>
> I don't think your added tests are covering everything they need to,
> are they?
>
> --
> David Woodhouse                            Open Source Technology Centre
> [email protected]                              Intel Corporation


------------------------------

Subject: Digest Footer

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


------------------------------

End of connman Digest, Vol 10, Issue 25
***************************************

Reply via email to