commit da494342cf7c537190eeea74b7a82ec37de89468
parent 5e530561254ce9091d389bbd263590de93a2e82d
Author: Tom Schuster <tschuster@mozilla.com>
Date: Mon, 20 Oct 2025 08:18:15 +0000
Bug 1991682 - Sanitizer: Improve the error messages. r=smaug
Differential Revision: https://phabricator.services.mozilla.com/D267560
Diffstat:
3 files changed, 98 insertions(+), 43 deletions(-)
diff --git a/dom/security/sanitizer/Sanitizer.cpp b/dom/security/sanitizer/Sanitizer.cpp
@@ -16,6 +16,7 @@
#include "mozilla/dom/SanitizerBinding.h"
#include "mozilla/dom/SanitizerDefaultConfig.h"
#include "nsContentUtils.h"
+#include "nsFmtString.h"
#include "nsGenericHTMLElement.h"
#include "nsIContentInlines.h"
#include "nsNameSpaceManager.h"
@@ -289,10 +290,12 @@ static CanonicalElementAttributes CanonicalizeElementAttributes(
for (const auto& attribute : elem.mAttributes.Value()) {
// Step 2.1.2.1. Append the result of canonicalize a sanitizer attribute
// with attribute to attributes.
- if (!attributes.EnsureInserted(CanonicalizeAttribute(attribute))) {
+ CanonicalName canonicalAttr = CanonicalizeAttribute(attribute);
+ if (!attributes.EnsureInserted(canonicalAttr)) {
if (aErrorMsg) {
- aErrorMsg->AssignLiteral(
- "Duplicate attribute in local 'attributes'");
+ aErrorMsg->Assign(nsFmtCString(
+ FMT_STRING("Duplicate attribute {} in 'attributes' of {}."),
+ canonicalAttr, CanonicalizeElement(aElement)));
return CanonicalElementAttributes();
}
}
@@ -311,10 +314,13 @@ static CanonicalElementAttributes CanonicalizeElementAttributes(
for (const auto& attribute : elem.mRemoveAttributes.Value()) {
// Step 2.2.2.1. Append the result of canonicalize a sanitizer attribute
// with attribute to attributes.
- if (!attributes.EnsureInserted(CanonicalizeAttribute(attribute))) {
+ CanonicalName canonicalAttr = CanonicalizeAttribute(attribute);
+ if (!attributes.EnsureInserted(canonicalAttr)) {
if (aErrorMsg) {
- aErrorMsg->AssignLiteral(
- "Duplicate attribute in local 'removeAttributes'");
+ aErrorMsg->Assign(nsFmtCString(
+ FMT_STRING(
+ "Duplicate attribute {} in 'removeAttributes' of {}."),
+ canonicalAttr, CanonicalizeElement(aElement)));
return CanonicalElementAttributes();
}
}
@@ -369,16 +375,18 @@ void Sanitizer::CanonicalizeConfiguration(const SanitizerConfig& aConfig,
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.
-
CanonicalName elementName = CanonicalizeElement(element);
if (elements.Contains(elementName)) {
- return aRv.ThrowTypeError("Duplicate element in 'elements'");
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING("Duplicate element {} in 'elements'."), elementName));
+ return;
}
CanonicalElementAttributes elementAttributes =
CanonicalizeElementAttributes(element, &errorMsg);
if (!errorMsg.IsEmpty()) {
- return aRv.ThrowTypeError(errorMsg);
+ aRv.ThrowTypeError(errorMsg);
+ return;
}
elements.InsertOrUpdate(elementName, std::move(elementAttributes));
@@ -397,8 +405,11 @@ void Sanitizer::CanonicalizeConfiguration(const SanitizerConfig& aConfig,
for (const auto& element : aConfig.mRemoveElements.Value()) {
// Step 4.2.1. Append the result of canonicalize a sanitizer element
// element to elements.
- if (!elements.EnsureInserted(CanonicalizeElement(element))) {
- aRv.ThrowTypeError("Duplicate element in 'removeElements'");
+ CanonicalName canonical = CanonicalizeElement(element);
+ if (!elements.EnsureInserted(canonical)) {
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING("Duplicate element {} in 'removeElements'."),
+ canonical));
return;
}
}
@@ -417,9 +428,12 @@ void Sanitizer::CanonicalizeConfiguration(const SanitizerConfig& aConfig,
for (const auto& element : aConfig.mReplaceWithChildrenElements.Value()) {
// Step 5.2.1. Append the result of canonicalize a sanitizer element
// element to elements.
- if (!elements.EnsureInserted(CanonicalizeElement(element))) {
- aRv.ThrowTypeError(
- "Duplicate element in 'replaceWithChildrenElements'");
+ CanonicalName canonical = CanonicalizeElement(element);
+ if (!elements.EnsureInserted(canonical)) {
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING(
+ "Duplicate element {} in 'replaceWithChildrenElements'."),
+ canonical));
return;
}
}
@@ -437,8 +451,10 @@ void Sanitizer::CanonicalizeConfiguration(const SanitizerConfig& aConfig,
for (const auto& attribute : aConfig.mAttributes.Value()) {
// Step 6.2.1. Append the result of canonicalize a sanitizer attribute
// attribute to attributes.
- if (!attributes.EnsureInserted(CanonicalizeAttribute(attribute))) {
- aRv.ThrowTypeError("Duplicate attribute in 'attributes'");
+ CanonicalName canonical = CanonicalizeAttribute(attribute);
+ if (!attributes.EnsureInserted(canonical)) {
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING("Duplicate attribute {} in 'attributes'."), canonical));
return;
}
}
@@ -456,8 +472,11 @@ void Sanitizer::CanonicalizeConfiguration(const SanitizerConfig& aConfig,
for (const auto& attribute : aConfig.mRemoveAttributes.Value()) {
// Step 7.2.2. Append the result of canonicalize a sanitizer attribute
// attribute to attributes.
- if (!attributes.EnsureInserted(CanonicalizeAttribute(attribute))) {
- aRv.ThrowTypeError("Duplicate attribute in 'removeAttributes'");
+ CanonicalName canonical = CanonicalizeAttribute(attribute);
+ if (!attributes.EnsureInserted(canonical)) {
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING("Duplicate attribute {} in 'removeAttributes'."),
+ canonical));
return;
}
}
@@ -494,7 +513,7 @@ void Sanitizer::IsValid(ErrorResult& aRv) {
"Must have either due to CanonicalizeConfiguration");
if (mElements && mRemoveElements) {
aRv.ThrowTypeError(
- "'elements' and 'removeElements' are not allowed at the same time");
+ "'elements' and 'removeElements' are not allowed at the same time.");
return;
}
@@ -505,7 +524,7 @@ void Sanitizer::IsValid(ErrorResult& aRv) {
if (mAttributes && mRemoveAttributes) {
aRv.ThrowTypeError(
"'attributes' and 'removeAttributes' are not allowed at the same "
- "time");
+ "time.");
return;
}
@@ -525,8 +544,9 @@ void Sanitizer::IsValid(ErrorResult& aRv) {
for (const CanonicalName& name : mElements->Keys()) {
if (mReplaceWithChildrenElements->Contains(name)) {
aRv.ThrowTypeError(
- "Element can't be in both 'elements' and "
- "'replaceWithChildrenElements'");
+ nsFmtCString(FMT_STRING("Element {} can't be in both 'elements' "
+ "and 'replaceWithChildrenElements'."),
+ name));
return;
}
}
@@ -538,9 +558,10 @@ void Sanitizer::IsValid(ErrorResult& aRv) {
if (mRemoveElements && mReplaceWithChildrenElements) {
for (const CanonicalName& name : *mRemoveElements) {
if (mReplaceWithChildrenElements->Contains(name)) {
- aRv.ThrowTypeError(
- "Element can't be in both 'removeElements' and "
- "'replaceWithChildrenElements'");
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING("Element {} can't be in both 'removeElements' and "
+ "'replaceWithChildrenElements'."),
+ name));
return;
}
}
@@ -562,8 +583,11 @@ void Sanitizer::IsValid(ErrorResult& aRv) {
if (elemAttributes.mAttributes) {
for (const CanonicalName& name : *elemAttributes.mAttributes) {
if (mAttributes->Contains(name)) {
- aRv.ThrowTypeError(
- "Same attribute both in local and global 'attributes'");
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING(
+ "Attribute {} can't be part of both the 'attributes' of "
+ "the element {} and the global 'attributes'."),
+ name, entry.GetKey()));
return;
}
}
@@ -574,9 +598,11 @@ void Sanitizer::IsValid(ErrorResult& aRv) {
if (elemAttributes.mRemoveAttributes) {
for (const CanonicalName& name : *elemAttributes.mRemoveAttributes) {
if (!mAttributes->Contains(name)) {
- aRv.ThrowTypeError(
- "Attribute in local 'removeAttributes' but not in global "
- "'attributes'");
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING(
+ "Attribute {} can't be in 'removeAttributes' of the "
+ "element {} but not in the global 'attributes'."),
+ name, entry.GetKey()));
return;
}
}
@@ -592,9 +618,11 @@ void Sanitizer::IsValid(ErrorResult& aRv) {
// data attribute.
for (const CanonicalName& name : *elemAttributes.mAttributes) {
if (name.IsDataAttribute()) {
- aRv.ThrowTypeError(
- "Local 'attributes' contains a data attribute, which is "
- "redundant with 'dataAttributes' being true");
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING(
+ "Data attribute {} in the 'attributes' of the element {} "
+ "is redundant with 'dataAttributes' being true."),
+ name, entry.GetKey()));
return;
}
}
@@ -611,9 +639,10 @@ void Sanitizer::IsValid(ErrorResult& aRv) {
// attribute.
for (const CanonicalName& name : *mAttributes) {
if (name.IsDataAttribute()) {
- aRv.ThrowTypeError(
- "Global 'attributes' contains a data attribute, which is "
- "redundant with 'dataAttributes' being true");
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING("Data attribute {} in the global 'attributes' is "
+ "redundant with 'dataAttributes' being true."),
+ name));
return;
}
}
@@ -633,9 +662,11 @@ void Sanitizer::IsValid(ErrorResult& aRv) {
if (elemAttributes.mAttributes) {
for (const CanonicalName& name : *elemAttributes.mAttributes) {
if (mRemoveAttributes->Contains(name)) {
- aRv.ThrowTypeError(
- "Same attribute both in local 'attributes' and global "
- "'removeAttributes'.");
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING(
+ "Attribute {} can't be in 'attributes' of the element {} "
+ "while in the global 'removeAttributes'."),
+ name, entry.GetKey()));
return;
}
}
@@ -646,9 +677,11 @@ void Sanitizer::IsValid(ErrorResult& aRv) {
if (elemAttributes.mRemoveAttributes) {
for (const CanonicalName& name : *elemAttributes.mRemoveAttributes) {
if (mRemoveAttributes->Contains(name)) {
- aRv.ThrowTypeError(
- "Same attribute both in local and global "
- "'removeAttributes'.");
+ aRv.ThrowTypeError(nsFmtCString(
+ FMT_STRING("Attribute {} can't be part of both the "
+ "'removeAttributes' of the element {} and the "
+ "global 'removeAttributes'."),
+ name, entry.GetKey()));
return;
}
}
@@ -660,7 +693,7 @@ void Sanitizer::IsValid(ErrorResult& aRv) {
if (mDataAttributes) {
aRv.ThrowTypeError(
"'removeAttributes' and 'dataAttributes' aren't allowed at the "
- "same time");
+ "same time.");
}
}
}
diff --git a/dom/security/sanitizer/SanitizerTypes.cpp b/dom/security/sanitizer/SanitizerTypes.cpp
@@ -50,6 +50,18 @@ CanonicalName::ToSanitizerElementNamespaceWithAttributes(
return result;
}
+std::ostream& operator<<(std::ostream& aStream, const CanonicalName& aName) {
+ nsAutoCString localName;
+ aName.mLocalName->ToUTF8String(localName);
+ aStream << '"' << localName << '"';
+ if (aName.mNamespace) {
+ nsAutoCString nameSpace;
+ aName.mNamespace->ToUTF8String(nameSpace);
+ return aStream << " (namespace: \"" << nameSpace << "\")";
+ }
+ return aStream << " (namespace: null)";
+}
+
bool CanonicalElementAttributes::Equals(
const CanonicalElementAttributes& aOther) const {
if (mAttributes.isSome() != aOther.mAttributes.isSome() ||
diff --git a/dom/security/sanitizer/SanitizerTypes.h b/dom/security/sanitizer/SanitizerTypes.h
@@ -5,6 +5,7 @@
#ifndef mozilla_dom_SanitizerTypes_h
#define mozilla_dom_SanitizerTypes_h
+#include "fmt/format.h"
#include "mozilla/Maybe.h"
#include "mozilla/dom/SanitizerBinding.h"
#include "nsHashtablesFwd.h"
@@ -54,6 +55,9 @@ class CanonicalName : public PLDHashEntryHdr {
CanonicalName Clone() const { return CanonicalName(mLocalName, mNamespace); }
protected:
+ friend std::ostream& operator<<(std::ostream& aStream,
+ const CanonicalName& aName);
+
template <typename SanitizerName>
void SetSanitizerName(SanitizerName& aName) const;
@@ -62,6 +66,8 @@ class CanonicalName : public PLDHashEntryHdr {
RefPtr<nsAtom> mNamespace;
};
+std::ostream& operator<<(std::ostream& aStream, const CanonicalName& aName);
+
using CanonicalNameSet = nsTHashSet<CanonicalName>;
struct CanonicalElementAttributes {
@@ -154,4 +160,8 @@ class MOZ_STACK_CLASS SanitizerComparator final {
} // namespace mozilla::dom::sanitizer
+template <>
+struct fmt::formatter<mozilla::dom::sanitizer::CanonicalName>
+ : ostream_formatter {};
+
#endif
