Skip to content

Commit

Permalink
AMP4Email: Set default "amp-allowed-url-macros" and "amp-action-white…
Browse files Browse the repository at this point in the history
…list" (#27561)

* Allow no URL macros for email

* Set default actions allowlist for email

* Update comments for variable-source

* Add missing actions

* Add unit tests

* Check existence of `hasAttribute`

* Delegate allowed actions to components

* Remove queryWhitelist method

* Remove action-white-list meta from tests

* Don't make every component check email format

* Move action allowlist tests to separate file

* Remove unparseable allowlist entries

* Use getActionInvocation helper method

* Add component-based  tests and associated fixes

* Stub maybeAddToEmailWhitelist in test-amp-form

* Remove extra line

* Do not allow duplicate actions

* Remove "amp-allowed-url-macros" meta

* Remove validator cases for meta tags

* Stub hasAttribute in test-url-replacements

* Fix type annotation

* Replace maybeAddToEmailWhitelist with opts

* Annotate void return

* Fixes to unit tests

* Take Arrays of methods, formats, in addToWhitelist

* Replace use of hasOwn

* Add warning for ignored whitelist entries

* Move action unit tests to component unit tests

* Move allowlist unit tests back to test-action.js

* Fix warning args

* Delete test-action-allowlist.js

* Remove unnecessary arg

* Remove validator tests for meta

* Assert valid whitelist entries

* Replace `invoke_` with `execute` in unit tests.

* Stub user().error

* Use sandbox.spy/stub

* Add wg-runtime to TODO comment

* (empty)
  • Loading branch information
caroqliu committed Apr 30, 2020
1 parent c8aa03a commit 3ddd4f4
Show file tree
Hide file tree
Showing 24 changed files with 699 additions and 274 deletions.
6 changes: 6 additions & 0 deletions extensions/amp-carousel/0.1/scrollable-carousel.js
Expand Up @@ -89,6 +89,12 @@ export class AmpScrollableCarousel extends BaseCarousel {
},
ActionTrust.LOW
);
/** If the element is in an email document, allow its `goToSlide` action. */
Services.actionServiceForDoc(this.element).addToWhitelist(
'amp-carousel',
'goToSlide',
['email']
);
}

/** @override */
Expand Down
2 changes: 2 additions & 0 deletions extensions/amp-carousel/0.1/slidescroll.js
Expand Up @@ -147,6 +147,8 @@ export class AmpSlideScroll extends BaseSlides {
buildSlides() {
this.vsync_ = this.getVsync();
this.action_ = Services.actionServiceForDoc(this.element);
/** If the element is in an email document, allow its `goToSlide` action. */
this.action_.addToWhitelist(TAG, 'goToSlide', ['email']);

this.hasNativeSnapPoints_ =
getStyle(this.element, 'scrollSnapType') != undefined;
Expand Down
66 changes: 65 additions & 1 deletion extensions/amp-carousel/0.1/test/test-scrollable-carousel.js
Expand Up @@ -15,13 +15,20 @@
*/

import '../amp-carousel';
import {ActionService} from '../../../../src/service/action-impl';
import {ActionTrust} from '../../../../src/action-constants';
import {Services} from '../../../../src/services';

import {
createElementWithAttributes,
whenUpgradedToCustomElement,
} from '../../../../src/dom';
import {user} from '../../../../src/log';
describes.realWin(
'test-scrollable-carousel',
{
amp: {
extensions: ['amp-carousel'],
runtimeOn: true,
},
},
(env) => {
Expand Down Expand Up @@ -496,5 +503,62 @@ describes.realWin(
});
}
);

it('should allow default actions in email documents', async () => {
env.win.document.documentElement.setAttribute('amp4email', '');
const action = new ActionService(env.ampdoc, env.win.document);
env.sandbox.stub(Services, 'actionServiceForDoc').returns(action);
const element = createElementWithAttributes(
env.win.document,
'amp-carousel',
{
'type': 'carousel',
'width': '400',
'height': '300',
}
);
env.win.document.body.appendChild(element);
env.sandbox.spy(element, 'enqueAction');
env.sandbox.stub(element, 'getDefaultActionAlias');
await whenUpgradedToCustomElement(element);
await element.whenBuilt();

action.execute(
element,
'goToSlide',
null,
'source',
'caller',
'event',
ActionTrust.HIGH
);
expect(element.enqueAction).to.be.calledWith(
env.sandbox.match({
actionEventType: '?',
args: null,
caller: 'caller',
event: 'event',
method: 'goToSlide',
node: element,
source: 'source',
trust: ActionTrust.HIGH,
})
);

const userErrorStub = env.sandbox.stub(user(), 'error');
action.execute(
element,
'toggleAutoplay',
null,
'source',
'caller',
'event',
ActionTrust.HIGH
);
expect(userErrorStub).to.be.calledOnce;
expect(userErrorStub.args[0][1]).to.match(
/"AMP-CAROUSEL.toggleAutoplay" is not whitelisted/
);
});
}
);
64 changes: 64 additions & 0 deletions extensions/amp-carousel/0.1/test/test-slidescroll.js
Expand Up @@ -15,14 +15,21 @@
*/

import '../amp-carousel';
import {ActionService} from '../../../../src/service/action-impl';
import {ActionTrust} from '../../../../src/action-constants';
import {Services} from '../../../../src/services';
import {
createElementWithAttributes,
whenUpgradedToCustomElement,
} from '../../../../src/dom';
import {user} from '../../../../src/log';

describes.realWin(
'SlideScroll',
{
amp: {
extensions: ['amp-carousel'],
runtimeOn: true,
},
},
(env) => {
Expand Down Expand Up @@ -1469,5 +1476,62 @@ describes.realWin(
});
});
});

it('should allow default actions in email documents', async () => {
env.win.document.documentElement.setAttribute('amp4email', '');
const action = new ActionService(env.ampdoc, env.win.document);
env.sandbox.stub(Services, 'actionServiceForDoc').returns(action);
const element = createElementWithAttributes(
env.win.document,
'amp-carousel',
{
'type': 'slides',
'width': '400',
'height': '300',
}
);
env.win.document.body.appendChild(element);
env.sandbox.spy(element, 'enqueAction');
env.sandbox.stub(element, 'getDefaultActionAlias');
await whenUpgradedToCustomElement(element);
await element.whenBuilt();

action.execute(
element,
'goToSlide',
null,
'source',
'caller',
'event',
ActionTrust.HIGH
);
expect(element.enqueAction).to.be.calledWith(
env.sandbox.match({
actionEventType: '?',
args: null,
caller: 'caller',
event: 'event',
method: 'goToSlide',
node: element,
source: 'source',
trust: ActionTrust.HIGH,
})
);

const userErrorStub = env.sandbox.stub(user(), 'error');
action.execute(
element,
'toggleAutoplay',
null,
'source',
'caller',
'event',
ActionTrust.HIGH
);
expect(userErrorStub).to.be.calledOnce;
expect(userErrorStub.args[0][1]).to.match(
/"AMP-CAROUSEL.toggleAutoplay" is not whitelisted/
);
});
}
);
16 changes: 11 additions & 5 deletions extensions/amp-form/0.1/amp-form.js
Expand Up @@ -128,8 +128,11 @@ export class AmpForm {
/** @private @const {string} */
this.id_ = id;

/** @private @const {?Document} */
this.doc_ = element.ownerDocument;

/** @const @private {!Window} */
this.win_ = toWin(element.ownerDocument.defaultView);
this.win_ = toWin(this.doc_.defaultView);

/** @const @private {!../../../src/service/timer-impl.Timer} */
this.timer_ = Services.timerFor(this.win_);
Expand Down Expand Up @@ -220,6 +223,8 @@ export class AmpForm {
/** @const @private {!./form-verifiers.FormVerifier} */
this.verifier_ = getFormVerifier(this.form_, () => this.handleXhrVerify_());

/** If the element is in an email document, allow its `clear` and `submit` actions. */
this.actions_.addToWhitelist('FORM', ['clear', 'submit'], ['email']);
this.actions_.installActionHandler(
this.form_,
this.actionHandler_.bind(this)
Expand All @@ -239,6 +244,9 @@ export class AmpForm {
Services.formSubmitForDoc(element).then((service) => {
this.formSubmitService_ = service;
});

/** @private */
this.isAmp4Email_ = this.doc_ && isAmp4Email(this.doc_);
}

/**
Expand Down Expand Up @@ -992,9 +1000,8 @@ export class AmpForm {
'See https://github.com/ampproject/amphtml/issues/24894.'
);
}
const doc = this.form_.ownerDocument;
// Only degrade trust across form submission in AMP4EMAIL for now.
return doc && isAmp4Email(doc)
return this.isAmp4Email_
? /** @type {!ActionTrust} */ (incomingTrust - 1)
: incomingTrust;
}
Expand Down Expand Up @@ -1182,9 +1189,8 @@ export class AmpForm {
}
const redirectTo = response.headers.get(REDIRECT_TO_HEADER);
if (redirectTo) {
const doc = this.form_.ownerDocument;
userAssert(
!(doc && isAmp4Email(doc)),
!this.isAmp4Email_,
'Redirects not supported in AMP4Email.',
this.form_
);
Expand Down
47 changes: 47 additions & 0 deletions extensions/amp-form/0.1/test/test-amp-form.js
Expand Up @@ -17,6 +17,7 @@
import '../../../amp-mustache/0.1/amp-mustache';
import '../../../amp-selector/0.1/amp-selector';
import * as xhrUtils from '../../../../src/utils/xhr-utils';
import {ActionService} from '../../../../src/service/action-impl';
import {ActionTrust} from '../../../../src/action-constants';
import {AmpEvents} from '../../../../src/amp-events';
import {
Expand Down Expand Up @@ -1535,6 +1536,7 @@ describes.repeated(
const actions = {
installActionHandler: () => {},
trigger: env.sandbox.spy(),
addToWhitelist: () => {},
};
env.sandbox.stub(Services, 'actionServiceForDoc').returns(actions);

Expand Down Expand Up @@ -2986,6 +2988,51 @@ describes.repeated(
});
});

it('should allow default actions in email documents', async () => {
env.win.document.documentElement.setAttribute('amp4email', '');
const action = new ActionService(env.ampdoc, env.win.document);
env.sandbox.stub(Services, 'actionServiceForDoc').returns(action);
const element = getForm();
document.body.appendChild(element);
const form = new AmpForm(element, 'test-id');
const clearSpy = env.sandbox.stub(form, 'handleClearAction_');
action.execute(
element,
'clear',
null,
'source',
'caller',
'event',
ActionTrust.HIGH
);
expect(clearSpy).to.be.called;

env.sandbox.stub(form, 'submit_');
const submitSpy = env.sandbox.stub(form, 'handleSubmitAction_');
action.execute(
element,
'submit',
null,
'source',
'caller',
'event',
ActionTrust.HIGH
);
await whenCalled(submitSpy);
expect(submitSpy).to.be.calledWith(
env.sandbox.match({
actionEventType: '?',
args: null,
caller: 'caller',
event: 'event',
method: 'submit',
node: element,
source: 'source',
trust: ActionTrust.HIGH,
})
);
});

describe('Async Inputs', () => {
it('NonXHRGet should submit sync if no Async Input', () => {
return getAmpForm(getForm()).then((ampForm) => {
Expand Down
5 changes: 5 additions & 0 deletions extensions/amp-image-lightbox/0.1/amp-image-lightbox.js
Expand Up @@ -792,6 +792,11 @@ class AmpImageLightbox extends AMP.BaseElement {
if (this.container_) {
return;
}
/** If the element is in an email document, allow its `open` action. */
Services.actionServiceForDoc(
this.element
).addToWhitelist('AMP-IMAGE-LIGHTBOX', 'open', ['email']);

this.container_ = this.element.ownerDocument.createElement('div');
this.container_.classList.add('i-amphtml-image-lightbox-container');
this.element.appendChild(this.container_);
Expand Down
45 changes: 45 additions & 0 deletions extensions/amp-image-lightbox/0.1/test/test-amp-image-lightbox.js
Expand Up @@ -17,6 +17,8 @@
import '../amp-image-lightbox';
import * as dom from '../../../../src/dom';
import * as lolex from 'lolex';
import {ActionService} from '../../../../src/service/action-impl';
import {ActionTrust} from '../../../../src/action-constants';
import {ImageViewer} from '../amp-image-lightbox';
import {Keys} from '../../../../src/utils/key-codes';
import {Services} from '../../../../src/services';
Expand All @@ -28,6 +30,7 @@ describes.realWin(
{
amp: {
extensions: ['amp-image-lightbox'],
runtimeOn: true,
},
},
(env) => {
Expand Down Expand Up @@ -246,6 +249,48 @@ describes.realWin(
expect(tryFocus).to.be.calledOnce;
});
});

it('should allow default actions in email documents', async () => {
env.win.document.documentElement.setAttribute('amp4email', '');
const action = new ActionService(env.ampdoc, env.win.document);
env.sandbox.stub(Services, 'actionServiceForDoc').returns(action);

const element = dom.createElementWithAttributes(
env.win.document,
'amp-image-lightbox',
{'layout': 'nodisplay'}
);
env.win.document.body.appendChild(element);
env.sandbox.spy(element, 'enqueAction');
env.sandbox.stub(element, 'getDefaultActionAlias');
await dom.whenUpgradedToCustomElement(element);

const impl = await element.getImpl();
impl.buildLightbox_();
env.sandbox.stub(impl, 'open_');
action.execute(
element,
'open',
null,
'source',
'caller',
'event',
ActionTrust.HIGH
);
expect(element.enqueAction).to.be.calledWith(
env.sandbox.match({
actionEventType: '?',
args: null,
caller: 'caller',
event: 'event',
method: 'open',
node: element,
source: 'source',
trust: ActionTrust.HIGH,
})
);
expect(impl.open_).to.be.calledOnce;
});
}
);

Expand Down

0 comments on commit 3ddd4f4

Please sign in to comment.