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

VERIFIED FIXED in mozilla1.9alpha1

Status

()

Core
XPConnect
P1
normal
VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: moz_bug_r_a4, Assigned: mrbkap)

Tracking

({regression, verified1.7.13})

1.7 Branch
mozilla1.9alpha1
regression, verified1.7.13
Points:
---
Bug Flags:
blocking1.7.13 +
blocking-aviary1.0.8 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: aviary1.0/moz1.7 only, tcfollowup)

Attachments

(4 attachments)

(Reporter)

Description

12 years ago
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()
(Reporter)

Comment 1

12 years ago
Created attachment 216498 [details]
testcase 1 - PAC script using eval()
(Reporter)

Comment 2

12 years ago
Created attachment 216499 [details]
testcase 2 - PAC script using new Function()
This is a bad regression -- are we able to fix before release?

/be
(Assignee)

Comment 4

12 years ago
Not hard to fix. Ugh -- we really need to test PAC when doing QA.
Assignee: general → mrbkap

Comment 5

12 years ago
Tracy/Marcia: Can we add PAC script testing to the BFTs for 1.0.x/1.5.x?
Flags: blocking1.7.13?
Flags: blocking-aviary1.0.8?
Keywords: regression
(Assignee)

Comment 6

12 years ago
Created attachment 216575 [details] [diff] [review]
Fix

This is very close to the patch that I wrote for bug 306467.
Attachment #216575 - Flags: review?(dveditz)
Comment on attachment 216575 [details] [diff] [review]
Fix

r=dveditz
Attachment #216575 - Flags: review?(dveditz) → review+
(Assignee)

Updated

12 years ago
Attachment #216575 - Flags: superreview?(bzbarsky)
Attachment #216575 - Flags: superreview?(bzbarsky) → superreview+
(Assignee)

Comment 8

12 years ago
Comment on attachment 216575 [details] [diff] [review]
Fix

What branch does this go on?
Attachment #216575 - Flags: approval-aviary1.0.8?

Comment 9

12 years ago
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!
Flags: blocking1.7.13?
Flags: blocking1.7.13+
Flags: blocking-aviary1.0.8?
Flags: blocking-aviary1.0.8+

Comment 10

12 years ago
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.
Attachment #216575 - Flags: approval1.7.13+
Attachment #216575 - Flags: approval-aviary1.0.8?
Attachment #216575 - Flags: approval-aviary1.0.8+
(Assignee)

Comment 11

12 years ago
Created attachment 216771 [details] [diff] [review]
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.
(Assignee)

Comment 12

12 years ago
Fix checked into the 1.7 branches.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Component: JavaScript Engine → XPConnect
Keywords: fixed-aviary1.0.8, fixed1.7.13
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
> 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.
Whiteboard: aviary1.0/moz1.7 only
verified with Firefox linux 1.0.8 build from 0401
Status: RESOLVED → VERIFIED
Keywords: fixed-aviary1.0.8 → verified-aviary1.0.8
also verified on Windows 1.7.13 build from 0403
Keywords: fixed1.7.13 → verified1.7.13
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?
> 
Whiteboard: aviary1.0/moz1.7 only → aviary1.0/moz1.7 only, tcfollowup
You need to log in before you can comment on or make changes to this bug.