Reviewers: Jasvir,
Description:
This change gets rid of the _d_attributes map and uses the
data-caja-* attribute namespace instead. TemplateCompiler now renames
attributes rather than stripping them out, and the *Attribute()
methods in Domado prefix attributes if they're not otherwise
whitelisted.
Please review this at http://codereview.appspot.com/5672083/
Affected files:
M src/com/google/caja/lang/html/HtmlSchema.java
M src/com/google/caja/plugin/domado.js
M src/com/google/caja/plugin/templates/TemplateCompiler.java
M src/com/google/caja/plugin/templates/TemplateSanitizer.java
M tests/com/google/caja/plugin/templates/TemplateSanitizerTest.java
M tests/com/google/caja/service/HtmlHandlerTest.java
Index: tests/com/google/caja/plugin/templates/TemplateSanitizerTest.java
===================================================================
--- tests/com/google/caja/plugin/templates/TemplateSanitizerTest.java
(revision 4782)
+++ tests/com/google/caja/plugin/templates/TemplateSanitizerTest.java
(working copy)
@@ -41,8 +41,7 @@
public final void testUnknownAttribute() throws Exception {
assertValid(
htmlFragment(fromString("<b unknown=\"bogus\">Hello</b>")),
- "<b>Hello</b>",
- "WARNING: removing unknown attribute unknown on b");
+ "<b data-caja-unknown=\"bogus\">Hello</b>");
}
public final void testKnownAttribute() throws Exception {
assertValid(htmlFragment(fromString("<b id=\"bold\">Hello</b>")),
@@ -59,8 +58,9 @@
assertValid(
htmlFragment(fromString("<bogus unknown=\"bogus\">Hello</bogus>")),
"Hello",
- "WARNING: removing unknown tag bogus"
- // No need to warn about unknown since we have warned about tag
+ "WARNING: removing unknown tag bogus",
+ "WARNING: removing attribute data-caja-unknown " +
+ "when folding bogus into parent"
);
}
public final void testDisallowedScriptElement() throws Exception {
@@ -251,12 +251,12 @@
+ "<x>Four</x>"
+ "</body>"
+ "</html>")),
- "<p>Foo</p><p>One</p><p>Two</p>ThreeFour",
+ "<p>Foo</p><p>One</p><p data-caja-styleo=\"color: red\">Two" +
+ "</p>ThreeFour",
"WARNING: folding element html into parent",
"WARNING: folding element head into parent",
"WARNING: removing disallowed tag title",
"WARNING: folding element body into parent",
- "WARNING: removing unknown attribute styleo on p",
"WARNING: removing unknown tag x");
}
public final void testElementFolding4() throws Exception {
Index: tests/com/google/caja/service/HtmlHandlerTest.java
===================================================================
--- tests/com/google/caja/service/HtmlHandlerTest.java (revision 4782)
+++ tests/com/google/caja/service/HtmlHandlerTest.java (working copy)
@@ -151,7 +151,8 @@
private void assertHtml2Json(String inputMimeType)
throws Exception {
registerUri(
- "http://foo/bar.html";, "<p>hi</p><script>42;</script><p>bye</p>",
+ "http://foo/bar.html";,
+ "<p foo align=center>hi</p><script>42;</script><p>bye</p>",
"text/html");
Object result = json((String) requestGet(
@@ -162,8 +163,10 @@
JSONObject json = (JSONObject) result;
// Check html generation is correct
- assertEquals("<p>hi<span id=\"id_2___\"></span></p><p>bye</p>",
- (String) json.get("html"));
+ assertEquals(
+ "<p align=\"center\" data-caja-foo=\"foo\">" +
+ "hi<span id=\"id_2___\"></span></p><p>bye</p>",
+ (String) json.get("html"));
assertEquals("{"
+ "___.loadModule({"
+ "'instantiate':function(___,IMPORTS___){"
Index: src/com/google/caja/lang/html/HtmlSchema.java
===================================================================
--- src/com/google/caja/lang/html/HtmlSchema.java (revision 4782)
+++ src/com/google/caja/lang/html/HtmlSchema.java (working copy)
@@ -229,7 +229,9 @@
public boolean isAttributeAllowed(AttribKey k) {
HTML.Attribute a = lookupAttribute(k);
- return a != null && allowedAttributes.contains(a.getKey());
+ if (a != null && allowedAttributes.contains(a.getKey())) { return
true; }
+ String keyName = k.localName;
+ return keyName.startsWith("data-caja-") && !keyName.endsWith("___");
}
public HTML.Attribute lookupAttribute(AttribKey k) {
Index: src/com/google/caja/plugin/domado.js
===================================================================
--- src/com/google/caja/plugin/domado.js (revision 4782)
+++ src/com/google/caja/plugin/domado.js (working copy)
@@ -33,7 +33,7 @@
* TODO(ihab.awad): Our implementation of getAttribute (and friends)
* is such that standard DOM attributes which we disallow for security
* reasons (like 'form:enctype') are placed in the "virtual" attributes
- * map (node._d_attributes). They appear to be settable and gettable,
+ * map (the data-caja-* namespace). They appear to be settable and
gettable,
* but their values are ignored and do not have the expected semantics
* per the DOM API. This is because we do not have a column in
* html4-defs.js stating that an attribute is valid but explicitly
@@ -3089,17 +3089,17 @@
return np(this).feral.hasFocus();
});
}
+ var cajaPrefix = "data-caja-";
TameElement.prototype.getAttribute = nodeMethod(function
(attribName) {
var feral = np(this).feral;
attribName = String(attribName).toLowerCase();
+ if (/__$/.test(attribName)) {
+ throw new TypeError('Attributes may not end with __');
+ }
var tagName = feral.tagName.toLowerCase();
var atype = getAttributeType(tagName, attribName);
if (atype === void 0) {
- // Unrecognized attribute; use virtual map
- if (feral._d_attributes) {
- return feral._d_attributes[attribName] || null;
- }
- return null;
+ return feral.getAttribute(cajaPrefix + attribName);
}
var value = bridal.getAttribute(feral, attribName);
if ('string' !== typeof value) { return value; }
@@ -3118,11 +3118,7 @@
var tagName = feral.tagName.toLowerCase();
var atype = getAttributeType(tagName, attribName);
if (atype === void 0) {
- // Unrecognized attribute; use virtual map
- return !!(
- feral._d_attributes &&
- Object.prototype.hasOwnProperty.call(
- feral._d_attributes, attribName));
+ return bridal.hasAttribute(feral, cajaPrefix + attribName);
} else {
return bridal.hasAttribute(feral, attribName);
}
@@ -3133,12 +3129,13 @@
var feral = np(this).feral;
if (!np(this).editable) { throw new Error(NOT_EDITABLE); }
attribName = String(attribName).toLowerCase();
+ if (/__$/.test(attribName)) {
+ throw new TypeError('Attributes may not end with __');
+ }
var tagName = feral.tagName.toLowerCase();
var atype = getAttributeType(tagName, attribName);
if (atype === void 0) {
- // Unrecognized attribute; use virtual map
- if (!feral._d_attributes) { feral._d_attributes = {}; }
- feral._d_attributes[attribName] = String(value);
+ bridal.setAttribute(feral, cajaPrefix + attribName, value);
} else {
var sanitizedValue = rewriteAttribute(
tagName, attribName, atype, value);
@@ -3152,13 +3149,13 @@
var feral = np(this).feral;
if (!np(this).editable) { throw new Error(NOT_EDITABLE); }
attribName = String(attribName).toLowerCase();
+ if (/__$/.test(attribName)) {
+ throw new TypeError('Attributes may not end with __');
+ }
var tagName = feral.tagName.toLowerCase();
var atype = getAttributeType(tagName, attribName);
if (atype === void 0) {
- // Unrecognized attribute; use virtual map
- if (feral._d_attributes) {
- delete feral._d_attributes[attribName];
- }
+ feral.removeAttribute(cajaPrefix + attribName);
} else {
feral.removeAttribute(attribName);
}
Index: src/com/google/caja/plugin/templates/TemplateSanitizer.java
===================================================================
--- src/com/google/caja/plugin/templates/TemplateSanitizer.java (revision
4782)
+++ src/com/google/caja/plugin/templates/TemplateSanitizer.java (working
copy)
@@ -143,12 +143,16 @@
HTML.Attribute a = schema.lookupAttribute(attrKey);
if (null == a) {
if (!Placeholder.ID_ATTR.is(attrib)) {
- if (!ignore) {
- mq.getMessages().add(new Message(
- PluginMessageType.UNKNOWN_ATTRIBUTE, MessageLevel.WARNING,
- Nodes.getFilePositionFor(attrib), attrKey, elKey));
+ if (attrKey.localName.endsWith("___")) { valid = false; }
+ else if (!attrKey.localName.startsWith("data-caja-")) {
+ // Remove this attribute and create another that
+ // starts with "data-caja-".
+ removeBadAttribute(el, attrKey);
+ el.setAttributeNS(
+ attrKey.ns.uri,
+ "data-caja-" + attrKey.localName,
+ attrib.getValue());
}
- valid &= removeBadAttribute(el, attrKey);
}
} else if ("target".equals(attrKey.localName)) {
if (!"_self".equals(attrib.getNodeValue())) {
Index: src/com/google/caja/plugin/templates/TemplateCompiler.java
===================================================================
--- src/com/google/caja/plugin/templates/TemplateCompiler.java (revision
4782)
+++ src/com/google/caja/plugin/templates/TemplateCompiler.java (working
copy)
@@ -21,6 +21,7 @@
import com.google.caja.parser.html.AttribKey;
import com.google.caja.parser.html.ElKey;
import com.google.caja.parser.html.Nodes;
+import com.google.caja.parser.js.StringLiteral;
import com.google.caja.parser.js.UncajoledModule;
import com.google.caja.plugin.JobEnvelope;
import com.google.caja.plugin.Placeholder;
@@ -41,6 +42,7 @@
import org.w3c.dom.Document;
import org.w3c.dom.DocumentFragment;
import org.w3c.dom.Element;
+import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
import org.w3c.dom.Text;
@@ -236,6 +238,17 @@
}
}
}
+ // Iterate over all attributes on el and allow any starting with
data-caja-
+ NamedNodeMap attrMap = el.getAttributes();
+ int attrLen = attrMap.getLength();
+ for (int i = 0; i < attrLen; ++i) {
+ Attr attr = (Attr) attrMap.item(i);
+ if (attr.getLocalName().startsWith("data-caja-")) {
+ scriptsPerNode.put(attr, new StringLiteral(
+ Nodes.getFilePositionForValue(attr),
+ attr.getValue()));
+ }
+ }
scriptsPerNode.put(el, null);
}
