Reviewers: MikeSamuel,
Description:
this makes the css pattern generator throw an error if the css
whitelist has a <foo> that isn't defined.
implementing this uncovered a symbol <identifier> that isn't
defined, which is used by counter-increment and counter-reset.
the identifier is a global name, and we were inappropriately
whitelisting counter-reset and counter-increment. this was
harmless because <identifier> was undefined and ignored,
but it's not harmless now that <identifier> is defined.
so I'm deleting counter-increment and counter-reset from the
whitelist, which is harmless because they didn't work anyway.
Please review this at https://codereview.appspot.com/10830045/
Affected files:
M src/com/google/caja/lang/css/CssPropertyPatterns.java
M src/com/google/caja/lang/css/css21-whitelist.json
M tests/com/google/caja/lang/css/CssPropertyPatternsTest.java
Index: src/com/google/caja/lang/css/CssPropertyPatterns.java
===================================================================
--- src/com/google/caja/lang/css/CssPropertyPatterns.java (revision 5470)
+++ src/com/google/caja/lang/css/CssPropertyPatterns.java (working copy)
@@ -391,6 +391,9 @@
Name symbolName = sig.getValue();
refsUsed.incr(symbolName.getCanonicalForm());
JSRE builtinMatch = builtinToPattern(symbolName);
+ if (builtinMatch == BUILTIN_PROP_BIT_MATCH) {
+ return null;
+ }
if (builtinMatch != null) {
String re = builtinMatch.toString();
boolean ident = isIdentChar(re.charAt(0));
@@ -419,9 +422,14 @@
JSRE.cat(identBefore && ident ? spaces : optSpaces,
builtinMatch));
}
CssSchema.SymbolInfo s = schema.getSymbol(symbolName);
+ if (s == null) {
+ throw new SomethingWidgyHappenedError(
+ "unknown CSS symbol " + symbolName);
+ }
if (COLOR.equals(symbolName) && complete) {
// Don't blow up the regexs by including the entire X11 color set
over
// and over.
+ // TODO(felix8a): why do this when standard-color doesn't exist?
CssSchema.SymbolInfo standard = schema.getSymbol(STANDARD_COLOR);
if (standard != null) { s = standard; }
}
@@ -523,7 +531,7 @@
if (zIndex) {
this.props.add(CssPropBit.Z_INDEX);
}
- if (!complete) { return null; }
+ if (!complete) { return BUILTIN_PROP_BIT_MATCH; }
}
JSRE p = BUILTINS.get(key);
if (p == null && key != baseKey) {
@@ -533,6 +541,8 @@
}
}
+ private static final JSRE BUILTIN_PROP_BIT_MATCH = JSRE.raw("");
+
private static final Map<String, CssPropBit> BUILTIN_PROP_BITS
= ImmutableMap.<String, CssPropBit>builder()
.put("number", CssPropBit.QUANTITY)
@@ -602,6 +612,7 @@
.put("integer:0,", digits)
.put("integer:0,255", digits)
.put("hex-color", JSRE.cat(hash, JSRE.rep(JSRE.rep(hex, 3, 3), 1,
2)))
+ .put("identifier", JSRE.raw("-?[_A-Za-z][-\\w]*"))
.put("specific-voice", quotedIdentifiers)
.put("family-name", quotedIdentifiers)
.put("uri", JSRE.cat(
@@ -632,7 +643,9 @@
// Seed with some known constant strings.
for (Name commonSymbol : new Name[] {
// Derived by counting symbol names into a bag in symbolToPattern
above.
- COLOR, Name.css("standard-color"),
+ COLOR,
+ // TODO(felix8a): this list used to include
+ // Name.css("standard-color"), which doesn't exist
Name.css("length"), Name.css("length:0,"),
Name.css("border-style"), Name.css("border-width"),
Name.css("bg-position"), Name.css("bg-size"),
Index: src/com/google/caja/lang/css/css21-whitelist.json
===================================================================
--- src/com/google/caja/lang/css/css21-whitelist.json (revision 5470)
+++ src/com/google/caja/lang/css/css21-whitelist.json (working copy)
@@ -36,8 +36,6 @@
"clear",
"clip",
"color",
- "counter-increment",
- "counter-reset",
"cue-after",
"cue-before",
"cue",
Index: tests/com/google/caja/lang/css/CssPropertyPatternsTest.java
===================================================================
--- tests/com/google/caja/lang/css/CssPropertyPatternsTest.java (revision
5470)
+++ tests/com/google/caja/lang/css/CssPropertyPatternsTest.java (working
copy)
@@ -14,6 +14,7 @@
package com.google.caja.lang.css;
+import com.google.caja.SomethingWidgyHappenedError;
import com.google.caja.lang.css.CssPropertyPatterns;
import com.google.caja.lang.css.CssPropertyPatterns.CssPropertyData;
import com.google.caja.lang.css.CssSchema;
@@ -30,6 +31,16 @@
import java.util.List;
public class CssPropertyPatternsTest extends CajaTestCase {
+
+ public final void testInvalidSymbol() {
+ try {
+ toPattern("<hiybbprqag>");
+ fail("invalid symbol succeeded");
+ } catch (SomethingWidgyHappenedError e) {
+ assertContains(e.getMessage(), "unknown CSS symbol hiybbprqag");
+ }
+ }
+
public final void testKeywordPattern() {
assertPattern("zoicks", "/^\\s*zoicks\\s*$/i");
assertMatches("zoicks", "zoicks", " zoicks", " ZOICKS ");
--
---
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.
