Rewrite client side CSS sanitization code.

Prior to this CL we turned CSS signatures into regular expressions.
This code was tricky, required shipping lots of (opaque and hard-to-debug) JS,
and the regular expressions blew up in size when functions (like rgb(...))
nest.

This changes our CSS sanitization to a simple token filtering, but with
functions treated as top-level entities.

Changes include
- Getting rid of JS regex generation and optimization code
- Revamping the CSS property bits to support unicode ranges, and make the
  distinction between quoted strings, URLs, and unreserved-words more obvious.
- Add a cssFns property to the css-defs.js property value maps.
- Gets rid of the cssExtras property in the css-defs.js property value maps.
- Gets rid of z-index special cases.
- Reworks sanitize-css.js and clients to use the new format.
  sanitizeCssProperty no longer takes a schema and just assumes cssSchema is
  present in the same way sanitizeStylesheet does.

Metrics
I measured the size of the JS before and after.

                       Before     After      Relative
Generated JS           43.75 kB   22.31 kB   51%
Minified Bundle JS     89.42 kB   69.52 kB   78%
GZipped JS              4.74 kB    4.18 kB   88%
GZipped Bundle JS      19.51 kB   18.85 kB   97%

So the generated JS schema went from half of our sanitizer bundle to a
third, and should scale linearly with the size of our schema, and not
explode as we flesh out CSS3.

----

Submitted @ r5493

[MikeSamuel, 2013-07-10 20:05:16]

Prior to this CL we turned CSS signatures into regular expressions.
This code was tricky, required shipping lots of (opaque and hard-to-debug) JS,
and the regular expressions blew up in size when functions (like rgb(...))
nest.

This changes our CSS sanitization to a simple token filtering, but with
functions treated as top-level entities.

Changes include
- Getting rid of JS regex generation and optimization code
- Revamping the CSS property bits to support unicode ranges, and make the
  distinction between quoted strings, URLs, and unreserved-words more obvious.
- Add a cssFns property to the css-defs.js property value maps.
- Gets rid of the cssExtras property in the css-defs.js property value maps.

NOT READY FOR COMMIT
This includes a rewrite of the code-generator which turns our CSS JSON into
a form that the client-side CSS rewriter can use but does not yet wire it
into sanitizecss.js.

[MikeSamuel, 2013-07-10 20:09:39]

Prior to this CL we turned CSS signatures into regular expressions.
This code was tricky, required shipping lots of (opaque and hard-to-debug) JS,
and the regular expressions blew up in size when functions (like rgb(...))
nest.

This changes our CSS sanitization to a simple token filtering, but with
functions treated as top-level entities.

Changes include
- Getting rid of JS regex generation and optimization code
- Revamping the CSS property bits to support unicode ranges, and make the
  distinction between quoted strings, URLs, and unreserved-words more obvious.
- Add a cssFns property to the css-defs.js property value maps.
- Gets rid of the cssExtras property in the css-defs.js property value maps.

NOT READY FOR COMMIT
This includes a rewrite of the code-generator which turns our CSS JSON into
a form that the client-side CSS rewriter can use but does not yet wire it
into sanitizecss.js.

[MikeSamuel, 2013-07-10 20:16:04]

Prior to this CL we turned CSS signatures into regular expressions.
This code was tricky, required shipping lots of (opaque and hard-to-debug) JS,
and the regular expressions blew up in size when functions (like rgb(...))
nest.

This changes our CSS sanitization to a simple token filtering, but with
functions treated as top-level entities.

Changes include
- Getting rid of JS regex generation and optimization code
- Revamping the CSS property bits to support unicode ranges, and make the
  distinction between quoted strings, URLs, and unreserved-words more obvious.
- Add a cssFns property to the css-defs.js property value maps.
- Gets rid of the cssExtras property in the css-defs.js property value maps.

NOT READY FOR COMMIT
This includes a rewrite of the code-generator which turns our CSS JSON into
a form that the client-side CSS rewriter can use but does not yet wire it
into sanitizecss.js.

[felix8a, 2013-07-11 22:33:36]

mostly looks fine so far

https://codereview.appspot.com/10943044/diff/14001/src/com/google/caja/lang/c...
File src/com/google/caja/lang/css/CssPropertyPatterns.java (right):

https://codereview.appspot.com/10943044/diff/14001/src/com/google/caja/lang/c...
src/com/google/caja/lang/css/CssPropertyPatterns.java:199: } else if (sig
instanceof CssPropertySignature.ProgIdSignature) {
I'm ok with dropping support for progid, it was really only for lack of alpha
support in old IE, and that's not an issue any more.

https://codereview.appspot.com/10943044/diff/14001/src/com/google/caja/plugin...
File src/com/google/caja/plugin/CssRewriter.java (right):

https://codereview.appspot.com/10943044/diff/14001/src/com/google/caja/plugin...
src/com/google/caja/plugin/CssRewriter.java:429: if
(!CSS21_COLOR_NAMES.contains(colorName.getCanonicalForm())) {
Is there a reason to whitelist color names instead of just allowing any
identifier?

https://codereview.appspot.com/10943044/diff/14001/tests/com/google/caja/lang...
File tests/com/google/caja/lang/css/CssPropertyPatternsTest.java (right):

https://codereview.appspot.com/10943044/diff/14001/tests/com/google/caja/lang...
tests/com/google/caja/lang/css/CssPropertyPatternsTest.java:80: public final
void testKeywordPatternsOverlyLoose() throws ParseException {
FailureIsAnOption is for things that should eventually be fixed. I think we
aren't interested in making 'zoicks' mismatch 'zoicks zoicks', so either this
should be an assertMatches, or it should be deleted. and similarly for all the
other OverlyLoose tests.

https://codereview.appspot.com/10943044/diff/14001/tests/com/google/caja/lang...
tests/com/google/caja/lang/css/CssPropertyPatternsTest.java:258: private void
assertMatch(
it seems strange to have all this matching logic in the test class. why test a
pattern-matching engine that only exists in test code?

https://codereview.appspot.com/10943044/diff/14001/tests/com/google/caja/lang...
tests/com/google/caja/lang/css/CssPropertyPatternsTest.java:273: for (boolean
adjacent = false; lexer.hasNext();) {
this seems like an odd use of for(), how about change it to while()?

[MikeSamuel, 2013-07-12 04:56:17]

Prior to this CL we turned CSS signatures into regular expressions.
This code was tricky, required shipping lots of (opaque and hard-to-debug) JS,
and the regular expressions blew up in size when functions (like rgb(...))
nest.

This changes our CSS sanitization to a simple token filtering, but with
functions treated as top-level entities.

Changes include
- Getting rid of JS regex generation and optimization code
- Revamping the CSS property bits to support unicode ranges, and make the
  distinction between quoted strings, URLs, and unreserved-words more obvious.
- Add a cssFns property to the css-defs.js property value maps.
- Gets rid of the cssExtras property in the css-defs.js property value maps.

NOT READY FOR COMMIT
This includes a rewrite of the code-generator which turns our CSS JSON into
a form that the client-side CSS rewriter can use but does not yet wire it
into sanitizecss.js.

[MikeSamuel, 2013-07-12 19:36:15]

Prior to this CL we turned CSS signatures into regular expressions.
This code was tricky, required shipping lots of (opaque and hard-to-debug) JS,
and the regular expressions blew up in size when functions (like rgb(...))
nest.

This changes our CSS sanitization to a simple token filtering, but with
functions treated as top-level entities.

Changes include
- Getting rid of JS regex generation and optimization code
- Revamping the CSS property bits to support unicode ranges, and make the
  distinction between quoted strings, URLs, and unreserved-words more obvious.
- Add a cssFns property to the css-defs.js property value maps.
- Gets rid of the cssExtras property in the css-defs.js property value maps.

NOT READY FOR COMMIT
This includes a rewrite of the code-generator which turns our CSS JSON into
a form that the client-side CSS rewriter can use but does not yet wire it
into sanitizecss.js.

[MikeSamuel, 2013-07-12 19:45:58]

https://codereview.appspot.com/10943044/diff/14001/src/com/google/caja/lang/c...
File src/com/google/caja/lang/css/CssPropertyPatterns.java (right):

https://codereview.appspot.com/10943044/diff/14001/src/com/google/caja/lang/c...
src/com/google/caja/lang/css/CssPropertyPatterns.java:199: } else if (sig
instanceof CssPropertySignature.ProgIdSignature) {
On 2013/07/11 22:33:36, felix8a wrote:
> I'm ok with dropping support for progid, it was really only for lack of alpha
> support in old IE, and that's not an issue any more.

Added a comment to that effect, but I'd rather not excise existing progid
support in the schema parser in this CL.

https://codereview.appspot.com/10943044/diff/14001/src/com/google/caja/plugin...
File src/com/google/caja/plugin/CssRewriter.java (right):

https://codereview.appspot.com/10943044/diff/14001/src/com/google/caja/plugin...
src/com/google/caja/plugin/CssRewriter.java:429: if
(!CSS21_COLOR_NAMES.contains(colorName.getCanonicalForm())) {
On 2013/07/11 22:33:36, felix8a wrote:
> Is there a reason to whitelist color names instead of just allowing any
> identifier?

I'd rather not white-list any identifier, though I don't see any reason not to
whitelist any in the CSS3 set now that more browsers are supporting it.

Some properties can contain "/" as a token.  "font" comes to mind.  I wouldn't
want that combined with an unrecognized color to masquerade as an unquoted URL.

Should I get rid of this CSS2/CSS3 distinction here or in a follow-on URL?

https://codereview.appspot.com/10943044/diff/14001/tests/com/google/caja/lang...
File tests/com/google/caja/lang/css/CssPropertyPatternsTest.java (right):

https://codereview.appspot.com/10943044/diff/14001/tests/com/google/caja/lang...
tests/com/google/caja/lang/css/CssPropertyPatternsTest.java:80: public final
void testKeywordPatternsOverlyLoose() throws ParseException {
On 2013/07/11 22:33:36, felix8a wrote:
> FailureIsAnOption is for things that should eventually be fixed. I think we
> aren't interested in making 'zoicks' mismatch 'zoicks zoicks', so either this
> should be an assertMatches, or it should be deleted. and similarly for all the
> other OverlyLoose tests.

Deleted all.

https://codereview.appspot.com/10943044/diff/14001/tests/com/google/caja/lang...
tests/com/google/caja/lang/css/CssPropertyPatternsTest.java:258: private void
assertMatch(
On 2013/07/11 22:33:36, felix8a wrote:
> it seems strange to have all this matching logic in the test class. why test a
> pattern-matching engine that only exists in test code?

Yeah.  Being able to test patterns that don't appear in the CSS schema helps
explore corner-cases in CssPropertyPatterns, but it is duplicating some portion
of sanitizecss's logic.

https://codereview.appspot.com/10943044/diff/14001/tests/com/google/caja/lang...
tests/com/google/caja/lang/css/CssPropertyPatternsTest.java:273: for (boolean
adjacent = false; lexer.hasNext();) {
On 2013/07/11 22:33:36, felix8a wrote:
> this seems like an odd use of for(), how about change it to while()?

Done.

[felix8a, 2013-07-12 19:47:29]

https://codereview.appspot.com/10943044/diff/14001/src/com/google/caja/plugin...
File src/com/google/caja/plugin/CssRewriter.java (right):

https://codereview.appspot.com/10943044/diff/14001/src/com/google/caja/plugin...
src/com/google/caja/plugin/CssRewriter.java:429: if
(!CSS21_COLOR_NAMES.contains(colorName.getCanonicalForm())) {
On 2013/07/12 19:45:59, MikeSamuel wrote:
> On 2013/07/11 22:33:36, felix8a wrote:
> > Is there a reason to whitelist color names instead of just allowing any
> > identifier?
> 
> I'd rather not white-list any identifier, though I don't see any reason not to
> whitelist any in the CSS3 set now that more browsers are supporting it.
> 
> Some properties can contain "/" as a token.  "font" comes to mind.  I wouldn't
> want that combined with an unrecognized color to masquerade as an unquoted
URL.
> 
> Should I get rid of this CSS2/CSS3 distinction here or in a follow-on URL?

either way is fine with me

[MikeSamuel, 2013-07-12 19:58:24]

Prior to this CL we turned CSS signatures into regular expressions.
This code was tricky, required shipping lots of (opaque and hard-to-debug) JS,
and the regular expressions blew up in size when functions (like rgb(...))
nest.

This changes our CSS sanitization to a simple token filtering, but with
functions treated as top-level entities.

Changes include
- Getting rid of JS regex generation and optimization code
- Revamping the CSS property bits to support unicode ranges, and make the
  distinction between quoted strings, URLs, and unreserved-words more obvious.
- Add a cssFns property to the css-defs.js property value maps.
- Gets rid of the cssExtras property in the css-defs.js property value maps.
- Gets rid of z-index special cases.
- Reworks sanitize-css.js and clients to use the new format.
  sanitizeCssProperty no longer takes a schema and just assumes cssSchema is
  present in the same way sanitizeStylesheet does.

Metrics
I measured the size of the JS before and after.

                       Before     After      Relative
Generated JS           43.75 kB   22.31 kB   51%
Minified Bundle JS     89.42 kB   69.52 kB   78%
GZipped JS              4.74 kB    4.18 kB   88%
GZipped Bundle JS      19.51 kB   18.85 kB   97%

So the generated JS schema went from half of our sanitizer bundle to a
third, and should scale linearly with the size of our schema, and not
explode as we flesh out CSS3.

[felix8a, 2013-07-15 20:07:11]

lgtm

https://codereview.appspot.com/10943044/diff/30001/src/com/google/caja/plugin...
File src/com/google/caja/plugin/sanitizecss.js (right):

https://codereview.appspot.com/10943044/diff/30001/src/com/google/caja/plugin...
src/com/google/caja/plugin/sanitizecss.js:188: // to treate ['Arial', 'Black']
equivalently to ['"Arial Black"'].
typo "treate"

[MikeSamuel, 2013-07-15 20:17:41]

Prior to this CL we turned CSS signatures into regular expressions.
This code was tricky, required shipping lots of (opaque and hard-to-debug) JS,
and the regular expressions blew up in size when functions (like rgb(...))
nest.

This changes our CSS sanitization to a simple token filtering, but with
functions treated as top-level entities.

Changes include
- Getting rid of JS regex generation and optimization code
- Revamping the CSS property bits to support unicode ranges, and make the
  distinction between quoted strings, URLs, and unreserved-words more obvious.
- Add a cssFns property to the css-defs.js property value maps.
- Gets rid of the cssExtras property in the css-defs.js property value maps.
- Gets rid of z-index special cases.
- Reworks sanitize-css.js and clients to use the new format.
  sanitizeCssProperty no longer takes a schema and just assumes cssSchema is
  present in the same way sanitizeStylesheet does.

Metrics
I measured the size of the JS before and after.

                       Before     After      Relative
Generated JS           43.75 kB   22.31 kB   51%
Minified Bundle JS     89.42 kB   69.52 kB   78%
GZipped JS              4.74 kB    4.18 kB   88%
GZipped Bundle JS      19.51 kB   18.85 kB   97%

So the generated JS schema went from half of our sanitizer bundle to a
third, and should scale linearly with the size of our schema, and not
explode as we flesh out CSS3.

[MikeSamuel, 2013-07-15 20:21:40]

Prior to this CL we turned CSS signatures into regular expressions.
This code was tricky, required shipping lots of (opaque and hard-to-debug) JS,
and the regular expressions blew up in size when functions (like rgb(...))
nest.

This changes our CSS sanitization to a simple token filtering, but with
functions treated as top-level entities.

Changes include
- Getting rid of JS regex generation and optimization code
- Revamping the CSS property bits to support unicode ranges, and make the
  distinction between quoted strings, URLs, and unreserved-words more obvious.
- Add a cssFns property to the css-defs.js property value maps.
- Gets rid of the cssExtras property in the css-defs.js property value maps.
- Gets rid of z-index special cases.
- Reworks sanitize-css.js and clients to use the new format.
  sanitizeCssProperty no longer takes a schema and just assumes cssSchema is
  present in the same way sanitizeStylesheet does.

Metrics
I measured the size of the JS before and after.

                       Before     After      Relative
Generated JS           43.75 kB   22.31 kB   51%
Minified Bundle JS     89.42 kB   69.52 kB   78%
GZipped JS              4.74 kB    4.18 kB   88%
GZipped Bundle JS      19.51 kB   18.85 kB   97%

So the generated JS schema went from half of our sanitizer bundle to a
third, and should scale linearly with the size of our schema, and not
explode as we flesh out CSS3.

[MikeSamuel, 2013-07-15 20:21:59]

https://codereview.appspot.com/10943044/diff/30001/src/com/google/caja/plugin...
File src/com/google/caja/plugin/sanitizecss.js (right):

https://codereview.appspot.com/10943044/diff/30001/src/com/google/caja/plugin...
src/com/google/caja/plugin/sanitizecss.js:188: // to treate ['Arial', 'Black']
equivalently to ['"Arial Black"'].
On 2013/07/15 20:07:12, felix8a wrote:
> typo "treate"

Done.