Revision: 4783
Author: metaweta
Date: Wed Feb 22 10:19:25 2012
Log: Allows arbitrary attributes in static HTML.
http://codereview.appspot.com/5672083
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.
R=jasvir
http://code.google.com/p/google-caja/source/detail?r=4783
Modified:
/trunk/src/com/google/caja/lang/html/HtmlSchema.java
/trunk/src/com/google/caja/plugin/domado.js
/trunk/src/com/google/caja/plugin/templates/TemplateCompiler.java
/trunk/src/com/google/caja/plugin/templates/TemplateSanitizer.java
/trunk/tests/com/google/caja/plugin/templates/TemplateSanitizerTest.java
/trunk/tests/com/google/caja/service/HtmlHandlerTest.java
=======================================
--- /trunk/src/com/google/caja/lang/html/HtmlSchema.java Tue Aug 24
13:37:35 2010
+++ /trunk/src/com/google/caja/lang/html/HtmlSchema.java Wed Feb 22
10:19:25 2012
@@ -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) {
=======================================
--- /trunk/src/com/google/caja/plugin/domado.js Thu Feb 9 21:05:25 2012
+++ /trunk/src/com/google/caja/plugin/domado.js Wed Feb 22 10:19:25 2012
@@ -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
@@ -59,6 +59,9 @@
var domitaModules;
var Domado = (function() {
'use strict';
+ var cajaPrefix = 'data-caja-';
+ var cajaPrefRe = new RegExp('^' + cajaPrefix);
+
if (!domitaModules) { domitaModules = {}; }
domitaModules.proxiesAvailable = typeof Proxy !== 'undefined';
@@ -1467,6 +1470,8 @@
html4.ATTRIBS.hasOwnProperty(attribKey))) {
atype = html4.ATTRIBS[attribKey];
value = rewriteAttribute(tagName, attribName, atype, value);
+ } else if (!/__$/.test(attribKey)) {
+ attribName = attribs[+i] = cajaPrefix + attribs[+i];
} else {
value = null;
}
@@ -1547,7 +1552,7 @@
startTag: function (tagName, attribs, out) {
out.push('<', tagName);
for (var i = 0; i < attribs.length; i += 2) {
- var aname = attribs[+i];
+ var aname = '' + attribs[+i];
var atype = getAttributeType(tagName, aname);
var value = attribs[i + 1];
if (aname !== 'target' && atype !== void 0) {
@@ -1555,6 +1560,9 @@
if (typeof value === 'string') {
out.push(' ', aname, '="',
html.escapeAttrib(value), '"');
}
+ } else if (cajaPrefRe.test(aname)) {
+ out.push(' ', aname.substring(10), '="',
+ html.escapeAttrib(value), '"');
}
}
out.push('>');
@@ -3092,14 +3100,13 @@
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 +3125,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 +3136,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 +3156,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);
}
=======================================
--- /trunk/src/com/google/caja/plugin/templates/TemplateCompiler.java Tue
Jan 24 11:04:08 2012
+++ /trunk/src/com/google/caja/plugin/templates/TemplateCompiler.java Wed
Feb 22 10:19:25 2012
@@ -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);
}
=======================================
--- /trunk/src/com/google/caja/plugin/templates/TemplateSanitizer.java Tue
Jan 24 11:04:08 2012
+++ /trunk/src/com/google/caja/plugin/templates/TemplateSanitizer.java Wed
Feb 22 10:19:25 2012
@@ -45,6 +45,7 @@
public final class TemplateSanitizer {
private final HtmlSchema schema;
private final MessageQueue mq;
+ private static String cajaPrefix = "data-caja-";
/**
* @param schema specifies which tags and attributes are allowed, and
which
@@ -143,12 +144,18 @@
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));
- }
- valid &= removeBadAttribute(el, attrKey);
+ if (attrKey.localName.endsWith("__")) { valid = false; }
+ else if (!attrKey.localName.startsWith(cajaPrefix)) {
+ // Remove this attribute and create another that
+ // starts with "data-caja-".
+ String localName = attrKey.localName;
+ String value = attrib.getValue();
+ removeBadAttribute(el, attrKey);
+ el.setAttributeNS(
+ attrKey.ns.uri,
+ cajaPrefix + localName,
+ value == null ? localName : value);
+ }
}
} else if ("target".equals(attrKey.localName)) {
if (!"_self".equals(attrib.getNodeValue())) {
@@ -214,9 +221,13 @@
for (Node child : Nodes.childrenOf(el)) { valid &= sanitize(child); }
for (Attr a : Nodes.attributesOf(el)) {
+ String attrName = a.getNodeName();
+ if (attrName.startsWith(cajaPrefix)) {
+ attrName = attrName.substring(10);
+ }
mq.addMessage(
PluginMessageType.CANNOT_FOLD_ATTRIBUTE,
Nodes.getFilePositionFor(a),
- MessagePart.Factory.valueOf(a.getNodeName()),
+ MessagePart.Factory.valueOf(attrName),
MessagePart.Factory.valueOf(el.getLocalName()));
}
=======================================
---
/trunk/tests/com/google/caja/plugin/templates/TemplateSanitizerTest.java
Tue Sep 28 09:41:05 2010
+++
/trunk/tests/com/google/caja/plugin/templates/TemplateSanitizerTest.java
Wed Feb 22 10:19:25 2012
@@ -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,8 @@
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 unknown when folding bogus into
parent"
);
}
public final void testDisallowedScriptElement() throws Exception {
@@ -251,12 +250,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 {
=======================================
--- /trunk/tests/com/google/caja/service/HtmlHandlerTest.java Tue Jan 24
11:04:08 2012
+++ /trunk/tests/com/google/caja/service/HtmlHandlerTest.java Wed Feb 22
10:19:25 2012
@@ -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___){"
