Skip to content

Commit

Permalink
Sync for validator/cpp/engine and Validator rollup (#35052)
Browse files Browse the repository at this point in the history 
* Simplify regex release version variable naming.
If the release version can't be determined then return unknown instead of standard.

PiperOrigin-RevId: 380675772

* cl/380675772 Simplify regex release version variable naming

Co-authored-by: honeybadgerdontcare <sedano@google.com>
  • Loading branch information
@antiphoton @honeybadgerdontcare
antiphoton and honeybadgerdontcare authored Jun 25, 2021
1 parent 5dcbc58 commit f2fa763
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 33 deletions.
27 changes: 10 additions & 17 deletions validator/cpp/engine/validator-internal.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,13 @@ ABSL_FLAG(bool, allow_module_nomodule, true,

namespace amp::validator {

// LTS JavaScript (note these are the same as kNomoduleLtsScriptPathRe):
// Standard and Nomodule JavaScript:
// v0.js
// v0/amp-ad-0.1.js
static const LazyRE2 kStandardScriptPathRe = {
R"re((v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\.js)re"};

// LTS and Nomodule LTS JavaScript:
// lts/v0.js
// lts/v0/amp-ad-0.1.js
static const LazyRE2 kLtsScriptPathRe = {
Expand All @@ -127,24 +133,12 @@ static const LazyRE2 kLtsScriptPathRe = {
static const LazyRE2 kModuleScriptPathRe = {
R"re((v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\.mjs)re"};

// Nomodule JavaScript:
// v0.js
// v0/amp-ad-0.1.js
static const LazyRE2 kNomoduleScriptPathRe = {
R"re((v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\.js)re"};

// Module LTS JavaScript:
// lts/v0.mjs
// lts/v0/amp-ad-0.1.mjs
static const LazyRE2 kModuleLtsScriptPathRe = {
R"re(lts/(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\.mjs)re"};

// Nomodule LTS JavaScript (note these are the same as kLtsScriptPathRe):
// lts/v0.js
// lts/v0/amp-ad-0.1.js
static const LazyRE2 kNomoduleLtsScriptPathRe = {
R"re(lts/(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\.js)re"};

// Runtime JavaScript:
// v0.js
// v0.mjs
Expand Down Expand Up @@ -332,17 +326,16 @@ ScriptTag ParseScriptTag(htmlparser::Node* node) {

// Determine the release version (LTS, module, standard, etc).
if ((has_module_attr && RE2::FullMatch(src, *kModuleLtsScriptPathRe)) ||
(has_nomodule_attr &&
RE2::FullMatch(src, *kNomoduleLtsScriptPathRe))) {
(has_nomodule_attr && RE2::FullMatch(src, *kLtsScriptPathRe))) {
script_tag.release_version = ScriptReleaseVersion::MODULE_NOMODULE_LTS;
} else if ((has_module_attr &&
RE2::FullMatch(src, *kModuleScriptPathRe)) ||
(has_nomodule_attr &&
RE2::FullMatch(src, *kNomoduleScriptPathRe))) {
RE2::FullMatch(src, *kStandardScriptPathRe))) {
script_tag.release_version = ScriptReleaseVersion::MODULE_NOMODULE;
} else if (RE2::FullMatch(src, *kLtsScriptPathRe)) {
script_tag.release_version = ScriptReleaseVersion::LTS;
} else {
} else if (RE2::FullMatch(src, *kStandardScriptPathRe)) {
script_tag.release_version = ScriptReleaseVersion::STANDARD;
}
}
Expand Down
26 changes: 10 additions & 16 deletions validator/js/engine/htmlparser-interface.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,13 @@ exports.ScriptReleaseVersion = ScriptReleaseVersion;
// AMP domain
const /** string */ ampProjectDomain = 'https://cdn.ampproject.org/';

// LTS JavaScript:
// Standard and Nomodule JavaScript:
// v0.js
// v0/amp-ad-0.1.js
const /** !RegExp */ standardScriptPathRegex =
new RegExp('(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\\.js$', 'i');

// LTS and Nomodule LTS JavaScript:
// lts/v0.js
// lts/v0/amp-ad-0.1.js
const /** !RegExp */ ltsScriptPathRegex =
Expand All @@ -101,24 +107,12 @@ const /** !RegExp */ ltsScriptPathRegex =
const /** !RegExp */ moduleScriptPathRegex =
new RegExp('(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\\.mjs$', 'i');

// Nomodule JavaScript:
// v0.js
// v0/amp-ad-0.1.js
const /** !RegExp */ nomoduleScriptPathRegex =
new RegExp('(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\\.js$', 'i');

// Module LTS JavaScript:
// lts/v0.mjs
// lts/v0/amp-ad-0.1.mjs
const /** !RegExp */ moduleLtsScriptPathRegex =
new RegExp('lts/(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\\.mjs$', 'i');

// Nomodule LTS JavaScript:
// lts/v0.js
// lts/v0/amp-ad-0.1.js
const /** !RegExp */ nomoduleLtsScriptPathRegex =
new RegExp('lts/(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\\.js$', 'i');

// Runtime JavaScript:
// v0.js
// v0.mjs
Expand Down Expand Up @@ -193,15 +187,15 @@ const ScriptTag = class {

// Determine the release version (LTS, module, standard, etc).
if ((is_module && moduleLtsScriptPathRegex.test(path)) ||
(is_nomodule && nomoduleLtsScriptPathRegex.test(path))) {
(is_nomodule && ltsScriptPathRegex.test(path))) {
this.release_version = ScriptReleaseVersion.MODULE_NOMODULE_LTS;
} else if (
(is_module && moduleScriptPathRegex.test(path)) ||
(is_nomodule && nomoduleScriptPathRegex.test(path))) {
(is_nomodule && standardScriptPathRegex.test(path))) {
this.release_version = ScriptReleaseVersion.MODULE_NOMODULE;
} else if (ltsScriptPathRegex.test(path)) {
this.release_version = ScriptReleaseVersion.LTS;
} else {
} else if (standardScriptPathRegex.test(path)) {
this.release_version = ScriptReleaseVersion.STANDARD;
}
}
Expand Down
2 changes: 2 additions & 0 deletions validator/testdata/feature_tests/regexps.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ feature_tests/regexps.html:27:2 The mandatory attribute 'amp-custom' is missing
| <script async custom-element="amp-vine" src="https://cdn.ampproject.org/v0/amp-vine-latest.js"> </script>
| <script async custom-element="amp-vine" src="https://cdn.ampproject.org/v0/amp-vine-0.1.js?foobar"></script>
>> ^~~~~~~~~
feature_tests/regexps.html:36:2 The script version for 'amp-vine' is a unknown version which mismatches with the first script on the page using the standard version. (see https://amp.dev/documentation/guides-and-tutorials/learn/spec/amphtml#required-markup)
>> ^~~~~~~~~
feature_tests/regexps.html:36:2 The attribute 'src' in tag 'amp-vine extension script' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-vine-0.1.js?foobar'. (see https://amp.dev/documentation/components/amp-vine)
| <script async custom-element="amp-vine" src="http://xss.com/https://cdn.ampproject.org/v0/amp-vine-0.1.js?foobar"></script>
>> ^~~~~~~~~
Expand Down

0 comments on commit f2fa763

Please sign in to comment.