Skip to content

Commit

Permalink
amp-script: Fetch locally, add size limit (#20427)
Browse files Browse the repository at this point in the history
* Fetch scripts in AMP land instead of in worker-dom.

* Add size cap.

* Upgrade worker-dom to 0.2.9.

* Update yarn.lock.

* Require element uniqueness until global size limit.

* Upgrade to worker-dom 0.2.10 so it downgrades to dompurify 1.0.8. :D

* Remove babel-regenerator-runtime.
  • Loading branch information
William Chou committed Jan 31, 2019
1 parent 08aebba commit 0933b9b
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 8 deletions.
21 changes: 20 additions & 1 deletion extensions/amp-script/0.1/amp-script.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import {Layout, isLayoutSizeDefined} from '../../../src/layout';
import {Services} from '../../../src/services';
import {addPurifyHooks, purifyConfig} from '../../../src/purifier';
import {
calculateExtensionScriptUrl,
Expand All @@ -31,6 +32,8 @@ import {isExperimentOn} from '../../../src/experiments';
/** @const {string} */
const TAG = 'amp-script';

/** @const {number} */
const MAX_SCRIPT_SIZE = 150000;

export class AmpScript extends AMP.BaseElement {
/** @override */
Expand Down Expand Up @@ -69,7 +72,23 @@ export class AmpScript extends AMP.BaseElement {
const authorUrl = this.element.getAttribute('src');
const workerUrl = this.workerThreadUrl_();
dev().info(TAG, 'Author URL:', authorUrl, ', worker URL:', workerUrl);
upgrade(this.element, authorUrl, workerUrl);

const xhr = Services.xhrFor(this.win);
const fetches = Promise.all([
// `workerUrl` is from CDN, so no need for `ampCors`.
xhr.fetchText(workerUrl, {ampCors: false}).then(r => r.text()),
xhr.fetchText(authorUrl).then(r => r.text()),
]);
upgrade(this.element, fetches.then(results => {
const workerScript = results[0];
const authorScript = results[1];
if (authorScript.length > MAX_SCRIPT_SIZE) {
user().error(TAG, `Max script size exceeded: ${authorScript.length} > `
+ MAX_SCRIPT_SIZE);
return [];
}
return [workerScript, authorScript, authorUrl];
}));
return Promise.resolve();
}

Expand Down
1 change: 1 addition & 0 deletions extensions/amp-script/validator-amp-script.protoascii
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
# tag_name: "AMP-SCRIPT"
# disallowed_ancestor: "AMP-SCRIPT"
# requires_extension: "amp-script"
# unique: true # TODO(choumx): Remove when global size limit is implemented.
# attrs: {
# name: "src"
# mandatory: true
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@
},
"dependencies": {
"@ampproject/animations": "0.1.1",
"@ampproject/worker-dom": "0.2.8",
"@ampproject/worker-dom": "0.2.10",
"document-register-element": "1.5.0",
"dompurify": "1.0.8",
"moment": "2.22.2",
"preact-compat": "3.18.4",
"preact": "8.4.2",
"preact-compat": "3.18.4",
"promise-pjs": "1.1.3",
"prop-types": "15.6.2",
"react-dates": "15.5.3",
Expand All @@ -53,8 +53,8 @@
"ansi-colors": "3.2.3",
"ast-replace": "1.1.3",
"atob": "2.1.2",
"ava": "1.0.1",
"autoprefixer": "9.4.3",
"ava": "1.0.1",
"babel-eslint": "10.0.1",
"babel-plugin-filter-imports": "2.0.4",
"babel-plugin-istanbul": "5.1.0",
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
resolved "https://registry.yarnpkg.com/@ampproject/animations/-/animations-0.1.1.tgz#35641d5032a4df312db8da060c27556e4c93c460"
integrity sha512-a2AKdZUnBOKWaWqGqFEVtIzIYWi9yvhH25dWHXSH8ZV0026ckV+i09PeTeDI69DoiMbKEQRlyl8a2RVZPepNnA==

"@ampproject/worker-dom@0.2.8":
version "0.2.8"
resolved "https://registry.yarnpkg.com/@ampproject/worker-dom/-/worker-dom-0.2.8.tgz#d3cb4b44d385233a50c745a82910e62999d1b6e4"
integrity sha512-v8AAq15vu93oJ+/ZB9iIpdq9pXfsjWgaq1ILrte84OAwJO4C2wNuvAMhqUjSW2w8kcFcjd5Tw5kcNO89ZwJH4Q==
"@ampproject/worker-dom@0.2.10":
version "0.2.10"
resolved "https://registry.yarnpkg.com/@ampproject/worker-dom/-/worker-dom-0.2.10.tgz#b9ac5f96a641d756277dab741632cd69e24d5971"
integrity sha512-XVhDiInqRs+lnqmr3tqjDe5vSykG/ILlNws8e2dEZFijJSBHYP2Ku8Ru0NHhPWj1MBItVukhJKqkaQ9GSPKKZw==
dependencies:
dompurify "1.0.8"

Expand Down

0 comments on commit 0933b9b

Please sign in to comment.