tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 9d4624d9f1111a080b258cc780418909b1be4102
parent 19122ed840a521174ab8f5f4ae93efb70b0eb642
Author: Tom Schuster <tschuster@mozilla.com>
Date:   Thu, 16 Oct 2025 21:25:28 +0000

Bug 1989215 - Sanitizer: Use unordered sets and a map instead of lists. r=smaug

Differential Revision: https://phabricator.services.mozilla.com/D267060

Diffstat:
Mdom/security/sanitizer/Sanitizer.cpp | 306+++++++++++++++++++++++++++++++++++++++++++------------------------------------
Mdom/security/sanitizer/Sanitizer.h | 14++++++--------
Mdom/security/sanitizer/SanitizerTypes.cpp | 109+++++++++++++++++++++++++++++--------------------------------------------------
Mdom/security/sanitizer/SanitizerTypes.h | 96+++++++++++++++++++++++++++++++------------------------------------------------
4 files changed, 250 insertions(+), 275 deletions(-)

diff --git a/dom/security/sanitizer/Sanitizer.cpp b/dom/security/sanitizer/Sanitizer.cpp @@ -281,12 +281,13 @@ static CanonicalName CanonicalizeAttribute( // https://wicg.github.io/sanitizer-api/#canonicalize-a-sanitizer-element-with-attributes template <typename SanitizerElementWithAttributes> -static CanonicalElementWithAttributes CanonicalizeElementWithAttributes( +static CanonicalElementAttributes CanonicalizeElementAttributes( const SanitizerElementWithAttributes& aElement) { // Step 1. Let result be the result of canonicalize a sanitizer element with // element. - CanonicalElementWithAttributes result = - CanonicalElementWithAttributes(CanonicalizeElement(aElement)); + // + // NOTE: CanonicalizeElement is the responsibility of the caller. + CanonicalElementAttributes result{}; // Step 2. If element is a dictionary: if (aElement.IsSanitizerElementNamespaceWithAttributes()) { @@ -295,7 +296,7 @@ static CanonicalElementWithAttributes CanonicalizeElementWithAttributes( // Step 2.1. If element["attributes"] exists: if (elem.mAttributes.WasPassed()) { // Step 2.1.1. Let attributes be « ». - ListSet<CanonicalName> attributes; + CanonicalNameSet attributes; // Step 2.1.2. For each attribute of element["attributes"]: for (const auto& attribute : elem.mAttributes.Value()) { @@ -311,7 +312,7 @@ static CanonicalElementWithAttributes CanonicalizeElementWithAttributes( // Step 2.2. If element["attributes"] exists: if (elem.mRemoveAttributes.WasPassed()) { // Step 2.2.1. Let attributes be « ». - ListSet<CanonicalName> attributes; + CanonicalNameSet attributes; // Step 2.2.2. For each attribute of element["removeAttributes"]: for (const auto& attribute : elem.mRemoveAttributes.Value()) { @@ -329,7 +330,8 @@ static CanonicalElementWithAttributes CanonicalizeElementWithAttributes( // result["removeAttributes"] exist: if (!result.mAttributes && !result.mRemoveAttributes) { // Step 3.1. Set result["removeAttributes"] to « ». - result.mRemoveAttributes = Some(ListSet<CanonicalName>()); + CanonicalNameSet set{}; + result.mRemoveAttributes = Some(std::move(set)); } // Step 4. Return result. @@ -339,6 +341,9 @@ static CanonicalElementWithAttributes CanonicalizeElementWithAttributes( // https://wicg.github.io/sanitizer-api/#configuration-canonicalize void Sanitizer::CanonicalizeConfiguration( const SanitizerConfig& aConfig, bool aAllowCommentsAndDataAttributes) { + // This function is only called while constructing a new Sanitizer object. + AssertNoLists(); + // Step 1. If neither configuration["elements"] nor // configuration["removeElements"] exist, then set // configuration["removeElements"] to « [] ». @@ -357,82 +362,93 @@ void Sanitizer::CanonicalizeConfiguration( // Step 3. If configuration["elements"] exists: if (aConfig.mElements.WasPassed()) { // Step 3.1. Let elements be « [] » - nsTArray<CanonicalElementWithAttributes> elements; + CanonicalElementMap elements; // Step 3.2. For each element of configuration["elements"] do: for (const auto& element : aConfig.mElements.Value()) { // Step 3.3.2.1. Append the result of canonicalize a sanitizer element // with attributes element to elements. - elements.AppendElement(CanonicalizeElementWithAttributes(element)); + + // XX duplicates. + CanonicalName elementName = CanonicalizeElement(element); + CanonicalElementAttributes elementAttributes = + CanonicalizeElementAttributes(element); + elements.InsertOrUpdate(elementName, std::move(elementAttributes)); } // Step 3.3. Set configuration["elements"] to elements. - mElements = Some(ListSet(std::move(elements))); + mElements = Some(std::move(elements)); } // Step 4. If configuration["removeElements"] exists: if (aConfig.mRemoveElements.WasPassed()) { // Step 4.1. Let elements be « [] » - nsTArray<CanonicalName> elements; + CanonicalNameSet elements; // Step 4.2. For each element of configuration["removeElements"] do: for (const auto& element : aConfig.mRemoveElements.Value()) { // Step 4.2.1. Append the result of canonicalize a sanitizer element // element to elements. - elements.AppendElement(CanonicalizeElement(element)); + + // xxx + elements.Insert(CanonicalizeElement(element)); } // Step 4.3. Set configuration["removeElements"] to elements. - mRemoveElements = Some(ListSet(std::move(elements))); + mRemoveElements = Some(std::move(elements)); } // Step 5. If configuration["replaceWithChildrenElements"] exists: if (aConfig.mReplaceWithChildrenElements.WasPassed()) { // Step 5.1. Let elements be « [] » - nsTArray<CanonicalName> elements; + CanonicalNameSet elements; // Step 5.2. For each element of // configuration["replaceWithChildrenElements"] do: for (const auto& element : aConfig.mReplaceWithChildrenElements.Value()) { // Step 5.2.1. Append the result of canonicalize a sanitizer element // element to elements. - elements.AppendElement(CanonicalizeElement(element)); + + // XXX + elements.Insert(CanonicalizeElement(element)); } // Step 5.3. Set configuration["replaceWithChildrenElements"] to elements. - mReplaceWithChildrenElements = Some(ListSet(std::move(elements))); + mReplaceWithChildrenElements = Some(std::move(elements)); } // Step 6. If configuration["attributes"] exists: if (aConfig.mAttributes.WasPassed()) { // Step 6.1. Let attributes be « [] » - nsTArray<CanonicalName> attributes; + CanonicalNameSet attributes; // Step 6.2. For each attribute of configuration["attributes"] do: for (const auto& attribute : aConfig.mAttributes.Value()) { // Step 6.2.1. Append the result of canonicalize a sanitizer attribute // attribute to attributes. - attributes.AppendElement(CanonicalizeAttribute(attribute)); + + // XXX Remember duplicates. + attributes.Insert(CanonicalizeAttribute(attribute)); } // Step 6.3. Set configuration["attributes"] to attributes. - mAttributes = Some(ListSet(std::move(attributes))); + mAttributes = Some(std::move(attributes)); } // Step 7. If configuration["removeAttributes"] exists: if (aConfig.mRemoveAttributes.WasPassed()) { // Step 7.1. Let attributes be « [] » - nsTArray<CanonicalName> attributes; + CanonicalNameSet attributes; // Step 7.2. For each attribute of configuration["removeAttributes"] do: for (const auto& attribute : aConfig.mRemoveAttributes.Value()) { // Step 7.2.2. Append the result of canonicalize a sanitizer attribute // attribute to attributes. - attributes.AppendElement(CanonicalizeAttribute(attribute)); + attributes.Insert(CanonicalizeAttribute(attribute)); } // Step 7.3. Set configuration["removeAttributes"] to attributes. - mRemoveAttributes = Some(ListSet(std::move(attributes))); + mRemoveAttributes = Some(std::move(attributes)); } // Step 8. If configuration["comments"] does not exist, then set @@ -486,33 +502,33 @@ void Sanitizer::IsValid(ErrorResult& aRv) { // Step 4. None of config[elements], config[removeElements], // config[replaceWithChildrenElements], config[attributes], or // config[removeAttributes], if they exist, has duplicates. - if (mElements && mElements->HasDuplicates()) { - aRv.ThrowTypeError("Duplicate element in 'elements'"); - return; - } - if (mRemoveElements && mRemoveElements->HasDuplicates()) { - aRv.ThrowTypeError("Duplicate element in 'removeElement'"); - return; - } - if (mReplaceWithChildrenElements && - mReplaceWithChildrenElements->HasDuplicates()) { - aRv.ThrowTypeError("Duplicate element in 'replaceWithChildrenElements'"); - return; - } - if (mAttributes && mAttributes->HasDuplicates()) { - aRv.ThrowTypeError("Duplicate attribute in 'attributes'"); - return; - } - if (mRemoveAttributes && mRemoveAttributes->HasDuplicates()) { - aRv.ThrowTypeError("Duplicate attribute in 'removeAttributes'"); - return; - } + // if (mElements && mElements->HasDuplicates()) { + // aRv.ThrowTypeError("Duplicate element in 'elements'"); + // return; + // } + // if (mRemoveElements && mRemoveElements->HasDuplicates()) { + // aRv.ThrowTypeError("Duplicate element in 'removeElement'"); + // return; + // } + // if (mReplaceWithChildrenElements && + // mReplaceWithChildrenElements->HasDuplicates()) { + // aRv.ThrowTypeError("Duplicate element in 'replaceWithChildrenElements'"); + // return; + // } + // if (mAttributes && mAttributes->HasDuplicates()) { + // aRv.ThrowTypeError("Duplicate attribute in 'attributes'"); + // return; + // } + // if (mRemoveAttributes && mRemoveAttributes->HasDuplicates()) { + // aRv.ThrowTypeError("Duplicate attribute in 'removeAttributes'"); + // return; + // } // Step 5. If both config[elements] and config[replaceWithChildrenElements] // exist, then the intersection of config[elements] and // config[replaceWithChildrenElements] is empty. if (mElements && mReplaceWithChildrenElements) { - for (const CanonicalElementWithAttributes& name : mElements->Values()) { + for (const CanonicalName& name : mElements->Keys()) { if (mReplaceWithChildrenElements->Contains(name)) { aRv.ThrowTypeError( "Element can't be in both 'elements' and " @@ -526,7 +542,7 @@ void Sanitizer::IsValid(ErrorResult& aRv) { // config[replaceWithChildrenElements] exist, then the intersection of // config[removeElements] and config[replaceWithChildrenElements] is empty. if (mRemoveElements && mReplaceWithChildrenElements) { - for (const CanonicalName& name : mRemoveElements->Values()) { + for (const CanonicalName& name : *mRemoveElements) { if (mReplaceWithChildrenElements->Contains(name)) { aRv.ThrowTypeError( "Element can't be in both 'removeElements' and " @@ -541,9 +557,12 @@ void Sanitizer::IsValid(ErrorResult& aRv) { // Step 7.1. If config[elements] exists: if (mElements) { // Step 7.1.1 For any element in config[elements]: - for (const CanonicalElementWithAttributes& elem : mElements->Values()) { + for (const auto& entry : *mElements) { + const CanonicalElementAttributes& elemAttributes = entry.GetData(); + // Step 7.1.1.1. Neither element[attributes] or // element[removeAttributes], if they exist, has duplicates. + /* if (elem.mAttributes && elem.mAttributes->HasDuplicates()) { aRv.ThrowTypeError("Duplicate attribute in local 'attributes'"); return; @@ -552,12 +571,13 @@ void Sanitizer::IsValid(ErrorResult& aRv) { aRv.ThrowTypeError("Duplicate attribute in local 'removeAttributes'"); return; } + */ // Step 7.1.1.2. The intersection of config[attributes] and // element[attributes] with default « [] » is empty. - if (elem.mAttributes) { - for (const CanonicalName& name : mAttributes->Values()) { - if (elem.mAttributes->Contains(name)) { + if (elemAttributes.mAttributes) { + for (const CanonicalName& name : *elemAttributes.mAttributes) { + if (mAttributes->Contains(name)) { aRv.ThrowTypeError( "Same attribute both in local and global 'attributes'"); return; @@ -567,8 +587,8 @@ void Sanitizer::IsValid(ErrorResult& aRv) { // Step 7.1.1.3. element[removeAttributes] is a subset of // config[attributes]. - if (elem.mRemoveAttributes) { - for (const CanonicalName& name : elem.mRemoveAttributes->Values()) { + if (elemAttributes.mRemoveAttributes) { + for (const CanonicalName& name : *elemAttributes.mRemoveAttributes) { if (!mAttributes->Contains(name)) { aRv.ThrowTypeError( "Attribute in local 'removeAttributes' but not in global " @@ -582,10 +602,11 @@ void Sanitizer::IsValid(ErrorResult& aRv) { "mDataAttributes exists iff mAttributes exists"); // Step 7.1.1.4. If dataAttributes is true: - if (*mDataAttributes && elem.mAttributes) { + if (*mDataAttributes && elemAttributes.mAttributes) { + // TODO: Merge with loop above? // Step 7.1.1.4.1. element[attributes] does not contain a custom // data attribute. - for (const CanonicalName& name : elem.mAttributes->Values()) { + for (const CanonicalName& name : *elemAttributes.mAttributes) { if (name.IsDataAttribute()) { aRv.ThrowTypeError( "Local 'attributes' contains a data attribute, which is " @@ -604,7 +625,7 @@ void Sanitizer::IsValid(ErrorResult& aRv) { if (*mDataAttributes) { // Step 7.2.1. config[attributes] does not contain a custom data // attribute. - for (const CanonicalName& name : mAttributes->Values()) { + for (const CanonicalName& name : *mAttributes) { if (name.IsDataAttribute()) { aRv.ThrowTypeError( "Global 'attributes' contains a data attribute, which is " @@ -620,11 +641,13 @@ void Sanitizer::IsValid(ErrorResult& aRv) { // Step 8.1. If config[elements] exists, then for any element in // config[elements]: if (mElements) { - for (const CanonicalElementWithAttributes& elem : mElements->Values()) { + for (const auto& entry : *mElements) { + const CanonicalElementAttributes& elemAttributes = entry.GetData(); + // Step 8.1.1. The intersection of config[removeAttributes] and // element[attributes] with default « [] » is empty. - if (elem.mAttributes) { - for (const CanonicalName& name : elem.mAttributes->Values()) { + if (elemAttributes.mAttributes) { + for (const CanonicalName& name : *elemAttributes.mAttributes) { if (mRemoveAttributes->Contains(name)) { aRv.ThrowTypeError( "Same attribute both in local 'attributes' and global " @@ -636,8 +659,8 @@ void Sanitizer::IsValid(ErrorResult& aRv) { // Step 8.1.2. The intersection of config[removeAttributes] and // element[removeAttributes] with default « [] » is empty. - if (elem.mRemoveAttributes) { - for (const CanonicalName& name : elem.mRemoveAttributes->Values()) { + if (elemAttributes.mRemoveAttributes) { + for (const CanonicalName& name : *elemAttributes.mRemoveAttributes) { if (mRemoveAttributes->Contains(name)) { aRv.ThrowTypeError( "Same attribute both in local and global " @@ -694,26 +717,26 @@ void Sanitizer::MaybeMaterializeDefaultConfig() { AssertNoLists(); - nsTArray<CanonicalElementWithAttributes> elements; + CanonicalElementMap elements; auto insertElements = [&elements]( mozilla::Span<nsStaticAtom* const> aElements, nsStaticAtom* aNamespace, nsStaticAtom* const* aElementWithAttributes) { size_t i = 0; for (nsStaticAtom* name : aElements) { - CanonicalElementWithAttributes element(CanonicalName(name, aNamespace)); + CanonicalElementAttributes elementAttributes{}; if (name == aElementWithAttributes[i]) { - nsTArray<CanonicalName> attributes; + CanonicalNameSet attributes; while (aElementWithAttributes[++i]) { - attributes.AppendElement( - CanonicalName(aElementWithAttributes[i], nullptr)); + attributes.Insert(CanonicalName(aElementWithAttributes[i], nullptr)); } i++; - element.mAttributes = Some(ListSet(std::move(attributes))); + elementAttributes.mAttributes = Some(std::move(attributes)); } - elements.AppendElement(std::move(element)); + CanonicalName elementName = CanonicalName(name, aNamespace); + elements.InsertOrUpdate(elementName, std::move(elementAttributes)); } }; insertElements(Span(kDefaultHTMLElements), nsGkAtoms::nsuri_xhtml, @@ -722,13 +745,13 @@ void Sanitizer::MaybeMaterializeDefaultConfig() { kMathMLElementWithAttributes); insertElements(Span(kDefaultSVGElements), nsGkAtoms::nsuri_svg, kSVGElementWithAttributes); - mElements = Some(ListSet(std::move(elements))); + mElements = Some(std::move(elements)); - nsTArray<CanonicalName> attributes; + CanonicalNameSet attributes; for (nsStaticAtom* name : kDefaultAttributes) { - attributes.AppendElement(CanonicalName(name, nullptr)); + attributes.Insert(CanonicalName(name, nullptr)); } - mAttributes = Some(ListSet(std::move(attributes))); + mAttributes = Some(std::move(attributes)); mIsDefaultConfig = false; } @@ -738,15 +761,15 @@ void Sanitizer::Get(SanitizerConfig& aConfig) { if (mElements) { nsTArray<OwningStringOrSanitizerElementNamespaceWithAttributes> elements; - for (const CanonicalElementWithAttributes& canonical : - mElements->Values()) { + for (const auto& entry : *mElements) { elements.AppendElement()->SetAsSanitizerElementNamespaceWithAttributes() = - canonical.ToSanitizerElementNamespaceWithAttributes(); + entry.GetKey().ToSanitizerElementNamespaceWithAttributes( + entry.GetData()); } aConfig.mElements.Construct(std::move(elements)); } else { nsTArray<OwningStringOrSanitizerElementNamespace> removeElements; - for (const CanonicalName& canonical : mRemoveElements->Values()) { + for (const CanonicalName& canonical : *mRemoveElements) { removeElements.AppendElement()->SetAsSanitizerElementNamespace() = canonical.ToSanitizerElementNamespace(); } @@ -756,8 +779,7 @@ void Sanitizer::Get(SanitizerConfig& aConfig) { if (mReplaceWithChildrenElements) { nsTArray<OwningStringOrSanitizerElementNamespace> replaceWithChildrenElements; - for (const CanonicalName& canonical : - mReplaceWithChildrenElements->Values()) { + for (const CanonicalName& canonical : *mReplaceWithChildrenElements) { replaceWithChildrenElements.AppendElement() ->SetAsSanitizerElementNamespace() = canonical.ToSanitizerElementNamespace(); @@ -786,29 +808,30 @@ bool Sanitizer::AllowElement( // Step 1. Set element to the result of canonicalize a sanitizer element // with attributes with element. - CanonicalElementWithAttributes element = - CanonicalizeElementWithAttributes(aElement); + CanonicalName elementName = CanonicalizeElement(aElement); + // NOTE: Duplicate attributes are removed/ignored. + CanonicalElementAttributes elementAttributes = + CanonicalizeElementAttributes(aElement); // Step 2 If configuration["elements"] exists: if (mElements) { // Step 2.1. Set modified to the result of remove element from // configuration["replaceWithChildrenElements"]. - bool modified = mReplaceWithChildrenElements - ? mReplaceWithChildrenElements->Remove(element) - : false; + bool modified = + mReplaceWithChildrenElements + ? mReplaceWithChildrenElements->EnsureRemoved(elementName) + : false; // Step 2.2. Comment: We need to make sure the per-element attributes do // not overlap with global attributes. // Step 2.3. If element["attributes"] exists: - if (element.mAttributes) { - nsTArray<CanonicalName> attributes; - for (const CanonicalName& attr : element.mAttributes->Values()) { + if (elementAttributes.mAttributes) { + CanonicalNameSet attributes; + for (const CanonicalName& attr : *elementAttributes.mAttributes) { // Step 2.3.1. Set element["attributes"] to remove duplicates from // element["attributes"]. - if (attributes.Contains(attr)) { - continue; - } + MOZ_ASSERT(!attributes.Contains(attr)); // Step 2.3.2. If configuration["attributes"] exists: if (mAttributes) { @@ -840,20 +863,18 @@ bool Sanitizer::AllowElement( } } - attributes.AppendElement(attr.Clone()); + attributes.Insert(attr.Clone()); } - element.mAttributes = Some(ListSet(std::move(attributes))); + elementAttributes.mAttributes = Some(std::move(attributes)); } // Step 2.4. If element["removeAttributes"] exists: - if (element.mRemoveAttributes) { - nsTArray<CanonicalName> removeAttributes; - for (const CanonicalName& attr : element.mRemoveAttributes->Values()) { + if (elementAttributes.mRemoveAttributes) { + CanonicalNameSet removeAttributes; + for (const CanonicalName& attr : *elementAttributes.mRemoveAttributes) { // Step 2.4.1. Set element["removeAttributes"] to remove duplicates from // element["removeAttributes"]. - if (removeAttributes.Contains(attr)) { - continue; - } + MOZ_ASSERT(!removeAttributes.Contains(attr)); // Step 2.4.2. If configuration["attributes"] exists: if (mAttributes) { @@ -873,19 +894,20 @@ bool Sanitizer::AllowElement( } } - removeAttributes.AppendElement(attr.Clone()); + removeAttributes.Insert(attr.Clone()); } - element.mRemoveAttributes = Some(ListSet(std::move(removeAttributes))); + elementAttributes.mRemoveAttributes = Some(std::move(removeAttributes)); } // Step 2.5. If configuration["elements"] does not contain element: - CanonicalElementWithAttributes* existingElement = mElements->Get(element); - if (!existingElement) { + const CanonicalElementAttributes* existingElementAttributes = + mElements->Lookup(elementName).DataPtrOrNull(); + if (!existingElementAttributes) { // Step 2.5.1. Comment: This is the case with a global allow-list that // does not yet contain element. // Step 2.5.2. Append element to configuration["elements"]. - mElements->Insert(std::move(element)); + mElements->InsertOrUpdate(elementName, std::move(elementAttributes)); // Step 2.5.3. Return true. return true; @@ -896,18 +918,16 @@ bool Sanitizer::AllowElement( // Step 2.7. Let current element be the item in configuration["elements"] // where item[name] equals element[name] and item[namespace] equals - // element[namespace]. (implict with existingElement) + // element[namespace]. (implict with existingElementAttributes) // Step 2.8. If element equals current element then return modified. - if (element.EqualAttributes(*existingElement)) { + if (elementAttributes.Equals(*existingElementAttributes)) { return modified; } // Step 2.9. Remove element from configuration["elements"]. - mElements->Remove(element); - // Step 2.10. Append element to configuration["elements"]. - mElements->Insert(std::move(element)); + mElements->InsertOrUpdate(elementName, std::move(elementAttributes)); // Step 2.11. Return true. return true; @@ -916,8 +936,9 @@ bool Sanitizer::AllowElement( // Step 3. Otherwise: // Step 3.1. If element["attributes"] exists or element["removeAttributes"] // with default « » is not empty: - if (element.mAttributes || - (element.mRemoveAttributes && !element.mRemoveAttributes->IsEmpty())) { + if (elementAttributes.mAttributes || + (elementAttributes.mRemoveAttributes && + !elementAttributes.mRemoveAttributes->IsEmpty())) { // Step 3.1.1. The user agent may report a warning to the console that this // operation is not supported. LogLocalizedString("SanitizerAllowElementIgnored", {}, @@ -930,11 +951,11 @@ bool Sanitizer::AllowElement( // Step 3.2. Set modified to the result of remove element from // configuration["replaceWithChildrenElements"]. bool modified = mReplaceWithChildrenElements - ? mReplaceWithChildrenElements->Remove(element) + ? mReplaceWithChildrenElements->EnsureRemoved(elementName) : false; // Step 3.3. If configuration["removeElements"] does not contain element: - if (!mRemoveElements->Contains(element)) { + if (!mRemoveElements->Contains(elementName)) { // Step 3.3.1. Comment: This is the case with a global remove-list that // does not contain element. @@ -946,7 +967,7 @@ bool Sanitizer::AllowElement( // contains element. // Step 3.5. Remove element from configuration["removeElements"]. - mRemoveElements->Remove(element); + mRemoveElements->Remove(elementName); // Step 3.6. Return true. return true; @@ -968,7 +989,7 @@ bool Sanitizer::RemoveElementCanonical(CanonicalName&& aElement) { // Step 2. Set modified to the result of remove element from // configuration["replaceWithChildrenElements"]. bool modified = mReplaceWithChildrenElements - ? mReplaceWithChildrenElements->Remove(aElement) + ? mReplaceWithChildrenElements->EnsureRemoved(aElement) : false; // Step 3. If configuration["elements"] exists: @@ -1080,18 +1101,21 @@ bool Sanitizer::AllowAttribute( // Step 2.5. If configuration["elements"] exists: if (mElements) { // Step 2.5.1. For each element in configuration["elements"]: - for (CanonicalElementWithAttributes& element : mElements->Values()) { + for (auto iter = mElements->Iter(); !iter.Done(); iter.Next()) { + CanonicalElementAttributes& elemAttributes = iter.Data(); + // Step 2.5.1.1. If element["attributes"] with default « [] » contains // attribute: - if (element.mAttributes && element.mAttributes->Contains(attribute)) { + if (elemAttributes.mAttributes && + elemAttributes.mAttributes->Contains(attribute)) { // Step 2.5.1.1.1. Remove attribute from element["attributes"]. - element.mAttributes->Remove(attribute); + elemAttributes.mAttributes->Remove(attribute); } // Step 2.5.1.2. Assert: element["removeAttributes"] with default « [] // » does not contain attribute. - MOZ_ASSERT_IF(element.mRemoveAttributes, - !element.mRemoveAttributes->Contains(attribute)); + MOZ_ASSERT_IF(elemAttributes.mRemoveAttributes, + !elemAttributes.mRemoveAttributes->Contains(attribute)); } } @@ -1150,14 +1174,15 @@ bool Sanitizer::RemoveAttributeCanonical(CanonicalName&& aAttribute) { // Step 2.4. If configuration["elements"] exists: if (mElements) { // Step 2.4.1. For each element in configuration["elements"]: - for (CanonicalElementWithAttributes& element : mElements->Values()) { + for (auto iter = mElements->Iter(); !iter.Done(); iter.Next()) { + CanonicalElementAttributes& elemAttributes = iter.Data(); // Step 2.4.1.1. If element["removeAttributes"] with default « [] » // contains attribute: - if (element.mRemoveAttributes && - element.mRemoveAttributes->Contains(aAttribute)) { + if (elemAttributes.mRemoveAttributes && + elemAttributes.mRemoveAttributes->Contains(aAttribute)) { // Step 2.4.1.1.1. Remove attribute from // element["removeAttributes"]. - element.mRemoveAttributes->Remove(aAttribute); + elemAttributes.mRemoveAttributes->Remove(aAttribute); } } } @@ -1183,20 +1208,22 @@ bool Sanitizer::RemoveAttributeCanonical(CanonicalName&& aAttribute) { // Step 3.4. If configuration["elements"] exists: if (mElements) { // Step 3.4.1. For each element in configuration["elements"]: - for (CanonicalElementWithAttributes& element : mElements->Values()) { + for (auto iter = mElements->Iter(); !iter.Done(); iter.Next()) { + CanonicalElementAttributes& elemAttributes = iter.Data(); // Step 3.4.1.1. If element["attributes"] with default « [] » contains // attribute: - if (element.mAttributes && element.mAttributes->Contains(aAttribute)) { + if (elemAttributes.mAttributes && + elemAttributes.mAttributes->Contains(aAttribute)) { // Step 3.4.1.1.1. Remove attribute from element["attributes"]. - element.mAttributes->Remove(aAttribute); + elemAttributes.mAttributes->Remove(aAttribute); } // Step 3.4.1.2. If element["removeAttributes"] with default « [] » // contains attribute: - if (element.mRemoveAttributes && - element.mRemoveAttributes->Contains(aAttribute)) { + if (elemAttributes.mRemoveAttributes && + elemAttributes.mRemoveAttributes->Contains(aAttribute)) { // Step 3.4.1.2.1. Remove attribute from element["removeAttributes"]. - element.mRemoveAttributes->Remove(aAttribute); + elemAttributes.mRemoveAttributes->Remove(aAttribute); } } } @@ -1251,19 +1278,20 @@ bool Sanitizer::SetDataAttributes(bool aAllow) { // Step 3.1. Remove any items attr from configuration["attributes"] where // attr is a custom data attribute. - mAttributes->Values().RemoveElementsBy([](const CanonicalName& aAttribute) { + mAttributes->RemoveIf([](const CanonicalName& aAttribute) { return aAttribute.IsDataAttribute(); }); // Step 3.2. If configuration["elements"] exists: if (mElements) { // Step 3.2.1. For each element in configuration["elements"]: - for (CanonicalElementWithAttributes& element : mElements->Values()) { + for (auto iter = mElements->Iter(); !iter.Done(); iter.Next()) { + CanonicalElementAttributes& elemAttributes = iter.Data(); // Step 3.2.1.1. If element[attributes] exists: - if (element.mAttributes) { + if (elemAttributes.mAttributes) { // Step 3.2.1.1.1. Remove any items attr from element[attributes] // where attr is a custom data attribute. - element.mAttributes->Values().RemoveElementsBy( + elemAttributes.mAttributes->RemoveIf( [](const CanonicalName& aAttribute) { return aAttribute.IsDataAttribute(); }); @@ -1652,10 +1680,11 @@ void Sanitizer::SanitizeAttributes(Element* aChild, // Step 7. Let elementWithLocalAttributes be « [] ». // Step 8. If configuration["elements"] exists and configuration["elements"] - // contains elementName: Step 8.1. Set elementWithLocalAttributes to + // contains elementName: + // Step 8.1. Set elementWithLocalAttributes to // configuration["elements"][elementName]. - const CanonicalElementWithAttributes* elementWithAttributes = - mElements ? mElements->Get(aElementName) : nullptr; + const CanonicalElementAttributes* elementAttributes = + mElements ? mElements->Lookup(aElementName).DataPtrOrNull() : nullptr; // Step 9. For each attribute in child’s attribute list: int32_t count = int32_t(aChild->GetAttrCount()); @@ -1678,9 +1707,8 @@ void Sanitizer::SanitizeAttributes(Element* aChild, // Step 9.2. If elementWithLocalAttributes["removeAttributes"] with default // « [] » contains attrName: - else if (elementWithAttributes && - elementWithAttributes->mRemoveAttributes && - elementWithAttributes->mRemoveAttributes->Contains(attrName)) { + else if (elementAttributes && elementAttributes->mRemoveAttributes && + elementAttributes->mRemoveAttributes->Contains(attrName)) { // Step 9.2.1. Remove attribute. remove = true; } @@ -1695,8 +1723,8 @@ void Sanitizer::SanitizeAttributes(Element* aChild, MOZ_ASSERT(mDataAttributes.isSome(), "mDataAttributes exists iff mAttributes exists"); if (!mAttributes->Contains(attrName) && - !(elementWithAttributes && elementWithAttributes->mAttributes && - elementWithAttributes->mAttributes->Contains(attrName)) && + !(elementAttributes && elementAttributes->mAttributes && + elementAttributes->mAttributes->Contains(attrName)) && !(*mDataAttributes && IsDataAttribute(attrLocalName, attrNs))) { // Step 9.3.1.1. Remove attribute. remove = true; @@ -1707,8 +1735,8 @@ void Sanitizer::SanitizeAttributes(Element* aChild, else { // Step 9.4.1. If elementWithLocalAttributes["attributes"] exists and // elementWithLocalAttributes["attributes"] does not contain attrName: - if (elementWithAttributes && elementWithAttributes->mAttributes && - !elementWithAttributes->mAttributes->Contains(attrName)) { + if (elementAttributes && elementAttributes->mAttributes && + !elementAttributes->mAttributes->Contains(attrName)) { // Step 9.4.1.1. Remove attribute. remove = true; } diff --git a/dom/security/sanitizer/Sanitizer.h b/dom/security/sanitizer/Sanitizer.h @@ -129,14 +129,12 @@ class Sanitizer final : public nsISupports, public nsWrapperCache { RefPtr<nsIGlobalObject> mGlobal; - Maybe<sanitizer::ListSet<sanitizer::CanonicalElementWithAttributes>> - mElements; - Maybe<sanitizer::ListSet<sanitizer::CanonicalName>> mRemoveElements; - Maybe<sanitizer::ListSet<sanitizer::CanonicalName>> - mReplaceWithChildrenElements; - - Maybe<sanitizer::ListSet<sanitizer::CanonicalName>> mAttributes; - Maybe<sanitizer::ListSet<sanitizer::CanonicalName>> mRemoveAttributes; + Maybe<sanitizer::CanonicalElementMap> mElements; + Maybe<sanitizer::CanonicalNameSet> mRemoveElements; + Maybe<sanitizer::CanonicalNameSet> mReplaceWithChildrenElements; + + Maybe<sanitizer::CanonicalNameSet> mAttributes; + Maybe<sanitizer::CanonicalNameSet> mRemoveAttributes; bool mComments = false; // mDataAttributes always exists when mAttributes exists after diff --git a/dom/security/sanitizer/SanitizerTypes.cpp b/dom/security/sanitizer/SanitizerTypes.cpp @@ -11,113 +11,84 @@ bool CanonicalName::IsDataAttribute() const { !mNamespace; } -SanitizerAttributeNamespace CanonicalName::ToSanitizerAttributeNamespace() - const { - SanitizerAttributeNamespace result; - mLocalName->ToString(result.mName); +template <typename SanitizerName> +void CanonicalName::SetSanitizerName(SanitizerName& aSanitizerName) const { + mLocalName->ToString(aSanitizerName.mName); if (mNamespace) { - mNamespace->ToString(result.mNamespace); + mNamespace->ToString(aSanitizerName.mNamespace); } else { - result.mNamespace.SetIsVoid(true); + aSanitizerName.mNamespace.SetIsVoid(true); } - return result; } -SanitizerElementNamespaceWithAttributes -CanonicalElementWithAttributes::ToSanitizerElementNamespaceWithAttributes() +SanitizerAttributeNamespace CanonicalName::ToSanitizerAttributeNamespace() const { - SanitizerElementNamespaceWithAttributes result; - mLocalName->ToString(result.mName); - if (mNamespace) { - mNamespace->ToString(result.mNamespace); - } else { - result.mNamespace.SetIsVoid(true); - } - if (mAttributes) { - result.mAttributes.Construct(ToSanitizerAttributes(*mAttributes)); - } - if (mRemoveAttributes) { - result.mRemoveAttributes.Construct( - ToSanitizerAttributes(*mRemoveAttributes)); - } + SanitizerAttributeNamespace result; + SetSanitizerName(result); return result; } SanitizerElementNamespace CanonicalName::ToSanitizerElementNamespace() const { SanitizerElementNamespace result; - mLocalName->ToString(result.mName); - if (mNamespace) { - mNamespace->ToString(result.mNamespace); - } else { - result.mNamespace.SetIsVoid(true); - } + SetSanitizerName(result); return result; } -// TODO(bug 1989215): This is obviously quadratic. Fix this! -template <typename ValueType> -bool ListSet<ValueType>::HasDuplicates() const { - for (size_t i = 0; i + 1 < mValues.Length(); i++) { - if (mValues.IndexOf(mValues[i], i + 1) != mValues.NoIndex) { - return true; - } +SanitizerElementNamespaceWithAttributes +CanonicalName::ToSanitizerElementNamespaceWithAttributes( + const CanonicalElementAttributes& aElementAttributes) const { + SanitizerElementNamespaceWithAttributes result; + SetSanitizerName(result); + if (aElementAttributes.mAttributes) { + result.mAttributes.Construct( + ToSanitizerAttributes(*aElementAttributes.mAttributes)); } - return false; + if (aElementAttributes.mRemoveAttributes) { + result.mRemoveAttributes.Construct( + ToSanitizerAttributes(*aElementAttributes.mRemoveAttributes)); + } + return result; } -template class ListSet<CanonicalName>; -template class ListSet<CanonicalElementWithAttributes>; - -bool CanonicalElementWithAttributes::EqualAttributes( - const CanonicalElementWithAttributes& aOther) const { - MOZ_ASSERT(*this == aOther); - +bool CanonicalElementAttributes::Equals( + const CanonicalElementAttributes& aOther) const { if (mAttributes.isSome() != aOther.mAttributes.isSome() || mRemoveAttributes.isSome() != aOther.mRemoveAttributes.isSome()) { return false; } if (mAttributes) { - if (mAttributes->Values() != aOther.mAttributes->Values()) { + if (mAttributes->Count() != aOther.mAttributes->Count()) { return false; } - } - if (mRemoveAttributes) { - if (mRemoveAttributes->Values() != aOther.mRemoveAttributes->Values()) { - return false; + for (const CanonicalName& attr : *mAttributes) { + if (!aOther.mAttributes->Contains(attr)) { + return false; + } } } - return true; -} - -CanonicalElementWithAttributes CanonicalElementWithAttributes::Clone() const { - CanonicalElementWithAttributes elem(CanonicalName::Clone()); - - if (mAttributes) { - nsTArray<CanonicalName> attributes; - for (const auto& attr : mAttributes->Values()) { - attributes.AppendElement(attr.Clone()); + if (mRemoveAttributes) { + if (mRemoveAttributes->Count() != aOther.mRemoveAttributes->Count()) { + return false; } - elem.mAttributes = Some(ListSet(std::move(attributes))); - } - if (mRemoveAttributes) { - nsTArray<CanonicalName> attributes; - for (const auto& attr : mRemoveAttributes->Values()) { - attributes.AppendElement(attr.Clone()); + for (const CanonicalName& attr : *mRemoveAttributes) { + if (!aOther.mRemoveAttributes->Contains(attr)) { + return false; + } } - elem.mRemoveAttributes = Some(ListSet(std::move(attributes))); } - return elem; + return true; } nsTArray<OwningStringOrSanitizerAttributeNamespace> ToSanitizerAttributes( - const ListSet<CanonicalName>& aList) { + const CanonicalNameSet& aSet) { + // XXX Sorting nsTArray<OwningStringOrSanitizerAttributeNamespace> attributes; - for (const CanonicalName& canonical : aList.Values()) { + for (const CanonicalName& canonical : aSet) { attributes.AppendElement()->SetAsSanitizerAttributeNamespace() = canonical.ToSanitizerAttributeNamespace(); } diff --git a/dom/security/sanitizer/SanitizerTypes.h b/dom/security/sanitizer/SanitizerTypes.h @@ -7,12 +7,21 @@ #include "mozilla/Maybe.h" #include "mozilla/dom/SanitizerBinding.h" +#include "nsHashtablesFwd.h" +#include "nsTHashSet.h" namespace mozilla::dom::sanitizer { +struct CanonicalElementAttributes; + // The name of an element/attribute combined with its namespace. -class CanonicalName { +class CanonicalName : public PLDHashEntryHdr { public: + using KeyType = const CanonicalName&; + using KeyTypePointer = const CanonicalName*; + + explicit CanonicalName(KeyTypePointer aKey) + : mLocalName(aKey->mLocalName), mNamespace(aKey->mNamespace) {} CanonicalName(CanonicalName&&) = default; CanonicalName(RefPtr<nsAtom> aLocalName, RefPtr<nsAtom> aNamespace) : mLocalName(std::move(aLocalName)), mNamespace(std::move(aNamespace)) {} @@ -20,84 +29,53 @@ class CanonicalName { : mLocalName(aLocalName), mNamespace(aNamespace) {} ~CanonicalName() = default; + KeyType GetKey() const { return *this; } + bool KeyEquals(KeyTypePointer aKey) const { + return mLocalName == aKey->mLocalName && mNamespace == aKey->mNamespace; + } + + static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; } + static PLDHashNumber HashKey(KeyTypePointer aKey) { + return mozilla::HashGeneric(aKey->mLocalName.get(), aKey->mNamespace.get()); + } + + enum { ALLOW_MEMMOVE = true }; + // Caution: Only use this for attribute names, not elements! // Returns true for names that start with data-* and have a null namespace. bool IsDataAttribute() const; - bool operator==(const CanonicalName& aOther) const { - return mLocalName == aOther.mLocalName && mNamespace == aOther.mNamespace; - } - - SanitizerElementNamespace ToSanitizerElementNamespace() const; SanitizerAttributeNamespace ToSanitizerAttributeNamespace() const; + SanitizerElementNamespace ToSanitizerElementNamespace() const; + SanitizerElementNamespaceWithAttributes + ToSanitizerElementNamespaceWithAttributes( + const CanonicalElementAttributes& aElementAttributes) const; CanonicalName Clone() const { return CanonicalName(mLocalName, mNamespace); } protected: + template <typename SanitizerName> + void SetSanitizerName(SanitizerName& aName) const; + RefPtr<nsAtom> mLocalName; // A "null" namespace is represented by the nullptr. RefPtr<nsAtom> mNamespace; }; -// TODO: Replace this with some kind of optimized ordered set. -template <typename ValueType> -class ListSet { - public: - ListSet() = default; - - explicit ListSet(nsTArray<ValueType>&& aValues) - : mValues(std::move(aValues)) {} +using CanonicalNameSet = nsTHashSet<CanonicalName>; - void Insert(ValueType&& aValue) { - if (Contains(aValue)) { - return; - } +struct CanonicalElementAttributes { + Maybe<CanonicalNameSet> mAttributes; + Maybe<CanonicalNameSet> mRemoveAttributes; - mValues.AppendElement(std::move(aValue)); - } - bool Remove(const CanonicalName& aValue) { - return mValues.RemoveElement(aValue); - } - bool Contains(const CanonicalName& aValue) const { - return mValues.Contains(aValue); - } - bool IsEmpty() const { return mValues.IsEmpty(); } - - ValueType* Get(const CanonicalName& aValue) { - auto index = mValues.IndexOf(aValue); - if (index == mValues.NoIndex) { - return nullptr; - } - return &mValues[index]; - } - - const nsTArray<ValueType>& Values() const { return mValues; } - nsTArray<ValueType>& Values() { return mValues; } - - bool HasDuplicates() const; - - private: - nsTArray<ValueType> mValues; + bool Equals(const CanonicalElementAttributes& aOther) const; }; -class CanonicalElementWithAttributes : public CanonicalName { - public: - explicit CanonicalElementWithAttributes(CanonicalName&& aName) - : CanonicalName(std::move(aName)) {} - - bool EqualAttributes(const CanonicalElementWithAttributes& aOther) const; - - SanitizerElementNamespaceWithAttributes - ToSanitizerElementNamespaceWithAttributes() const; - - CanonicalElementWithAttributes Clone() const; - - Maybe<ListSet<CanonicalName>> mAttributes; - Maybe<ListSet<CanonicalName>> mRemoveAttributes; -}; +using CanonicalElementMap = + nsTHashMap<CanonicalName, CanonicalElementAttributes>; nsTArray<OwningStringOrSanitizerAttributeNamespace> ToSanitizerAttributes( - const ListSet<CanonicalName>& aList); + const CanonicalNameSet& aSet); } // namespace mozilla::dom::sanitizer