whitelist of variables

src/service/url-replacements-impl.js

@@ -68,11 +68,11 @@ function encodeValue(val) {
  * Class to provide variables that pertain to top level AMP window.
  */
 export class GlobalVariableSource extends VariableSource {
-
+  /**
+   * @param {!./ampdoc-impl.AmpDoc} ampdoc
+   */
   constructor(ampdoc) {
-    super();
-    /** @const {!./ampdoc-impl.AmpDoc} */
-    this.ampdoc = ampdoc;
+    super(ampdoc);
 
     /**
      * @private

src/service/variable-source.js

@@ -109,7 +109,13 @@ export function getNavigationData(win, attribute) {
  * and override initialize() to add more supported variables.
  */
 export class VariableSource {
-  constructor() {
+  /**
+   * @param {?./ampdoc-impl.AmpDoc} ampdoc
+   */
+  constructor(ampdoc) {
+    /** @protected @const {?./ampdoc-impl.AmpDoc} */
+    this.ampdoc = ampdoc;
+
     /** @private {!RegExp|undefined} */
     this.replacementExpr_ = undefined;
 
@@ -121,6 +127,47 @@ export class VariableSource {
 
     /** @private {boolean} */
     this.initialized_ = false;
+
+    /** 
+     * The whitelist of variables allowed for variable substitution.
+     * @private @const {?Array<string>} 
+     */
+    this.variableWhitelist_ = this.getVariableWhitelist_();
+  }
+
+  /**
+   * @return {?Array<string>} The whitelist of allowed AMP variables. (if provided in 
+   *     a meta tag).
+   * @private
+   */
+  getVariableWhitelist_() {
+    if (this.variableWhitelist_) {
+      return this.variableWhitelist_;
+    }
+
+    // TODO(hamousavi): Remove this conditional and change the type annotation 
+    // for this.ampdoc to non-nullable. This is a temporary measures because
+    // tests currently do not respect the non-nullability measure. 
+    if (!this.ampdoc) {
+      return null;
+    }
+
+    const head = this.ampdoc.getRootNode().head;
+    if (!head) {
+      return null;
+    }
+
+    // A meta[name="amp-variable-substitution-whitelist"] tag, if present, 
+    // contains, in its content attribute, a whitelist of variable substitution.
+    const meta =
+      head.querySelector('meta[name="amp-variable-substitution-whitelist"]');
+    if (!meta) {
+      return null;
+    }
+
+    this.variableWhitelist_ = meta.getAttribute('content').split(',')
+        .map(variable => variable.trim());
+    return this.variableWhitelist_;
   }
 
   /**
@@ -164,6 +211,10 @@ export class VariableSource {
    */
   set(varName, syncResolver) {
     dev().assert(varName.indexOf('RETURN') == -1);
+    if (!this.isWhitelisted_(varName)) {
+      return this;
+    }
+
     this.replacements_[varName] =
         this.replacements_[varName] || {sync: undefined, async: undefined};
     this.replacements_[varName].sync = syncResolver;
@@ -184,6 +235,9 @@ export class VariableSource {
    */
   setAsync(varName, asyncResolver) {
     dev().assert(varName.indexOf('RETURN') == -1);
+    if (!this.isWhitelisted_(varName)) {
+      return this;
+    }
     this.replacements_[varName] =
         this.replacements_[varName] || {sync: undefined, async: undefined};
     this.replacements_[varName].async = asyncResolver;
@@ -246,6 +300,12 @@ export class VariableSource {
    * @private
    */
   buildExpr_(keys, isV2) {
+    // If a whitelist is present, the keys must belong to the whitelist.
+    // We filter the keys one last time to ensure no unwhitelisted key is 
+    // allowed.
+    if (this.getVariableWhitelist_()) {
+      keys = keys.filter(key => this.getVariableWhitelist_().includes(key));
+    }
     // The keys must be sorted to ensure that the longest keys are considered
     // first. This avoids a problem where a RANDOM conflicts with RANDOM_ONE.
     keys.sort((s1, s2) => s2.length - s1.length);
@@ -264,4 +324,16 @@ export class VariableSource {
     }
     return new RegExp(regexStr, 'g');
   }
+
+  /**
+   * Returns `true` if a variable whitelist is *not* present or the present
+   * whitelist contains the given variable name.
+   * @param {string} varName
+   * @return {boolean} 
+   * @private
+   */
+  isWhitelisted_(varName) {
+    return !this.getVariableWhitelist_() ||
+      this.getVariableWhitelist_().includes(varName);
+  }
 }

test/functional/test-variable-source.js

@@ -19,11 +19,17 @@ import {
   getTimingDataAsync,
 } from '../../src/service/variable-source';
 
+import {createElementWithAttributes} from '../../src/dom';
 
-describe('VariableSource', () => {
+
+describes.fakeWin('VariableSource', {
+    amp: {
+      ampdoc: 'single',
+    },
+}, env => {
   let varSource;
   beforeEach(() => {
-    varSource = new VariableSource();
+    varSource = new VariableSource(env.ampdoc);
   });
 
   it('Works without any variables', () => {
@@ -84,6 +90,39 @@ describe('VariableSource', () => {
     });
   });
 
+  describes.realWin('Whitelist of variable substitutions', {
+    amp: {
+      ampdoc: 'single',
+    },
+  }, env => {
+    let variableSource;
+    beforeEach(() => {
+      env.win.document.head.appendChild(
+        createElementWithAttributes(env.win.document, 'meta', {
+          name: 'amp-variable-substitution-whitelist',
+          content: 'ABC,ABCD,CANONICAL',
+        }));
+      variableSource = new VariableSource(env.ampdoc);
+    });
+
+    it('Works with whitelisted variables', () => {
+      variableSource.setAsync('ABCD', () => Promise.resolve('abcd'));
+      expect(variableSource.getExpr()).to.be.ok;
+
+      return variableSource.get('ABCD')['async']().then(value => {
+        expect(value).to.equal('abcd');
+      });
+    });
+
+    it('Should not work with unwhitelisted variables', () => {
+      variableSource.setAsync('RANDOM', () => Promise.resolve('0.1234'));
+      expect(variableSource.getExpr()).to.be.ok;
+
+      expect(variableSource.get('RANDOM')).to.be.undefined;
+    });
+
+  });
+
   describes.fakeWin('getTimingData', {}, env => {
     let win;
 
@@ -116,4 +155,4 @@ describe('VariableSource', () => {
       });
     });
   });
-});
+});

test/functional/url-expander/test-expander.js

@@ -14,8 +14,10 @@
  * limitations under the License.
  */
 
+import {AmpDocSingle} from '../../../src/service/ampdoc-impl';
 import {Expander} from '../../../src/service/url-expander/expander';
 import {GlobalVariableSource} from '../../../src/service/url-replacements-impl';
+import {createElementWithAttributes} from '../../../src/dom';
 
 describes.realWin('Expander', {
   amp: {
@@ -86,6 +88,41 @@ describes.realWin('Expander', {
     });
   });
 
+  describe('Whitelist of variables', () => {
+    let variableSource;
+    let expander;
+
+    const mockBindings = {
+      RANDOM: () => 0.1234,
+      ABC: () => 'three',
+      ABCD: () => 'four',
+    };
+    beforeEach(() => {
+      env.win.document.head.appendChild(
+        createElementWithAttributes(env.win.document, 'meta', {
+          name: 'amp-variable-substitution-whitelist',
+          content: 'ABC,ABCD,CANONICAL',
+        }));
+
+      variableSource = new GlobalVariableSource(env.ampdoc);
+      expander = new Expander(variableSource);
+    });
+
+    it('should not replace unwhitelisted RANDOM', () => {
+      const url = 'http://www.google.com/?test=RANDOM';
+      const expected = 'http://www.google.com/?test=RANDOM';
+      return expect(expander.expand(url, mockBindings))
+          .to.eventually.equal(expected);
+    });
+
+    it('should replace whitelisted ABCD', () => {
+      const url = 'http://www.google.com/?test=ABCD';
+      const expected = 'http://www.google.com/?test=four';
+      return expect(expander.expand(url, mockBindings))
+          .to.eventually.equal(expected);
+    });
+  });
+
   describe('#expand', () => {
     function mockClientIdFn(str) {
       if (str === '__ga') {