Граф коммитов

5 Коммитов

Автор SHA1 Сообщение Дата
Nicholas Nethercote 038a72de3b Bug 440908 - Add support for `sticky` and `locked` attributes to default prefs. r=glandium
Sticky prefs are already specifiable with `sticky_pref`, but this is a more
general attribute mechanism. The ability to specify a locked pref in the data
file is new.

The patch also adds nsIPrefService.readDefaultPrefsFromFile, to match the
existing nsIPrefService.readUserPrefsFromFile method, and converts a number of
the existing tests to use it.

MozReview-Commit-ID: 9LLMBJVZfg7

--HG--
extra : rebase_source : fa25bad87c4d9fcba6dc13cd2cc04ea6a2354f51
2018-03-02 15:31:40 +11:00
Nicholas Nethercote 135442b9ce Bug 107264 - Add error recovery to the prefs parser. r=glandium
This was first suggested 17 years ago!

The error recovery works by just scanning forward for the next ';' token.

This change allows a lot of the gtest tests to be combined.

MozReview-Commit-ID: CbZ2OFtdIxf

--HG--
extra : rebase_source : 5a43fff06e88b45a095725856bbe1e6b5470c9a0
2018-02-21 08:29:23 +11:00
Nicholas Nethercote eeb14c6c69 Bug 1423840 (attempt 2) - Rewrite the prefs parser. r=glandium,Manishearth
The prefs parser has two significant problems.

- It doesn't separate tokenizing from parsing.

- It is implemented as a loop around a big switch on a "current state"
  variable.

As a result, it is hard to understand and modify, slower than it could be, and
in obscure cases (involving comments and whitespace) it fails to parse what
should be valid input.

This patch replaces it with a recursive descent parser (albeit one without any
recursion!) that has separate tokenization. The new parser is easier to
understand and modify, more correct, and has better error messages. It doesn't
do error recovery, but that would be much easier to add than in the old parser.

The new parser also runs about 1.9x faster than the existing parser. (As
measured by parsing greprefs.js's contents from memory 1000 times in
succession, omitting the prefs hash table construction. If the table
construction is included, it's about 1.6x faster.)

The new parser is slightly stricter than the old parser in a few ways.

- Disconcertingly, the old parser allowed arbitrary junk between prefs
  (including at the start and end of the prefs file) so long as that junk
  didn't include any of the following chars: '/', '#', 'u', 's', 'p'. I.e.
  lines like these:

    !foo@bar&pref("prefname", true);
    ticky_pref("prefname", true);       // missing 's' at start
    User_pref("prefname", true);        // should be 'u' at start

  would all be treated the same as this:

    pref("prefname", true);

  The new parser disallows such junk because it isn't necessary and seems like
  an unintentional botch by the old parser.

- The old parser allowed character 0x1a (SUB) between tokens and treated it
  like '\n'.

  The new parser does not allow this character. SUB was used to indicate
  end-of-file (*not* end-of-line) in some old operating systems such as MS-DOS,
  but this doesn't seem necessary today.

- The old parser tolerated (with a warning) invalid escape sequences within
  string literals -- such as "\q" (not a valid escape) and "\x1" and "\u12"
  (both of which have insufficient hex digits) -- accepting them literally.

  The new parser does not tolerate invalid escape sequences because it doesn't
  seem necessary and would complicate things.

- The old parser tolerated character 0x00 (NUL) within string literals; this is
  dangerous because C++ code that manipulates string values with embedded NULs
  will almost certainly consider those chars as end-of-string markers.

  The new parser treats NUL chars as end-of-file, to avoid this danger and
  because it facilitates a significant optimization (described within the
  code).

- The old parser allowed integer literals to overflow, silently wrapping them.

  The new parser treats integer overflow as a parse error. This seems better,
  and it caught existing overflows of places.database.lastMaintenance, in
  testing/profiles/prefs_general.js (bug 1424030) and
  testing/talos/talos/config.py (bug 1434813).

The first of these changes meant that a couple of existing prefs with ";;" at
the end had to be changed (done in the preceding patch).

The minor increase in strictness shouldn't be a problem for default pref files
such as greprefs.js within the application (which we can modify), nor for
app-written prefs files such as prefs.js. It could affect user-written prefs
files such as user.js; the experience above suggests that integer overflow and
";;" are the most likely problems in practice. In my opinion, the risk here is
acceptable.

The new parser also does a better job of tracking line numbers because it (a)
treats "\r\n" sequences as a single end-of-line marker, and (a) pays attention
to end-of-line sequences within string literals.

Finally, the patch adds thorough tests of both valid and invalid syntax.

MozReview-Commit-ID: JD3beOQl4AJ
2018-02-01 16:21:47 +11:00
Cosmin Sabou 9efa17a39e Backed out 2 changesets (bug 1423840) for mass Talos failures due to forbidden connections. CLOSED TREE
Backed out changeset e8b798a5205a (bug 1423840)
Backed out changeset e500592d3551 (bug 1423840)
2018-02-01 03:05:08 +02:00
Nicholas Nethercote 67e80b725b Bug 1423840 - Rewrite the prefs parser. r=glandium,Manishearth
The prefs parser has two significant problems.

- It doesn't separate tokenizing from parsing.

- It is implemented as a loop around a big switch on a "current state"
  variable.

As a result, it is hard to understand and modify, slower than it could be, and
in obscure cases (involving comments and whitespace) it fails to parse what
should be valid input.

This patch replaces it with a recursive descent parser (albeit one without any
recursion!) that has separate tokenization. The new parser is easier to
understand and modify, more correct, and has better error messages. It doesn't
do error recovery, but that would be much easier to add than in the old parser.

The new parser also runs about 1.9x faster than the existing parser. (As
measured by parsing greprefs.js's contents from memory 1000 times in
succession, omitting the prefs hash table construction. If the table
construction is included, it's about 1.6x faster.)

The new parser is slightly stricter than the old parser in a few ways.

- Disconcertingly, the old parser allowed arbitrary junk between prefs
  (including at the start and end of the prefs file) so long as that junk
  didn't include any of the following chars: '/', '#', 'u', 's', 'p'. I.e.
  lines like these:

    !foo@bar&pref("prefname", true);
    ticky_pref("prefname", true);       // missing 's' at start
    User_pref("prefname", true);        // should be 'u' at start

  would all be treated the same as this:

    pref("prefname", true);

  The new parser disallows such junk because it isn't necessary and seems like
  an unintentional botch by the old parser.

- The old parser allowed character 0x1a (SUB) between tokens and treated it
  like '\n'.

  The new parser does not allow this character. SUB was used to indicate
  end-of-file (*not* end-of-line) in some old operating systems such as MS-DOS,
  but this doesn't seem necessary today.

- The old parser tolerated (with a warning) invalid escape sequences within
  string literals -- such as "\q" (not a valid escape) and "\x1" and "\u12"
  (both of which have insufficient hex digits) -- accepting them literally.

  The new parser does not tolerate invalid escape sequences because it doesn't
  seem necessary and would complicate things.

- The old parser tolerated character 0x00 (NUL) within string literals; this is
  dangerous because C++ code that manipulates string values with embedded NULs
  will almost certainly consider those chars as end-of-string markers.

  The new parser treats NUL chars as end-of-file, to avoid this danger and
  because it facilitates a significant optimization (described within the
  code).

- The old parser allowed integer literals to overflow, silently wrapping them.

  The new parser treats integer overflow as a parse error. This seems better,
  and it caught an existing overflow in testing/profiles/prefs_general.js, for
  places.database.lastMaintenance (see bug 1424030).

The first of these changes meant that a couple of existing prefs with ";;" at
the end had to be changed (done in the preceding patch).

The minor increase in strictness shouldn't be a problem for default pref files
such as greprefs.js within the application (which we can modify), nor for
app-written prefs files such as prefs.js. It could affect user-written prefs
files such as user.js; the experience above suggests that ";;" is the most
likely problem in practice. In my opinion, the risk here is acceptable.

The new parser also does a better job of tracking line numbers because it (a)
treats "\r\n" sequences as a single end-of-line marker, and (a) pays attention
to end-of-line sequences within string literals.

Finally, the patch adds thorough tests of both valid and invalid syntax.

MozReview-Commit-ID: 8EYWH7KxGG
* * *
[mq]: win-fix

MozReview-Commit-ID: 91Bxjfghqfw

--HG--
extra : rebase_source : a8773413e5d68c33e4329df6819b6e1f82c22b85
2017-12-03 00:26:36 +11:00