Fix AtomicStringTable lowercase translator.
It was erroneously checking if a candidate entry in the atomic string
table was an existing lowercase version of the provided string by
comparing the two strings for lower-ASCII equality, but this doesn't
imply that the string found in the atomic string table is lowercase
ASCII.
For urgent merging to release branches, this is a minimal fix.
A followup CL will land a more efficient version of this.
(cherry picked from commit 2f19541f156707c517f8bd2b3bdf9d8f729e2587)
(cherry picked from commit 486c4092effb4d632d3bef8f35b7d2832fb88552)
Bug: 1138487
Change-Id: I3af7411879ea831c9210e43cee16bca607b51b30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2480844
Reviewed-by: Daniel Cheng <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Jeremy Roman <[email protected]>
Cr-Original-Original-Commit-Position: refs/heads/master@{#818060}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2480731
Reviewed-by: Krishna Govind <[email protected]>
Cr-Original-Commit-Position: refs/branch-heads/4294@{#8}
Cr-Original-Branched-From: 945a64b5e6cd310d9fb6863165edf69d808e0c74-refs/heads/master@{#817614}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2480732
Reviewed-by: Jeremy Roman <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#488}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/third_party/blink/renderer/platform/wtf/text/atomic_string_table.cc b/third_party/blink/renderer/platform/wtf/text/atomic_string_table.cc
index f7c4174d..92af32dc0 100644
--- a/third_party/blink/renderer/platform/wtf/text/atomic_string_table.cc
+++ b/third_party/blink/renderer/platform/wtf/text/atomic_string_table.cc
@@ -166,7 +166,13 @@
}
static bool Equal(StringImpl* const& str, const StringView& buf) {
- return EqualIgnoringASCIICase(StringView(str), buf);
+ // This is similar to EqualIgnoringASCIICase, but not the same.
+ // In particular, it validates that |str| is a lowercase version of |buf|.
+ // Unlike EqualIgnoringASCIICase, it returns false if they are equal
+ // ignoring ASCII case but |str| contains an uppercase ASCII character.
+ // TODO(crbug.com/1138487): Replace this with a more efficient version.
+ StringView str_view(str);
+ return EqualIgnoringASCIICase(str_view, buf) && str_view.IsLowerASCII();
}
};
@@ -298,6 +304,8 @@
const auto& it = table_.Find<LowercaseStringViewLookupTranslator>(string);
if (it == table_.end())
return WeakResult();
+ DCHECK(StringView(*it).IsLowerASCII());
+ DCHECK(EqualIgnoringASCIICase(*it, string));
return WeakResult(*it);
}
diff --git a/third_party/blink/web_tests/external/wpt/dom/nodes/Element-setAttribute-crbug-1138487.html b/third_party/blink/web_tests/external/wpt/dom/nodes/Element-setAttribute-crbug-1138487.html
new file mode 100644
index 0000000..9aa9ed8
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/dom/nodes/Element-setAttribute-crbug-1138487.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+// Regression test for crbug.com/1138487.
+//
+// It was possible for a non-ASCII-lowercase string to be used when inserting
+// into the attribute collection if a hashtable encountered it during probing
+// while looking for the ASCII-lowercase equivalent.
+//
+// This caused such a string to be illegally used as an attribute name, thus
+// causing inconsistent behavior in future attribute lookup.
+test(() => {
+ const el = document.createElement('div');
+ el.setAttribute('labelXQL', 'abc');
+ el.setAttribute('_valueXQL', 'def');
+ assert_equals(el.getAttribute('labelXQL'), 'abc');
+ assert_equals(el.getAttribute('labelxql'), 'abc');
+ assert_equals(el.getAttribute('_valueXQL'), 'def');
+ assert_equals(el.getAttribute('_valuexql'), 'def');
+}, "Attributes first seen in mixed ASCII case should not be corrupted.");
+</script>
