Bug 1498497 - Stop using eval in TBDistCustomizer.jsm to match Firefox. r=jorgk
authoraceman <acelists@atlas.sk>
Thu, 24 Jan 2019 13:11:00 +0100
changeset 34298 0a405faf8b1dd4ed743633103fadbdaa93e9d86a
parent 34297 881cf0d594f227667269c03cc35a09959d5f700d
child 34299 376574bc14becba3bf53d5c3491dc0a0ffc56985
push id389
push userclokep@gmail.com
push dateMon, 18 Mar 2019 19:01:53 +0000
reviewersjorgk
bugs1498497
Bug 1498497 - Stop using eval in TBDistCustomizer.jsm to match Firefox. r=jorgk
mail/base/modules/TBDistCustomizer.jsm
mail/base/test/unit/test_mailGlue_distribution.js
--- a/mail/base/modules/TBDistCustomizer.jsm
+++ b/mail/base/modules/TBDistCustomizer.jsm
@@ -1,15 +1,12 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-// This file uses eval, but really shouldn't. See bug 1498497.
-/* eslint-disable no-eval */
-
 this.EXPORTED_SYMBOLS = ["TBDistCustomizer"];
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 var TBDistCustomizer = {
   applyPrefDefaults() {
     this._prefDefaultsApplied = true;
@@ -47,17 +44,17 @@ var TBDistCustomizer = {
     defaults.setStringPref("distribution.about", partnerAbout);
 
     if (sections.Preferences) {
       let keys = this._ini.getKeys("Preferences");
       while (keys.hasMore()) {
         let key = keys.getNext();
         try {
           // Get the string value of the key
-          let value = eval(this._ini.getString("Preferences", key));
+          let value = this.parseValue(this._ini.getString("Preferences", key));
           // After determining what type it is, set the pref
           switch (typeof value) {
           case "boolean":
             defaults.setBoolPref(key, value);
             break;
           case "number":
             defaults.setIntPref(key, value);
             break;
@@ -71,53 +68,61 @@ var TBDistCustomizer = {
           }
         } catch (e) {
           Cu.reportError(e);
         }
       }
     }
 
     // Set the prefs in the other sections
-
-    // We eval() the localizable prefs as well (even though they'll
-    // always get set as a string) to keep the INI format consistent:
-    // string prefs always need to be in quotes
-
     let localizedStr = Cc["@mozilla.org/pref-localizedstring;1"]
                          .createInstance(Ci.nsIPrefLocalizedString);
 
     if (sections.LocalizablePreferences) {
       let keys = this._ini.getKeys("LocalizablePreferences");
       while (keys.hasMore()) {
         let key = keys.getNext();
         try {
-          let value = eval(this._ini.getString("LocalizablePreferences", key));
+          let value = this.parseValue(this._ini.getString("LocalizablePreferences", key));
           value = value.replace(/%LOCALE%/g, this._locale);
           localizedStr.data = "data:text/plain," + key + "=" + value;
           defaults.setComplexValue(key, Ci.nsIPrefLocalizedString, localizedStr);
         } catch (e) {
           Cu.reportError(e);
         }
       }
     }
 
     if (sections["LocalizablePreferences-" + this._locale]) {
       let keys = this._ini.getKeys("LocalizablePreferences-" + this._locale);
       while (keys.hasMore()) {
         let key = keys.getNext();
         try {
-          let value = eval(this._ini.getString("LocalizablePreferences-" + this._locale, key));
+          let value = this.parseValue(this._ini.getString("LocalizablePreferences-" + this._locale, key));
           localizedStr.data = "data:text/plain," + key + "=" + value;
           defaults.setComplexValue(key, Ci.nsIPrefLocalizedString, localizedStr);
         } catch (e) {
           Cu.reportError(e);
         }
       }
     }
   },
+
+  parseValue(value) {
+    try {
+      value = JSON.parse(value);
+    } catch (e) {
+      // JSON.parse catches numbers and booleans.
+      // Anything else, we assume is a string.
+      // Remove the quotes that aren't needed anymore.
+      value = value.replace(/^"/, "");
+      value = value.replace(/"$/, "");
+    }
+    return value;
+  },
 };
 
 XPCOMUtils.defineLazyGetter(TBDistCustomizer, "_ini", function() {
   let ini = null;
   let iniFile = Services.dirsvc.get("XCurProcD", Ci.nsIFile);
   iniFile.append("distribution");
   iniFile.append("distribution.ini");
   if (iniFile.exists()) {
--- a/mail/base/test/unit/test_mailGlue_distribution.js
+++ b/mail/base/test/unit/test_mailGlue_distribution.js
@@ -1,11 +1,8 @@
-// This file uses eval, but really shouldn't. See bug 1498497.
-/* eslint-disable no-eval */
-
 ChromeUtils.import("resource:///modules/TBDistCustomizer.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 function run_test() {
   do_test_pending();
 
   Services.locale.requestedLocales = ["en-US"];
 
@@ -70,17 +67,17 @@ function run_test() {
   pref = Services.prefs.getCharPref("distribution.about");
   Assert.equal(aboutLocale, pref);
 
   // Test Preferences section
   let s = "Preferences";
   let keys = testIni.getKeys(s);
   while (keys.hasMore()) {
     let key = keys.getNext();
-    let value = eval(testIni.getString(s, key));
+    let value = TBDistCustomizer.parseValue(testIni.getString(s, key));
     switch (typeof value) {
     case "boolean":
         Assert.equal(value, Services.prefs.getBoolPref(key));
         break;
     case "number":
         Assert.equal(value, Services.prefs.getIntPref(key));
         break;
     case "string":
@@ -93,30 +90,30 @@ function run_test() {
 
   // Test the LocalizablePreferences-[locale] section
   // Add any prefs found in it to the overrides array
   let overrides = [];
   s = "LocalizablePreferences-en-US";
   keys = testIni.getKeys(s);
   while (keys.hasMore()) {
     let key = keys.getNext();
-    let value = eval(testIni.getString(s, key));
+    let value = TBDistCustomizer.parseValue(testIni.getString(s, key));
     value = "data:text/plain," + key + "=" + value;
     Assert.equal(value, Services.prefs.getCharPref(key));
     overrides.push(key);
   }
 
   // Test the LocalizablePreferences section
   // Any prefs here that aren't found in overrides are not overridden
   //   by LocalizablePrefs-[locale] and should be tested
   s = "LocalizablePreferences";
   keys = testIni.getKeys(s);
   while (keys.hasMore()) {
     let key = keys.getNext();
     if (!overrides.includes(key)) {
-      let value = eval(testIni.getString(s, key));
+      let value = TBDistCustomizer.parseValue(testIni.getString(s, key));
       value =  value.replace(/%LOCALE%/g, "en-US");
       value = "data:text/plain," + key + "=" + value;
       Assert.equal(value, Services.prefs.getCharPref(key));
     }
   }
   do_test_finished();
 }