Skip to content

Commit

Permalink
Removing CSP related dead code. (ampproject#12287)
Browse files Browse the repository at this point in the history 
* Removing CSP related dead code.

* Unused import.

* Added test for violation listener inclusion.
  • Loading branch information
@glevitzky @gzgogo
glevitzky authored and gzgogo committed Jan 26, 2018
1 parent a1c2740 commit 1fc8232
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 50 deletions.
14 changes: 0 additions & 14 deletions extensions/amp-a4a/0.1/amp-a4a.js
View file
Expand Up @@ -83,12 +83,6 @@ export const SAFEFRAME_VERSION_HEADER = 'X-AmpSafeFrameVersion';
/** @type {string} @visibleForTesting */
export const EXPERIMENT_FEATURE_HEADER_NAME = 'amp-ff-exps';

/**
* Controls if Content Security Policy is enabled for FIE render.
* @type {string} @visibleForTesting
*/
export const CSP_ENABLED_EXP_NAME = 'csp_enabled';

/** @type {string} */
const TAG = 'amp-a4a';

Expand Down Expand Up @@ -364,9 +358,6 @@ export class AmpA4A extends AMP.BaseElement {
*/
this.postAdResponseExperimentFeatures = {};

/** @private {boolean} whether CSP for FIE is enabled */
this.cspEnabled_ = false;

/**
* The configuration for amp-analytics. If null, no amp-analytics element
* will be inserted and no analytics events will be fired.
Expand Down Expand Up @@ -703,9 +694,6 @@ export class AmpA4A extends AMP.BaseElement {
tryDecodeUriComponent(match[1]));
}
}
this.cspEnabled_ =
this.postAdResponseExperimentFeatures[CSP_ENABLED_EXP_NAME] ==
'true';
// If the response has response code 204, or arrayBuffer is null,
// collapse it.
if (!fetchResponse.arrayBuffer || fetchResponse.status == 204) {
Expand Down Expand Up @@ -1074,7 +1062,6 @@ export class AmpA4A extends AMP.BaseElement {
this.experimentalNonAmpCreativeRenderMethod_ =
this.getNonAmpCreativeRenderingMethod();
this.postAdResponseExperimentFeatures = {};
this.cspEnabled_ = false;
}

/**
Expand Down Expand Up @@ -1365,7 +1352,6 @@ export class AmpA4A extends AMP.BaseElement {
html: creativeMetaData.minifiedCreative,
extensionIds: creativeMetaData.customElementExtensions || [],
fonts: fontsArray,
cspEnabled: this.cspEnabled_,
}, embedWin => {
installUrlReplacementsForEmbed(this.getAmpDoc(), embedWin,
new A4AVariableSource(this.getAmpDoc(), embedWin));
Expand Down
7 changes: 0 additions & 7 deletions extensions/amp-a4a/0.1/test/test-amp-a4a.js
View file
Expand Up @@ -24,8 +24,6 @@ import {
SAFEFRAME_VERSION_HEADER,
protectFunctionWrapper,
assignAdUrlToError,
EXPERIMENT_FEATURE_HEADER_NAME,
CSP_ENABLED_EXP_NAME,
} from '../amp-a4a';
import {AMP_SIGNATURE_HEADER} from '../signature-verifier';
import {FriendlyIframeEmbed} from '../../../../src/friendly-iframe-embed';
Expand Down Expand Up @@ -322,18 +320,13 @@ describe('amp-a4a', () => {
});

it('populates postAdResponseExperimentFeatures', () => {
adResponse.headers[EXPERIMENT_FEATURE_HEADER_NAME] =
`foo=bar,bad,${CSP_ENABLED_EXP_NAME}=true`;
a4a.buildCallback();
a4a.onLayoutMeasure();
return a4a.layoutCallback().then(() => {
const child = a4aElement.querySelector('iframe[srcdoc]');
expect(child).to.be.ok;
expect(child.srcdoc.indexOf('meta http-equiv=Content-Security-Policy'))
.to.not.equal(-1);
expect(a4a.postAdResponseExperimentFeatures).to.jsonEqual({
foo: 'bar', [CSP_ENABLED_EXP_NAME]: 'true',
});
});
});

Expand Down
8 changes: 1 addition & 7 deletions src/friendly-iframe-embed.js
View file
Expand Up @@ -56,7 +56,6 @@ const EXCLUDE_INI_LOAD = ['AMP-AD', 'AMP-ANALYTICS', 'AMP-PIXEL'];
* html: string,
* extensionIds: (?Array<string>|undefined),
* fonts: (?Array<string>|undefined),
* cspEnabled: boolean,
* }}
*/
export let FriendlyIframeSpec;
Expand Down Expand Up @@ -147,9 +146,6 @@ export function installFriendlyIframeEmbed(iframe, container, spec,
iframe.readyState = 'complete';
};
const registerViolationListener = () => {
if (!spec.cspEnabled) {
return;
}
iframe.contentWindow.addEventListener('securitypolicyviolation',
violationEvent => {
dev().warn('FIE', 'security policy violation', violationEvent);
Expand Down Expand Up @@ -280,10 +276,8 @@ function mergeHtml(spec) {
}

// Load CSP
if (spec.cspEnabled) {
result.push('<meta http-equiv=Content-Security-Policy ' +
result.push('<meta http-equiv=Content-Security-Policy ' +
'content="script-src \'none\';object-src \'none\';child-src \'none\'">');
}

// Postambule.
if (ip > 0) {
Expand Down
67 changes: 45 additions & 22 deletions test/functional/test-friendly-iframe-embed.js
View file
Expand Up @@ -390,56 +390,56 @@ describe('friendly-iframe-embed', () => {

it('should pre-pend to html', () => {
const html = mergeHtmlForTesting(spec);
expect(html).to.equal('<base href="https://acme.org/embed1"><a></a>');
expect(html).to.equal('<base href="https://acme.org/embed1">'
+ '<meta http-equiv=Content-Security-Policy content='
+ '"script-src \'none\';object-src \'none\';child-src \'none\'">'
+ '<a></a>');
});

it('should insert into head', () => {
spec.html = '<html><head>head</head><body>body';
const html = mergeHtmlForTesting(spec);
expect(html).to.equal(
'<html><head>'
+ '<base href="https://acme.org/embed1">'
+ 'head</head><body>body');
expect(html).to.equal('<html><head><base href="https://acme.org/embed1">'
+ '<meta http-equiv=Content-Security-Policy content='
+ '"script-src \'none\';object-src \'none\';'
+ 'child-src \'none\'">head</head><body>body');
});

it('should insert into head w/o html', () => {
spec.html = '<head>head</head><body>body';
const html = mergeHtmlForTesting(spec);
expect(html).to.equal(
'<head>'
+ '<base href="https://acme.org/embed1">'
+ 'head</head><body>body');
expect(html).to.equal('<head><base href="https://acme.org/embed1">'
+ '<meta http-equiv=Content-Security-Policy content="'
+ 'script-src \'none\';object-src \'none\';child-src \'none\'">head'
+ '</head><body>body');
});

it('should insert before body', () => {
spec.html = '<html><body>body';
const html = mergeHtmlForTesting(spec);
expect(html).to.equal(
'<html>'
+ '<base href="https://acme.org/embed1">'
+ '<body>body');
expect(html).to.equal('<html><base href="https://acme.org/embed1">'
+ '<meta http-equiv=Content-Security-Policy content="script-src '
+ '\'none\';object-src \'none\';child-src \'none\'"><body>body');
});

it('should insert before body w/o html', () => {
spec.html = '<body>body';
const html = mergeHtmlForTesting(spec);
expect(html).to.equal(
'<base href="https://acme.org/embed1">'
+ '<body>body');
expect(html).to.equal('<base href="https://acme.org/embed1">'
+ '<meta http-equiv=Content-Security-Policy content="script-src '
+ '\'none\';object-src \'none\';child-src \'none\'"><body>body');
});

it('should insert after html', () => {
spec.html = '<html>content';
const html = mergeHtmlForTesting(spec);
expect(html).to.equal(
'<html>'
+ '<base href="https://acme.org/embed1">'
+ 'content');
expect(html).to.equal('<html><base href="https://acme.org/embed1">'
+ '<meta http-equiv=Content-Security-Policy content="script-src '
+ '\'none\';object-src \'none\';child-src \'none\'">content');
});

it('should insert CSP', () => {
spec.html = '<html><head></head><body></body></html>';
spec.cspEnabled = true;
expect(mergeHtmlForTesting(spec)).to.equal(
'<html><head><base href="https://acme.org/embed1">' +
'<meta http-equiv=Content-Security-Policy ' +
Expand Down Expand Up @@ -504,6 +504,24 @@ describe('friendly-iframe-embed', () => {
return embed.whenWindowLoaded();
});
});

it('should add violation listener', () => {
let eventListenerSpy;
const container = {
appendChild: child => {
document.body.appendChild(child);
eventListenerSpy =
sandbox.spy(child.contentWindow, 'addEventListener');
},
};
const embedPromise = installFriendlyIframeEmbed(iframe, container, {
url: 'https://acme.org/url1',
html: '<a id="a1"></a>',
});
return embedPromise.then(() => {
expect(eventListenerSpy).to.be.calledOnce;
});
});
});

describe('child document ready polling', () => {
Expand Down Expand Up @@ -561,7 +579,9 @@ describe('friendly-iframe-embed', () => {
},
removeEventListener: () => {},
};
contentWindow = {};
contentWindow = {
addEventListener: () => {},
};
contentDocument = {};
contentBody = {nodeType: 1, style: {}};
container = {
Expand Down Expand Up @@ -595,6 +615,7 @@ describe('friendly-iframe-embed', () => {
});

it('should poll until ready', () => {
iframe.contentWindow = contentWindow;
const embedPromise = installFriendlyIframeEmbed(iframe, container, {
url: 'https://acme.org/url1',
html: '<body></body>',
Expand Down Expand Up @@ -643,6 +664,7 @@ describe('friendly-iframe-embed', () => {
});

it('should stop polling when loaded', () => {
iframe.contentWindow = contentWindow;
const embedPromise = installFriendlyIframeEmbed(iframe, container, {
url: 'https://acme.org/url1',
html: '<body></body>',
Expand All @@ -657,6 +679,7 @@ describe('friendly-iframe-embed', () => {
});

it('should stop polling when loading failed', () => {
iframe.contentWindow = contentWindow;
const embedPromise = installFriendlyIframeEmbed(iframe, container, {
url: 'https://acme.org/url1',
html: '<body></body>',
Expand Down

0 comments on commit 1fc8232

Please sign in to comment.