New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 735637 link

Starred by 18 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 22
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 644030
issue 827533



Sign in to add a comment

Re-implement myIpAddress() and myIpAddressEx() more reliably (possibly on a per-platform basis)

Project Member Reported by eroman@chromium.org, Jun 21 2017

Issue description

myIpAddress() is a helper function available to PAC scripts, intended to return the local host's IP address (with some ambiguity on what that means in various situations).

Chrome today is essentially doing this to implement myIpAddress() on non-ChromeOS platforms:

   getaddrinfo(gethostname());

This doesn't always work as intended:
  * May map to 127.0.0.1  (which is never what users want)
  * May timeout or be slow (depending on how name resolving is configured)
  * May return in nonsensical IPs (if hostname is numeric may be interpreted as an IP by our resolver)
  * There may be multiple interfaces with addresses to pick from, priority is ambiguous

Part of this bug is about finding a technique that works reliably and quickly.

The obvious approach is to enumerate the interfaces, and use some heuristics for prioritizing which of their addresses to use. This requires platform-specific code, and getting the heuristics good enough.

For comparison, Firefox uses a variety of hacks including connecting UDP sockets and reading back their local address [1] [2]

Someone should also compare what Internet Explorer / Edge do, especially in the case of myIpAddressEx(), for which it is effectively the reference implementation (as Microsoft introduced that extension).

(ChromeOS currently has some workarounds as it was always returning 127.0.0.1)

[1] https://hg.mozilla.org/mozilla-central/annotate/e53e37d2c2bf/netwerk/base/src/ProxyAutoConfig.cpp#l798
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=347307
 

Comment 1 by eroman@chromium.org, Jun 21 2017

Cc: mmenke@chromium.org eroman@chromium.org pbomm...@chromium.org
 Issue 734063  has been merged into this issue.

Comment 2 by eroman@chromium.org, Jun 21 2017

Cc: dskaram@chromium.org
 Issue 731749  has been merged into this issue.

Comment 3 by eroman@chromium.org, Jun 21 2017

Cc: mge...@chromium.org pauljensen@chromium.org
 Issue 267101  has been merged into this issue.

Comment 4 by mmenke@chromium.org, Jun 21 2017

Worth noting that to discover if we have IPv6 connectivity, we use a UDP hack that may not be too dissimilar from the kinda of things FireFox does (Though our logic is probably much simpler than what would be needed for this).

Comment 5 by eroman@chromium.org, Jun 22 2017

Description: Show this description
Connecting a UDP socket to 8.8.8.8 and checking the source address seems like a good idea.
Here's an example of similar code in Chrome's DNS resolver:
https://cs.chromium.org/chromium/src/net/dns/address_sorter_posix.cc?rcl=da14fb28cd3ccd701355b1e163e3d1d84ef2a73a&l=286
Here's an example of similar code in Android:
http://androidxref.com/7.1.1_r6/xref/bionic/libc/dns/net/getaddrinfo.c#1757

If we fix this in Chrome, we should fix this in Android too:
http://androidxref.com/7.1.1_r6/xref/external/chromium-libpac/src/proxy_resolver_js_bindings.cc#54

Comment 7 by mmenke@chromium.org, Jun 22 2017

Of course, we presumably still want to do something when users are connected to networks without internet access....
When there is no route to 8.8.8.8, perhaps the IP address used to get the PAC could be used (but if it's a local file, there is no way to choose between several potential LAN addresses).
PAC scripts are used in networks where more than one proxy is required. Usualy 1 for internal and 1 for external URLs. Usualy there is no direct internet connection to 8.8.8.8 and also no dns server which would resolve external addresses available.

The only usecase of myIpAddress(), i am aware of, is loadbalancing. i.e. by checking if the IP address is even or odd a proxy or another is selected.
Also random numbers would be fine here.

The main problem that I do see here is that myIpAddress() does always do something which causes a lag. Wouldn't it be possible to evaluate the local IP only once at startup and remember it during runtime? Maybe chromium could detect (by event from OS?) if the link status of network interfaces has changed and only in that case evaluate again?

We do watch for network changes.  The issue here isn't performance, but getting the right IP in the first place.

Also worth noting that of the bugs merged into this one, two were about getting the wrong IP address, so clearly returning random numbers isn't enough.
 Issue 736638  has been merged into this issue.
There are other use cases for myIpAddress():  The one that caused me to
sign up for this issue for one, is that I use a laptop, and the laptop may
be connected with any of a number of different networks, with different
proxy requirements.  Frequently, they don't even have a PAC file that is
automatically loaded unless you're using THEIR version of Windoze, and
since I use Linux anyway, I have to script the proxy myself.

A properly functional myIpAddress(), that actually returns IP address of
the interface I'm using to connect with the internet, would be a huge
benefit.
 Issue 812439  has been merged into this issue.
I've only discovered this to be the underlying issue for us also, this is what worked for me, being hit and miss between client devices it was very hard to uncover:
else
if (isInNet(myIpAddress(), "127.0.0.1", "255.255.255.0"))
    return "PROXY IP:port;
else
if (isInNet(myIpAddress(), "0.0.0.0", "0.0.0.0"))
    return "PROXY IP:port;

my underlying suggestion for chrome developers, would be to identify the clients IP based on the network adapter with the default gateway assigned. 
Blocking: 644030
IMHO the underlying problem is that myIpAddress() is a naive API. The source IP address used depends on the destination. myIpAddressForHost(host) would have been a better API.

Since we can't rewrite history, the suggestion in #14 seems good, combined with the idea in #8. As a further fallback, we could look for an RFC1918 address, failing that any IPv4 or IPv6 address at all, and finally localhost.
 Issue 886578  has been merged into this issue.
Cc: ericorth@chromium.org
Owner: eroman@chromium.org
Status: Assigned (was: Available)
Blocking: 827533
Status: Started (was: Assigned)
Work in progress: https://chromium-review.googlesource.com/c/chromium/src/+/1336063
Project Member

Comment 22 by bugdroid1@chromium.org, Nov 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7193c9faf699e4fbb3e713b6f9d1adea59680a0e

commit 7193c9faf699e4fbb3e713b6f9d1adea59680a0e
Author: Eric Roman <eroman@chromium.org>
Date: Thu Nov 22 01:39:58 2018

Re-implement myIpAddress() and myIpAddressEx().

The previous implementation was effectively:
  getaddrinfo(gethostname())

The new approach uses a similar strategy to Firefox, by first prioritizing the default local address used for connecting to the internet, followed by various fallbacks including testing routes to private IPs.

This also deletes the ChromeOS specific implementation, as the generic one addresses the original problem it was fixing.

Bug:  735637 ,  827533 
Change-Id: I31a5d1a58af48398afad787aefc1edd6cc4510ac
Reviewed-on: https://chromium-review.googlesource.com/c/1336063
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610278}
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/chrome/browser/io_thread.cc
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/chrome/browser/net/proxy_browsertest.cc
[add] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/chrome/test/data/my_ip_address.pac
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/chromeos/network/BUILD.gn
[delete] https://crrev.com/cb59771be7f119f04b1a3b4fe2ed55be724fecb5/chromeos/network/host_resolver_impl_chromeos.cc
[delete] https://crrev.com/cb59771be7f119f04b1a3b4fe2ed55be724fecb5/chromeos/network/host_resolver_impl_chromeos.h
[delete] https://crrev.com/cb59771be7f119f04b1a3b4fe2ed55be724fecb5/chromeos/network/host_resolver_impl_chromeos_unittest.cc
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/net/BUILD.gn
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/net/base/ip_address.cc
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/net/base/ip_address.h
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/net/dns/BUILD.gn
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/net/dns/host_resolver.h
[add] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/net/proxy_resolution/pac_library.cc
[add] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/net/proxy_resolution/pac_library.h
[add] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/net/proxy_resolution/pac_library_unittest.cc
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/net/proxy_resolution/proxy_bypass_rules.cc
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/net/proxy_resolution/proxy_resolver_v8.cc
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/net/proxy_resolution/proxy_resolver_v8_tracing.cc
[modify] https://crrev.com/7193c9faf699e4fbb3e713b6f9d1adea59680a0e/services/network/proxy_resolver_factory_mojo.cc

Status: Fixed (was: Started)
I'm an end user affected by this issue.
How can I test it? Is it available on Canary version already?
Looks like it just made the cut for Canary 72.0.3618.0, which should have gone out this morning.
When I reported the issue I was using the chrome://net-internals/#proxy to check for the applied proxy settings. But now it does not show anything.

Instead I went to chrome://net-export as indicated and recorded a website.
I think is fixed as I could read my proxy IP address. I'm attaching the log.
chrome-net-export-log.json
105 KB View Download
The proxy being used in that log is:
   PROXY 192.168.101.139:3128
#26: Regarding net-internals#proxy see  Issue 892454 .
For those wishing to test more are are some steps:

Save this PAC script to a file, say "pac.js"


function FindProxyForURL(url, host) {
  alert("XXX: myIpAddress(): " + myIpAddress());

  if (myIpAddressEx)
     alert("myIpAddressEx(): " + myIpAddressEx());

  return "DIRECT";
}


Now launch the Canary version of Chrome with the flag:

   --proxy-pac-url=file://PATH_TO_PAC_JS


Then capture a NetLog using chrome://net-export/ and view it using https://netlog-viewer.appspot.com/. Finally go to events tab and set filter to "PAC_JAVASCRIPT_ALERT"

Sign in to add a comment