Bug 1632279 - Disallow default pref definitions in user pref files. r=KrisWright

We distinguish between two kinds of pref syntax.
- "Default pref files" are the ones that come with Firefox, constructed from
  `all.js` and similar files.
- "User pref files" are the ones that get created in the user's profile.
  `prefs.js` is the one that Firefox creates and overwrites every time a pref
  changes. `user.js` is the one that users can create themselves.

We also have two basic kinds of pref.
- Default: `pref(...)` and the unfortunate `sticky_pref(...)`.
- User: `user_pref(...)`, which override but don't replace the default.

It only makes sense for user pref files to contain user prefs; users shouldn't
be able to create default prefs or change default pref values.

But it turns out that user pref files have been able to define default prefs
pretty much forever. This appears to be an oversight, and this commit restricts
things so that user pref files cannot contain default prefs.

The commit also fixes an incorrect comment in testParser.js.

Differential Revision: https://phabricator.services.mozilla.com/D73003

modules/libpref/docs/index.md

@@ -181,6 +181,11 @@ reads it.
 These files are not JavaScript; the `.js` suffix is present for historical
 reasons. They are read by a custom parser within libpref.
 
+User pref file syntax is slightly more restrictive than default pref file
+syntax. In user pref files `user_pref` definitions are allowed but `pref` and
+`sticky_pref` definitions are not, and attributes (such as `locked`) are not
+allowed.
+
 **Problem:** geckodriver has a separate prefs parser in the mozprofile crate.
 
 **Problem:** there is no versioning of these files, for either the syntax or

modules/libpref/parser/src/lib.rs

@@ -10,7 +10,8 @@
 //!
 //! <pref-file>   = <pref>*
 //! <pref>        = <pref-spec> "(" <pref-name> "," <pref-value> <pref-attrs> ")" ";"
-//! <pref-spec>   = "user_pref" | "pref" | "sticky_pref"
+//! <pref-spec>   = "user_pref" | "pref" | "sticky_pref" // in default pref files
+//! <pref-spec>   = "user_pref"                          // in user pref files
 //! <pref-name>   = <string-literal>
 //! <pref-value>  = <string-literal> | "true" | "false" | <int-value>
 //! <int-value>   = <sign>? <int-literal>
@@ -373,14 +374,22 @@ impl<'t> Parser<'t> {
         loop {
             // <pref-spec>
             let (pref_value_kind, mut is_sticky) = match token {
-                Token::Pref => (PrefValueKind::Default, false),
-                Token::StickyPref => (PrefValueKind::Default, true),
+                Token::Pref if self.kind == PrefValueKind::Default => {
+                    (PrefValueKind::Default, false)
+                }
+                Token::StickyPref if self.kind == PrefValueKind::Default => {
+                    (PrefValueKind::Default, true)
+                }
                 Token::UserPref => (PrefValueKind::User, false),
                 Token::SingleChar(EOF) => return !self.has_errors,
                 _ => {
                     token = self.error_and_recover(
                         token,
-                        "expected pref specifier at start of pref definition",
+                        if self.kind == PrefValueKind::Default {
+                            "expected pref specifier at start of pref definition"
+                        } else {
+                            "expected 'user_pref' at start of pref definition"
+                        },
                     );
                     continue;
                 }

modules/libpref/test/gtest/Parser.cpp

@@ -374,8 +374,17 @@ pref("parse.error", true))",
   );
 
   USER(R"(
-pref("parse.error", true;
-pref("int.ok", 1);
+pref("parse.error", true);
+sticky_pref("parse.error", true);
+user_pref("int.ok", 1);
+    )",
+    "test:2: prefs parse error: expected 'user_pref' at start of pref definition\n"
+    "test:3: prefs parse error: expected 'user_pref' at start of pref definition\n"
+  );
+
+  USER(R"(
+user_pref("parse.error", true;
+user_pref("int.ok", 1);
     )",
     "test:2: prefs parse error: expected ')' after pref value\n"
   );
@@ -418,7 +427,7 @@ pref("parse.error", true)",
   );
 
   USER(R"(
-pref("parse.error", true)",
+user_pref("parse.error", true)",
     "test:2: prefs parse error: expected ')' after pref value\n"
   );
 

modules/libpref/test/unit/data/testParser.js

@@ -1,5 +1,5 @@
-// Note: this file tests only valid syntax (of user pref files, not default
-// pref files). See modules/libpref/test/gtest/Parser.cpp for tests if invalid
+// Note: this file tests only valid syntax (of default pref files, not user
+// pref files). See modules/libpref/test/gtest/Parser.cpp for tests of invalid
 // syntax.
 
 #