Discussion: What are allowed version strings when self-hosting the runtime?

[mdmower]

I'd like to clear-up the requirements on allowed version strings passed to

gulp dist --version_override=<version>

As part of #25873, publishers can opt to build and host the AMP runtime themselves. A guide is in preparation to support this feature (#27100). Some publishers might appreciate the ability to use their own versioning system.

The only technical requirement I'm aware of is the expectation that version must contain only characters allowed by a URL path segment (https://tools.ietf.org/html/rfc3986#section-3.3). One example where this restriction is assumed is in method calculateExtensionScriptUrl(), where rtv is not encoded before inserted in the URL. A simple way to enforce this requirement could be to stipulate version === encodeURIComponent(version), but this is admittedly more stringent than RFC3986.

Here are some simple examples that show version v1.2a doesn't blow up the world:

• AMP: https://ampdemo.cmphys.com/amp-sh/self-hosted-amprt-opt-rtv.html
• PWA: https://ampdemo.cmphys.com/amp-sh/self-hosted-amprt-pwa.html?page=opt-rtv

[mdmower]

Ping @rsimha @ampproject/wg-infra

[rsimha]

I have no fundamental objections to publishers using non-numeric versions for self hosting AMP. However, I'm not sure if there are other places in runtime code that make implicit assumptions about the format / length of the version string.

Tagging others for comment:

@danielrozenberg wrote the date-based versioning format we currently use
@ampproject/wg-runtime @ampproject/wg-performance @ampproject/wg-ads @ampproject/wg-ui-and-a11y may be aware of other pitfalls

[jridgewell]

It seems the JS doesn't actually depend on it being a number (at least as far as I can tell).

[kristoferbaxter]

What do we think about requiring a number and adding a test?

[mdmower]

What do we think about requiring a number and adding a test?

If I can weigh in, I think it would be great to give publishers a bit more flexibility. I'm partial to

version === encodeURIComponent(version)

but would also be fine with with something like

/^[a-z0-9.\-]+$/i.test(version)

Ultimately, I'll be happy to update my self-hosting guide with your decision and would also be willing to add a test to the build system to ensure a valid version is passed to --version_override. Thanks for the responses!

[jridgewell]

@mdmower: Do you see a reason to use a non-standard version? Using the same 15 digit RTV pattern would be the safest, in case there's some new code in the future that expects it.

[mdmower]

Do you see a reason to use a non-standard version?

In general, I avoid imposing restrictions unless there is a demonstrated need for the restriction. Here, I see an opportunity for publishers who self-host the runtime to use a versioning system they are more familiar with. The AMP Project's preference of "Date of release" style versioning is just one of many schemes. I think it'd be great to allow the flexibility, but I'm not going to complain if 15-digits is the decision.

[kristoferbaxter]

I'd also prefer to use the same versioning scheme used by the officially released versions on the AMP CDN.

Reasons are similar to Justin, but mostly due to fear of error logging systems and similar that expect a version number of the currently official but not enforced pattern.

[mdmower]

No problem. I'll create a PR to enforce the version string requirement in --version_override that will close this issue. Thanks much for the discussion!

[jridgewell]

Also note that version_override replaces the datetime portion (the AMP version), which is 13 digits.

The RTV (15 digits) is created when a config is concatted on the v0.js file. The config is differentiated by the two leading digits. All source code is the same between AMP versions, even with different RTV prefixes.