Last Comment Bug 311952 - eval(string) broken in XPInstall
: eval(string) broken in XPInstall
Status: VERIFIED FIXED
: fixed1.8, regression
Product: Core Graveyard
Classification: Graveyard
Component: Installer: XPInstall Engine (show other bugs)
: Trunk
: x86 Windows 2000
: P2 major (vote)
: mozilla1.8rc1
Assigned To: Blake Kaplan (:mrbkap)
:
:
Mentors:
Depends on:
Blocks: 311403
  Show dependency treegraph
 
Reported: 2005-10-10 12:42 PDT by Karsten Düsterloh
Modified: 2015-12-11 07:21 PST (History)
6 users (show)
asa: blocking1.8rc1+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Run xpinstall on its own runtime (3.32 KB, patch)
2005-10-10 23:55 PDT, Blake Kaplan (:mrbkap)
dveditz: review+
brendan: superreview+
asa: approval1.8rc1+
Details | Diff | Splinter Review

Description Karsten Düsterloh 2005-10-10 12:42:41 PDT
Having a line like this

     eval("666");

in your install.js makes XPInstall break with this (bogus?) error message in
install.log:

     "function eval must be called directly, and not by way of a function of
another name" in install.log"

eval(666) just works fine.
(This is very similar to bug 298054, just without the crash. :-/)

I tracked down this trunk regression range:
Win32 SeaMonkey trunk build 2005-10-06-06 is still okay,
Win32 SeaMonkey trunk build 2005-10-07-06 is broken.

If I set jsobj.c back to revision 3.214 to get rid of mrbkap's checkins there in
the regression range, eval is working again.
Comment 1 shutdown 2005-10-10 14:04:18 PDT
regression from bug 311025 and bug 311403.
Comment 2 Blake Kaplan (:mrbkap) 2005-10-10 16:17:28 PDT
I've never dealt with xpinstall before. How exactly do I reproduce this (does it
involve the xpi from bug 298054?)?

eval(<non-string>) is exactly equivilant to |<non-string>|, which is why you
don't see any bugs with it.
Comment 3 Karsten Düsterloh 2005-10-10 22:53:17 PDT
Yes, the XPI from attachment 186678 [details] in bug 298054 should suffice as a testcase.
Comment 4 Blake Kaplan (:mrbkap) 2005-10-10 23:55:47 PDT
Created attachment 199142 [details] [diff] [review]
Run xpinstall on its own runtime

For the record: the problem is that with the checkin to bug 311403, the JS
engine says that if you provide a non-null findObjectPrincipals hook in the
runtime, you must always provide non-null (not-ambiguous) principals to the
engine or you'll be stopped. This patch makes xpinstall run on its own runtime.
Comment 5 Daniel Veditz [:dveditz] 2005-10-10 23:58:29 PDT
Comment on attachment 199142 [details] [diff] [review]
Run xpinstall on its own runtime

r=dveditz
Comment 6 Brendan Eich [:brendan] 2005-10-11 00:09:12 PDT
Comment on attachment 199142 [details] [diff] [review]
Run xpinstall on its own runtime

Use JS_NewRuntime, not the obsolete JS_Init synonym.

Looks good otherwise.  Pretty funny that this "wizard context?" fallback to a
private runtime was always there.

/be
Comment 7 Blake Kaplan (:mrbkap) 2005-10-11 00:11:15 PDT
Note to triagers: This bug does not yet exist on the branch, but is caused by
bug 311403, which is a security fix that we will want to take on the branch. The
patch is very simple -- to always create a new runtime instead of only creating
one if a runtime cannot be found. dveditz informed me on IRC that xpinstall runs
in its own little world (so it does not need to access the DOM and does not rely
on the sharing of runtimes between its context and the global context).
Comment 8 Brendan Eich [:brendan] 2005-10-11 01:21:44 PDT
(In reply to comment #7)
> dveditz informed me on IRC that xpinstall runs
> in its own little world (so it does not need to access the DOM and does not rely
> on the sharing of runtimes between its context and the global context).

On the last line, -e s/runtimes/objects/ -e s/context/runtime/g ;-)

/be
Comment 9 Blake Kaplan (:mrbkap) 2005-10-11 11:04:50 PDT
Fix checked into trunk.
Comment 10 Karsten Düsterloh 2005-10-11 12:01:25 PDT
Yeah, works well in my local tree; will verify with a nightly asap.
Thanks for the very quick fix!
Comment 11 Blake Kaplan (:mrbkap) 2005-10-11 13:47:48 PDT
Checked in on MOZILLA_1_8_BRANCH.
Comment 12 Karsten Düsterloh 2005-10-12 15:15:17 PDT
Verified with Win32 Seamonkey trunk build 2005101205.

Note You need to log in before you can comment on or make changes to this bug.