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) mikesamuel Mon Feb 13 13:01:40 2012

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-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') {
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/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);
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/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');
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/html-emitter.js#newcode510
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/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());
On 2012/02/11 22:41:37, Jasvir wrote:
cruft

Done.

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