Draft of a sanitizer for CSS selectors in JavaScript

sanitizerSelectors(cssText) performs the same sanitization as
CssRewriter.  This CL is just a testfile that contains both the code
under test and the testcases in CssRewriterTest and does not attempt
to move the code into it's final location since I want to discuss
where this should fit as part of this CL.

One issue is whether CssRewriterTest.java should run its tests against the
JavaScript version.  To do that, we would need to

1. either load html-emitter.js and domado.js into Rhino to get the property
sanitization function in domado.js, OR

2. move the property sanitization out of domado.js so that less needs
to be pulled into Rhino to test equivalence of the java and javascript
CSS rewritings.

I will follow this CL shortly with another that wires
sanitizeSelectors into html-emitter.js.

[Jasvir, 2012-02-11 22:41:37]

LGTM

Pulling the selector sanitization functions out of domado seems like the way to
go for your testing question.

http://codereview.appspot.com/5649058/diff/1/src/com/google/caja/plugin/html-...
File src/com/google/caja/plugin/html-emitter.js (right):

http://codereview.appspot.com/5649058/diff/1/src/com/google/caja/plugin/html-...
src/com/google/caja/plugin/html-emitter.js:422: // } else if (atIdent ===
'@import') {
For the moment can we leave this uncommented and issue a console warning here
that @imports aren being deliberately elided.

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/cs...
File src/com/google/caja/plugin/cssparser.js (right):

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/cs...
src/com/google/caja/plugin/cssparser.js:123: handler.endAtrule(atident);
Are you using the argument anywhere?  function endAtrule doesn't seem to use it.

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/ht...
File src/com/google/caja/plugin/html-emitter.js (right):

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/ht...
src/com/google/caja/plugin/html-emitter.js:465: || 'head > html');
Clever.  It's still possible to construct something that this selector matches
but it's not unsafe.  It may be better to avoid this trick and just hold off
emitting the block until you knew if the any selector would actually match.

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/ht...
src/com/google/caja/plugin/html-emitter.js:510:
document.getElementsByTagName('head')[0].appendChild(
Are we guaranteed gEBTN("head").length > 0 on all supported browsers/docs?

http://codereview.appspot.com/5649058/diff/2001/tests/com/google/caja/plugin/...
File tests/com/google/caja/plugin/CssRewriterTest.java (right):

http://codereview.appspot.com/5649058/diff/2001/tests/com/google/caja/plugin/...
tests/com/google/caja/plugin/CssRewriterTest.java:497:
System.out.println("test:\n\t" + css.replace('\n', '
').replaceAll("[{][^}]*[}]", "\n\t").trim());
cruft

[MikeSamuel, 2012-02-13 20:46:08]

I reworked stuff and realized that there's no actual need to pull bits out of
Domado.  This is still tentative but there's more flesh on the skeleton now.  A
bit more work and I should be able to migrate some tests.

http://codereview.appspot.com/5649058/diff/1/src/com/google/caja/plugin/html-...
File src/com/google/caja/plugin/html-emitter.js (right):

http://codereview.appspot.com/5649058/diff/1/src/com/google/caja/plugin/html-...
src/com/google/caja/plugin/html-emitter.js:422: // } else if (atIdent ===
'@import') {
On 2012/02/11 22:41:37, Jasvir wrote:
> For the moment can we leave this uncommented and issue a console warning here
> that @imports aren being deliberately elided.

Done.

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/cs...
File src/com/google/caja/plugin/cssparser.js (right):

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/cs...
src/com/google/caja/plugin/cssparser.js:123: handler.endAtrule(atident);
On 2012/02/11 22:41:37, Jasvir wrote:
> Are you using the argument anywhere?  function endAtrule doesn't seem to use
it.

No.  I realized I needed to keep a stack after all.  Removed.

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/ht...
File src/com/google/caja/plugin/html-emitter.js (right):

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/ht...
src/com/google/caja/plugin/html-emitter.js:465: || 'head > html');
On 2012/02/11 22:41:37, Jasvir wrote:
> Clever.  It's still possible to construct something that this selector matches
> but it's not unsafe.  It may be better to avoid this trick and just hold off
> emitting the block until you knew if the any selector would actually match.

I added code to remember that this was done and later elide it.  The selector is
still used so the code on safeCss is always a prefix of a valid CSS string.

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/ht...
src/com/google/caja/plugin/html-emitter.js:510:
document.getElementsByTagName('head')[0].appendChild(
On 2012/02/11 22:41:37, Jasvir wrote:
> Are we guaranteed gEBTN("head").length > 0 on all supported browsers/docs?

As long as a <head>, <body> or </html> has been seen, yes.  The head is a
required but implied element.

http://codereview.appspot.com/5649058/diff/2001/tests/com/google/caja/plugin/...
File tests/com/google/caja/plugin/CssRewriterTest.java (right):

http://codereview.appspot.com/5649058/diff/2001/tests/com/google/caja/plugin/...
tests/com/google/caja/plugin/CssRewriterTest.java:497:
System.out.println("test:\n\t" + css.replace('\n', '
').replaceAll("[{][^}]*[}]", "\n\t").trim());
On 2012/02/11 22:41:37, Jasvir wrote:
> cruft

Done.

[Jasvir, 2012-02-23 23:19:49]

LGTM

http://codereview.appspot.com/5649058/diff/12001/src/com/google/caja/plugin/h...
File src/com/google/caja/plugin/html-emitter.js (right):

http://codereview.appspot.com/5649058/diff/12001/src/com/google/caja/plugin/h...
src/com/google/caja/plugin/html-emitter.js:528: console.log('safeCssText=' +
safeCssText);
Cruft.

[MikeSamuel, 2012-02-23 23:39:00]

http://codereview.appspot.com/5649058/diff/12001/src/com/google/caja/plugin/h...
File src/com/google/caja/plugin/html-emitter.js (right):

http://codereview.appspot.com/5649058/diff/12001/src/com/google/caja/plugin/h...
src/com/google/caja/plugin/html-emitter.js:528: console.log('safeCssText=' +
safeCssText);
On 2012/02/23 23:19:50, Jasvir wrote:
> Cruft.

Removed.