1247959 - window.dump() does not work on Windows

Status: REOPENED

(Core :: Security: Process Sandboxing, defect)

Description

[Jukka Jylänki]

+++ This bug was initially created as a clone of Bug #1247958 +++

STR:

1. run firefox.exe --console
2. open console and type "window.dump('hello!\n');"

Observed: the result is not printed in the console window.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

It is a tool for developers, but dump() is also part of the DOM code, so let's try moving the bug there where it may get more attention.

Comment 2

[Jukka Jylänki]

This looks like it is caused by E10s. Setting the pref browser.tabs.remote.autostart.2=false, closing the browser and restarting, then the STR works.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Ok, then this is a dupe of bug 1173698.

Comment 4

[Jukka Jylänki]

I really don't like the bugzilla workflow of closing issues as duplicate like this, for two reasons.

a) Bug 1173698 does not have a test case. It has happened more than once in the past that bug A with STR is closed as dupe of bug B, which later gets partially fixed, but STR in A is not verified to pass, and the bug in the original STR remains and falls through unnoticed.

b) This bug has a descriptive user story as a title and comment 0, as opposed to a programmer's technical description as a title in bug 1173698. Quick search only searches open bugs, so it does no longer find this bug when searching with keywords "window dump windows". People who hit this issue are less likely to make the connection to the programmer's description in bug 1173698 when searching for dupes.

Don't mean this to say "my writeup of the bug is better than yours" or anything like that (a programmer's writeup of the bug is important!), but to raise that issues a) and b) have been a recurring trend.

I don't know how to resolve this workflow, but in general I'd prefer all bugs with separate STRs to be kept open until the STRs are explicitly verified by someone (committer who fixed it is ok) to be fixed. I wish bugzilla had a way to mark duplicates without closing. I think I'd rather prefer this bug was marked to depend on bug 1173698 instead, and just comment on them being duplicate, since I think that a bug with an STR should not be closed until there is someone who performs the proposed steps and witnesses the fix to be in.

What I'll do is I'll copy the STR comment from this bug to bug 1173698, which should future-proof that a) can't happen.

Comment 5

[Jukka Jylänki]

Sorry for the noise in advance if you don't agree with the above comment! Feel free to close as duplicate if you think the above points are not that critical here.

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

We can dupe them the other way then :)

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

I guess this is fine, but the problem extends to more than window.dump().  C++ printf_stderr() doesn't work from the child either.

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

And the other bug was known by the e10s team.

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

minus for e10s since this is a sandbox issue, not a core e10s issue.

As far as the sandbox goes, I believe this is behaving as intended. NI'ing Bob to confirm

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

I have to say breaking functionality as basic as printf and dump is terrible.  I don't understand why these basic tools can't be supported.

Comment 12

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Bug 1083701 and bug 1037445 comment #3 look relevant here.  It looks like it's only WinXP where allowing inheriting stdout/stderr would cause security problems?

Comment 13

[(No longer employed by Mozilla) Aaron Klotz]

Yup. The ability to explicitly whitelist handles for inheritance at the time of process creation wasn't added until Vista. Prior to that, it was either "inherit all handles created as inheritable" or nothing.

Comment 14

[Bob Owen (:bobowen)]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #10)
> minus for e10s since this is a sandbox issue, not a core e10s issue.
> 
> As far as the sandbox goes, I believe this is behaving as intended. NI'ing
> Bob to confirm

If this is the same as bug 1173698, then I definitely want to fix this.

The reason I've treated bug 1173698 as fairly low priority is that it works when you redirect the output to a file, so there is a workaround.

It seems from bug 1247958 that that workaround doesn't work in this case, so I'll take a quicker look.
We should already be inheriting stdout and stderr on Vista+.

Leaving NI to remind me.

Comment 15

[Bob Owen (:bobowen)]

(In reply to Bob Owen (:bobowen) from comment #14)

> It seems from bug 1247958 that that workaround doesn't work in this case, so
> I'll take a quicker look.

If you redirect to a file the dump output does appear.

Also, when running normal Nightly (so opt not debug) it doesn't log to the launched console for me, even in non-e10s, but did log if redirected to a file.

I still need to get this working at some point, but I think redirecting to a file is a reasonable workaround.

Comment 17

[Jukka Jylänki]

Does "when sandboxed" here mean "when running e10s" enabled?

If so, then this is not accurate to be a duplicate of bug 1189223. STR:

1. Navigate to about:config and set pref "browser.tabs.remote.autostart.2;false" to disable e10s.
2. Close and restart browser with "firefox -console"
3. open page console and type "window.dump('hello!\n');"

but nothing gets printed on screen. I'll reopen this pre-emptively as not a duplicate of 1189223, since that talks about e10s.

Comment 18

[Masatoshi Kimura [:emk]]

(In reply to Jukka Jylänki from comment #17)

Does "when sandboxed" here mean "when running e10s" enabled?

If so, then this is not accurate to be a duplicate of bug 1189223. STR:

1. Navigate to about:config and set pref
   "browser.tabs.remote.autostart.2;false" to disable e10s.
2. Close and restart browser with "firefox -console"
3. open page console and type "window.dump('hello!\n');"

but nothing gets printed on screen. I'll reopen this pre-emptively as not a
duplicate of 1189223, since that talks about e10s.

Did you flip browser.dom.window.dump.enabled? Worked for me if I set this pref to true. Also, I didn't have to disable e10s to see the dump output in the console.