Closed Bug 170128 Opened 22 years ago Closed 20 years ago

PAC: myIpAddress() does not update to network changes

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.8alpha1

People

(Reporter: benc, Assigned: darin.moz)

References

Details

(Keywords: verified1.7, Whiteboard: checklinux)

Attachments

(1 file, 3 obsolete files)

from bug 79244: chase said in comment #10

There's another possibly relnote-worthy behavior, but I'm not entirely sure of
the scope of it.  If the address of the network interface changes (this is not
that uncommon for mobile users), myIPAddress() will continue to report the old
address.  This happens because we save dns.myIPAddress when the PAC is loaded
and then return that value every time.  I think this may run deeper as well --
I'm not sure if nsDNSService ever detects the change either.  I've been trading
email with a mobile user about this; I may just split it off into a separate bug.
this would affect any number of our more sophisticated network users
Keywords: helpwanted
Target Milestone: --- → Future
I'm surprised that there isn't more traffic surrounding this bug.  Maybe it is
because it is just simply annoying more than anything else.  I go between work
and home with my laptop, at each stop getting a DHCP assigned IP address.  I
have my proxy script set up to check the IP address to decide whether to return
DIRECT or the proxy address.  Works great except I have to shut down Mozilla
everytime I get a new address.  Anyway, I just didn't want anyone to think this
wasn't important to at least someone!
so myIPAddress will get reread when going from offline to online.

Won't automatically change, except perhaps when combined with a fix to bug
76111.
Attached patch Better patch (obsolete) — Splinter Review
Clears when going offline, and again when going from offline to online. 
(Otherwise a myIPAddress check while in offline mode would cache the value
forever again.)
Attachment #121613 - Attachment is obsolete: true
cc me
Comment on attachment 121700 [details] [diff] [review]
Better patch

Can't resist requesting review on a 5-line patch :)
Attachment #121700 - Flags: review?(darin)
Comment on attachment 121700 [details] [diff] [review]
Better patch

sr=darin
Attachment #121700 - Flags: superreview+
Attachment #121700 - Flags: review?(gordon)
Attachment #121700 - Flags: review?(darin)
i think this bug is now fixed with the DNS rewrite.

myIPAddress does not exist anymore, but that aside... i think the underlying bug
is fixed.  now, whenever nsIDNSService::myHostName is accessed, we call
PR_GetSystemInfo, which calls into the OS to fetch the current hostname of the
local system.  DNS caching is also now limited to no longer than 1 minute, and
moreover when the user toggles the online/offline switch, we flush the entire
DNS cache.  so, i think we can say that this bug is fixed as a result of the
patch for bug 205726.
Status: NEW → RESOLVED
Closed: 21 years ago
Depends on: 205726
Resolution: --- → FIXED
Attachment #121700 - Flags: review?(gordon)
Barry Hathaway wrote me to say:

>I see from Bugzilla that you had worked on bug #170128 and now it is closed.
>I just recently installed Mozilla 1.6 and I still seem to see the same behavior
>with myIpAddress() - it always returns the same address. Any ideas how I can
get >either the current IP address or hostname of my Windows machine in
autoproxy.pac?
>Thanks in advance.

Upon further inspection of this problem, I think the problem has to do with the
fact that nsProxyAutoConfig.js only computes the value for myIP once (after the
PAC file has been loaded).

A workaround for this problem is to force reload the PAC file from the proxy
preferences panel.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
Target Milestone: Future → mozilla1.7beta
is this a matter of moving the code out of the onStopRequest into myIpAddress(),
or is it more complicated?

It sounds like we can call nsIDNSService::myHostName whenever we want, because
it is hooked in to cache (I think we changed myIpAddress() from a method to a
property a long time ago because we didn't have caching).
Summary: PAC: myIpaddress() does not update to network changes → PAC: myIpAddress() does not update to network changes
UPDATE: Mozilla 1.7b
Mac and Linux seem to always return 127.0.0.1.
> is this a matter of moving the code out of the onStopRequest into myIpAddress(),
> or is it more complicated?

This doesn't make any sense to me.


> It sounds like we can call nsIDNSService::myHostName whenever we want...

Right, it is probably just a matter of modifying nsProxyAutoConfig.js such that
it always hits nsIDNSService to satisfy "myIP."
> > is this a matter of moving the code out of the onStopRequest into myIpAddress(),
> > or is it more complicated?
> 
> This doesn't make any sense to me.

I understand now... the answer I think is "yes."  I'm working on a patch now.
Attached patch v2 patch (obsolete) — Splinter Review
Ok, this patch fixes two problems:

 (1) It makes myIpAddress() always talk to the DNS service, and
 (2) It eliminates the bogus single-level of caching in dnsResolve.
Assignee: general → darin
Attachment #121700 - Attachment is obsolete: true
Attachment #147702 - Flags: superreview?(bzbarsky)
Attachment #147702 - Flags: review?(cbiesinger)
Target Milestone: mozilla1.7beta → mozilla1.8alpha
darin, I'm not doing srs, remember?  If there's absolutely no one else qualified
to review this, I can try to take a look in a few days, but otherwise...
Attachment #147702 - Flags: superreview?(bzbarsky) → superreview?(bryner)
Attachment #147702 - Flags: superreview?(bryner) → superreview+
Comment on attachment 147702 [details] [diff] [review]
v2 patch

hm... this assumes the hostname stays the same.... can't DHCP assign you a
different hostname? or what if the user changes the hostname himself?

well, in any case, this is an improvement. r=me.
Attachment #147702 - Flags: review?(cbiesinger) → review+
> hm... this assumes the hostname stays the same.... can't DHCP assign you a
> different hostname? or what if the user changes the hostname himself?

Yeah, good point.  I'll write up a better patch.
Attached patch v2.1 patchSplinter Review
revised per biesi's review
Attachment #147702 - Attachment is obsolete: true
Attachment #147738 - Flags: review?(cbiesinger)
Comment on attachment 147738 [details] [diff] [review]
v2.1 patch

thanks
Attachment #147738 - Flags: review?(cbiesinger) → review+
Attachment #147738 - Flags: superreview?(bryner)
Looks like a lot of functional changes, but will behave much better, as good as
we can considering the fact it can only return one IPv4 address.

So, I'm assuming the cache-of-one is removed b/c of the DNS rewrite?

What is also worth noting is that we get the IPaddress by doing a lookup on our
hostname, which is not perfect, but the best we can do... (if we enumerated the
interfaces, which would we return), because hostnames sometimes aren't set, or
are set incorrectly).

If your system returns 127.0.0.1, type nslookup $HOST (mac) or nslookup
$HOSTNAME (linux), if it returns an error, you know why. 

(putting all this here before I forget, and isn't worth filing a new bug b/c it
can't be fixed).
> So, I'm assuming the cache-of-one is removed b/c of the DNS rewrite?

yeah, that's right, and in fact the bugs mentioned in the comments had all been
fixed.
*** Bug 243403 has been marked as a duplicate of this bug. ***
Attachment #147738 - Flags: superreview?(bryner) → superreview+
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 21 years ago20 years ago
Resolution: --- → FIXED
Comment on attachment 147738 [details] [diff] [review]
v2.1 patch

This is a low-risk patch that would be good to get on the 1.7 branch if time
permits.
Attachment #147738 - Flags: approval1.7?
Comment on attachment 147738 [details] [diff] [review]
v2.1 patch

a=mkaply
Attachment #147738 - Flags: approval1.7? → approval1.7+
fixed1.7
Keywords: fixed1.7
verified 1.7: 20040524, Win 2K.

I'm not going to get to Mac or Linux soon, unless I get lucky.

The steps I'm using are:

using PAC:
http://www.mozilla.org/quality/networking/testing/PAC/myIpAddress.pac

view the javascript console. each accessed URL will cause the current result of
myIpAddress() to be logged.

click on any link. confirm IP address is correct.
go offline. click on any link. confirm IP address is 127.0.0.1
go online. click on any link. confirm the IP address is correct again.
Whiteboard: checklinux, checkmac
VERIFIED: Mozilla 1.7RC2, Mac OS X.
Same steps, although you have to reload PAC to update.

You also have to remember that the value returned comes from a lookup of
"hostname -s".
Whiteboard: checklinux, checkmac → checklinux
(In reply to comment #28)

I can confirm that this bug is present in FireFox 1.0 running on OSX 10.3.7.

myIpAddress() is always returning 127.0.0.1, regardless of the on-line /
off-line state. I assume it is a FireFox problem and not some wierd settings in
my OSX, as other browsers return correct address using the same call.

I believe this might be the same issue as #250249.
Ivica:

What does hostname -s say? What happens if you telnet to that returend hostname?
> If your system returns 127.0.0.1, type nslookup $HOST (mac) or nslookup
> $HOSTNAME (linux), if it returns an error, you know why. 
> 
[Loris.Santamaria@guasipati ~]$ nslookup $HOSTNAME
Server:         192.168.0.100
Address:        192.168.0.100#53

Name:   guasipati.pzo.open-world.com.ve
Address: 192.168.0.213

...but here lies the problem:

[Loris.Santamaria@guasipati ~]$ cat /etc/hosts
# Do not remove the following line, or various programs
# that require network functionality will fail.
127.0.0.1 guasipati.pzo.open-world.com.ve guasipati localhost.localdomain  
localhost


And this is the default configuration in Fedora with DHCP enabled. It seems that
mozilla does not look th Ip address as "nslookup $HOSTNAME" but as "hostname -i"
Lori, Ivica: necko does use the system calls for resolver, which in most cases
means using /etc/hosts. That is really not the topic of this bug, which was that
myIpAddress() needs to change when the underlying values change.

I'm going to verify and close up this bug, so I'm asking you to create a new bug
so we can address the problems there.
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: