Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨Random Subdomains for SafeFrame #27393

Merged
merged 67 commits into from
Apr 22, 2020

Conversation

ryanhugh
Copy link
Contributor

@ryanhugh ryanhugh commented Mar 24, 2020

Adds an experiment to use random subdomains for Safeframe. This will provide more isolation between creatives. We intend to make each subdomain completely random - a random string will be generated for each ad slot on every page. Google is experimenting with this same change on non-AMP pages too.

AMP has had similar discussions recently about changing this behavior for NameFrame: #27091

Let me know if I should make an i2i.

@amp-owners-bot amp-owners-bot bot requested a review from cramforce March 24, 2020 21:34
@cramforce cramforce requested review from lannka and removed request for lannka and cramforce March 24, 2020 23:12
@lgtm-com
Copy link

lgtm-com bot commented Mar 25, 2020

This pull request introduces 1 alert when merging 142bac8 into 054d3e3 - view on LGTM.com

new alerts:

  • 1 for Assignment to constant

@ryanhugh
Copy link
Contributor Author

Just wondering if there was any update here. No worries if things are delayed a bit! :)

extensions/amp-a4a/0.1/amp-a4a.js Outdated Show resolved Hide resolved
extensions/amp-a4a/0.1/amp-a4a.js Outdated Show resolved Hide resolved
src/service/crypto-impl.js Outdated Show resolved Hide resolved
src/service/crypto-impl.js Outdated Show resolved Hide resolved
src/service/crypto-impl.js Outdated Show resolved Hide resolved
extensions/amp-a4a/0.1/amp-a4a.js Outdated Show resolved Hide resolved
extensions/amp-a4a/0.1/amp-a4a.js Outdated Show resolved Hide resolved
extensions/amp-a4a/0.1/amp-a4a.js Outdated Show resolved Hide resolved
src/service/crypto-impl.js Outdated Show resolved Hide resolved
@keithwrightbos keithwrightbos merged commit 701c62c into ampproject:master Apr 22, 2020
@ryanhugh ryanhugh deleted the random-subdomain branch April 22, 2020 20:31
ldoroshe pushed a commit to ldoroshe/amphtml that referenced this pull request May 8, 2020
* random subdomain for each safeframe

* fixed some tests

* experiment

* fix

* added experiment

* fix

* fix

* fix

* linting

* linting

* fixes

* linting

* experiment changes

* experiment changes

* experiment changes

* linting

* linting

* linting

* experiment fix

* revert adsense changes

* stuff

* stuff

* added tests

* added more tests

* changed crypto code a bit

* linting

* linting

* linting

* linting

* more changes

* update experiment

* use the existing fn

* lint

* update

* update

* updatetests

* update

* update

* whitespace commit just to re-run the tests

* whitespace commit just to re-run the tests

* empty commit to re-run tests

* empty commit to re-run tests

* empty commit to re-run flakey tests

* empty commit to re-run flakey tests

* empty commit to re-run flakey tests

* empty commit to re-run flakey tests

* empty commit to re-run flakey tests 2

* empty commit to re-run flakey tests 3

* empty commit to re-run flakey tests 4

* empty commit to re-run flakey tests 5

* empty commit to re-run flakey tests 6

* empty commit to re-run flakey tests 7

* empty commit to re-run flakey tests 8

* empty commit to re-run flakey tests 9

* empty commit to re-run flakey tests 10

* empty commit to re-run flakey tests 11

* empty commit to re-run flakey tests 12

* empty commit to re-run flakey tests 13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants