Allow colons in CSS class names in selectors and allow some attribute tests

This addresses issue 1617, where we don't properly sanitize attribute selectors and don't allow colons in classes and IDs.

Submitted as r5306

[metaweta, 2013-02-26 15:24:21]

lgtm

https://codereview.appspot.com/7393064/diff/1/src/com/google/caja/plugin/sani...
File src/com/google/caja/plugin/sanitizecss.js (right):

https://codereview.appspot.com/7393064/diff/1/src/com/google/caja/plugin/sani...
src/com/google/caja/plugin/sanitizecss.js:356: // Split the element selector
into three parts.
four parts

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/sa...
File tests/com/google/caja/plugin/sanitizecss_test.js (right):

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/sa...
tests/com/google/caja/plugin/sanitizecss_test.js:225: var source =
'input.cl\\:a\\3a ss[type = "text"], input#foo\\:bar';
The 3a here won't be turned into a colon; you need \x instead of \\.

[kpreid2, 2013-02-26 17:45:48]

https://codereview.appspot.com/7393064/diff/1/src/com/google/caja/plugin/sani...
File src/com/google/caja/plugin/sanitizecss.js (right):

https://codereview.appspot.com/7393064/diff/1/src/com/google/caja/plugin/sani...
src/com/google/caja/plugin/sanitizecss.js:317: if (selectors[i] == ',') {  //
TODO: ignore ',' inside brackets.
===

https://codereview.appspot.com/7393064/diff/1/src/com/google/caja/plugin/sani...
src/com/google/caja/plugin/sanitizecss.js:451: + attrs + pseudoSelector;
four space indent

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/cs...
File tests/com/google/caja/plugin/cssparser_test.js (right):

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/cs...
tests/com/google/caja/plugin/cssparser_test.js:246: ['startStylesheet', [],
four space indent

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/sa...
File tests/com/google/caja/plugin/sanitizecss_test.js (right):

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/sa...
tests/com/google/caja/plugin/sanitizecss_test.js:225: var source =
'input.cl\\:a\\3a ss[type = "text"], input#foo\\:bar';
On 2013/02/26 15:24:21, metaweta wrote:
> The 3a here won't be turned into a colon; you need \x instead of \\.

You mean \\x?

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/sa...
tests/com/google/caja/plugin/sanitizecss_test.js:228: tokens, 'sfx', function
(el, args) { return { tagName: el }; });
no space before (

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/sa...
tests/com/google/caja/plugin/sanitizecss_test.js:230: [['.sfx
input.cl\\:a\\:ss[type="text"]',
four space indent

[MikeSamuel, 2013-02-26 21:45:43]

This addresses issue 1617, where we don't properly sanitize attribute selectors
and don't allow colons in classes and IDs.

[MikeSamuel, 2013-02-26 21:48:14]

https://codereview.appspot.com/7393064/diff/1/src/com/google/caja/plugin/sani...
File src/com/google/caja/plugin/sanitizecss.js (right):

https://codereview.appspot.com/7393064/diff/1/src/com/google/caja/plugin/sani...
src/com/google/caja/plugin/sanitizecss.js:317: if (selectors[i] == ',') {  //
TODO: ignore ',' inside brackets.
On 2013/02/26 17:45:48, kpreid2 wrote:
> ===

Done.

https://codereview.appspot.com/7393064/diff/1/src/com/google/caja/plugin/sani...
src/com/google/caja/plugin/sanitizecss.js:356: // Split the element selector
into three parts.
On 2013/02/26 15:24:21, metaweta wrote:
> four parts

Done.

https://codereview.appspot.com/7393064/diff/1/src/com/google/caja/plugin/sani...
src/com/google/caja/plugin/sanitizecss.js:451: + attrs + pseudoSelector;
On 2013/02/26 17:45:48, kpreid2 wrote:
> four space indent

Done.

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/cs...
File tests/com/google/caja/plugin/cssparser_test.js (right):

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/cs...
tests/com/google/caja/plugin/cssparser_test.js:246: ['startStylesheet', [],
On 2013/02/26 17:45:48, kpreid2 wrote:
> four space indent

Done.

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/sa...
File tests/com/google/caja/plugin/sanitizecss_test.js (right):

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/sa...
tests/com/google/caja/plugin/sanitizecss_test.js:225: var source =
'input.cl\\:a\\3a ss[type = "text"], input#foo\\:bar';
On 2013/02/26 15:24:21, metaweta wrote:
> The 3a here won't be turned into a colon; you need \x instead of \\.

It should be by decodeCssEscape at csslexer.js:213 and 32.

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/sa...
tests/com/google/caja/plugin/sanitizecss_test.js:228: tokens, 'sfx', function
(el, args) { return { tagName: el }; });
On 2013/02/26 17:45:48, kpreid2 wrote:
> no space before (

Done.

https://codereview.appspot.com/7393064/diff/1/tests/com/google/caja/plugin/sa...
tests/com/google/caja/plugin/sanitizecss_test.js:230: [['.sfx
input.cl\\:a\\:ss[type="text"]',
On 2013/02/26 17:45:48, kpreid2 wrote:
> four space indent

Done.

[kpreid2, 2013-02-26 21:53:31]

LGTM

[metaweta, 2013-02-26 21:56:52]

lgtm

Thanks, Mike!

[metaweta, 2013-02-26 23:05:57]

The committed CL doesn't fix issue 1617; for some reason, sanitizeCssSelectors
is still stripping out selectors with colons.

[MikeSamuel, 2013-02-27 19:40:05]

On 2013/02/26 23:05:57, metaweta wrote:
> The committed CL doesn't fix issue 1617; for some reason, sanitizeCssSelectors
> is still stripping out selectors with colons.

Ack.  Will fix in a follow up CL.

[kpreid2, 2013-03-07 17:59:28]

On 2013/02/27 19:40:05, MikeSamuel wrote:
> On 2013/02/26 23:05:57, metaweta wrote:
> > The committed CL doesn't fix issue 1617; for some reason,
sanitizeCssSelectors
> > is still stripping out selectors with colons.
> 
> Ack.  Will fix in a follow up CL.

Please close this review since it is committed (r5306, for the record).

[MikeSamuel, 2013-03-08 02:03:34]

done

2013/3/7  <kpreid.switchb.org@gmail.com>:
> On 2013/02/27 19:40:05, MikeSamuel wrote:
>>
>> On 2013/02/26 23:05:57, metaweta wrote:
>> > The committed CL doesn't fix issue 1617; for some reason,
>
> sanitizeCssSelectors
>>
>> > is still stripping out selectors with colons.
>
>
>> Ack.  Will fix in a follow up CL.
>
>
> Please close this review since it is committed (r5306, for the record).
>
> https://codereview.appspot.com/7393064/