ampproject · Gregable · Dec 11, 2015 · Dec 11, 2015
diff --git a/validator/htmlparser.js b/validator/htmlparser.js
@@ -307,8 +307,11 @@ amp.htmlparser.HtmlParser.INSIDE_TAG_TOKEN_ = new RegExp(
     // interpreters are inconsistent in whether a group that matches nothing
     // is null, undefined, or the empty string.
     ('(?:' +
-    '([^\\t\\r\\n /=>\\"]+)' +          // attribute name
-    ('(' +                              // optionally followed
+    // We don't allow attribute names starting with '/', but we allow them
+    // to contain '/' (differing from HTML5 spec), so that we can identify
+    // full mustache template variables and emit matching errors.
+    '([^\\t\\r\\n /=>\\"][^\\t\\r\\n =>\\"]*)' +  // attribute name
+    ('(' +                                        // optionally followed
     '\\s*=\\s*' +
     ('(' +
              // A double quoted string.

diff --git a/validator/testdata/feature_tests/template.html b/validator/testdata/feature_tests/template.html
@@ -0,0 +1,93 @@
+<!--
+  Copyright 2015 The AMP HTML Authors. All Rights Reserved.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS-IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the license.
+-->
+<!--
+  Test Description:
+  This tests the logic for <template> tags and mustache variable replacements.
+-->
+<!doctype html>
+<html ⚡>
+<head>
+  <meta charset="utf-8">
+  <link rel="canonical" href="./regular-html-version.html" />
+  <meta name="viewport" content="width=device-width,minimum-scale=1">
+  <style>body {opacity: 0}</style><noscript><style>body {opacity: 1}</style></noscript>
+  <script async src="https://cdn.ampproject.org/v0.js"></script>
+</head>
+<body>
+
+<template type="amp-mustache">
+  <{{not-actually-parsed-as-an-html-tag-so-allowed}}>
+  <p title="{{allowed}}">{{allowed}}</p>
+  <p {{notallowed}}></p>
+  <p {{notallowed}}=0></p>
+  <p data-{notallowed}=0></p>
+  <p data-{{notallowed}}=0></p>
+  <p data-{{{notallowed}}}=0></p>
+  <p data-{{&notallowed}}=0></p>
+  <p data-{{#notallowed}}=0></p>
+  <p data-{{/notallowed}}=0></p>
+  <p data-{{^notallowed}}=0></p>
+  <p data-{{>notallowed}}=0></p>
+  <p {{#notallowed}}class=foo{{/notallowed}}>
+  <p {{#notallowed}}class{{/notallowed}}>
+  <p title="{{{notallowed}}}"></p>
+  <p title="{{&notallowed}}"></p>
+  <p title="{{>notallowed}}"></p>
+
+  <!-- now with some whitespace inside the mustache tags -->
+  <{{ not-actually-parsed-as-an-html-tag-so-allowed }}>
+  <p title="{{ allowed }}">{{ allowed }}</p>
+  <p {{ notallowed }}></p>
+  <p {{ notallowed }}=0></p>
+  <p data-{{ notallowed }}=0></p>
+  <p data-{{{ notallowed }}}=0></p>
+  <p data-{{ &notallowed }}=0></p>
+  <p data-{{ #notallowed }}=0></p>
+  <p data-{{ /notallowed }}=0></p>
+  <p data-{{ ^notallowed }}=0></p>
+  <p data-{{ >notallowed }}=0></p>
+  <p {{ #notallowed }}class=foo{{ /notallowed }}>
+  <p {{ #notallowed }}class{{ /notallowed }}>
+  <p title="{{{ notallowed }}}"></p>
+  <p title="{{ &notallowed }}"></p>
+  <p title="{{ >notallowed }}"></p>
+  <p title="{{& notallowed }}"></p>
+  <p title="{{> notallowed }}"></p>
+  <p title="{{ & notallowed }}"></p>
+  <p title="{{ > notallowed }}"></p>
+
+  <!-- Note, this is allowed by the validator, but it is critical that it
+       be sanitized by the runtime. If the runtime allowed this, then after
+       rendering (assuming #off was null) we would have:
+      <a href="javascript:alert('foo')"></a>
+   -->
+  <a href="{{#off}}"></a>
+    {{/off}}javascript:alert('foo'){{#off}}
+  <a href="{{/off}"></a>
+
+  <!-- Allowed by the validator, but could lead to script execution
+       depending on the value. -->
+  <a href="{{foo}}"></a>
+  <a href="java{{foo}}script:alert('foo')"></a>
+
+  <!-- Really tricky example that the validator allows, but the runtime
+       must handle. Essentially if {{foo}} is an empty string, this becomes
+       a <script> tag, otherwise, it's just a harmless comment -->
+  <!-- comment --{{foo}}><script><!-- -->
+
+</template>
+</body>
+</html>
diff --git a/validator/testdata/feature_tests/template.out b/validator/testdata/feature_tests/template.out
@@ -0,0 +1,34 @@
+FAIL
+feature_tests/template.html:34:2 TEMPLATE_IN_ATTR_NAME {{notallowed}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:35:2 TEMPLATE_IN_ATTR_NAME {{notallowed}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:36:2 DISALLOWED_ATTR data-{notallowed}
+feature_tests/template.html:37:2 TEMPLATE_IN_ATTR_NAME data-{{notallowed}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:38:2 TEMPLATE_IN_ATTR_NAME data-{{{notallowed}}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:39:2 TEMPLATE_IN_ATTR_NAME data-{{&notallowed}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:40:2 TEMPLATE_IN_ATTR_NAME data-{{#notallowed}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:41:2 TEMPLATE_IN_ATTR_NAME data-{{/notallowed}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:42:2 TEMPLATE_IN_ATTR_NAME data-{{^notallowed}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:43:2 TEMPLATE_IN_ATTR_NAME data-{{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:44:2 TEMPLATE_IN_ATTR_NAME {{#notallowed}}class (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:45:2 TEMPLATE_IN_ATTR_NAME {{#notallowed}}class{{/notallowed}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:46:2 UNESCAPED_TEMPLATE_IN_ATTR_VALUE title={{{notallowed}}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:47:2 UNESCAPED_TEMPLATE_IN_ATTR_VALUE title={{&notallowed}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:48:2 TEMPLATE_PARTIAL_IN_ATTR_VALUE title={{>notallowed}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:53:2 TEMPLATE_IN_ATTR_NAME {{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:54:2 TEMPLATE_IN_ATTR_NAME {{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:55:2 TEMPLATE_IN_ATTR_NAME data-{{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:56:2 TEMPLATE_IN_ATTR_NAME data-{{{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:57:2 TEMPLATE_IN_ATTR_NAME data-{{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:58:2 TEMPLATE_IN_ATTR_NAME data-{{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:59:2 TEMPLATE_IN_ATTR_NAME data-{{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:60:2 TEMPLATE_IN_ATTR_NAME data-{{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:61:2 TEMPLATE_IN_ATTR_NAME data-{{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:62:2 TEMPLATE_IN_ATTR_NAME {{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:63:2 TEMPLATE_IN_ATTR_NAME {{ (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:64:2 UNESCAPED_TEMPLATE_IN_ATTR_VALUE title={{{ notallowed }}} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:65:2 UNESCAPED_TEMPLATE_IN_ATTR_VALUE title={{ &notallowed }} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:66:2 TEMPLATE_PARTIAL_IN_ATTR_VALUE title={{ >notallowed }} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:67:2 UNESCAPED_TEMPLATE_IN_ATTR_VALUE title={{& notallowed }} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:68:2 TEMPLATE_PARTIAL_IN_ATTR_VALUE title={{> notallowed }} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:69:2 UNESCAPED_TEMPLATE_IN_ATTR_VALUE title={{ & notallowed }} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
+feature_tests/template.html:70:2 TEMPLATE_PARTIAL_IN_ATTR_VALUE title={{ > notallowed }} (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md)
diff --git a/validator/validator.js b/validator/validator.js
@@ -149,12 +149,25 @@ function specificity(code) {
       return 13;
     case amp.validator.ValidationError.Code.MUTUALLY_EXCLUSIVE_ATTRS:
       return 14;
-    case amp.validator.ValidationError.Code.DEV_MODE_ENABLED:
+    case amp.validator.ValidationError.Code.UNESCAPED_TEMPLATE_IN_ATTR_VALUE:
       return 15;
-    case amp.validator.ValidationError.Code.DEPRECATED_ATTR:
+    case amp.validator.ValidationError.Code.TEMPLATE_PARTIAL_IN_ATTR_VALUE:
       return 16;
-    case amp.validator.ValidationError.Code.DEPRECATED_TAG:
+    case amp.validator.ValidationError.Code.TEMPLATE_IN_ATTR_NAME:
       return 17;
+
+    // TODO(johannes): The last three are fairly specific but not necessarily
+    // errors. We may need to take that into account to prevent a non-error
+    // from getting rendered as the sole explanation for an overall failure.
+    // I have not seen this happening but it probably could for edge cases
+    // (e.g. maxerrors set too low, etc.).
+    case amp.validator.ValidationError.Code.DEV_MODE_ENABLED:
+      return 100;
+    case amp.validator.ValidationError.Code.DEPRECATED_ATTR:
+      return 101;
+    case amp.validator.ValidationError.Code.DEPRECATED_TAG:
+      return 102;
+
     default:
       goog.asserts.fail('Unrecognized Code: ' + code);
   }
@@ -1035,12 +1048,14 @@ function getDetailOrName(tagSpec) {
 /**
  * This wrapper class provides access to a TagSpec and a tag id
  * which is unique within its context, the ParsedValidatorRules.
+ * @param {!string} templateSpecUrl
  * @param {!amp.validator.TagSpec} tagSpec
  * @param {number} tagId
  * @param {!goog.structs.Map<string, !amp.validator.AttrList>} attrListsByName
  * @constructor
  */
-const ParsedTagSpec = function ParsedTagSpec(tagSpec, tagId, attrListsByName) {
+const ParsedTagSpec = function ParsedTagSpec(
+    templateSpecUrl, tagSpec, tagId, attrListsByName) {
   /**
    * @type {!amp.validator.TagSpec}
    * @private
@@ -1075,6 +1090,11 @@ const ParsedTagSpec = function ParsedTagSpec(tagSpec, tagId, attrListsByName) {
    * @private
    */
   this.mandatoryOneofs_ = [];
+  /**
+   * @type {string}
+   * @private
+   */
+  this.templateSpecUrl_ = /** @type {string} */ templateSpecUrl;
 
   const attrs = GetAttrsFor(tagSpec, attrListsByName);
   for (let i = 0; i < attrs.length; ++i) {
@@ -1119,6 +1139,23 @@ ParsedTagSpec.prototype.getSpec = function() {
   return this.spec_;
 };
 
+ParsedTagSpec.prototype.valueHasUnescapedTemplateSyntax = function(value) {
+  // Mustache (https://mustache.github.io/mustache.5.html), our template
+  // system, supports {{{unescaped}}} or {{{&unescaped}}} and there can
+  // be whitespace after the 2nd '{'. We disallow these in attribute Values.
+  const unescapedOpenTag = new RegExp('{{\\s*[&{]', 'g');
+  return unescapedOpenTag.test(value);
+};
+
+ParsedTagSpec.prototype.valueHasPartialsTemplateSyntax = function(value) {
+  // Mustache (https://mustache.github.io/mustache.5.html), our template
+  // system, supports 'partials' which include other Mustache templates
+  // in the format of {{>partial}} and there can be whitespace after the {{.
+  // We disallow partials in attribute values.
+  const unescapedOpenTag = new RegExp('{{\\s*>', 'g');
+  return unescapedOpenTag.test(value);
+};
+
 /**
  * Validates whether the attributes set on |encountered_tag| conform to this
  * tag specification. All mandatory attributes must appear. Only attributes
@@ -1151,14 +1188,42 @@ ParsedTagSpec.prototype.validateAttributes = function(
       // in practice, some type of ad or perhaps other custom elements require
       // particular data attributes.
       // http://www.w3.org/TR/html5/single-page.html#attr-data-*
-      if (goog.string.startsWith(encounteredAttrKey, 'data-'))
+      // http://w3c.github.io/aria-in-html/
+      // However, mostly to avoid confusion, we want to make sure that
+      // nobody tries to make a Mustache template data attribute,
+      // e.g. <div data-{{foo}}>, so we also exclude those characters.
+      if (goog.string.startsWith(encounteredAttrKey, 'data-') &&
+          !goog.string.contains(encounteredAttrKey, '}') &&
+          !goog.string.contains(encounteredAttrKey, '{')) {
         continue;
+      }
 
-      context.addError(amp.validator.ValidationError.Code.DISALLOWED_ATTR,
-                       encounteredAttrName, this.spec_.specUrl,
-                       resultForAttempt);
+      // At this point, it's an error either way, but we try to give a
+      // more specific error in the case of Mustache template characters.
+      if (encounteredAttrName.indexOf('{{') != -1) {
+        context.addError(
+            amp.validator.ValidationError.Code.TEMPLATE_IN_ATTR_NAME,
+            encounteredAttrName, this.templateSpecUrl_, resultForAttempt);
+      } else {
+        context.addError(amp.validator.ValidationError.Code.DISALLOWED_ATTR,
+                         encounteredAttrName, this.spec_.specUrl,
+                         resultForAttempt);
+      }
       return;
     }
+    if (parsedSpec.valueHasUnescapedTemplateSyntax(encounteredAttrValue)) {
+      context.addError(
+          amp.validator.ValidationError.Code.UNESCAPED_TEMPLATE_IN_ATTR_VALUE,
+          encounteredAttrName + '=' + encounteredAttrValue,
+          this.templateSpecUrl_, resultForAttempt);
+    }
+    if (parsedSpec.valueHasPartialsTemplateSyntax(encounteredAttrValue)) {
+      context.addError(
+          amp.validator.ValidationError.Code.TEMPLATE_PARTIAL_IN_ATTR_VALUE,
+          encounteredAttrName + '=' + encounteredAttrValue,
+          this.templateSpecUrl_, resultForAttempt);
+    }
+
     if (parsedSpec.getSpec().deprecation !== null) {
       context.addError(amp.validator.ValidationError.Code.DEPRECATED_ATTR,
                        encounteredAttrName + ' - ' +
@@ -1310,7 +1375,8 @@ const ParsedValidatorRules = function ParsedValidatorRules() {
 
   for (let i = 0; i < rules.tags.length; ++i) {
     const spec = rules.tags[i];
-    const parsedTagSpec = new ParsedTagSpec(spec, i, attrListsByName);
+    const parsedTagSpec = new ParsedTagSpec(
+        rules.templateSpecUrl, spec, i, attrListsByName);
     this.tagSpecById_.push(parsedTagSpec);
     goog.asserts.assert(spec.name !== null);
     if (!this.tagSpecByTagName_.containsKey(spec.name)) {

diff --git a/validator/validator.proto b/validator/validator.proto
@@ -210,6 +210,9 @@ message ValidatorRules {
   // See comment in validator.protoascii.
   optional int32 spec_file_revision = 6 [ default = -1 ];
 
+  // Spec URL for information about mustache templates.
+  optional string template_spec_url = 8;
+
   // Deprecated fields, do not reuse.
   extensions 5, 2;
 }
@@ -250,6 +253,9 @@ message ValidationError {
     INVALID_PROPERTY_VALUE_IN_ATTR_VALUE = 15;
     DISALLOWED_PROPERTY_IN_ATTR_VALUE = 16;
     MUTUALLY_EXCLUSIVE_ATTRS = 17;
+    UNESCAPED_TEMPLATE_IN_ATTR_VALUE = 18;
+    TEMPLATE_PARTIAL_IN_ATTR_VALUE = 19;
+    TEMPLATE_IN_ATTR_NAME = 20;
   }
   optional Code code = 1;
   optional int32 line = 2 [ default = 1 ];

diff --git a/validator/validator.protoascii b/validator/validator.protoascii
@@ -20,20 +20,24 @@
 # in production from crashing. This id is not relevant to validator.js
 # because thus far, engine (validator.js) and spec file (validator.protoascii)
 # are always released together.
-min_validator_revision_required: 65
+min_validator_revision_required: 66
 # The spec file revision allows the validator engine to distinguish
 # newer versions of the spec file. This is currently a Google internal
 # mechanism, validator.js does not use this facility. However, any
 # change to this file requires updating this revision id.
-spec_file_revision: 95
-
+spec_file_revision: 96
 # Rules for AMP HTML
 # (https://github.com/google/amphtml/blob/master/spec/amp-html-format.md).
 # These rules are just enough to validate the example from the spec.
 # They are certainly not sufficent for release: many tags are missing.
 # Also we may want to allow some attribute definitions to be shared
 # between tags. As in, we may almost certainly need to revisit this format,
 # but it's a starting point.
+#
+# Specific URL to reference for violations having to do with mustache
+# templates. These are specific to any particular tag, so must live at
+# the top level of this rules file.
+template_spec_url: "https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md"
 tags: {
   name: "!doctype"
   detail: "html doctype"
@@ -1479,6 +1483,27 @@ tags: {
     mandatory: true
   }
 }
+tags: {
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-mustache runtime"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md"
+  attrs: {
+    name: "custom-template"
+    value: "amp-mustache"
+    mandatory: true
+  }
+  attrs: {
+    name: "async"
+    value: ""
+    mandatory: true
+  }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-mustache-0.1.js"
+    mandatory: true
+  }
+}
 # 4.11.2 Non-conforming features
 tags: { name: "acronym" }
 tags: { name: "big" }
@@ -1494,6 +1519,17 @@ tags: { name: "strike" }
 tags: { name: "tt" }
 tags: { name: "xmp" }
 
+# 4.11.3 The template element
+tags: {
+  name: "template"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md"
+  attrs: {
+    name: "type"
+    value: "amp-mustache"
+    mandatory: true
+  }
+}
+
 # Some additional tags, microformats, allowances.
 #