Loading...

google-caja-discuss@googlegroups.com

[Prev] Thread [Next]  |  [Prev] Date [Next]

[Caja] Re: Draft of a sanitizer for CSS selectors in JavaScript (issue 5649058) jasvir Sat Feb 11 15:00:31 2012

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-emitter.js
File src/com/google/caja/plugin/html-emitter.js (right):

http://codereview.appspot.com/5649058/diff/1/src/com/google/caja/plugin/html-emitter.js#newcode422
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/cssparser.js
File src/com/google/caja/plugin/cssparser.js (right):

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/cssparser.js#newcode123
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/html-emitter.js
File src/com/google/caja/plugin/html-emitter.js (right):

http://codereview.appspot.com/5649058/diff/2001/src/com/google/caja/plugin/html-emitter.js#newcode465
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/html-emitter.js#newcode510
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/CssRewriterTest.java
File tests/com/google/caja/plugin/CssRewriterTest.java (right):

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

http://codereview.appspot.com/5649058/