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

I2I: Change AMP Cache Origins #26205

Closed
Gregable opened this issue Jan 3, 2020 · 11 comments
Closed

I2I: Change AMP Cache Origins #26205

Gregable opened this issue Jan 3, 2020 · 11 comments
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code TSC Resolved Issues marked for TSC attention that have been resolved

Comments

@Gregable
Copy link
Member

Gregable commented Jan 3, 2020

Summary

A small number of document origins on the AMP Cache will change to a new origin to improve readability of AMP Cache origns.

Background

See AMP Cache URL Format, specifically the section "Domain Name Prefix" for the current AMP Cache origin mapping.

When a document is served on the AMP cache, the publisher's origin is mapped to a new AMP Cache origin, such as example-com.cdn.ampproject.org. The mapping algorithm ensures unique AMP Cache origins, while trying to make the origin as human readable as possible.

Publisher Origins containing a "-" (hyphen) in byte position 3 currently result in an AMP Cache origin using the fallback algorithm described in the above document. For example, the publisher origin:

en-us.example.com

currently maps to the fallback AMP Cache origin of:

covfmowbjemnekovz6xlilc56old3te5kbdukxjd6ypb7buewh5a.cdn.ampproject.org

This occurs because the standard mapping algorithm would otherwise produce:

en--us-example-com.cdn.ampproject.org

Which is an invalid hostname due to a "-" hyphen in byte position 3 and 4. See RFC5891 4.2.3.1:

The Unicode string MUST NOT contain "--" (two consecutive hyphens) in the
third and fourth character positions and MUST NOT start or end with a "-"
(hyphen).

The fallback AMP Cache origin is also used in cases of very long publisher origins that would cause the standard mapping algorithm to produce a hostname component longer than is legally allowed.

Design

Changes to the Origin mapping are disruptive to publishers who may rely on specific AMP Cache origins when authorizing CORS requests from the AMP Cache. Therefore this design only modifies the AMP Cache origin mapping for publisher origins which currently map to a fallback AMP Cache origin due to the position 3 "-" (hyphen) case described in the Background.

The proposed design modifies the Basic Algorithm to add a new step 4 between current steps 3 and 4:

  1. If the output of step 3 has a "-" (hyphen) at both positions 3 and 4, then to the output of step 3, add a prefix of "0-" and add a suffix of "-0".

In the example above, this would result in:

0-en--us-example-com-0.cdn.ampproject.org

Domains cannot start with a hyphen, so the "0-" prefix will push a non-hyphen byte into position 3 in all cases.

The "0-" prefix by itself could collide with another publisher domain, in this example: 0.en-us.example.com. Therefore, when the prefix is added in step 4, the "-0" suffix is also added. No domain names end in ".0", so this results in a unique AMP Cache origin.

Release timing recommendation

After this change is approved, the AMP Cache would begin accepting requests using either the previous algorithm or the new algorithm as soon as could be implemented.

The documentation would be updated as soon as could be implemented.

An announcement would then be sent to amphtml-announce regarding the change, to give advanced warning in the event that any publisher is relying on a specific fallback AMP Cache origin in CORS processing.

4 weeks after that, clients such as Google Search could begin sending traffic to the new AMP Cache Origins and the AMP Cache could begin rejecting the old origins by redirecting to the new one.

/cc @ampproject/wg-approvers

@Gregable Gregable added the INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code label Jan 3, 2020
@cramforce
Copy link
Member

I'm not 100% sure this needs to be approved by the AMP Project. But it definitely LGTM.

CC @ampproject/tsc for awareness.

@ssantosms
Copy link
Contributor

Took me a while to comment here, but I don't have any concerns.

@jridgewell
Copy link
Contributor

I approve.

@kristoferbaxter
Copy link
Contributor

Approved. Quite reasonable and well targeted for minimal disruption.

@src-code
Copy link
Contributor

This resolves issue #16779

@dvoytenko
Copy link
Contributor

LGTM

@Gregable
Copy link
Member Author

FYI, this was announced on amphtml-announce yesterday, with a no-sooner-than date of Mar 30th, 2020.

@rudygalfi
Copy link
Contributor

Is the plan to close this issue out once the change has been rolled out?

@rudygalfi rudygalfi added the TSC Resolved Issues marked for TSC attention that have been resolved label Mar 11, 2020
twifkak pushed a commit to ampproject/amppackager that referenced this issue Mar 12, 2020
twifkak pushed a commit to ampproject/amppackager that referenced this issue Mar 12, 2020
twifkak pushed a commit to ampproject/amppackager that referenced this issue Mar 12, 2020
@Gregable
Copy link
Member Author

@rudygalfi Yes. Let me know if there is a different process for this you'd like to see.

@rudygalfi
Copy link
Contributor

@Gregable Thanks for confirming. Sounds good to close out once fully resolved. I was mainly asking because until this week, we re-reviewing in TSC meetings because this is tagged to the TSC, but we've also now applied a "TSC Resolved" label since it required no further TSC input.

@Gregable
Copy link
Member Author

This change has been live for some time now, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code TSC Resolved Issues marked for TSC attention that have been resolved
Projects
None yet
Development

No branches or pull requests

8 participants