Skip to content

Commit

Permalink
Add UseCounter for incompatible pattern attribute values
Browse files Browse the repository at this point in the history
A proposed change to the HTML pattern attribute [1] swaps the `u`
RegExp flag for the new `v` flag [2], resulting in some potential
incompatibility.

Some previously valid patterns are now errors, specifically those
with a character class including either an unescaped special
character or a double punctuator:

    pattern="[(]"
    pattern="[)]"
    pattern="[[]"
    pattern="[{]"
    pattern="[}]"
    pattern="[/]"
    pattern="[-]"
    pattern="[|]"
    pattern="[&&]"
    pattern="[!!]"
    pattern="[##]"
    pattern="[$$]"
    pattern="[%%]"
    pattern="[**]"
    pattern="[++]"
    pattern="[,,]"
    pattern="[..]"
    pattern="[::]"
    pattern="[;;]"
    pattern="[<<]"
    pattern="[==]"
    pattern="[>>]"
    pattern="[??]"
    pattern="[@@]"
    pattern="[``]"
    pattern="[~~]"
    pattern="[_^^]"

We don’t expect such patterns to be very common. This UseCounter
aims to validate that assumption.

Note that throwing patterns result in `inputElement.validity.valid
=== true` for any input value, so the only compatibility risk is
that some value/pattern combinations that would previously result
in `inputElement.validity.valid === false` now result in
`inputElement.validity.valid === true`.

[1]: whatwg/html#7908
[2]: https://v8.dev/features/regexp-v-flag

Bug: chromium:1412729
Change-Id: Ifa8bcc27dbf6e8a2a7098643dbb27a7633bb97de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4249120
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Mathias Bynens <[email protected]>
Reviewed-by: Alexei Svitkine <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1105129}
  • Loading branch information
mathiasbynens authored and Chromium LUCI CQ committed Feb 14, 2023
1 parent 675e1b5 commit 100082a
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,8 @@ UseCounterMetricsRecorder::GetAllowedUkmFeatures() {
WebFeature::kRequestFileSystem,
WebFeature::kRequestFileSystemWorker,
WebFeature::kRequestFileSystemSyncWorker,
WebFeature::
kHTMLPatternRegExpUnicodeSetIncompatibilitiesWithUnicodeMode,
}));
return *opt_in_features;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3801,6 +3801,7 @@ enum WebFeature {
kTopicsAPIFetch = 4460,
kTopicsAPIXhr = 4461,
kParseFromString = 4462,
kHTMLPatternRegExpUnicodeSetIncompatibilitiesWithUnicodeMode = 4463,

// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots.
Expand Down
33 changes: 26 additions & 7 deletions third_party/blink/renderer/core/html/forms/base_text_input_type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "third_party/blink/renderer/bindings/core/v8/script_regexp.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/frame/web_feature.h"
#include "third_party/blink/renderer/core/html/forms/email_input_type.h"
#include "third_party/blink/renderer/core/html/forms/html_input_element.h"
#include "third_party/blink/renderer/core/html_names.h"
Expand Down Expand Up @@ -98,22 +99,40 @@ bool BaseTextInputType::PatternMismatch(const String& value) const {
bool BaseTextInputType::PatternMismatchPerValue(const String& value) const {
const AtomicString& raw_pattern =
GetElement().FastGetAttribute(html_names::kPatternAttr);
// Empty values can't be mismatched
// Empty values can't be mismatched.
if (raw_pattern.IsNull() || value.empty())
return false;
if (!regexp_ || pattern_for_regexp_ != raw_pattern) {
ScriptRegexp* raw_regexp = MakeGarbageCollected<ScriptRegexp>(
ScriptRegexp* raw_regexp_u = MakeGarbageCollected<ScriptRegexp>(
raw_pattern, kTextCaseSensitive, MultilineMode::kMultilineDisabled,
UnicodeMode::kUnicode);
if (!raw_regexp->IsValid()) {
ScriptRegexp* raw_regexp_v = MakeGarbageCollected<ScriptRegexp>(
raw_pattern, kTextCaseSensitive, MultilineMode::kMultilineDisabled,
UnicodeMode::kUnicodeSets);
if (raw_regexp_u->IsValid() && !raw_regexp_v->IsValid()) {
UseCounter::Count(
GetElement().GetDocument(),
WebFeature::
kHTMLPatternRegExpUnicodeSetIncompatibilitiesWithUnicodeMode);
GetElement().GetDocument().AddConsoleMessage(
MakeGarbageCollected<ConsoleMessage>(
mojom::blink::ConsoleMessageSource::kRendering,
mojom::blink::ConsoleMessageLevel::kWarning,
"Pattern attribute value " + raw_pattern +
" is valid with the RegExp `u` flag, but not with the `v` "
"flag: " +
raw_regexp_v->ExceptionMessage() +
". See https://crbug.com/1412729"));
}
if (!raw_regexp_u->IsValid()) {
GetElement().GetDocument().AddConsoleMessage(
MakeGarbageCollected<ConsoleMessage>(
mojom::ConsoleMessageSource::kRendering,
mojom::ConsoleMessageLevel::kError,
mojom::blink::ConsoleMessageSource::kRendering,
mojom::blink::ConsoleMessageLevel::kError,
"Pattern attribute value " + raw_pattern +
" is not a valid regular expression: " +
raw_regexp->ExceptionMessage()));
regexp_ = raw_regexp;
raw_regexp_u->ExceptionMessage()));
regexp_ = raw_regexp_u;
pattern_for_regexp_ = raw_pattern;
return false;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
CONSOLE WARNING: Pattern attribute value [a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])? is valid with the RegExp `u` flag, but not with the `v` flag: Uncaught SyntaxError: Invalid regular expression: /[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?/: Invalid character in character class. See https://crbug.com/1412729
CONSOLE WARNING: Pattern attribute value [a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])? is valid with the RegExp `u` flag, but not with the `v` flag: Uncaught SyntaxError: Invalid regular expression: /[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?/: Invalid character in character class. See https://crbug.com/1412729
CONSOLE ERROR: Pattern attribute value )foo( is not a valid regular expression: Uncaught SyntaxError: Invalid regular expression: /)foo(/: Unmatched ')'
CONSOLE ERROR: Pattern attribute value )foo( is not a valid regular expression: Uncaught SyntaxError: Invalid regular expression: /)foo(/: Unmatched ')'
CONSOLE ERROR: Pattern attribute value )foo( is not a valid regular expression: Uncaught SyntaxError: Invalid regular expression: /)foo(/: Unmatched ')'
Expand Down
2 changes: 2 additions & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42196,6 +42196,8 @@ Called by update_use_counter_feature_enum.py.-->
<int value="4460" label="TopicsAPIFetch"/>
<int value="4461" label="TopicsAPIXhr"/>
<int value="4462" label="ParseFromString"/>
<int value="4463"
label="HTMLPatternRegExpUnicodeSetIncompatibilitiesWithUnicodeMode"/>
</enum>

<enum name="FeaturePolicyAllowlistType">
Expand Down

0 comments on commit 100082a

Please sign in to comment.