Remove "amp-allowed-url-macros" meta

ampproject · Apr 13, 2020 · 56cfae3 · 56cfae3
1 parent 0e243a6
commit 56cfae3
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 34 deletions.
diff --git a/src/service/variable-source.js b/src/service/variable-source.js
@@ -331,8 +331,7 @@ export class VariableSource {
   }
 
   /**
-   * Searches for the "amp-allowed-url-macros" meta tag, parses and returns its contents.
-   * For email documents, the allowlist defaults to empty and supercedes the meta tag.
+   * For email documents, all URL macros are disallowed by default.
    * @return {?Array<string>} The allowlist of substitutable AMP variables
    * @private
    */
@@ -341,8 +340,7 @@ export class VariableSource {
       return this.variableWhitelist_;
     }
 
-    // Allow no URL macros for AMP4Email format documents.
-    // This cannot be overwritten even if the meta tag is provided.
+    // Disallow all URL macros for AMP4Email format documents.
     if (this.ampdoc.isSingleDoc()) {
       const doc = /** @type {Document} */ (this.ampdoc.getRootNode());
       if (
@@ -351,24 +349,13 @@ export class VariableSource {
         doc.documentElement.hasAttribute &&
         isAmp4Email(doc)
       ) {
-        return [];
+        /**
+         * The whitelist of variables allowed for variable substitution.
+         * @private {?Array<string>}
+         */
+        this.variableWhitelist_ = [''];
+        return this.variableWhitelist_;
       }
     }
-
-    // A meta[name="amp-allowed-url-macros"] tag, if present,
-    // contains, in its content attribute, a whitelist of variable substitution.
-    const meta = this.ampdoc.getMetaByName('amp-allowed-url-macros');
-    if (meta === null) {
-      return null;
-    }
-
-    /**
-     * The whitelist of variables allowed for variable substitution.
-     * @private {?Array<string>}
-     */
-    this.variableWhitelist_ = meta
-      .split(',')
-      .map((variable) => variable.trim());
-    return this.variableWhitelist_;
   }
 }
diff --git a/test/unit/test-variable-source.js b/test/unit/test-variable-source.js
@@ -128,10 +128,8 @@ describes.fakeWin(
       (env) => {
         let variableSource;
         beforeEach(() => {
-          env.sandbox.stub(env.ampdoc, 'getMeta').returns({
-            'amp-allowed-url-macros': 'ABC,ABCD,CANONICAL',
-          });
           variableSource = new VariableSource(env.ampdoc);
+          variableSource.variableWhitelist_ = ['ABC', 'ABCD', 'CANONICAL'];
         });
 
         it('Works with whitelisted variables', () => {
@@ -173,10 +171,8 @@ describes.fakeWin(
     );
 
     it('Should not work with empty variable whitelist', () => {
-      env.sandbox.stub(env.ampdoc, 'getMeta').returns({
-        'amp-allowed-url-macros': '',
-      });
       const variableSource = new VariableSource(env.ampdoc);
+      variableSource.variableWhitelist_ = [''];
 
       variableSource.setAsync('RANDOM', () => Promise.resolve('0.1234'));
       expect(variableSource.getExpr()).to.be.ok;

diff --git a/test/unit/url-expander/test-expander.js b/test/unit/url-expander/test-expander.js
@@ -99,17 +99,14 @@ describes.realWin(
       };
 
       function createExpanderWithWhitelist(whitelist, mockBindings) {
-        env.sandbox.stub(env.ampdoc, 'getMeta').returns({
-          'amp-allowed-url-macros': whitelist,
-        });
-
         variableSource = new GlobalVariableSource(env.ampdoc);
+        variableSource.variableWhitelist_ = whitelist;
         return new Expander(variableSource, mockBindings);
       }
 
       it('should not replace unwhitelisted RANDOM', () => {
         const expander = createExpanderWithWhitelist(
-          'ABC,ABCD,CANONICAL',
+          ['ABC', 'ABCD', 'CANONICAL'],
           mockBindings
         );
         const url = 'http://www.google.com/?test=RANDOM';
@@ -119,7 +116,7 @@ describes.realWin(
 
       it('should replace whitelisted ABCD', () => {
         const expander = createExpanderWithWhitelist(
-          'ABC,ABCD,CANONICAL',
+          ['ABC', 'ABCD', 'CANONICAL'],
           mockBindings
         );
         const url = 'http://www.google.com/?test=ABCD';
@@ -128,7 +125,7 @@ describes.realWin(
       });
 
       it('should not replace anything with empty whitelist', () => {
-        const expander = createExpanderWithWhitelist('', mockBindings);
+        const expander = createExpanderWithWhitelist([''], mockBindings);
         const url = 'http://www.google.com/?test=ABCD';
         const expected = 'http://www.google.com/?test=ABCD';
         return expect(expander.expand(url)).to.eventually.equal(expected);