Last Comment Bug 312124 - no longer works: "function eval must be called directly, and not by way of a function of another name"
: no longer works: "function eval must be call...
: fixed1.8, regression, testcase
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: PowerPC Mac OS X
: P2 normal (vote)
: mozilla1.8rc1
Assigned To: Blake Kaplan (:mrbkap)
: Hixie (not reading bugmail)
: Andrew Overholt [:overholt]
Depends on:
  Show dependency treegraph
Reported: 2005-10-11 20:13 PDT by Jesse Ruderman
Modified: 2011-08-05 21:27 PDT (History)
12 users (show)
asa: blocking1.8rc1+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

simple testcase (311 bytes, text/html)
2005-10-11 20:14 PDT, Jesse Ruderman
no flags Details
Potential patch (2.26 KB, patch)
2005-10-12 17:05 PDT, Blake Kaplan (:mrbkap)
mrbkap: superreview-
Details | Diff | Splinter Review
With COM rules followed (2.29 KB, patch)
2005-10-12 23:01 PDT, Blake Kaplan (:mrbkap)
caillon: review+
brendan: superreview+
asa: approval1.8rc1+
Details | Diff | Splinter Review

Description Jesse Ruderman 2005-10-11 20:13:04 PDT
Steps to reproduce:
1. Go to
2. Click "Execute"

Expected: Print 0, 1, 4, 9, 16

Result: "Error on line 1: function eval must be called directly, and not by way
of a function of another name"

Regression range:
PASS: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b3)
Gecko/20050714 Firefox/1.0+
FAIL: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4)
Gecko/20050715 Firefox/1.0+
Comment 1 Jesse Ruderman 2005-10-11 20:14:13 PDT
Created attachment 199252 [details]
simple testcase
Comment 2 Blake Kaplan (:mrbkap) 2005-10-11 21:46:21 PDT
So the problem here is that the javascript URL principals inherit the principals
of their owner (the attachment page) and the iframe has about:blank principals,
so eval does not think that its principals (of the attachment page) can subsume
the principals of the scope object (the about:blank window) and says, "go away,
you can't do that."

Brendan and I were talking about making the principals of newly-opened windows
be that of their parent document (e.g., the attachment page for the iframe in
the testcase) or their opener if there was no parent document. I think this
should be possible with changes (in docshell?). Thoughts appreciated.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2005-10-11 22:04:02 PDT
This would only be for about:blank pages, right?

I'd have to think through changing the security context of those pretty
Comment 4 Blake Kaplan (:mrbkap) 2005-10-11 22:09:06 PDT
Right. As soon as a page has loaded something in it, it would have new
principals (which would be subject to all of the same-origin checks that we do now).
Comment 5 Jesse Ruderman 2005-10-11 22:11:22 PDT
How about treating about:blank like a data: URL?  With data: URLs, if the page
has both a parent and an opener, I think the opener's principal is used.
Comment 6 Jesse Ruderman 2005-10-11 22:29:29 PDT
Are you saying that about:blank would have no principals defined until a script
"claims" it by loading a JavaScript URL, sticking nodes in, etc?  I guess that
could work too, but it might be more complicated than it needs to be.
Comment 7 Blake Kaplan (:mrbkap) 2005-10-11 22:34:39 PDT
No, my idea was closer to what you said in comment 5.
Comment 8 Blake Kaplan (:mrbkap) 2005-10-12 16:32:58 PDT
Actually, this is more easily implemented in nsPrincipal::Subsume().
Non-about:blank principals subsuming about:blank principals fixes this bug. I'm
still cleaning up my patch, and I still need to make sure that this isn't going
to open up any security problems.
Comment 9 Jesse Ruderman 2005-10-12 16:51:23 PDT
I think the approach in comment 5 is better than the approach in comment 8, at
least in the long term.  I don't want scripts from other sites to be able to
access my content/scripts just because I put it in a frame with the URL about:blank.
Comment 10 Blake Kaplan (:mrbkap) 2005-10-12 17:05:41 PDT
Created attachment 199367 [details] [diff] [review]
Potential patch

Jesse, the approach in comment 5 is incompatible with current behavior. This
patch is specific to the |eval| and |new Script| cases (bringing them up to
speed with our current treatment of about:blank documents, which is the
"anybody can touch me" treatment). This approach is compatible, and, I believe
fixes the original oversight in Subsume() [see also
Comment 11 Jesse Ruderman 2005-10-12 17:13:48 PDT
Fair enough. I'll file a new bug for treating about:blank differently, since
that change would affect compatibility.
Comment 12 Brendan Eich [:brendan] 2005-10-12 18:37:31 PDT
Comment on attachment 199367 [details] [diff] [review]
Potential patch

sr=me if it helps (I originated subsumes).

We should endeavor on the trunk to undo this change, by making about: URLs like
data: URLs (and javascript: URls on a good day): their origin is the origin of
the script that loaded them, or if statically src'ed, the origin of the
embedding or parent doc.

We should make not-a-principal real on the trunk too.  Followup bugs welcome.

Comment 13 Brendan Eich [:brendan] 2005-10-12 22:29:47 PDT
Comment on attachment 199367 [details] [diff] [review]
Potential patch

> nsPrincipal::Subsumes(nsIPrincipal *aOther, PRBool *aResult)
> {
>+  // First, check if aOther is an about:blank principal. If it is, then we can
>+  // subsume it.
>+  nsCOMPtr<nsIURI> otherOrigin;
>+  aOther->GetURI(getter_AddRefs(otherOrigin));
>+  if (otherOrigin) {
>+    PRBool isAbout = PR_FALSE;
>+    if (NS_SUCCEEDED(otherOrigin->SchemeIs("about", &isAbout)) && isAbout) {
>+      nsCAutoString str;
>+      otherOrigin->GetSpec(str);
>+      // Note: about:blank principals do not necessarily subsume about:blank
>+      // principals (unless aOther == this, which is checked in the Equals call
>+      // below).
>+      if (str.Equals("about:blank")) {
>+        PRBool isEqual = PR_FALSE;
>+        if (NS_SUCCEEDED(otherOrigin->Equals(mCodebase, &isEqual)) && !isEqual) {
>+          return PR_TRUE;

Argh, you are in a NS_IMETHODIMP -- need to set *aResult and return NS_OK.
Comment 14 Blake Kaplan (:mrbkap) 2005-10-12 22:59:46 PDT
Comment on attachment 199367 [details] [diff] [review]
Potential patch

Comment 15 Blake Kaplan (:mrbkap) 2005-10-12 23:01:13 PDT
Created attachment 199390 [details] [diff] [review]
With COM rules followed
Comment 16 Christopher Aillon (sabbatical, not receiving bugmail) 2005-10-13 07:49:54 PDT
Comment on attachment 199390 [details] [diff] [review]
With COM rules followed

You probably don't need to initialize PRBool isAbout or PRBool isEqual.  If the
methods fail, you won't look at their value, but if they succeed, COM rules
should warrant you get a valid value in them.  But that's a very minor nit.

Comment 17 Brendan Eich [:brendan] 2005-10-13 19:05:11 PDT
Comment on attachment 199390 [details] [diff] [review]
With COM rules followed

I say don't trust nsIURI impls, they could come from anything (some wacky
plugin).  The cost of initializing is teeny.  sr=me.

Comment 18 Blake Kaplan (:mrbkap) 2005-10-14 11:58:01 PDT
Fix checked into trunk.
Comment 19 Blake Kaplan (:mrbkap) 2005-10-17 14:41:56 PDT
Fix checked into MOZILLA_1_8_BRANCH.
Comment 20 Jesse Ruderman 2005-10-19 17:45:59 PDT
Filed followup bug 313070, "Treat about:blank like a data: URL".

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