331943 - [1.0.8rc regression] PAC script that uses eval() or new Function() no longer works

Status: VERIFIED FIXED

(Core :: XPConnect, defect, P1)

Description

[moz_bug_r_a4]

This was caused by the fix for Bug 311403 (the patch for 1.0.x is in Bug 311025 comment #24).

js_CheckPrincipalsAccess doesn't allow null scopePrincipals, but, in 1.0.x, a
sandbox object doesn't have principals.

Steps to Reproduce:
1. Setup a PAC file testcase on web server, or save it to local hard disk.
2. Set Automatic proxy configuration URL to its URL.
3. Open any web site.
4. See JS console.

Actual Results:
testcase 1:
Error: function eval must be called directly, and not by way of a function of another name.

testcase 2:
Error: function Function must be called directly, and not by way of a function of another name.

Expected Results:
testcase 1:
PAC-alert: OK: eval()

testcase 2:
PAC-alert: OK: new Function()

Comment 1

[moz_bug_r_a4]

Attachment: testcase 1 - PAC script using eval()

Comment 2

[moz_bug_r_a4]

Attachment: testcase 2 - PAC script using new Function()

Comment 3

[Brendan Eich [:brendan]]

This is a bad regression -- are we able to fix before release?

/be

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Not hard to fix. Ugh -- we really need to test PAC when doing QA.

Comment 5

[Bob Clary [:bc] (inactive)]

Tracy/Marcia: Can we add PAC script testing to the BFTs for 1.0.x/1.5.x?

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

This is very close to the patch that I wrote for bug 306467.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 216575 [details] [diff] [review]
Fix

r=dveditz

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 216575 [details] [diff] [review]
Fix

What branch does this go on?

Comment 9

[Tim Riley [:timr]]

a=timr for drivers.  This should land on 1.0.8 (aviary-1.0.1) and 1.7.13.  This was discussed by schrep, dveditz, mrbkap, darin, brendan, timr.  It causes breakage to limited usage but important PAC scripts and we cannot backout the critical patch that causes this.  Let's land it and respin ASAP!

Comment 10

[Tim Riley [:timr]]

Comment on attachment 216575 [details] [diff] [review]
Fix

a=timr for drivers.  This should land on both 1.0.8 and 1.7.13.  mrbkap says the fix is the same for both.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: What I'm about to check in

Note the added JSPRINCIPALS_DROP calls in the failure cases. I'm about to check this fix in.

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into the 1.7 branches.

Comment 13

[Daniel Veditz [:dveditz]]

> Let's land it and respin ASAP!

And move the _RELEASE tags for ff108 and moz1.7.13 and any "pull by date" on the build machines.

Comment 14

[Tracy Walker [:tracy]]

verified with Firefox linux 1.0.8 build from 0401

Comment 15

[Tracy Walker [:tracy]]

also verified on Windows 1.7.13 build from 0403

Comment 16

[Marcia Knous [:marcia]]

bc: yes, we can, and I have it on my todo list. I have added a note in the sw to follow up on this.

(In reply to comment #15)
> also verified on Windows 1.7.13 build from 0403
> 

(In reply to comment #5)
> Tracy/Marcia: Can we add PAC script testing to the BFTs for 1.0.x/1.5.x?
>