790905 - Processing the mozilla.cfg file fails if the file contains JavaScript: "Failed to read the configuration file"

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[dickvl]

Attachment: mozilla.cfg with no ROT13 applied

I use a mozilla.cfg file to add some information about the current profile to the prefs.js file for backup purposes and set some default prefs.

That is done by adding some JavaScript to the mozilla.cfg file to get the profile location and the user account name.

//
const {classes:Cc,interfaces:Ci} = Components;
envUN = Cc["@mozilla.org/process/environment;1"].getService(Ci.nsIEnvironment).get("USERNAME");
profD = Cc["@mozilla.org/file/directory_service;1"].getService(Ci.nsIProperties).get("ProfD", Ci.nsIFile).path;

pref("profile-info.username", envUN);
pref("profile-info.profD", profD);

pref("browser.startup.homepage", "about:about");
pref("browser.newtab.url", "about:blank");

I get a "Failed to read the configuration file. Please contact your system administrator." error and Firefox doesn't start.
It works if I comment out the first lines and only leave the last two.

This still worked in the  20120911030553 (96287ad60bef) build.
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-09-10-03-05-24-mozilla-central/
No longer works in the 20120911140351 (fdfaef738a00) build.
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-09-11-14-03-51-mozilla-central/

Regression range (it is a big one 2039 entries, so I haven't checked it):
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=96287ad60bef&tochange=fdfaef738a00

Comment 1

[Alice0775 White]

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/96287ad60bef
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20120910183953
Bad:
http://hg.mozilla.org/mozilla-central/rev/d042ad078f43
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20120911065552
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=96287ad60bef&tochange=d042ad078f43


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/c8e5053af5ad
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20120911004454
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/4f5b4f0ecf01
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20120911010554
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c8e5053af5ad&tochange=4f5b4f0ecf01

Comment 2

[Alice0775 White]

In local build,
Last good:382dce79a998
First bad: 1cef36c730eb

Comment 3

[Bobby Holley (:bholley)]

Hm, just setting general.config.obscure_value and general.config.filename doesn't seem to cause anything to happen. According to gdb, nsReadConfig::Init is never called (so the observer doesn't get registered). Do I need an extension or something to make this stuff work?

Comment 4

[Alice0775 White]

Attachment: local-settings.js

No need add-on.

STR
1. Install Firfox to "Installation directory"
2. Copy mozilla.cfg to Installation directory/mozilla.cfg
   Copy local-settings.js to Installation directory/defaults/pref/local-settings.js
3. Start Firefox

Comment 5

[Bobby Holley (:bholley)]

Looks like the fix in bug 776490 wasn't quite right, but continued to work because of dynamic UniversalXPConnect checks, which are now gone. Patch coming up.

Comment 6

[Bobby Holley (:bholley)]

Attachment: Make the compartment principal of autoconfig_glob match the principal passed to JS::Evaluate. v1

Comment 7

[Bobby Holley (:bholley)]

Attachment: Replace no-op principal check with something real. v1

This would have caught this bug.

Comment 8

[Mike Kaply [:mkaply]]

defaultPref has been broken all along as well. Any chance this will fix that?

https://bugzilla.mozilla.org/show_bug.cgi?id=786875

Also, this will obviously have to be backported to at least 17.

Comment 9

[Bobby Holley (:bholley)]

(In reply to Michael Kaply (mkaply) from comment #8)
> defaultPref has been broken all along as well. Any chance this will fix that?

I'm really not sure. It's sort of an accident that the patch in bug 776490 worked, so it might not have covered all the cases.

> Also, this will obviously have to be backported to at least 17.

Well, I believe this only broke in 18, right? Though I'm fine to backport this for correctness' sake (especially if it fixes defaultPref).

Comment 10

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Comment on attachment 661204 [details] [diff] [review]
Make the compartment principal of autoconfig_glob match the principal passed to JS::Evaluate. v1

r=me

Comment 11

[Luke Wagner [:luke]]

Comment on attachment 661205 [details] [diff] [review]
Replace no-op principal check with something real. v1

Can we just remove options.principals then?  It seems like it only exists to be gotten wrong.

Comment 12

[Bobby Holley (:bholley)]

(In reply to Luke Wagner [:luke] from comment #11)
> Comment on attachment 661205 [details] [diff] [review]
> Replace no-op principal check with something real. v1
> 
> Can we just remove options.principals then?  It seems like it only exists to
> be gotten wrong.

Eventually yes - I plan to gut a bunch of obsolete security stuff from the js-engine. But at the moment that's more surgery than I'd like to do right now, and I'd like to just land this to ensure that, at the very least, the consumers aren't getting the wrong thing here.

Comment 13

[Luke Wagner [:luke]]

Comment on attachment 661205 [details] [diff] [review]
Replace no-op principal check with something real. v1

Excellent.

Comment 14

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=eaa76cb02c37

Comment 15

[Bobby Holley (:bholley)]

Looks green, modulo one make-check jsapi-test that I fixed. Luke, I took the liberty of carrying over your review on that one: https://hg.mozilla.org/integration/mozilla-inbound/rev/acec40170961#l1.42 - I hope that's ok?

Pushed to inbound:
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/ef528ae38f04
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/acec40170961

Comment 16

[Graeme McCutcheon [:graememcc]]

https://hg.mozilla.org/mozilla-central/rev/ef528ae38f04
https://hg.mozilla.org/mozilla-central/rev/acec40170961

Comment 17

[Luke Wagner [:luke]]

Fine by me.

So are JS_(Get|Set)FrameAnnotation going away now?  If so, don't forget to zap StackFrame::annotation_ and HAS_ANNOTATION or file a bug on me to do so.

Comment 18

[Mike Kaply [:mkaply]]

This will need to make Firefox 17 as well so it is in the ESR

Comment 19

[bhavana bajaj [:bajaj]]

Can we get a try build so that mkaply can help us confirm this is fixed.

Comment 20

[Bobby Holley (:bholley)]

(In reply to Michael Kaply (mkaply) from comment #18)
> This will need to make Firefox 17 as well so it is in the ESR

Is it actually broken on 17? I thought that the dynamic UniversalXPConnect checks were saving us there. Anyhow, happy to backport it just to be safe.

Comment 21

[Alex Keybl [:akeybl]]

(In reply to Michael Kaply (mkaply) from comment #18)
> This will need to make Firefox 17 as well so it is in the ESR

mkaply - are you definitely hitting this bug on FF17? We can hold on a try build until that's confirmed.

Comment 22

[Mike Kaply [:mkaply]]

It looks like we are good on Firefox 17.