Use Android proxy's exclusion list to enable proxy bypass

Use Android proxy's exclusion list to enable proxy bypass

Based on Android AOSP submission
https://android-review.googlesource.com/#/c/102054/
Author: Jianzheng Zhou jianzheng.zhou@freescale.com

BUG=

Committed: https://crrev.com/9ac8056f252382a8fdffa89a8db3df9675d0bfd8
Cr-Commit-Position: refs/heads/master@{#299231}

[sgurun-gerrit only, 2014-07-25 15:46:05]

PTAL, thanks!

[mmenke, 2014-07-25 15:51:03]

[+eroman]:  Mind taking this?  I'm unfamiliar with the ProxyRules logic, and
been pretty slammed with reviews this week.

[eroman, 2014-07-25 21:11:15]

https://codereview.chromium.org/421493002/diff/20001/net/proxy/proxy_config_s...
File net/proxy/proxy_config_service_android.cc (right):

https://codereview.chromium.org/421493002/diff/20001/net/proxy/proxy_config_s...
net/proxy/proxy_config_service_android.cc:170: if (!exclusion_list.empty()) {
This block is essentially equivalent to:

config->proxy_rules().bypass_rules.ParseFromString(exclusion_list);

With the caveat that ParseFromString also accepts semicolons as a separator.

Can you point me to any documentation on the android format for rules?
BypassProxyRules is most similar to the Windows proxy rules foramt, and will
interpret things in a very particular way. For instance "<local>" has special
meaning, CIDR style masks can be added, and there is a difference in meaning for
"foo.com" vs ".foo.com"

[sgurun-gerrit only, 2014-07-26 00:57:09]

https://codereview.chromium.org/421493002/diff/20001/net/proxy/proxy_config_s...
File net/proxy/proxy_config_service_android.cc (right):

https://codereview.chromium.org/421493002/diff/20001/net/proxy/proxy_config_s...
net/proxy/proxy_config_service_android.cc:170: if (!exclusion_list.empty()) {
Thanks. Actually this made me realize that there are some other issues to be
aware of.

Android is dropping the hidden API proxyproperties (which chrome was using
reflection to access) and providing a new Public API (that was publicisized in
l-preview sdk). Do you want me to open a bug about that?

The new API (tentative) is returning a list of hosts
public String[] getExclusionList()

My understanding is the hosts can contain "*" in bypass rules.

However this CL is relevant for support for devices using current sdk and below,
so it is still meaningful. I need to do more investigation to answer your
question about current rules 




On 2014/07/25 21:11:15, eroman wrote:
> This block is essentially equivalent to:
> 
> config->proxy_rules().bypass_rules.ParseFromString(exclusion_list);
> 
> With the caveat that ParseFromString also accepts semicolons as a separator.
> 
> Can you point me to any documentation on the android format for rules?
> BypassProxyRules is most similar to the Windows proxy rules foramt, and will
> interpret things in a very particular way. For instance "<local>" has special
> meaning, CIDR style masks can be added, and there is a difference in meaning
for
> "foo.com" vs ".foo.com"

[sgurun-gerrit only, 2014-07-26 02:17:34]

On 2014/07/26 00:57:09, sgurun wrote:
>
https://codereview.chromium.org/421493002/diff/20001/net/proxy/proxy_config_s...
> File net/proxy/proxy_config_service_android.cc (right):
> 
>
https://codereview.chromium.org/421493002/diff/20001/net/proxy/proxy_config_s...
> net/proxy/proxy_config_service_android.cc:170: if (!exclusion_list.empty()) {
> Thanks. Actually this made me realize that there are some other issues to be
> aware of.
> 
> Android is dropping the hidden API proxyproperties (which chrome was using
> reflection to access) and providing a new Public API (that was publicisized in
> l-preview sdk). Do you want me to open a bug about that?
> 
> The new API (tentative) is returning a list of hosts
> public String[] getExclusionList()
> 
> My understanding is the hosts can contain "*" in bypass rules.
> 
> However this CL is relevant for support for devices using current sdk and
below,
> so it is still meaningful. I need to do more investigation to answer your
> question about current rules 
> 
> 
Looking at the code for proxy exclusion list 
https://android.googlesource.com/platform/frameworks/base/+/2a00329c6d55c6cd9...
the domains in the exclusion list are comma separated. Each domain is considered
for both "."+domain name version and just domainname. The code speaks the best:

            String splitExclusionList[] =
exclusionList.toLowerCase().split(",");
            mParsedExclusionList = new String[splitExclusionList.length * 2];
            for (int i = 0; i < splitExclusionList.length; i++) {
                String s = splitExclusionList[i].trim();
                if (s.startsWith(".")) s = s.substring(1);
                mParsedExclusionList[i*2] = s;
                mParsedExclusionList[(i*2)+1] = "." + s;
            }
when domains are checked if they are in exclusion list or not, they are checked
using:
 public boolean isExcluded(String url) {
   ...
   for (int i = 0; i< mParsedExclusionList.length; i+=2) {
            if (urlDomain.equals(mParsedExclusionList[i]) ||
                    urlDomain.endsWith(mParsedExclusionList[i+1])) {
                return true;
            }
        }
   ...
}

> 
> 
> On 2014/07/25 21:11:15, eroman wrote:
> > This block is essentially equivalent to:
> > 
> > config->proxy_rules().bypass_rules.ParseFromString(exclusion_list);
> > 
> > With the caveat that ParseFromString also accepts semicolons as a separator.
> > 
> > Can you point me to any documentation on the android format for rules?
> > BypassProxyRules is most similar to the Windows proxy rules foramt, and will
> > interpret things in a very particular way. For instance "<local>" has
special
> > meaning, CIDR style masks can be added, and there is a difference in meaning
> for
> > "foo.com" vs ".foo.com"

[sgurun-gerrit only, 2014-08-29 01:27:15]

On 2014/07/26 02:17:34, sgurun_OOO_until_Oct5 wrote:
> On 2014/07/26 00:57:09, sgurun wrote:
> >
>
https://codereview.chromium.org/421493002/diff/20001/net/proxy/proxy_config_s...
> > File net/proxy/proxy_config_service_android.cc (right):
> > 
> >
>
https://codereview.chromium.org/421493002/diff/20001/net/proxy/proxy_config_s...
> > net/proxy/proxy_config_service_android.cc:170: if (!exclusion_list.empty())
{
> > Thanks. Actually this made me realize that there are some other issues to be
> > aware of.
> > 
> > Android is dropping the hidden API proxyproperties (which chrome was using
> > reflection to access) and providing a new Public API (that was publicisized
in
> > l-preview sdk). Do you want me to open a bug about that?
> > 
> > The new API (tentative) is returning a list of hosts
> > public String[] getExclusionList()
> > 
> > My understanding is the hosts can contain "*" in bypass rules.
> > 
> > However this CL is relevant for support for devices using current sdk and
> below,
> > so it is still meaningful. I need to do more investigation to answer your
> > question about current rules 
> > 
> > 
> Looking at the code for proxy exclusion list 
>
https://android.googlesource.com/platform/frameworks/base/+/2a00329c6d55c6cd9...
> the domains in the exclusion list are comma separated. Each domain is
considered
> for both "."+domain name version and just domainname. The code speaks the
best:
> 
>             String splitExclusionList[] =
> exclusionList.toLowerCase().split(",");
>             mParsedExclusionList = new String[splitExclusionList.length * 2];
>             for (int i = 0; i < splitExclusionList.length; i++) {
>                 String s = splitExclusionList[i].trim();
>                 if (s.startsWith(".")) s = s.substring(1);
>                 mParsedExclusionList[i*2] = s;
>                 mParsedExclusionList[(i*2)+1] = "." + s;
>             }
> when domains are checked if they are in exclusion list or not, they are
checked
> using:
>  public boolean isExcluded(String url) {
>    ...
>    for (int i = 0; i< mParsedExclusionList.length; i+=2) {
>             if (urlDomain.equals(mParsedExclusionList[i]) ||
>                     urlDomain.endsWith(mParsedExclusionList[i+1])) {
>                 return true;
>             }
>         }
>    ...
> }
> 
> > 
> > 
> > On 2014/07/25 21:11:15, eroman wrote:
> > > This block is essentially equivalent to:
> > > 
> > > config->proxy_rules().bypass_rules.ParseFromString(exclusion_list);
> > > 
> > > With the caveat that ParseFromString also accepts semicolons as a
separator.
> > > 
> > > Can you point me to any documentation on the android format for rules?
> > > BypassProxyRules is most similar to the Windows proxy rules foramt, and
will
> > > interpret things in a very particular way. For instance "<local>" has
> special
> > > meaning, CIDR style masks can be added, and there is a difference in
meaning
> > for
> > > "foo.com" vs ".foo.com"


Hello,

sorry I just found some time to work on this CL more. Basically, I discovered an
error related to the API differences. The old APIs used to return a string for
excluded hosts, while the L preview API returns an array of hosts.

The important question from eroman was the format of exclusion list. I have
discussed it with a few people here and they said that it is compatible with
Java (http://docs.oracle.com/javase/7/docs/technotes/guides/net/proxies.html).
What confused me was the java page describes the separator as "|",  while in the
android UI it is a ",", and it seems that the comma is not translated to '|'
before being passed to the bypass string here.

Chrome proxy logic already tries to read proxy exclusion list from Java
properties here:
https://code.google.com/p/chromium/codesearch#chromium/src/net/proxy/proxy_co...
This CL complements it by reading the exclusion list from the intent.

PTAL, thanks

[eroman, 2014-09-23 21:17:09]

LGTM assuming you have tested it

https://codereview.chromium.org/421493002/diff/40001/net/proxy/proxy_config_s...
File net/proxy/proxy_config_service_android.cc (right):

https://codereview.chromium.org/421493002/diff/40001/net/proxy/proxy_config_s...
net/proxy/proxy_config_service_android.cc:172:
std::vector<std::string>::const_iterator itr;
nit: "it" is a more common iterator variable name in net code.

https://codereview.chromium.org/421493002/diff/40001/net/proxy/proxy_config_s...
net/proxy/proxy_config_service_android.cc:178: VLOG(0) << "Add bypass rules:" <<
pattern.c_str();
Is this vlog necessary?

[sgurun-gerrit only, 2014-10-10 00:58:55]

https://codereview.chromium.org/421493002/diff/40001/net/proxy/proxy_config_s...
File net/proxy/proxy_config_service_android.cc (right):

https://codereview.chromium.org/421493002/diff/40001/net/proxy/proxy_config_s...
net/proxy/proxy_config_service_android.cc:172:
std::vector<std::string>::const_iterator itr;
On 2014/09/23 21:17:09, eroman wrote:
> nit: "it" is a more common iterator variable name in net code.

Done.

https://codereview.chromium.org/421493002/diff/40001/net/proxy/proxy_config_s...
net/proxy/proxy_config_service_android.cc:178: VLOG(0) << "Add bypass rules:" <<
pattern.c_str();
On 2014/09/23 21:17:09, eroman wrote:
> Is this vlog necessary?

Done.

https://codereview.chromium.org/421493002/diff/140001/net/proxy/proxy_config_...
File net/proxy/proxy_config_service_android.cc (right):

https://codereview.chromium.org/421493002/diff/140001/net/proxy/proxy_config_...
net/proxy/proxy_config_service_android.cc:182:
config->proxy_rules().bypass_rules.AddRuleForHostname("", pattern, -1);
himm, I am not fully sure if I am doing it right here. If  example.com is in the
bypass list of Android, I think it means bypass *.example.com. However here it
means only exclude example.com but not www.example.com. I will verify the
behavior here before submitting.

[sgurun-gerrit only, 2014-10-11 00:53:59]

https://codereview.chromium.org/421493002/diff/140001/net/proxy/proxy_config_...
File net/proxy/proxy_config_service_android.cc (right):

https://codereview.chromium.org/421493002/diff/140001/net/proxy/proxy_config_...
net/proxy/proxy_config_service_android.cc:182:
config->proxy_rules().bypass_rules.AddRuleForHostname("", pattern, -1);
So it seems that the exclusion list that is manually entered is simply
transformed from a comma separated list to a "|" separated list and then it is
written to http.nonProxyHosts System property.
This happens in Android.net.Proxy.setHttpProxySystemProperty().

The Java http.nonProxyHosts property is a list of patterns that may include "*".
It is described in detail here: 
http://docs.oracle.com/javase/6/docs/technotes/guides/net/proxies.html
It looks to me the behaviors are equivalent. Also I verified they behave same by
writing a simple program using HTTPUrlConnection via ProxySelector and comparing
to the chromium after adding this feature.


On 2014/10/10 00:58:55, sgurun_OOO_until_Oct5 wrote:
> himm, I am not fully sure if I am doing it right here. If  http://example.com
is in the
> bypass list of Android, I think it means bypass *.example.com. However here it
> means only exclude http://example.com but not http://www.example.com. I will
verify the
> behavior here before submitting.

[sgurun-gerrit only, 2014-10-11 00:54:08]

The CQ bit was checked by sgurun@chromium.org

[commit-bot: I haz the power, 2014-10-11 00:54:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/421493002/140001

[commit-bot: I haz the power, 2014-10-11 02:19:15]

Committed patchset #5 (id:140001)

[commit-bot: I haz the power, 2014-10-11 02:19:50]

Patchset 5 (id:??) landed as
https://crrev.com/9ac8056f252382a8fdffa89a8db3df9675d0bfd8
Cr-Commit-Position: refs/heads/master@{#299231}