Closed Bug 1193625 Opened 9 years ago Closed 9 years ago

mozilla.cfg/override.ini/policies.js are ignored when the encoding is incorrect

Categories

(Toolkit :: Preferences, defect)

Product:
Toolkit
Component:
Preferences
Version:
40 Branch
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla44
Tracking Flags:
Tracking Status
firefox40 + wontfix
firefox41 + wontfix
firefox42 + fixed
firefox43 + fixed
firefox44 --- fixed

People

(Reporter: enz-coast, Assigned: mkaply, NeedInfo)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Fallback to ASCII if conversion fails
1.16 KB, patch
mossop
: review+
Details | Diff | Splinter Review
1193625.diff
1.71 KB, patch
mrbkap
: review+
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta+
ritu
: approval-mozilla-release-
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150806001005

Steps to reproduce:

Yesterday we deployed Firefox 40 as an update for 39.0.3 to 6500 clients ( windows 7 SP1 Pro 64Bit ) in good faith it would work like all the times before.


Actual results:

After Firefox 40 being deployed to the machines our users nearly killed us, because all of preconfigured options were gone... We tried to figure it out but after a 18 hour nightshift for my IT crew and me, nothing useful showed up and 10 minutes ago we rolled back to 39.0.3 until we can figure out what the f**k is producing this error for us. The next logical step for us was reaching out directly to Mozilla - so here we are...

Help would be much appreciated !


Expected results:

As about 50-60 times / updates before we expected Firefox to continue working with our mozilla.cfg, policies.js and override.ini, which we deploy along with Firefox itself and which inherits essential configuration options for our systems like proxy support, disabling autoupdate and many others... We tried to elaborate why the Firefox is ignoring the files ( filemon, regmon, some other tracking tools ), checked about:config, tried to find information via the web and the mozilla site but nothing that helped was showing up.

Thanks in advance !
If wanted / needed I can provide our configuration files and the WPKG XML Files which we use to deploy FF to our environment. Just tell me what you need !
Same problem here. Realized before it went into production state. i confirm it. Prior versions working.
Severity: normal → critical
Component: Untriaged → General
OS: Unspecified → Windows 7
Hardware: Unspecified → x86_64
The version 40.0.1 still contains the error !
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0
20150812163655

Push log:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8f57f60ee58a&tochange=fec90cbfbaad

I'm not seeing any likely suspects in the respective time range.


Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:43.0) Gecko/20100101 Firefox/43.0
20150813030208

No issue in the latest Nightly, so whatever was broken was fixed at some point.
Status: UNCONFIRMED → NEW
status-firefox40: --- → affected
status-firefox43: --- → fixed
Component: General → Preferences: Backend
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
the duplicate bug isnt fixed.. same problem with the 40.0.2.
startup shows the standard browser question.
the updates are not turned off.

im far off to deploy a nightly version to our productive clients.
shall i open a new bug or is there an updated version 40.0.3 coming soon.
still running with security issues on the 39.0.3
i uploaded some screenshots

firefox_about_39.03
http://abload.de/img/firefox_about_39.03bpad1.jpg

firefox about 40.0.2
http://abload.de/img/firefox_about_40.0.28saay.jpg

firefox_startup_40.0.2
http://abload.de/img/firefox_startup_40.0.ulzbo.jpg
i meant security on version before 39.0.3 my bad
Error still present with 40.0.2. Will there be a Bugfix for the 40.x version-tree or do we have to sit this bug out and live with the old 39.0.3 version?

this is our override.ini:
[XRE]
EnableProfileMigrator=false
[/align]

this is our policies.js:
pref("general.config.obscure_value", 0);
pref("general.config.filename", "mozilla.cfg");

this is our mozilla.cfg: ( servernames etc are altered in favour of privacy )
lockPref("browser.startup.homepage_override.mstone", "ignore");
lockPref("browser.shell.checkDefaultBrowser", false);
lockPref("app.update.enabled", false);
lockPref("browser.startup.homepage", "https://xxx.xxx.xxx.xxx.de");
lockPref("browser.startup.page", 1);
lockPref("network.proxy.type", 2);
lockPref("network.proxy.autoconfig_url", "https://xxx.xxx.xxx.xxx.xxx.de/squid.pac");
lockPref("network.negotiate-auth.allow-proxies", false);
lockPref("toolkit.telemetry.enabled", false);
lockPref("toolkit.telemetry.rejected", true);
lockPref("toolkit.telemetry.prompted", 2);
lockPref("datareporting.healthreport.service.firstRun", true);
lockPref("datareporting.healthreport.uploadEnabled", false);
lockPref("browser.cache.disk.capacity", 100000);
lockPref("browser.cache.disk.smart_size.enabled", false);
lockPref("browser.cache.disk.smart_size.first_run", false);
pref("browser.sessionstore.resume_from_crash", false);
pref("browser.rights.3.shown", true);

Help and/or answers from an official source regarding a bugfix/new version would be very helpful.
[Tracking Requested - why for this release]: affects system administrators managing multi-user environments.

―――――

(In reply to Gingerbread Man from comment #5)
> No issue in the latest Nightly, so whatever was broken was fixed at some
> point.

It turns out that's incorrect. Nightly had a different file than the other versions I tested, which is why it didn't exhibit the issue. In Release, Beta, FDE and Nightly, parsing chokes on the following line, resulting in the whole file being ignored:
// Pocket — social bookmarking
The — character is the problem, though it wasn't an issue in 39.0.3 and prior.

(In reply to enz-coast from comment #9)
> this is our mozilla.cfg: ( servernames etc are altered in favour of privacy )

Are you absolutely sure that's it, verbatim, aside from the server strings? If so, it looks like the root of the problem is different in your case.
status-firefox41: --- → affected
status-firefox42: --- → affected
status-firefox43: fixed → affected
tracking-firefox40: --- → ?
Same problem...
@Gingerbread Man:
Yes, that is our config. We tried the config file line for line but nothing, it´s ignored completely so still nothing new here !
What a mess.  I am looking into Chrome for my company due to all of this non-sense.  Until then I am deploying a work around that does not use any custom config files.

I configure Firefox with the settings, addons, bookmarks, etc that needs to be deployed to our company.  Once configured I copied the "%program files (x86)%\Mozilla Firefox" and the "%appdata%\Mozilla" directories to a share for deployment.

I then made a script that performs a backup of the user firefox profile, uninstalls firefox, and then performs the following to install without a configuration file.


Copy the following to target computer:

%program files (x86)%\Mozilla Firefox
%userprofile%\appdata\roaming\mozilla
%programdata%\Microsoft\Windows\Start Menu\Programs\Mozilla Firefox\Mozilla Firefox.lnk
%userprofile%\Desktop\Mozilla Firefox.lnk


Import the following registry key to add firefox back into Programs and Features:

HKLM\Software\Wow6432Node\Microsoft\Windows\CurrentVersion\Uninstall\Mozilla Firefox 40.0 (x86 en-US)


Run the command "ie4uinit.exe -ClearIconCache" if the Firefox icon does not appear for the shortcuts.


This might seem like a pain but when you are talking over 3,000 devices it is well worth the band aide until we can look into Chrome more.

Let me know if anyone has questions.
We had the same problem in our environment. We fixed it through removing all special characters (German characters) from the comments in the mozilla.cfg. 

I would try a test with a mozilla.cfg without all comments in your case...
The first comment (e. g. //Firefox Default Settings) must left in the cfg file of course
I saved the file mozilla.cfg with UTF-8 encoding, it is not ignored anymore.
(In reply to enz-coast from comment #0)
> Yesterday we deployed Firefox 40 as an update for 39.0.3 to 6500 clients (
> windows 7 SP1 Pro 64Bit ) in good faith it would work like all the times
> before.
You should use ESR for this kind of usage: https://www.mozilla.org/en-US/firefox/organizations/faq/

Too late for 40 but we should improve the error messages in case of incorrect encoding (or manage the previous encoding) for 41 or 42.
status-firefox40: affected → wontfix
tracking-firefox40: ? → +
tracking-firefox41: --- → +
Summary: mozilla.cfg/override.ini/policies.js are ignored since update from Firefox 39.0.3 to Firefox 40 → mozilla.cfg/override.ini/policies.js are ignored when the encoding is incorrect
It is getting too late to fix this in FF41 given that we don't have a patch ready. Let's try to get this fixed in 42.
status-firefox41: affected → wontfix
tracking-firefox42: --- → +
tracking-firefox43: --- → +
Will we be able to comment in our native language again or do we have quit commenting in order to make it work in future versions ? I mean hey, it worked for over 30 versions without any problem and then it´s broken and needs 3 major ( 39.03 to 42.xx ) versions to get fixed?
(In reply to enz-coast from comment #20)
> Will we be able to comment in our native language again

Save the file in UTF-8 encoding. This seems necessary for any non-ASCII characters (I tested é and —).
Still in FF44. When the file mozilla.cfg is saved as ANSI with some accented letters, Firefox fails to parse it.

m-i:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6b1f01887b3eebe3168d4eb3f56e506546454986&tochange=dd23898c54d1661dc09ce7b291e6cc47869e0093

Mike Kaply — Bug 1137799 - AutoConfig file should be read as UTF-8, not ASCII, r=mossop


Mike, is it normal the user needs to save mozilla.cfg in UTF-8 format?
If ANSI encoding is not supported anymore, it should be documented.
Blocks: 1137799
Flags: needinfo?(mozilla)
Component: Preferences: Backend → Preferences
Product: Core → Toolkit
My apologies for this.

I announced this on the enterprise mailing list and on my blog.

Unfortunately there really isn't any good place to announce these types of changes.

As far as "Fixing" this goes, switching to UTF-8 was the fix.

Requiring ANSI for AutoConfig was wrong from the beginning. I'll look into the possibility of detecting this situation and handling both.
Flags: needinfo?(mozilla)
Attached patch Use ASCII when UTF8 fails (obsolete) — — Splinter Review 
I should have done this in the first place.

I didn't realize there was so much use of this.
Attachment #8665463 - Flags: review?(dtownsend)
Assignee: nobody → mozilla
Status: NEW → UNCONFIRMED
Ever confirmed: false
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8665463 [details] [diff] [review]
Use ASCII when UTF8 fails

Review of attachment 8665463 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not sure that this is the right fix. Looks like the EvalInSandboxObject call will fail if the script throws an exception, even if that is unrelated to encoding issues which would mean you could end up running the script twice in some cases. Is that a problem? Is there a way we can check the scripts encoding early, maybe by compiling it in the JS engine without running up front or something?
> I'm not sure that this is the right fix. Looks like the EvalInSandboxObject call will fail if the script throws an exception, even if that is unrelated to encoding issues which would mean you could end up running the script twice in some cases. Is that a problem?

That's a good point.

> Is there a way we can check the scripts encoding early, maybe by compiling it in the JS engine without running up front or something?

I wonder if I can just run the conversion as a standalone thing?
Blake: Any idea how to solve this? See https://bugzilla.mozilla.org/show_bug.cgi?id=1193625#c26

Thanks.
Flags: needinfo?(mrbkap)
The AppendUTF8toUTF16 fails here:

http://mxr.mozilla.org/mozilla-central/source/xpcom/string/nsReadableUtils.cpp#250

but there doesn't appear to be a way to detect the error at that point...
So it looks like the conversion does fail and we get a 0 length string back.

I've tested this repeatedly in and out of the debugger and it's working for me.

Note that this only affects AutoConfig.
Attachment #8665463 - Attachment is obsolete: true
Attachment #8665463 - Flags: review?(dtownsend)
Attachment #8665563 - Flags: review?(dtownsend)
Comment on attachment 8665563 [details] [diff] [review]
Fallback to ASCII if conversion fails

Review of attachment 8665563 [details] [diff] [review]:
-----------------------------------------------------------------

Ok that looks safer. Thanks.

::: extensions/pref/autoconfig/src/nsJSConfigTriggers.cpp
@@ +113,5 @@
> +      rv = xpc->EvalInSandboxObject(NS_ConvertASCIItoUTF16(script), filename, cx,
> +                                    autoconfigSb, JSVERSION_LATEST, &v);
> +    } else {
> +      rv = xpc->EvalInSandboxObject(utf8Script, filename, cx,
> +                                    autoconfigSb, JSVERSION_LATEST, &v);        

Some trailing whitespace here
Attachment #8665563 - Flags: review?(dtownsend) → review+
I'd probably try a little harder to avoid duplicating the rest of those arguments in the call. That being said, I wonder if we can deprecate ANSI encoding in favor of UTF-8 (as is happening elsewhere on the web) since even with this fix, I think we risk misinterpreting some of the values in these files.
Flags: needinfo?(mrbkap)
Attached patch 1193625.diff — — Splinter Review 
Per mrbkap's suggestion, make one call to the API and add a deprecation warning.
Attachment #8666012 - Flags: review?(mrbkap)
Attachment #8666012 - Flags: review?(mrbkap) → review+
(In reply to enz-coast from comment #9)
> Error still present with 40.0.2. Will there be a Bugfix for the 40.x
> version-tree or do we have to sit this bug out and live with the old 39.0.3
> version?
> 
> this is our override.ini:
> [XRE]
> EnableProfileMigrator=false
> [/align]
> 
> this is our policies.js:
> pref("general.config.obscure_value", 0);
> pref("general.config.filename", "mozilla.cfg");

I've been using a .cfg and lockPrefs for the better part of 10 years (based on instructions originally from (http://ilias.ca/blog/2005/03/locking-mozilla-firefox-settings/). I do not put my pref("general.config.filename", "mozilla.cfg"); in policies.js. I use all.js located in \defaults\prefs.

> this is our mozilla.cfg: ( servernames etc are altered in favour of privacy )
> lockPref("browser.startup.homepage_override.mstone", "ignore");
> lockPref("browser.shell.checkDefaultBrowser", false);
> lockPref("app.update.enabled", false);
> lockPref("browser.startup.homepage", "https://xxx.xxx.xxx.xxx.de");
> lockPref("browser.startup.page", 1);
> lockPref("network.proxy.type", 2);
> lockPref("network.proxy.autoconfig_url",
> "https://xxx.xxx.xxx.xxx.xxx.de/squid.pac");
> lockPref("network.negotiate-auth.allow-proxies", false);
> lockPref("toolkit.telemetry.enabled", false);
> lockPref("toolkit.telemetry.rejected", true);
> lockPref("toolkit.telemetry.prompted", 2);
> lockPref("datareporting.healthreport.service.firstRun", true);
> lockPref("datareporting.healthreport.uploadEnabled", false);
> lockPref("browser.cache.disk.capacity", 100000);
> lockPref("browser.cache.disk.smart_size.enabled", false);
> lockPref("browser.cache.disk.smart_size.first_run", false);
> pref("browser.sessionstore.resume_from_crash", false);
> pref("browser.rights.3.shown", true);
> 
> Help and/or answers from an official source regarding a bugfix/new version
> would be very helpful.

I didn't experience any .cgf bustage with 40.0.x or 41.0.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4a33a9b8c218f3c1bd3ce83c8178f67396b9296
Bug 1193625 - Fallback to ASCII if AutoConfig UTF-8 conversion fails. r=Mossop, mrbkap
https://hg.mozilla.org/mozilla-central/rev/b4a33a9b8c21
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Mike, I think we want to uplift that to aurora & beta, could you fill the uplift request? Thanks
Flags: needinfo?(mozilla)
Comment on attachment 8666012 [details] [diff] [review]
1193625.diff

Approval Request Comment
[Feature/regressing bug #]: bug 1137799
[User impact if declined]: AutoConfig for Firefox doesn't work in certain cases where international characters are involved.
[Describe test coverage new/current, TreeHerder]: Unfortunately AutoConfig is not tested properly (due to the difficulty of writing testcases), but I have tested this multiple times and multiple ways verifying the fix works.
[Risks and why]: Very low risk. It is AutoConfig only and adds a fallback case.
[String/UUID change made/needed]: None.

I've added release as well just in case there is a chemspill.
Flags: needinfo?(mozilla)
Attachment #8666012 - Flags: approval-mozilla-release?
Attachment #8666012 - Flags: approval-mozilla-beta?
Attachment #8666012 - Flags: approval-mozilla-aurora?
Comment on attachment 8666012 [details] [diff] [review]
1193625.diff

I don't think we will take in 41 but Ritu will make the call.
Thanks!
Attachment #8666012 - Flags: approval-mozilla-beta?
Attachment #8666012 - Flags: approval-mozilla-beta+
Attachment #8666012 - Flags: approval-mozilla-aurora?
Attachment #8666012 - Flags: approval-mozilla-aurora+
IMHO, it should be included in a possible chemspill release, because it affects corporate installations, especially non-English users using accented letters to comment lines in mozilla.cfg. And all these users don't have necessarily the idea to convert the file from ANSI to UTF-8.
Well it's been in two releases so far and it didn't product many tickets. My guess is most people worked around.

But if we do a chemspill, it would be nice to have.
enz-coast, Loic: Would you be able to confirm that the fix works as expected on the latest Nightly build? Thanks.
Flags: needinfo?(epinal99-bugzilla2)
Flags: needinfo?(enz-coast)
Yes, it works with the latest Nightly.


Homepage defined as "https://www.mozilla.org/".

Added the ANSI-encoded file "mozilla.cfg" to /firefox:
//Test: é
pref("browser.startup.homepage","intranet");

Restart. New homepage is "intranet".
Flags: needinfo?(epinal99-bugzilla2)
Comment on attachment 8666012 [details] [diff] [review]
1193625.diff

While this is a valid issue, at this point, we are considering bugs such as crashes/hangs, regressions that make Firefox unusable, data loss scenarios and major bugs without any workaround. Given that this issue has a workaround and can be circumvented by re-encoding the config file, we will have to wontfix this for 41. I hope we get a fix that is verified and stable in time for 42 release.
Attachment #8666012 - Flags: approval-mozilla-release? → approval-mozilla-release-
Comment on attachment 8665563 [details] [diff] [review]
Fallback to ASCII if conversion fails

Review of attachment 8665563 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/pref/autoconfig/src/nsJSConfigTriggers.cpp
@@ +106,5 @@
>  
>      nsAutoCString script(js_buffer, length);
>      JS::RootedValue v(cx);
> +
> +    nsString utf8Script = NS_ConvertUTF8toUTF16(script);

This should be NS_ConvertUTF8toUTF16 utf8Script(script);
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: