ali's comments

ampproject · Jun 15, 2017 · 143d929 · 143d929
1 parent e0b7047
commit 143d929
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 2 deletions.
diff --git a/src/service/standard-actions-impl.js b/src/service/standard-actions-impl.js
@@ -19,11 +19,12 @@ import {OBJECT_STRING_ARGS_KEY} from '../service/action-impl';
 import {Layout, getLayoutClass} from '../layout';
 import {actionServiceForDoc, urlReplacementsForDoc} from '../services';
 import {bindForDoc} from '../services';
+import {computedStyle, getStyle, toggle} from '../style';
 import {dev, user} from '../log';
-import {registerServiceBuilderForDoc} from '../service';
 import {historyForDoc} from '../services';
+import {isProtocolValid} from '../url';
+import {registerServiceBuilderForDoc} from '../service';
 import {resourcesForDoc} from '../services';
-import {computedStyle, getStyle, toggle} from '../style';
 import {vsyncFor} from '../services';
 
 /**
@@ -136,6 +137,9 @@ export class StandardActions {
       return;
     }
     const url = invocation.args['url'];
+    if (!isProtocolValid(url)) {
+      return;
+    }
     const expandedUrl = this.urlReplacements_.expandUrlSync(url);
     const node = invocation.target;
     const win = (node.ownerDocument || node).defaultView;

diff --git a/test/functional/test-standard-actions.js b/test/functional/test-standard-actions.js
@@ -193,6 +193,12 @@ describes.sandboxed('StandardActions', {}, () => {
       standardActions.handleAmpTarget(invocation);
       expect(win.location).to.equal('http://bar.com');
       expect(expandUrlStub.calledOnce);
+
+      // Invalid protocols should fail.
+      invocation.args.url = /*eslint no-script-url: 0*/ 'javascript:alert(1)';
+      standardActions.handleAmpTarget(invocation);
+      expect(win.location).to.equal('http://bar.com');
+      expect(expandUrlStub.calledOnce);
     });
 
     it('should implement goBack', () => {