diff --git a/examples/bind/forms.amp.html b/examples/bind/forms.amp.html new file mode 100644 index 0000000000000..704af2d3cbf9f --- /dev/null +++ b/examples/bind/forms.amp.html @@ -0,0 +1,47 @@ + + + + + Forms Examples in AMP + + + + + + + + + + +

Submit the form and then press a button to change the text in the below template

+ + + + + +

Enter your name and email.

+
+
+ + + +
+ +
+ +
+
+ + diff --git a/extensions/amp-bind/0.1/bind-impl.js b/extensions/amp-bind/0.1/bind-impl.js index 3a471932881d2..1d8f3c98c9f34 100644 --- a/extensions/amp-bind/0.1/bind-impl.js +++ b/extensions/amp-bind/0.1/bind-impl.js @@ -28,6 +28,7 @@ import {reportError} from '../../../src/error'; import {resourcesForDoc} from '../../../src/resources'; import {filterSplice} from '../../../src/utils/array'; import {rewriteAttributeValue} from '../../../src/sanitizer'; +import {timerFor} from '../../../src/timer'; const TAG = 'amp-bind'; @@ -100,10 +101,15 @@ export class Bind { this.resources_ = resourcesForDoc(ampdoc); /** - * True if a digest is triggered before scan for bindings completes. - * @private {boolean} + * @const @private {!Array} */ - this.digestQueuedAfterScan_ = false; + this.mutationPromises_ = []; + + /** + * @const @private {MutationObserver} + */ + this.mutationObserver_ = + new MutationObserver(this.onMutationsObserved_.bind(this)); /** @const @private {boolean} */ this.workerExperimentEnabled_ = isExperimentOn(this.win_, 'web-worker'); @@ -282,7 +288,7 @@ export class Bind { /** * Scans `node` for attributes that conform to bind syntax and returns * a tuple containing bound elements and binding data for the evaluator. - * @param {!Element} node + * @param {!Node} node * @return { * !Promise<{ * boundElements: !Array, @@ -307,11 +313,18 @@ export class Bind { // Helper function for scanning the tree walker's next node. // Returns true if the walker has no more nodes. const scanNextNode_ = () => { - const element = walker.nextNode(); - if (!element) { + const node = walker.currentNode; + if (!node) { return true; } + // Walker is filtered to only return elements + const element = dev().assertElement(node); const tagName = element.tagName; + if (tagName === 'TEMPLATE') { + // Listen for changes in amp-mustache templates + this.observeElementForMutations_(element); + } + const boundProperties = this.scanElement_(element); if (boundProperties.length > 0) { boundElements.push({element, boundProperties}); @@ -325,7 +338,7 @@ export class Bind { } expressionToElements[expressionString].push(element); }); - return false; + return !walker.nextNode(); }; return new Promise(resolve => { @@ -653,6 +666,61 @@ export class Bind { } } + /** + * Begin observing mutations to element. Presently, all supported elements + * that can add/remove bindings add new elements to their parent, so parent + * node should be observed for mutations. + * @private + */ + observeElementForMutations_(element) { + // TODO(kmh287): What if parent is the body tag? + // TODO(kmh287): Generify logic for node observation strategy + // when bind supprots more dynamic nodes. + const elementToObserve = element.parentElement; + this.mutationObserver_.observe(elementToObserve, {childList: true}); + } + + /** + * Respond to observed mutations. Adds all bindings for newly added elements + * removes bindings for removed elements, then immediately applies the current + * scope to the new bindings. + * + * @param mutations {Array} + * @private + */ + onMutationsObserved_(mutations) { + mutations.forEach(mutation => { + // Add bindings for new nodes first to ensure that a binding isn't removed + // and then subsequently re-added. + const addPromises = []; + const addedNodes = mutation.addedNodes; + for (let i = 0; i < addedNodes.length; i++) { + const addedNode = addedNodes[i]; + if (addedNode.nodeType == Node.ELEMENT_NODE) { + const addedElement = dev().assertElement(addedNode); + addPromises.push(this.addBindingsForNode_(addedElement)); + } + } + const mutationPromise = Promise.all(addPromises).then(() => { + const removePromises = []; + const removedNodes = mutation.removedNodes; + for (let i = 0; i < removedNodes.length; i++) { + const removedNode = removedNodes[i]; + if (removedNode.nodeType == Node.ELEMENT_NODE) { + const removedElement = dev().assertElement(removedNode); + removePromises.push(this.removeBindingsForNode_(removedElement)); + } + } + return Promise.all(removePromises); + }).then(() => { + return this.digest_(); + }); + if (getMode().test) { + this.mutationPromises_.push(mutationPromise); + } + }); + } + /** * Returns true if both arrays contain the same strings. * @param {!(IArrayLike|Array)} a @@ -723,4 +791,23 @@ export class Bind { return false; } + + /** + * Wait for DOM mutation observer callbacks to fire. Returns a promise + * that resolves when mutation callbacks have fired. + * + * @return {Promise} + * + * @visibleForTesting + */ + waitForAllMutationsForTesting() { + return timerFor(this.win_).poll(5, () => { + return this.mutationPromises_.length > 0; + }).then(() => { + return Promise.all(this.mutationPromises_); + }).then(() => { + this.mutationPromises_.length = 0; + }); + } + } diff --git a/extensions/amp-bind/0.1/test/test-bind-impl.js b/extensions/amp-bind/0.1/test/test-bind-impl.js index 27aff5932935a..4ead8f6bae53f 100644 --- a/extensions/amp-bind/0.1/test/test-bind-impl.js +++ b/extensions/amp-bind/0.1/test/test-bind-impl.js @@ -21,6 +21,7 @@ import {chunkInstanceForTesting} from '../../../../src/chunk'; import {toArray} from '../../../../src/types'; import {toggleExperiment} from '../../../../src/experiments'; import {user} from '../../../../src/log'; +import {installTimerService} from '../../../../src/service/timer-impl'; describes.realWin('amp-bind', { amp: { @@ -33,6 +34,7 @@ describes.realWin('amp-bind', { let canBindStub; beforeEach(() => { + installTimerService(env.win); toggleExperiment(env.win, 'amp-bind', true); // Stub validator methods to return true for ease of testing. @@ -144,6 +146,68 @@ describes.realWin('amp-bind', { }); }); + describe('under dynamic tags', () => { + it('should dynamically detect new bindings', () => { + const doc = env.win.document; + const template = doc.createElement('template'); + doc.getElementById('parent').appendChild(template); + return onBindReady().then(() => { + expect(bind.boundElements_.length).to.equal(0); + createElementWithBinding('[onePlusOne]="1+1"'); + return bind.waitForAllMutationsForTesting(); + }).then(() => { + expect(bind.boundElements_.length).to.equal(1); + }); + }); + + //TODO(kmh287): Move to integration test + it('should NOT bind blacklisted attributes', () => { + // Restore real implementations of canBind and isResultValid + sandbox.restore(); + const doc = env.win.document; + const template = doc.createElement('template'); + let textElement; + doc.getElementById('parent').appendChild(template); + return onBindReady().then(() => { + expect(bind.boundElements_.length).to.equal(0); + const binding = '[onclick]="\'alert(document.cookie)\'" ' + + '[onmouseover]="\'alert()\'" ' + + '[style]="\'background=color:black\'"'; + textElement = createElementWithBinding(binding); + return bind.waitForAllMutationsForTesting(); + }).then(() => { + expect(bind.boundElements_.length).to.equal(0); + expect(textElement.getAttribute('onclick')).to.be.null; + expect(textElement.getAttribute('onmouseover')).to.be.null; + expect(textElement.getAttribute('style')).to.be.null; + }); + }); + + //TODO(kmh287): Move to integration test + it('should NOT allow unsecure attribute values', () => { + // Restore real implementations of canBind and isResultValid + sandbox.restore(); + const doc = env.win.document; + const template = doc.createElement('template'); + let aElement; + doc.getElementById('parent').appendChild(template); + return onBindReady().then(() => { + expect(bind.boundElements_.length).to.equal(0); + const binding = '[href]="javascript:alert(1)"'; + const div = env.win.document.createElement('div'); + div.innerHTML = ''; + aElement = div.firstElementChild; + // Templated HTML is added as a sibling to the template, + // not as a child + doc.getElementById('parent').appendChild(aElement); + return bind.waitForAllMutationsForTesting(); + }).then(() => { + expect(aElement.getAttribute('href')).to.be.null; + }); + }); + }); + + it('should NOT apply expressions on first load', () => { const element = createElementWithBinding('[onePlusOne]="1+1"'); expect(element.getAttribute('onePlusOne')).to.equal(null); diff --git a/extensions/amp-mustache/0.1/test/test-amp-mustache.js b/extensions/amp-mustache/0.1/test/test-amp-mustache.js index 7d23dc5a0817b..5cf7d54b9da44 100644 --- a/extensions/amp-mustache/0.1/test/test-amp-mustache.js +++ b/extensions/amp-mustache/0.1/test/test-amp-mustache.js @@ -40,7 +40,47 @@ describe('amp-mustache template', () => { expect(result./*OK*/innerHTML).to.equal('value = abc'); }); + it('should sanitize templated tag names', () => { + const templateElement = document.createElement('div'); + templateElement./*OK*/innerHTML = + 'value = <{{value}} href="javascript:alert(0)">abc'; + const template = new AmpMustache(templateElement); + template.compileCallback(); + const result = template.render({ + value: 'a', + }); + expect(result./*OK*/innerHTML).to.not + .equal('abc'); + expect(result.firstElementChild).to.be.null; + }); + describe('Sanitizing data- attributes', () => { + + it('should sanitize templated attribute names', () => { + const templateElement = document.createElement('div'); + templateElement./*OK*/innerHTML = + 'value = abc'; + let template = new AmpMustache(templateElement); + template.compileCallback(); + let result = template.render({ + value: 'href', + }); + expect(result).to.not.equal('abc'); + expect(result.firstElementChild.getAttribute('href')).to.be.null; + + templateElement./*OK*/innerHTML = + 'value =

ALERT

'; + template = new AmpMustache(templateElement); + template.compileCallback(); + result = template.render({ + value: 'onclick', + }); + expect(result).to.not + .equal('

ALERT

'); + expect(result.firstElementChild.getAttribute('[onclick]')).to.be.null; + expect(result.firstElementChild.getAttribute('onclick')).to.be.null; + }); + it('should parse data-&style=value output correctly', () => { const templateElement = document.createElement('div'); templateElement./*OK*/innerHTML = 'value = {{allowed}}

+

+

diff --git a/extensions/amp-mustache/0.1/test/validator-amp-mustache.out b/extensions/amp-mustache/0.1/test/validator-amp-mustache.out index 8affa8725d184..866983a6ecd86 100644 --- a/extensions/amp-mustache/0.1/test/validator-amp-mustache.out +++ b/extensions/amp-mustache/0.1/test/validator-amp-mustache.out @@ -1,41 +1,43 @@ FAIL amp-mustache/0.1/test/validator-amp-mustache.html:34:2 Mustache template syntax in attribute name '{{notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] amp-mustache/0.1/test/validator-amp-mustache.html:35:2 Mustache template syntax in attribute name '{{notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:36:2 The attribute 'data-{notallowed}' may not appear in tag 'p'. [DISALLOWED_HTML] -amp-mustache/0.1/test/validator-amp-mustache.html:37:2 Mustache template syntax in attribute name 'data-{{notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:38:2 Mustache template syntax in attribute name 'data-{{{notallowed}}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:39:2 Mustache template syntax in attribute name 'data-{{¬allowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:40:2 Mustache template syntax in attribute name 'data-{{#notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:41:2 Mustache template syntax in attribute name 'data-{{/notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:42:2 Mustache template syntax in attribute name 'data-{{^notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:43:2 Mustache template syntax in attribute name 'data-{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:44:2 Mustache template syntax in attribute name '{{#notallowed}}class' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:45:2 Mustache template syntax in attribute name '{{#notallowed}}class{{/notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:46:2 The attribute 'title' in tag 'p' is set to '{{{notallowed}}}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:47:2 The attribute 'title' in tag 'p' is set to '{{¬allowed}}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:48:2 The attribute 'title' in tag 'p' is set to '{{>notallowed}}', which contains a Mustache template partial. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:49:2 The attribute 'data-title' in tag 'p' is set to '{{{notallowed}}}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:50:2 The attribute 'data-title' in tag 'p' is set to '{{¬allowed}}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:51:2 The attribute 'data-title' in tag 'p' is set to '{{>notallowed}}', which contains a Mustache template partial. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:56:2 Mustache template syntax in attribute name '{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:57:2 Mustache template syntax in attribute name '{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:58:2 Mustache template syntax in attribute name 'data-{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:59:2 Mustache template syntax in attribute name 'data-{{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:36:2 Mustache template syntax in attribute name '[{{notallowed}}]' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:37:2 The attribute 'data-{notallowed}' may not appear in tag 'p'. [DISALLOWED_HTML] +amp-mustache/0.1/test/validator-amp-mustache.html:38:2 Mustache template syntax in attribute name 'data-{{notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:39:2 Mustache template syntax in attribute name 'data-[{{notallowed}}]' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:40:2 Mustache template syntax in attribute name 'data-{{{notallowed}}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:41:2 Mustache template syntax in attribute name 'data-{{¬allowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:42:2 Mustache template syntax in attribute name 'data-{{#notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:43:2 Mustache template syntax in attribute name 'data-{{/notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:44:2 Mustache template syntax in attribute name 'data-{{^notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:45:2 Mustache template syntax in attribute name 'data-{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:46:2 Mustache template syntax in attribute name '{{#notallowed}}class' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:47:2 Mustache template syntax in attribute name '{{#notallowed}}class{{/notallowed}}' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:48:2 The attribute 'title' in tag 'p' is set to '{{{notallowed}}}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:49:2 The attribute 'title' in tag 'p' is set to '{{¬allowed}}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:50:2 The attribute 'title' in tag 'p' is set to '{{>notallowed}}', which contains a Mustache template partial. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:51:2 The attribute 'data-title' in tag 'p' is set to '{{{notallowed}}}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:52:2 The attribute 'data-title' in tag 'p' is set to '{{¬allowed}}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:53:2 The attribute 'data-title' in tag 'p' is set to '{{>notallowed}}', which contains a Mustache template partial. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:58:2 Mustache template syntax in attribute name '{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:59:2 Mustache template syntax in attribute name '{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] amp-mustache/0.1/test/validator-amp-mustache.html:60:2 Mustache template syntax in attribute name 'data-{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:61:2 Mustache template syntax in attribute name 'data-{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:61:2 Mustache template syntax in attribute name 'data-{{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] amp-mustache/0.1/test/validator-amp-mustache.html:62:2 Mustache template syntax in attribute name 'data-{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] amp-mustache/0.1/test/validator-amp-mustache.html:63:2 Mustache template syntax in attribute name 'data-{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] amp-mustache/0.1/test/validator-amp-mustache.html:64:2 Mustache template syntax in attribute name 'data-{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:65:2 Mustache template syntax in attribute name '{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:66:2 Mustache template syntax in attribute name '{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:67:2 The attribute 'title' in tag 'p' is set to '{{{ notallowed }}}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:68:2 The attribute 'title' in tag 'p' is set to '{{ ¬allowed }}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:69:2 The attribute 'title' in tag 'p' is set to '{{ >notallowed }}', which contains a Mustache template partial. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:70:2 The attribute 'title' in tag 'p' is set to '{{& notallowed }}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:71:2 The attribute 'title' in tag 'p' is set to '{{> notallowed }}', which contains a Mustache template partial. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:72:2 The attribute 'title' in tag 'p' is set to '{{ & notallowed }}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:73:2 The attribute 'title' in tag 'p' is set to '{{ > notallowed }}', which contains a Mustache template partial. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:98:4 The tag 'template' may not appear as a descendant of tag 'template'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:110:0 The attribute 'autoplay' in tag 'amp-audio' is set to the invalid value '{{invalid}}'. (see https://www.ampproject.org/docs/reference/extended/amp-audio.html) [AMP_TAG_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:124:7 The tag 'amp-audio extension .js script' is missing or incorrect, but required by 'amp-audio'. (see https://www.ampproject.org/docs/reference/extended/amp-audio.html) [AMP_TAG_PROBLEM] -amp-mustache/0.1/test/validator-amp-mustache.html:124:7 The tag 'amp-mustache extension .js script' is missing or incorrect, but required by 'template'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_TAG_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:65:2 Mustache template syntax in attribute name 'data-{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:66:2 Mustache template syntax in attribute name 'data-{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:67:2 Mustache template syntax in attribute name '{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:68:2 Mustache template syntax in attribute name '{{' in tag 'p'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:69:2 The attribute 'title' in tag 'p' is set to '{{{ notallowed }}}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:70:2 The attribute 'title' in tag 'p' is set to '{{ ¬allowed }}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:71:2 The attribute 'title' in tag 'p' is set to '{{ >notallowed }}', which contains a Mustache template partial. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:72:2 The attribute 'title' in tag 'p' is set to '{{& notallowed }}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:73:2 The attribute 'title' in tag 'p' is set to '{{> notallowed }}', which contains a Mustache template partial. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:74:2 The attribute 'title' in tag 'p' is set to '{{ & notallowed }}', which contains unescaped Mustache template syntax. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:75:2 The attribute 'title' in tag 'p' is set to '{{ > notallowed }}', which contains a Mustache template partial. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:100:4 The tag 'template' may not appear as a descendant of tag 'template'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_HTML_TEMPLATE_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:112:0 The attribute 'autoplay' in tag 'amp-audio' is set to the invalid value '{{invalid}}'. (see https://www.ampproject.org/docs/reference/extended/amp-audio.html) [AMP_TAG_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:126:7 The tag 'amp-audio extension .js script' is missing or incorrect, but required by 'amp-audio'. (see https://www.ampproject.org/docs/reference/extended/amp-audio.html) [AMP_TAG_PROBLEM] +amp-mustache/0.1/test/validator-amp-mustache.html:126:7 The tag 'amp-mustache extension .js script' is missing or incorrect, but required by 'template'. (see https://www.ampproject.org/docs/reference/extended/amp-mustache.html) [AMP_TAG_PROBLEM] diff --git a/src/sanitizer.js b/src/sanitizer.js index b1f4d0e1b9ddd..52ff095e6e683 100644 --- a/src/sanitizer.js +++ b/src/sanitizer.js @@ -25,6 +25,8 @@ import { import {parseSrcset} from './srcset'; import {user} from './log'; import {urls} from './config'; +import {map} from './utils/object'; +import {startsWith} from './string'; /** @private @const {string} */ @@ -106,6 +108,9 @@ const WHITELISTED_ATTRS = [ 'href', 'on', 'placeholder', + /* Attributes added for amp-bind */ + // TODO(kmh287): Add more whitelisted attributes for bind? + 'text', ]; @@ -125,7 +130,6 @@ const BLACKLISTED_ATTR_VALUES = [ /*eslint no-script-url: 0*/ '>} */ const BLACKLISTED_TAG_SPECIFIC_ATTR_VALUES = { 'input': { @@ -186,9 +190,19 @@ export function sanitizeHtml(html) { } return; } + const bindAttribsIndices = map(); + // Special handling for attributes for amp-bind which are formatted as + // [attr]. The brackets are restored at the end of this function. + for (let i = 0; i < attribs.length; i += 2) { + const attr = attribs[i]; + if (attr && attr[0] == '[' && attr[attr.length - 1] == ']') { + bindAttribsIndices[i] = true; + attribs[i] = attr.slice(1, -1); + } + } if (BLACKLISTED_TAGS[tagName]) { ignore++; - } else if (tagName.indexOf('amp-') != 0) { + } else if (!startsWith(tagName, 'amp-')) { // Ask Caja to validate the element as well. // Use the resulting properties. const savedAttribs = attribs.slice(0); @@ -198,11 +212,12 @@ export function sanitizeHtml(html) { } else { attribs = scrubbed.attribs; // Restore some of the attributes that AMP is directly responsible - // for, such as "on". + // for, such as "on" for (let i = 0; i < attribs.length; i += 2) { - if (WHITELISTED_ATTRS.indexOf(attribs[i]) != -1) { + const attrib = attribs[i]; + if (WHITELISTED_ATTRS.includes(attrib)) { attribs[i + 1] = savedAttribs[i + 1]; - } else if (attribs[i].search(WHITELISTED_ATTR_PREFIX_REGEX) == 0) { + } else if (attrib.search(WHITELISTED_ATTR_PREFIX_REGEX) == 0) { attribs[i + 1] = savedAttribs[i + 1]; } } @@ -251,7 +266,11 @@ export function sanitizeHtml(html) { continue; } emit(' '); - emit(attrName); + if (bindAttribsIndices[i]) { + emit('[' + attrName + ']'); + } else { + emit(attrName); + } emit('="'); if (attrValue) { emit(htmlSanitizer.escapeAttrib(rewriteAttributeValue( @@ -311,7 +330,7 @@ export function sanitizeFormattingHtml(html) { export function isValidAttr(tagName, attrName, attrValue) { // "on*" attributes are not allowed. - if (attrName.indexOf('on') == 0 && attrName != 'on') { + if (startsWith(attrName, 'on') && attrName != 'on') { return false; } @@ -320,6 +339,14 @@ export function isValidAttr(tagName, attrName, attrValue) { return false; } + // See validator-main.protoascii + // https://github.com/ampproject/amphtml/blob/master/validator/validator-main.protoascii + if (attrName == 'class' && + attrValue && + /(^|\W)i-amphtml-/i.test(attrValue)) { + return false; + } + // No attributes with "javascript" or other blacklisted substrings in them. if (attrValue) { const attrValueNorm = attrValue.toLowerCase().replace(/[\s,\u0000]+/g, ''); @@ -336,7 +363,8 @@ export function isValidAttr(tagName, attrName, attrValue) { return false; } - // Remove blacklisted values for specific attributes e.g. input[type=image]. + // Remove blacklisted values for specific attributes for specific tags + // e.g. input[type=image]. const attrBlacklist = BLACKLISTED_TAG_SPECIFIC_ATTR_VALUES[tagName]; if (attrBlacklist) { const blacklistedValuesRegex = attrBlacklist[attrName]; @@ -384,7 +412,7 @@ export function resolveUrlAttr(tagName, attrName, attrValue, windowLocation) { const isProxyHost = isProxyOrigin(windowLocation); const baseUrl = parseUrl(getSourceUrl(windowLocation)); - if (attrName == 'href' && attrValue.indexOf('#') != 0) { + if (attrName == 'href' && !startsWith(attrValue, '#')) { return resolveRelativeUrl(attrValue, baseUrl); } diff --git a/src/service/timer-impl.js b/src/service/timer-impl.js index 31e9d97b80bbf..3e3f679b864f6 100644 --- a/src/service/timer-impl.js +++ b/src/service/timer-impl.js @@ -150,6 +150,25 @@ export class Timer { } return Promise.race([delayPromise, opt_racePromise]); } + + /** + * Returns a promise that resolves after `predicate` returns true. + * Polls with interval `delay` + * @param {number} delay + * @param {function():boolean} predicate + * @return {!Promise} + */ + poll(delay, predicate) { + return new Promise(resolve => { + const interval = this.win.setInterval(() => { + if (predicate()) { + this.win.clearInterval(interval); + resolve(); + } + }, delay); + }); + } + } /** diff --git a/test/functional/test-sanitizer.js b/test/functional/test-sanitizer.js index 4e0470cc1350e..10d15a2978cb7 100644 --- a/test/functional/test-sanitizer.js +++ b/test/functional/test-sanitizer.js @@ -155,6 +155,8 @@ describe('sanitizeHtml', () => { it('should NOT output security-sensitive attributes', () => { expect(sanitizeHtml('ab')).to.be.equal('ab'); + expect(sanitizeHtml('ab')).to.be + .equal('ab'); }); it('should apply html4/caja restrictions', () => { @@ -163,6 +165,51 @@ describe('sanitizeHtml', () => { expect(sanitizeHtml('
b
')).to.be .equal('
b
'); }); + + it('should output [text] and [class] attributes', () => { + expect(sanitizeHtml('

')).to.be + .equal('

'); + }); + + it('should NOT output blacklisted values for class attributes', () => { + expect(sanitizeHtml('

hello

')).to.be + .equal('

hello

'); + expect(sanitizeHtml('

hello

')).to.be + .equal('

hello

'); + expect(sanitizeHtml('

hello

')).to.be + .equal('

hello

'); + expect(sanitizeHtml('

hello

')).to.be + .equal('

hello

'); + expect(sanitizeHtml('

hello

')).to.be + .equal('

hello

'); + expect(sanitizeHtml('

hello

')).to.be + .equal('

hello

'); + }); + + it('should NOT output security-sensitive amp-bind attributes', () => { + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + }); }); diff --git a/test/functional/test-timer.js b/test/functional/test-timer.js index cf8a3ac9dbbc3..f24da80c1de37 100644 --- a/test/functional/test-timer.js +++ b/test/functional/test-timer.js @@ -17,7 +17,7 @@ import {Timer} from '../../src/service/timer-impl'; import * as sinon from 'sinon'; -describe('Timer', () => { +describes.fakeWin('Timer', {}, env => { let sandbox; let windowMock; @@ -31,6 +31,7 @@ describe('Timer', () => { WindowApi.prototype.document = {}; const windowApi = new WindowApi(); windowMock = sandbox.mock(windowApi); + timer = new Timer(windowApi); }); @@ -142,4 +143,31 @@ describe('Timer', () => { expect(reason.message).to.contain('timeout'); }); }); + + it('poll - resolves only when condition is true', done => { + const realTimer = new Timer(env.win); + let predicate = false; + realTimer.poll(10, () => { + return predicate; + }).then(() => { + expect(predicate).to.be.true; + done(); + }); + setTimeout(() => { + predicate = true; + }, 15); + }); + + it('poll - clears out interval when complete', done => { + const realTimer = new Timer(env.win); + const clearIntervalStub = sandbox.stub(); + env.win.clearInterval = clearIntervalStub; + realTimer.poll(111, () => { + return true; + }).then(() => { + expect(clearIntervalStub).to.have.been.calledOnce; + done(); + }); + }); + }); diff --git a/third_party/caja/README.amp b/third_party/caja/README.amp index 48d23bfa88b52..7a8c5db246f79 100644 --- a/third_party/caja/README.amp +++ b/third_party/caja/README.amp @@ -9,3 +9,4 @@ Run compile.sh to generate a new version. Local Modifications: Export to ES6. Change unicode literal to unicode escaped character. (see https://github.com/google/caja/issues/1978) +Change html attribute regex to optionally accept bracketed attribute names for amp-bind diff --git a/third_party/caja/html-sanitizer.js b/third_party/caja/html-sanitizer.js index ecc76676f8fa7..7e7ee7f12784b 100644 --- a/third_party/caja/html-sanitizer.js +++ b/third_party/caja/html-sanitizer.js @@ -411,7 +411,7 @@ URI.prototype.setPath = function (newPath) { URI.prototype.setRawPath = function (newPath) { if (newPath) { newPath = String(newPath); - this.path_ = + this.path_ = // Paths must start with '/' unless this is a path-relative URL. (!this.domain_ || /^\//.test(newPath)) ? newPath : '/' + newPath; } else { @@ -1608,7 +1608,7 @@ var html = (function(html4) { var ATTR_RE = new RegExp( '^\\s*' + - '([-.:\\w]+)' + // 1 = Attribute name + '(\\[[-.:\\w]+\\]|[-.:\\w]+)' + // 1 = Attribute name '(?:' + ( '\\s*(=)\\s*' + // 2 = Is there a value? '(' + ( // 3 = Attribute value