I'd like you to do a code review. To review this change, run gvn review --project https://google-caja.googlecode.com/svn mikesamuel/[EMAIL PROTECTED]
Alternatively, to review the latest snapshot of this change branch, run gvn --project https://google-caja.googlecode.com/svn review mikesamuel/bug-681 to review the following change: *mikesamuel/[EMAIL PROTECTED] | mikesamuel | 2008-10-21 20:52:05 -0800 (Tue, 21 Oct 2008) Description: Fix bug: multi-word style properties not passing the sanitizer since domita.js was using CSS property names (e.g. font-size) when it should've used style member names (e.g. fontSize) to lookup into the css schema. Affected Paths: M //trunk/src/com/google/caja/plugin/domita.js M //trunk/tests/com/google/caja/plugin/domita_test.html M //trunk/tests/com/google/caja/plugin/domita_test_untrusted.html This is a semiautomated message from "gvn mail". See <http://code.google.com/p/gvn/> to learn more. Index: src/com/google/caja/plugin/domita.js =================================================================== --- src/com/google/caja/plugin/domita.js (^/trunk/src/com/google/caja/plugin/[EMAIL PROTECTED]) +++ src/com/google/caja/plugin/domita.js (^/changes/mikesamuel/bug-681/trunk/src/com/google/caja/plugin/[EMAIL PROTECTED]) @@ -141,10 +141,15 @@ attachDocumentStub = (function () { for (var i = 0; declarations && i < declarations.length; i++) { var propertyAndValue = declarations[i].split(':'); - var property = trimCssSpaces(propertyAndValue[0]); + var property = trimCssSpaces(propertyAndValue[0]).toLowerCase(); var value = trimCssSpaces(propertyAndValue[1]); - if (css.properties.hasOwnProperty(property) - && css.properties[property].test(value + ' ')) { + // TODO(mikesamuel): make a separate function to map between + // CSS property names and style object members while handling + // float/cssFloat properly. + var stylePropertyName = property.replace( // font-size -> fontSize + /-[a-z]/g, function (m) { return m.substring(1).toUpperCase(); }); + if (css.properties.hasOwnProperty(stylePropertyName) + && css.properties[stylePropertyName].test(value + ' ')) { sanitizedDeclarations.push(property + ': ' + value); } } Index: tests/com/google/caja/plugin/domita_test.html =================================================================== --- tests/com/google/caja/plugin/domita_test.html (^/trunk/tests/com/google/caja/plugin/[EMAIL PROTECTED]) +++ tests/com/google/caja/plugin/domita_test.html (^/changes/mikesamuel/bug-681/trunk/tests/com/google/caja/plugin/[EMAIL PROTECTED]) @@ -122,7 +122,11 @@ getComputedStyle: ___.simpleFrozenFunc(function (tameNode, styleProp) { var node = tameNode.node___; if (node.currentStyle) { - return node.currentStyle[styleProp]; + return node.currentStyle[styleProp.replace( + /-([a-z])/g, + function (_, letter) { + return letter.toUpperCase(); + })]; } else if (window.getComputedStyle) { return document.defaultView.getComputedStyle(node, null) .getPropertyValue(styleProp); Index: tests/com/google/caja/plugin/domita_test_untrusted.html =================================================================== --- tests/com/google/caja/plugin/domita_test_untrusted.html (^/trunk/tests/com/google/caja/plugin/[EMAIL PROTECTED]) +++ tests/com/google/caja/plugin/domita_test_untrusted.html (^/changes/mikesamuel/bug-681/trunk/tests/com/google/caja/plugin/[EMAIL PROTECTED]) @@ -91,6 +91,8 @@ </table> </div> +<p class="testcontainer" id="test-inner-HTML-style-sanitizer"></p> + <script type="text/javascript"> var setUp, tearDown; @@ -114,6 +116,24 @@ function assertFailsSafe(canFail, assertionsIfPass assertionsIfPasses(); } +function assertColor(expected, cssColorString) { + if (typeof cssColorString === 'string') { + cssColorString = cssColorString.toLowerCase(); + } + if (cssColorString === expected.name) { return; } + var hexSix = expected.rgb.toString(16); + while (hexSix.length < 6) { hexSix = '0' + hexSix; } + if (cssColorString === '#' + hexSix) { return; } + var hexThree = hexSix.charAt(0) + hexSix.charAt(2) + hexSix.charAt(4); + if (cssColorString === '#' + hexThree) { return; } + if (('rgb(' + (expected.rgb >> 16) + ', ' + ((expected.rgb >> 8) & 0xff) + + ', ' + (expected.rgb & 0xff) + ')') === cssColorString) { + return; + } + + fail(cssColorString + ' != #' + hexSix); +} + /** * Canonicalize well formed innerHTML output, by making sure that attributes * are ordered by name, and have quoted values. @@ -577,14 +597,9 @@ jsunitRegister('testEmitCss', function testCss() { directAccess.emitCssHook(['.', ' a { color: #00ff00 }']); var computedColor = directAccess.getComputedStyle( - document.getElementById('not-blue'), 'color').toLowerCase(); - if (!(computedColor === 'green' || computedColor === '#00ff00' - || computedColor === '#0f0' || computedColor === 'rgb(0, 255, 0)' - || computedColor === 'rgb(0, 100%, 0)')) { - fail(computedColor + ' is not green'); - } else { - pass('test-emit-css'); - } + document.getElementById('not-blue'), 'color'); + assertColor({ rgb: 0x00ff00, name: 'green' }, computedColor); + pass('test-emit-css'); }); jsunitRegister('testBug731', @@ -652,4 +667,28 @@ jsunitRegister('testCaseInsensitiveAttrs', pass('test-case-insensitive-attrs'); }); }); + +jsunitRegister('testInnerHTMLStyleSanitizer', + function testInnerHTMLStyleSanitizer() { + function colorOf(id) { + return directAccess.getComputedStyle(document.getElementById(id), 'color'); + } + function backgroundOf(id) { + return directAccess.getComputedStyle( + document.getElementById(id), 'background-color'); + } + + var cont = document.getElementById('test-inner-HTML-style-sanitizer'); + + cont.innerHTML = '<span style="color: #00f" id="style-html">Blue</span>'; + assertColor({ name: 'blue', rgb: 0x0000ff }, colorOf('style-html')); + + cont.innerHTML = ( + '<span style="color: red; background-color: purple"' + + ' id="style-html">Red on purple</span>'); + assertColor({ name: 'red', rgb: 0xff0000 }, colorOf('style-html')); + assertColor({ name: 'purple', rgb: 0x800080 }, backgroundOf('style-html')); + + pass('test-inner-HTML-style-sanitizer'); +}); </script> --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to http://groups.google.com/group/google-caja-discuss To unsubscribe, email [EMAIL PROTECTED] -~----------~----~----~----~------~----~------~--~---
