Closed Bug 246872 Opened 16 years ago Closed 16 years ago

PAC: myIpAddress gives "myIpAddress is not defined" error

Categories

(Core :: Networking, defect, major)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla1.7final

People

(Reporter: bugzilla_mozilla_org.20.ngc, Assigned: darin.moz)

References

Details

(Keywords: testcase, verified1.7, Whiteboard: fixed-aviary1.0)

Attachments

(2 files)

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 !
This really should be fixed before 1.7 final.
This breaks Mozilla for alot of people in corporate enviroments.
Flags: blocking1.7+
(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.
Flags: blocking1.7+
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?
Assignee: general → darin
Component: JavaScript Engine → Networking
QA Contact: pschwartau → benc
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.
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
Flags: blocking1.7+
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 ?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: 4xp
(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 ??
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 drivers@mozilla.org can set
the blocking1.7+ flag.  We have to set "blocking1.7?" to get drivers' attention.
Status: NEW → ASSIGNED
Flags: blocking1.7+ → blocking1.7?
Target Milestone: --- → mozilla1.7final
(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.
Serge: did you try calling myIpAddress at global scope?
This patch fixes the problem for me.  Thanks to shaver for figuring this out.
Attachment #150873 - Flags: superreview?(shaver)
Attachment #150873 - Flags: review?(shaver)
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 on attachment 150873 [details] [diff] [review]
v1 patch
[Checked in: Comment 17]

Go strong, fast and true, for the 1.7 approval!
Attachment #150873 - Flags: superreview?(shaver)
Attachment #150873 - Flags: superreview+
Attachment #150873 - Flags: review?(shaver)
Attachment #150873 - Flags: review+
Attachment #150873 - Flags: approval1.7?
Comment on attachment 150873 [details] [diff] [review]
v1 patch
[Checked in: Comment 17]

aieeeee
Attachment #150873 - Flags: approval1.7? → approval1.7+
{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 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
?
fixed-on-trunk
fixed-1.7
fixed-aviary1.0
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: fixed1.7
Resolution: --- → FIXED
Whiteboard: fixed-aviary1.0
Attachment #150873 - Attachment description: v1 patch → v1 patch [Checked in: Comment 17]
Attachment #150873 - Attachment is obsolete: true
OS: Windows 2000 → All
Hardware: PC → All
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);
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
Removing blocking1.7=?, as this bug has been fixed1.7 in the meantime.
Flags: blocking1.7?
verified on Mozilla 1.7 branch. Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.7) Gecko/20040617
Keywords: fixed1.7verified1.7
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...
Keywords: testcase
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.
Attachment #150873 - Attachment is obsolete: false
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.