246872 - PAC: myIpAddress gives "myIpAddress is not defined" error

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Michael]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040614 Firefox/0.9
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040608

Error: myIpAddress is not defined
Source File: http://*******/proxy.pac
Line: 249

Reproducible: Always
Steps to Reproduce:
1. set PAC script to a script the calls myIpAddress();
2. Try to surf the web
3. Look in javascript console

Actual Results:  
get time out error when trying to access site that goes through proxy.

Expected Results:  
myIpAddress(); should return local IP address to PAC script

I confirmed this in 1.7rc3 and Firefox 0.9 !

Comment 1

[Michael]

This really should be fixed before 1.7 final.
This breaks Mozilla for alot of people in corporate enviroments.

Comment 2

[Andrew Schultz]

(In reply to comment #1)
> This really should be fixed before 1.7 final.
> This breaks Mozilla for alot of people in corporate enviroments.

It's a bit late for 1.7 and only drivers can set blocking flags anyway.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

hm... bug 170128 changed myIpAddress behaviour, but benc verified that the
function still worked...

can we see your proxy.pac file, or maybe a simplified version of it that also
shows this bug?

Comment 4

[Darin Fisher]

reporter:

how did you install mozilla?  did you perhaps use the .zip file and unzip on top
of an old mozilla install?  if you did, that may have caused this problem.  i
suspect you might want to try deleting the compreg.dat file that is inside the
mozilla/components directory.

we had problems in the past where changes to nsProxyAuthConfig.js were not
recognized when a user unzipped a new mozilla .zip file ontop of an existing
mozilla directory.

Comment 5

[Michael]

Attachment: A test PAC script that doesn't work in 1.7rc3 or Firefox

Here is a test case. This script should work. It works in older builds.
It works in IE even!! Set your PAC script to this file.
Then try to surf the web.
You will time out.
Then look in the Javascript console.
You will see something like:

Error: myIpAddress is not defined
Source File: file:///c:/test.pac
Line: 207

Comment 6

[Serge Gautherie (:sgautherie)]

Confirming, with '4xp'.

I tested using
<http://bugzilla.mozilla.org/attachment.cgi?id=150848&action=view> as my PAC value.

[Microsoft Internet Explorer, version 5 (5.00.3105.0106) (128b, SP1)] (W98SE)

Surfs; no error.

[Netscape® Communicator 4.8 : en-20020722] (W98SE)

Had to use <file:///x%7C/test.pac>;
but Surfs; no error.

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7) Gecko/20040608] (<-- 1.7rc3 !)
(W98SE)

This is from the .exe full installer file.

Surfs;
but I get an error each time the file is (re)loaded (when setting the pref.)
{{
Error: myIpAddress is not defined
Source File: http://bugzilla.mozilla.org/attachment.cgi?id=150848&action=view
Line: 207
}}

Obviously, the line number does not refer to that 6 line PAC attachment.!.

Could someone test v1.8a1/Trunk too ?

Comment 7

[Serge Gautherie (:sgautherie)]

(In reply to comment #3)
> hm... bug 170128 changed myIpAddress behaviour, but benc verified that the
> function still worked...

Then, I tried
<http://www.mozilla.org/quality/networking/testing/PAC/myIpAddress.pac>
from bug 170128 comment 27:

MsIEv5.0: I'm getting |alert()| dialogs with my current IP@ :-)

Mv1.7rc3: I'm getting JS.C. outputs with my current IP@ :-)
{{
PAC-alert: aaa.bbb.ccc.ddd
}}

Nv4.8: (I did not try.)


Then |myIpAddress()| seems to work...
Could there be something "special" about the testcase attachment ??

Comment 8

[Darin Fisher]

I can reproduce the problem.  myIpAddress works fine if called from within
FindProxyForURL; however, it does not seem to work at global scope.  We load the
PAC file inside a SandBox, and we expose myIpAddress as a property on the
SandBox instance.  I don't know the details of how the SandBox works.  CC'ing
folks who might be able to help.  Only members of [e-mail] can set
the blocking1.7+ flag.  We have to set "blocking1.7?" to get drivers' attention.

Comment 9

[Serge Gautherie (:sgautherie)]

(In reply to comment #8)
> I can reproduce the problem.  myIpAddress works fine if called from within
> FindProxyForURL; however, it does not seem to work at global scope.

Oh well, I was just testing and confirming this too:
I used <file:///x:/test.pac>;
|myIpAddress()| work fine inside the function with alert()/dump()/var.

Comment 10

[Darin Fisher]

Serge: did you try calling myIpAddress at global scope?

Comment 11

[Darin Fisher]

Attachment: v1 patch [Checked in: Comment 17]

This patch fixes the problem for me.  Thanks to shaver for figuring this out.

Comment 12

[Darin Fisher]

The problem was that we were not setting the properties on the sandbox object
until after we had evaluated the PAC script.  Since global JS code runs at
evaluation time, we prevented code at global scope from having access to the
properties set on the sandbox :-(

The fix is trivial.

Comment 13

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 150873 [details] [diff] [review]
v1 patch
[Checked in: Comment 17]

Go strong, fast and true, for the 1.7 approval!

Comment 14

[Daniel (Leaf) Nunes]

Comment on attachment 150873 [details] [diff] [review]
v1 patch
[Checked in: Comment 17]

aieeeee

Comment 15

[Serge Gautherie (:sgautherie)]

{Why do I find myself too slow for the second time ?? Anyway, I confirm !}

(In reply to comment #10)
> Serge: did you try calling myIpAddress at global scope?

Yes, my current test file is +/- like
{{
var myipaddr2 = myIpAddress();

function FindProxyForURL(url, host)
{
var myipaddr = myIpAddress();

//dump(myIpAddress() + "\n" );
alert('1 ' + myIpAddress());
alert('2 ' + myipaddr2);

return "DIRECT";
}
}}

Nv4.8: I'm getting the two alert() with correct value :-)

Mv1.7rc3: JS error on the global |var| when loading the PAC file :-(

Now, the interesting part:
1. Start Mozilla, and load the file for the first time: JS error.
2. Comment out the global |myipaddr2|, and reload the file (pref. button): no
error. (Don't "run" it: the var is used in the function...)
3a. Uncomment out the global |myipaddr2|, and reload the file (pref. button): no
error ...
3b. ... Run it by going to some site: the 2 alerts show as expected.

1) It IS a '4xp';
2) It is an "initialization" issue (obviously ;->>):
I guess |myIpAddress()| gets defined into the sandbox at step 2 (only from
inside the function);
Then, it remains available, even at global scope, at steps 3.


Well, I believe this is all the (testing) help I can provide: up to the SandBox
experts now.
{{
/netwerk/base/src/nsProxyAutoConfig.js, line 147 -- function myIpAddress() {
}}

Comment 16

[Serge Gautherie (:sgautherie)]

Comment on attachment 150873 [details] [diff] [review]
v1 patch
[Checked in: Comment 17]

While we're here: Could you change
+	 // evaluate loded js file
into
+	 // evaluate loaded js file
?

Comment 17

[Darin Fisher]

fixed-on-trunk
fixed-1.7
fixed-aviary1.0

Comment 18

[benc]

Not being too expert at javascript, why would you do this? The PAC spec says
that a .pac file should be w/in the raw js code of the FindProxyForURL() function:

The proxy autoconfig file is written in JavaScript. The file must define the
function:

        function FindProxyForURL(url, host)
        {
            ...
        }

which will be called by the Navigator in the following way for every URL that is
retrieved by it:

        ret = FindProxyForURL(url, host);

Comment 19

[Brendan Eich [:brendan]]

benc: there's no problem, PAC has always loaded a .js file, which could contain
any declarations or statements, but which should define FindProxyForURL.  We
need to be totally backward compatible, and the patch restores that.

/be

Comment 20

[Serge Gautherie (:sgautherie)]

Removing blocking1.7=?, as this bug has been fixed1.7 in the meantime.

Comment 21

[David Epstein]

verified on Mozilla 1.7 branch. Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.7) Gecko/20040617

Comment 22

[benc]

Brendan: I'll add a test case for this. Limited javascript skills means I try to
stick w/ what has worked for me in the past...

Comment 23

[benc]

So, I guess on broader level, this means that all PACUtil functions should work
at the global scope?

I'll just create one test file for that.

Comment 24

[benc]

V/fixed.

Finally had time to verify in trunk. Will make this a pac testcase.

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20051028 SeaMonkey/1.1a