Revision: 5169
Author: [email protected]
Date: Thu Nov 29 12:41:07 2012
Log: Improve document.title behavior; cleanup.
https://codereview.appspot.com/6843122
* Implement HTML5-conformant behavior for document.title if there is no
<title> element.
* Remove attribute/property "blacklist" code from TameIFrameElement
which is no longer necessary since the whitelist is per-attribute.
* Move misplaced @param comment for taming membrane parameter.
[email protected]
http://code.google.com/p/google-caja/source/detail?r=5169
Modified:
/trunk/src/com/google/caja/plugin/domado.js
/trunk/tests/com/google/caja/plugin/es53-test-domado-dom-guest.html
/trunk/tests/com/google/caja/plugin/es53-test-domado-global.js
=======================================
--- /trunk/src/com/google/caja/plugin/domado.js Wed Nov 28 10:24:16 2012
+++ /trunk/src/com/google/caja/plugin/domado.js Thu Nov 29 12:41:07 2012
@@ -922,7 +922,6 @@
* objects, and a shared map allowing separate virtual documents to
dispatch
* events across them. (TODO(kpreid): Confirm this explanation is good.)
*
- * @param {Object} taming. An interface to a taming membrane.
* @param {Object} opt_rulebreaker. If necessary, authorities to break
the
* ES5/3 taming membrane and work with the taming-frame system. If
* running under SES, pass null instead.
@@ -1519,6 +1518,7 @@
* virtual Document node provided to Cajoled code.
* @param {Object} optTargetAttributePresets a record containing the
presets
* (default and whitelist) for the HTML "target" attribute.
+ * @param {Object} taming. An interface to a taming membrane.
* @return {Object} A collection of privileged access tools, plus the
tamed
* {@code document} and {@code window} objects under those names.
This
* object is known as a "domicile".
@@ -3576,6 +3576,11 @@
bridal.getAttribute(feral, 'target')));
}
}
+ } else {
+ if (typeof console !== 'undefined') {
+ console.warn('Rejecting <' + tagName + '>.setAttribute(',
+ attribName, ',', value, ')');
+ }
}
}
return value;
@@ -4618,15 +4623,6 @@
domClass: 'HTMLHtmlElement'
});
- var P_blacklist = {
- enumerable: true,
- extendedAccessors: true,
- get: nodeMethod(function () { return undefined; }),
- set: nodeMethod(function (value, prop) {
- if (typeof console !== 'undefined')
- console.error('Cannot set the [', prop, '] property of an
iframe.');
- })
- };
var TameIFrameElement = defineElement({
domClass: 'HTMLIFrameElement',
construct: function () {
@@ -4664,8 +4660,8 @@
},
height: NP.filterProp(identity, Number),
width: NP.filterProp(identity, Number),
- src: P_blacklist,
- name: P_blacklist,
+ src: NP.filterAttr(identity, identity), // rewrite handled for
attr
+ name: NP.filterAttr(identity, identity), // rejection handled
for attr
contentDocument: {
enumerable: true,
get: cajaVM.def(function () {
@@ -4680,32 +4676,6 @@
}
}
});
- // TODO(kpreid): Check these two (straight from Domita) for
correctness
- // vs. TameElement's version
- setOwn(TameIFrameElement.prototype, 'getAttribute',
- nodeMethod(function (attr) {
- var attrLc = String(attr).toLowerCase();
- if (attrLc !== 'name' && attrLc !== 'src') {
- return TameElement.prototype.getAttribute.call(this, attr);
- }
- return null;
- }));
- setOwn(TameIFrameElement.prototype, 'setAttribute',
- nodeMethod(function (attr, value) {
- var attrLc = String(attr).toLowerCase();
- // The 'name' and 'src' attributes are whitelisted for all tags in
- // html4-attributes-whitelist.json, since they're needed on tags
- // like <img>. Because there's currently no way to filter
attributes
- // based on the tag, we have to blacklist these two here.
- // TODO(kpreid): Don't we have per-attribute filtering now?
- if (attrLc !== 'name' && attrLc !== 'src') {
- return TameElement.prototype.setAttribute.call(this, attr,
value);
- }
- if (typeof console !== 'undefined')
- console.error('Cannot set the [' + attrLc +
- '] attribute of an iframe.');
- return value;
- }));
function contentDomicile(tameIFrame) {
// TODO(kpreid): Once we support loading content via src=, we will
need
// to consider whether this should always allow access to said
content,
@@ -5318,10 +5288,19 @@
enumerable: true,
get: nodeMethod(function() {
var titleEl = this.getElementsByTagName('title')[0];
- return trimHTML5Spaces(titleEl.textContent);
+ return titleEl ? trimHTML5Spaces(titleEl.textContent) : "";
}),
set: nodeMethod(function(value) {
var titleEl = this.getElementsByTagName('title')[0];
+ if (!titleEl) {
+ var head = this.getElementsByTagName('head')[0];
+ if (head) {
+ titleEl = this.createElement('title');
+ head.appendChild(titleEl);
+ } else {
+ return;
+ }
+ }
titleEl.textContent = value;
})
},
=======================================
--- /trunk/tests/com/google/caja/plugin/es53-test-domado-dom-guest.html Wed
Nov 28 10:24:16 2012
+++ /trunk/tests/com/google/caja/plugin/es53-test-domado-dom-guest.html Thu
Nov 29 12:41:07 2012
@@ -2484,6 +2484,12 @@
<script type="text/javascript">
jsunitRegister('testIframeShim',
function testIframeShim() {
+ // Tests that iframes can be used as visual layout tools, as some HTML
in
+ // the wild does. Historical note: This test was written to test a
+ // specialized attribute-blacklisting case for HTMLIFrameElement,
which no
+ // longer exists; all of this is now the default behavior for
+ // non-whitelisted attributes, but as it is important to security this
+ // specific test remains.
var ifr = document.createElement('iframe');
ifr.height = 5;
ifr.width = 6;
@@ -2495,10 +2501,12 @@
assertEquals('5', ifr.getAttribute('height'));
assertEquals('6', ifr.getAttribute('width'));
assertEquals('1', ifr.getAttribute('frameBorder'));
- assertEquals(null, ifr.getAttribute('name'));
- assertEquals(null, ifr.getAttribute('src'));
- assertEquals(void 0, ifr.name);
- assertEquals(void 0, ifr.src);
+ // Hazardous attributes are virtualized normally...
+ assertEquals('test', ifr.getAttribute('name'));
+ assertEquals('http://www.google.com', ifr.getAttribute('src'));
+ // ...but do not actually get set.
+ assertEquals(null, directAccess.getAttribute(ifr, 'name'));
+ assertEquals(null, directAccess.getAttribute(ifr, 'src'));
pass('testIframeShim');
});
</script>
=======================================
--- /trunk/tests/com/google/caja/plugin/es53-test-domado-global.js Tue Oct
9 17:52:36 2012
+++ /trunk/tests/com/google/caja/plugin/es53-test-domado-global.js Thu Nov
29 12:41:07 2012
@@ -104,6 +104,29 @@
}());
+ /**
+ * Tests of global structure-referencing property behavior.
+ */
+ (function () {
+ // Behavior of document.title without any title element present
+ registerGuestTest('testNoElementTitleProp',
+ '<body>' + // TODO(kpreid): That this is required is a
HtmlEmitter bug
+ '<script>window.globalGuestTest =' +
+ ' function globalGuestTest() {' +
+ ' assertEquals("head before get", "", ' +
+ ' document.getElementsByTagName("head")[0].innerHTML);' +
+ ' assertEquals("title before set", "", document.title);' +
+ ' assertEquals("head after get", "", ' +
+ ' document.getElementsByTagName("head")[0].innerHTML);' +
+ ' document.title = "t";' +
+ ' assertEquals("title after set", "t", document.title);' +
+ ' assertEquals("head after set", "<title>t</title>", ' +
+ ' document.getElementsByTagName("head")[0].innerHTML);' +
+ '};</script>');
+
+ // TODO(kpreid): Test document.body
+ })();
+
/**
* Tests of onload handlers, which are a special case because on*
attributes
* on <body> are actually aliases for event handlers of the _window_
object,
