Reviewers: MikeSamuel, ihab.awad,
Description:
This fixes the css sanitizer to be Closure friendly and replaces the
minified version with a closured version. This reduces the size from
76k to 52k uncompressed, 24k to 16k compressed.
The fix is all about how Closure treats a.foo differently from
a['foo']. These particular foos are external linkage names, so
I changed the to a['foo'] so Closure won't rename them.
The change in RhinoTestBed is because the Closured js will emit
code like
if (condition) /foo/.test(bar)
and our java JS parser barfs on the regexp in that position.
The barf is a known problem, it's complicated to fix, but it
turns out we don't really need to parse js in RhinoTestBed,
so that's what the change is.
Please review this at https://codereview.appspot.com/13258043/
Affected files:
M build.xml
M src/com/google/caja/plugin/cssparser.js
M src/com/google/caja/plugin/html-sanitizer.js
M src/com/google/caja/plugin/sanitizecss.js
M tests/com/google/caja/parser/js/ParserTest.java
M tests/com/google/caja/util/RhinoTestBed.java
Index: build.xml
===================================================================
--- build.xml (revision 5579)
+++ build.xml (working copy)
@@ -823,11 +823,8 @@
<transform>
<output file="${lib}/html-css-sanitizer.jslint.js"
language="jslint"/>
- <!-- TODO(felix8a): -closured doesn't work -->
- <output file="${lib.caja}/plugin/html-css-sanitizer-closured.js"
+ <output file="${lib.caja}/plugin/html-css-sanitizer-minified.js"
language="closure"/>
- <output file="${lib.caja}/plugin/html-css-sanitizer-minified.js"
- language="javascript" renderer="minify"/>
<output file="${lib.caja}/plugin/html-css-sanitizer-bundle.js"
language="javascript" renderer="concat"/>
<input file="${lib.caja}/plugin/css-defs.js"/>
@@ -1110,21 +1107,7 @@
<!-- For RhinoTestBed tests; arrange files so that they can use the
same
relative URLs in browser. TODO(kpreid): Clean up the whole
mess. -->
<fileset dir="${lib}">
- <include name="**/caja/plugin/csslexer.js"/>
- <include name="**/caja/plugin/cssparser.js"/>
- <include name="**/caja/plugin/css-defs.js"/>
- <include name="**/caja/plugin/html4-defs.js"/>
- <include name="**/caja/plugin/uri.js"/>
- <include name="**/caja/plugin/sanitizecss.js"/>
- <include name="**/caja/plugin/html-sanitizer.js"/>
- <include name="**/caja/plugin/bridal.js"/>
- <include name="**/caja/plugin/html-schema.js"/>
- <include name="**/caja/plugin/html-emitter.js"/>
- <include name="**/caja/plugin/html-sanitizer-minified.js"/>
- <include name="**/caja/plugin/html-sanitizer-legacy.js"/>
- <include name="**/caja/plugin/html-css-sanitizer-bundle.js"/>
- <include name="**/caja/plugin/html-css-sanitizer-minified.js"/>
- <include name="**/caja/plugin/html-sanitizer-bundle.js"/>
+ <include name="**/caja/plugin/*.js"/>
</fileset>
</copy>
<copy todir="${testlib}">
@@ -1352,7 +1335,6 @@
<include name="html-sanitizer-bundle.js" />
<include name="html-sanitizer-minified.js" />
<include name="html-css-sanitizer-bundle.js" />
- <include name="html-css-sanitizer-closured.js" />
<include name="html-css-sanitizer-minified.js" />
<include name="taming-membrane.out.js" />
</fileset>
Index: src/com/google/caja/plugin/cssparser.js
===================================================================
--- src/com/google/caja/plugin/cssparser.js (revision 5579)
+++ src/com/google/caja/plugin/cssparser.js (working copy)
@@ -84,12 +84,12 @@
// stylesheet : [ CDO | CDC | S | statement ]*;
parseCssStylesheet = function(cssText, handler) {
var toks = lexCss(cssText);
- if (handler.startStylesheet) { handler.startStylesheet(); }
+ if (handler['startStylesheet']) { handler['startStylesheet'](); }
for (var i = 0, n = toks.length; i < n;) {
// CDO and CDC ("<!--" and "-->") are converted to space by the
lexer.
i = toks[i] === ' ' ? i+1 : statement(toks, i, n, handler);
}
- if (handler.endStylesheet) { handler.endStylesheet(); }
+ if (handler['endStylesheet']) { handler['endStylesheet'](); }
};
// statement : ruleset | at-rule;
@@ -116,14 +116,14 @@
var s = start+1, e = i;
if (s < n && toks[s] === ' ') { ++s; }
if (e > s && toks[e-1] === ' ') { --e; }
- if (handler.startAtrule) {
- handler.startAtrule(toks[start].toLowerCase(), toks.slice(s, e));
+ if (handler['startAtrule']) {
+ handler['startAtrule'](toks[start].toLowerCase(), toks.slice(s,
e));
}
i = (toks[i] === '{')
? block(toks, i, n, handler)
: i+1; // Skip over ';'
- if (handler.endAtrule) {
- handler.endAtrule();
+ if (handler['endAtrule']) {
+ handler['endAtrule']();
}
}
// Else we reached end of input or are missing a semicolon.
@@ -135,7 +135,7 @@
// Assumes the leading '{' has been verified by callers.
function block(toks, i, n, handler) {
++i; // skip over '{'
- if (handler.startBlock) { handler.startBlock(); }
+ if (handler['startBlock']) { handler['startBlock'](); }
while (i < n) {
var ch = toks[i].charAt(0);
if (ch == '}') {
@@ -159,7 +159,7 @@
i = ruleset(toks, i, n, handler);
}
}
- if (handler.endBlock) { handler.endBlock(); }
+ if (handler['endBlock']) { handler['endBlock'](); }
return i;
}
@@ -181,8 +181,8 @@
i = e+1; // Skip over '{'
// Don't include any trailing space in the selector slice.
if (e > s && toks[e-1] === ' ') { --e; }
- if (handler.startRuleset) {
- handler.startRuleset(toks.slice(s, e));
+ if (handler['startRuleset']) {
+ handler['startRuleset'](toks.slice(s, e));
}
while (i < n) {
tok = toks[i];
@@ -196,8 +196,8 @@
i = declaration(toks, i, n, handler);
}
}
- if (handler.endRuleset) {
- handler.endRuleset();
+ if (handler['endRuleset']) {
+ handler['endRuleset']();
}
return i;
}
@@ -298,9 +298,9 @@
++e;
}
}
- if (valuelen && handler.declaration) {
+ if (valuelen && handler['declaration']) {
// TODO: coerce non-keyword ident tokens to quoted strings.
- handler.declaration(property.toLowerCase(), value);
+ handler['declaration'](property.toLowerCase(), value);
}
}
return e;
Index: src/com/google/caja/plugin/html-sanitizer.js
===================================================================
--- src/com/google/caja/plugin/html-sanitizer.js (revision 5579)
+++ src/com/google/caja/plugin/html-sanitizer.js (working copy)
@@ -900,7 +900,7 @@
parseCssDeclarations(
value,
{
- declaration: function (property, tokens) {
+ 'declaration': function (property, tokens) {
var normProp = property.toLowerCase();
sanitizeCssProperty(
normProp, tokens,
Index: src/com/google/caja/plugin/sanitizecss.js
===================================================================
--- src/com/google/caja/plugin/sanitizecss.js (revision 5579)
+++ src/com/google/caja/plugin/sanitizecss.js (working copy)
@@ -162,7 +162,7 @@
return;
}
- var propBits = propertySchema.cssPropBits;
+ var propBits = propertySchema['cssPropBits'];
/**
* Recurse to apply the appropriate function schema to the function
call
@@ -192,7 +192,7 @@
var bareFnToken = withoutVendorPrefix(fnToken);
// Cut out the originals, so the caller can step by one token.
var fnTokens = tokens.splice(start, end - start, '');
- var fns = propertySchema.cssFns;
+ var fns = propertySchema['cssFns'];
// Look for a function that matches the name.
for (var i = 0, nFns = fns.length; i < nFns; ++i) {
if (fns[i].substring(0, bareFnToken.length) == bareFnToken) {
@@ -259,11 +259,12 @@
)
: (
- litGroup = propertySchema.cssLitGroup,
+ litGroup = propertySchema['cssLitGroup'],
litMap = (litGroup
- ? (propertySchema.cssLitMap
+ ? (propertySchema['cssLitMap']
// Lazily compute the union from litGroup.
- || (propertySchema.cssLitMap =
unionArrays(litGroup)))
+ || (propertySchema['cssLitMap'] =
+ unionArrays(litGroup)))
: ALLOWED_LITERAL), // A convenient empty object.
(litMap[withoutVendorPrefix(token)] === ALLOWED_LITERAL)
)
@@ -871,12 +872,12 @@
parseCssStylesheet(
cssText,
{
- startStylesheet: function () {
+ 'startStylesheet': function () {
safeCss = [];
},
- endStylesheet: function () {
+ 'endStylesheet': function () {
},
- startAtrule: function (atIdent, headerArray) {
+ 'startAtrule': function (atIdent, headerArray) {
if (elide) {
atIdent = null;
} else if (atIdent === '@media') {
@@ -936,14 +937,14 @@
elide = !atIdent;
blockStack.push(atIdent);
},
- endAtrule: function () {
+ 'endAtrule': function () {
blockStack.pop();
if (!elide) {
safeCss.push(';');
}
checkElide();
},
- startBlock: function () {
+ 'startBlock': function () {
// There are no bare blocks in CSS, so we do not change the
// block stack here, but instead in the events that bracket
// blocks.
@@ -951,13 +952,13 @@
safeCss.push('{');
}
},
- endBlock: function () {
+ 'endBlock': function () {
if (!elide) {
safeCss.push('}');
elide = true; // skip any semicolon from endAtRule.
}
},
- startRuleset: function (selectorArray) {
+ 'startRuleset': function (selectorArray) {
var historySensitiveSelectors = void 0;
var removeHistoryInsensitiveSelectors = false;
if (!elide) {
@@ -1012,7 +1013,7 @@
removeHistoryInsensitiveSelectors
});
},
- endRuleset: function () {
+ 'endRuleset': function () {
var rules = blockStack.pop();
var propertiesEnd = safeCss.length;
if (!elide) {
@@ -1034,7 +1035,7 @@
}
checkElide();
},
- declaration: function (property, valueArray) {
+ 'declaration': function (property, valueArray) {
if (!elide) {
var isImportant = false;
var nValues = valueArray.length;
Index: tests/com/google/caja/parser/js/ParserTest.java
===================================================================
--- tests/com/google/caja/parser/js/ParserTest.java (revision 5579)
+++ tests/com/google/caja/parser/js/ParserTest.java (working copy)
@@ -519,6 +519,7 @@
// requires a nontrivial rewrite of the js lexer stack.
@FailureIsAnOption
public final void testRegexpContext() throws Exception {
+ assertParseSucceeds("if(true)/bar/;");
assertParseSucceeds("{} /foo/;");
}
Index: tests/com/google/caja/util/RhinoTestBed.java
===================================================================
--- tests/com/google/caja/util/RhinoTestBed.java (revision 5579)
+++ tests/com/google/caja/util/RhinoTestBed.java (working copy)
@@ -147,14 +147,14 @@
scriptBody = textContentOf(script);
}
String scriptText;
- Block js = parseJavascript(scriptBody, mq);
+ InputSource isrc = scriptBody.getSourceBreaks(0).source();
// Add blank lines at the front so that Rhino stack traces have
correct
// line numbers.
scriptText = prefixWithBlankLines(
scriptBody.toString(0, scriptBody.getLimit()),
Nodes.getFilePositionFor(script).startLineNo() - 1);
- scriptContent.add(Pair.pair(scriptText,
js.getFilePosition().source()));
- mc.addInputSource(js.getFilePosition().source());
+ scriptContent.add(Pair.pair(scriptText, isrc));
+ mc.addInputSource(isrc);
script.getParentNode().removeChild(script);
}
for (Pair<String, InputSource> script : scriptContent) {
--
---
You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
