Skip to content

Commit

Permalink
[Sanitizer] Remove name normalization and adapt tests.
Browse files Browse the repository at this point in the history
Adapt Sanitizer API to the proposed
WICG/sanitizer-api#145
This removes normalization of element and attribute names
and makes comparisons case-insensitive.

Bug: 1326827
Change-Id: Ib7cebd9e74cec895c2716aa88b4eeb71bb7d169e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3706742
Commit-Queue: Daniel Vogelheim <[email protected]>
Reviewed-by: Yifan Luo <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1017597}
  • Loading branch information
otherdaniel authored and Chromium LUCI CQ committed Jun 24, 2022
1 parent 7babdb5 commit 13a47b1
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 81 deletions.
46 changes: 0 additions & 46 deletions third_party/blink/renderer/modules/sanitizer_api/builtins.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,6 @@ namespace {
// arguments. Thus we have this typedef as a work-around.
typedef HashMap<String, String> StringMap;

StringMap MixedCaseNames(const char* const* names) {
HashMap<String, String> map;
for (const char* const* iter = names; *iter; ++iter) {
String name(*iter);
if (!name.IsLowerASCII()) {
map.insert(name.LowerASCII(), name);
}
}
return map;
}

SanitizerConfigImpl::ElementList ElementsFromAPI(const char* const* elements) {
SanitizerConfigImpl::ElementList element_list;
for (const char* const* elem = elements; *elem; ++elem) {
Expand Down Expand Up @@ -91,18 +80,6 @@ const SanitizerConfigImpl::AttributeList& GetBaselineAllowAttributes() {
return attributes_;
}

const HashMap<String, String>& GetMixedCaseElementNames() {
DEFINE_STATIC_LOCAL(StringMap, element_names_,
(MixedCaseNames(kBaselineElements)));
return element_names_;
}

const HashMap<String, String>& GetMixedCaseAttributeNames() {
DEFINE_STATIC_LOCAL(StringMap, attribute_names_,
(MixedCaseNames(kBaselineAttributes)));
return attribute_names_;
}

} // namespace default_config_names

namespace with_namespace_names {
Expand All @@ -126,18 +103,6 @@ const SanitizerConfigImpl::AttributeList& GetBaselineAllowAttributes() {
return attributes_;
}

const HashMap<String, String>& GetMixedCaseElementNames() {
DEFINE_STATIC_LOCAL(StringMap, element_names_,
(MixedCaseNames(kBaselineElements)));
return element_names_;
}

const HashMap<String, String>& GetMixedCaseAttributeNames() {
DEFINE_STATIC_LOCAL(StringMap, attribute_names_,
(MixedCaseNames(kBaselineAttributes)));
return attribute_names_;
}

} // namespace with_namespace_names

bool WithNamespaces() {
Expand All @@ -161,15 +126,4 @@ const SanitizerConfigImpl::AttributeList& GetBaselineAllowAttributes() {
return WithNamespaces() ? with_namespace_names::GetBaselineAllowAttributes()
: default_config_names::GetBaselineAllowAttributes();
}

const HashMap<String, String>& GetMixedCaseElementNames() {
return WithNamespaces() ? with_namespace_names::GetMixedCaseElementNames()
: default_config_names::GetMixedCaseElementNames();
}

const HashMap<String, String>& GetMixedCaseAttributeNames() {
return WithNamespaces() ? with_namespace_names::GetMixedCaseAttributeNames()
: default_config_names::GetMixedCaseAttributeNames();
}

} // namespace blink
5 changes: 0 additions & 5 deletions third_party/blink/renderer/modules/sanitizer_api/builtins.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ const SanitizerConfigImpl& GetDefaultConfig();
const SanitizerConfigImpl::ElementList& GetBaselineAllowElements();
const SanitizerConfigImpl::AttributeList& GetBaselineAllowAttributes();

// We derive a map of lower-case to mixed-case names from the built-ins, for
// use in name normalization.
const HashMap<String, String>& GetMixedCaseElementNames();
const HashMap<String, String>& GetMixedCaseAttributeNames();

} // namespace blink

#endif // THIRD_PARTY_BLINK_RENDERER_MODULES_SANITIZER_API_BUILTINS_H_
31 changes: 9 additions & 22 deletions third_party/blink/renderer/modules/sanitizer_api/config_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ bool IsValidCharacter(UChar ch) {
// obtuse, but it seems to allow all XML names. The HTML parser however
// allows only ascii. Here, we settle for the simplest, most restrictive
// variant. May it's too restrictive, though.
// TODO(vogelheim): "HTML parser allows only ascii" is no longer true.
return (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') ||
(ch >= '0' && ch <= '9') || ch == ':' || ch == '-' || ch == '_';
}
Expand All @@ -113,22 +114,15 @@ String ElementFromAPI(const String& name) {
if (!IsValidName(name))
return Invalid();

// Normalize element name, using the GetMixedCaseElementNames table.
String normalized = name.LowerASCII();
const auto& mixed_case_names = GetMixedCaseElementNames();
const auto iter = mixed_case_names.find(normalized);
if (iter != mixed_case_names.end())
normalized = iter->value;

// Handle namespace prefixes:
wtf_size_t pos = normalized.find(':');
wtf_size_t pos = name.find(':');
// Two (or more) colons => invalid.
if (pos != WTF::kNotFound && normalized.find(':', pos + 1) != WTF::kNotFound)
if (pos != WTF::kNotFound && name.find(':', pos + 1) != WTF::kNotFound)
return Invalid();
// No prefix, or the ones explicitly allowed by the spec: okay.
if (pos == WTF::kNotFound || normalized.StartsWith("svg:") ||
normalized.StartsWith("math:")) {
return normalized;
if (pos == WTF::kNotFound || name.StartsWith("svg:") ||
name.StartsWith("math:")) {
return name;
}
// All else: invalid.
return Invalid();
Expand All @@ -138,19 +132,12 @@ String AttributeFromAPI(const String& name) {
if (!IsValidName(name))
return Invalid();

// Normalize attribute name, using the GetMixedCaseAttributeNames table.
String normalized = name.LowerASCII();
const auto& mixed_case_names = GetMixedCaseAttributeNames();
const auto iter = mixed_case_names.find(normalized);
if (iter != mixed_case_names.end())
normalized = iter->value;

// The spec allows only a specific list of prefixed attributes. Use the
// GetBaselineAllowAttributes() table to check for those. All other uses
// of colon are invalid.
if (normalized.find(':') == WTF::kNotFound ||
GetBaselineAllowAttributes().Contains(normalized))
return normalized;
if (name.find(':') == WTF::kNotFound ||
GetBaselineAllowAttributes().Contains(name))
return name;
return Invalid();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,11 @@
assert_equals(sanitize(elem, probe), probe);
}, `Mixed-case element names #${index}: "${elem}"`);
test(t => {
assert_equals(sanitize(elem.toLowerCase(), probe), probe);
assert_not_equals(sanitize(elem.toLowerCase(), probe), probe);
}, `Mixed-case element names #${index}: "${elem.toLowerCase()}"`);
test(t => {
assert_equals(sanitize(elem.toUpperCase(), probe), probe);
assert_not_equals(sanitize(elem.toUpperCase(), probe), probe);
}, `Mixed-case element names #${index}: "${elem.toUpperCase()}"`);
test(t => {
assert_equals(sanitize(elem, probe.toLowerCase()), probe);
}, `Mixed-case element names #${index}: "${elem}" with lowercase probe.`);
test(t => {
const elems = [elem];
assert_array_equals(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"onclick": ["*"] },
allowCustomElements: true,
};
assert_deep_equals(configs[0],
assert_deep_equals(config_0_mixed,
new Sanitizer(config_0_mixed).getConfiguration());
}, "SanitizerAPI getConfiguration() reflects creation config.");

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 13a47b1

Please sign in to comment.