зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1731587 - [sanitizer] Remove string-based APIs, handle HTML comments, align exceptions with wpt r=hsivonen
This patch combines three relatively small changes: 1) Removing string-based APIs -- Adjusting the Sanitize() function, its WebIDl definition etc. to only accept Document/DocumentFragment 2) Parse allowComments from the Sanitizer constructor and pass into the underlying nsTreeSanitizer 3) Ensure that exceptions and error cases align with current wpt Differential Revision: https://phabricator.services.mozilla.com/D126673
This commit is contained in:
Frederik Braun
Родитель
fc8b433cc4
Коммит
cd84e68a73
|
|
@ -4525,6 +4525,20 @@ void Element::RegUnRegAccessKey(bool aDoReg) {
|
|||
void Element::SetHTML(const nsAString& aInnerHTML,
|
||||
const SetHTMLOptions& aOptions, ErrorResult& aError) {
|
||||
FragmentOrElement* target = this;
|
||||
// Throw for disallowed elements
|
||||
if (IsHTMLElement(nsGkAtoms::script)) {
|
||||
aError.ThrowTypeError("This does not work on <script> elements");
|
||||
return;
|
||||
}
|
||||
if (IsHTMLElement(nsGkAtoms::object)) {
|
||||
aError.ThrowTypeError("This does not work on <object> elements");
|
||||
return;
|
||||
}
|
||||
if (IsHTMLElement(nsGkAtoms::iframe)) {
|
||||
aError.ThrowTypeError("This does not work on <iframe> elements");
|
||||
return;
|
||||
}
|
||||
|
||||
// Handle template case.
|
||||
if (target->IsTemplateElement()) {
|
||||
DocumentFragment* frag =
|
||||
|
|
|
|||
|
|
@ -1608,6 +1608,9 @@ void nsTreeSanitizer::WithWebSanitizerOptions(
|
|||
if (!aOptions.IsAnyMemberPresent()) {
|
||||
return;
|
||||
}
|
||||
if (aOptions.mAllowComments.WasPassed()) {
|
||||
mAllowComments = aOptions.mAllowComments.Value();
|
||||
}
|
||||
if (aOptions.mAllowElements.WasPassed()) {
|
||||
mIsCustomized = true;
|
||||
const Sequence<nsString>& allowedElements = aOptions.mAllowElements.Value();
|
||||
|
|
|
|||
|
|
@ -45,9 +45,8 @@ already_AddRefed<Sanitizer> Sanitizer::Constructor(
|
|||
|
||||
/* static */
|
||||
already_AddRefed<DocumentFragment> Sanitizer::InputToNewFragment(
|
||||
const mozilla::dom::StringOrDocumentFragmentOrDocument& aInput,
|
||||
ErrorResult& aRv) {
|
||||
// turns an StringOrDocumentFragmentOrDocument into a DocumentFragment for
|
||||
const mozilla::dom::DocumentFragmentOrDocument& aInput, ErrorResult& aRv) {
|
||||
// turns an DocumentFragmentOrDocument into a new DocumentFragment for
|
||||
// internal use with nsTreeSanitizer
|
||||
|
||||
nsCOMPtr<nsPIDOMWindowInner> window = do_QueryInterface(mGlobal);
|
||||
|
|
@ -63,8 +62,6 @@ already_AddRefed<DocumentFragment> Sanitizer::InputToNewFragment(
|
|||
if (aInput.IsDocumentFragment()) {
|
||||
RefPtr<DocumentFragment> inFragment = &aInput.GetAsDocumentFragment();
|
||||
inFragment->GetInnerHTML(innerHTML);
|
||||
} else if (aInput.IsString()) {
|
||||
innerHTML.Assign(aInput.GetAsString());
|
||||
} else if (aInput.IsDocument()) {
|
||||
RefPtr<Document> doc = &aInput.GetAsDocument();
|
||||
nsCOMPtr<Element> docElement = doc->GetDocumentElement();
|
||||
|
|
@ -106,8 +103,7 @@ already_AddRefed<DocumentFragment> Sanitizer::InputToNewFragment(
|
|||
}
|
||||
|
||||
already_AddRefed<DocumentFragment> Sanitizer::Sanitize(
|
||||
const mozilla::dom::StringOrDocumentFragmentOrDocument& aInput,
|
||||
ErrorResult& aRv) {
|
||||
const mozilla::dom::DocumentFragmentOrDocument& aInput, ErrorResult& aRv) {
|
||||
nsCOMPtr<nsPIDOMWindowInner> window = do_QueryInterface(mGlobal);
|
||||
if (!window || !window->GetDoc()) {
|
||||
aRv.Throw(NS_ERROR_FAILURE);
|
||||
|
|
@ -143,6 +139,21 @@ already_AddRefed<Element> Sanitizer::SanitizeFor(const nsAString& aElement,
|
|||
if (aRv.Failed()) {
|
||||
return nullptr;
|
||||
}
|
||||
RefPtr<nsAtom> elemName = NS_Atomize(aElement);
|
||||
// FIXME(freddyb): Invalid parameters for SanitizeFor should throw, just like
|
||||
// Element.setHTML does. Cf. https://github.com/WICG/sanitizer-api/issues/125
|
||||
if (elemName == nsGkAtoms::script) {
|
||||
// aRv.ThrowTypeError("This does not work on <script> elements");
|
||||
return nullptr;
|
||||
}
|
||||
if (elemName == nsGkAtoms::object) {
|
||||
// aRv.ThrowTypeError("This does not work on <object> elements");
|
||||
return nullptr;
|
||||
}
|
||||
if (elemName == nsGkAtoms::iframe) {
|
||||
// aRv.ThrowTypeError("This does not work on <iframe> elements");
|
||||
return nullptr;
|
||||
}
|
||||
RefPtr<Document> inertDoc = nsContentUtils::CreateInertHTMLDocument(nullptr);
|
||||
if (!inertDoc) {
|
||||
aRv.Throw(NS_ERROR_FAILURE);
|
||||
|
|
@ -151,7 +162,6 @@ already_AddRefed<Element> Sanitizer::SanitizeFor(const nsAString& aElement,
|
|||
RefPtr<DocumentFragment> fragment = new (inertDoc->NodeInfoManager())
|
||||
DocumentFragment(inertDoc->NodeInfoManager());
|
||||
|
||||
RefPtr<nsAtom> elemName = NS_Atomize(aElement);
|
||||
aRv = nsContentUtils::ParseFragmentHTML(aInput, fragment, elemName,
|
||||
kNameSpaceID_XHTML, false, true);
|
||||
|
||||
|
|
|
|||
|
|
@ -35,9 +35,7 @@ class Sanitizer final : public nsISupports, public nsWrapperCache {
|
|||
NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(Sanitizer);
|
||||
|
||||
explicit Sanitizer(nsIGlobalObject* aGlobal, const SanitizerConfig& aOptions)
|
||||
: mGlobal(aGlobal),
|
||||
mTreeSanitizer(nsIParserUtils::SanitizerAllowStyle |
|
||||
nsIParserUtils::SanitizerAllowComments) {
|
||||
: mGlobal(aGlobal), mTreeSanitizer(nsIParserUtils::SanitizerAllowStyle) {
|
||||
MOZ_ASSERT(aGlobal);
|
||||
mTreeSanitizer.WithWebSanitizerOptions(aOptions);
|
||||
}
|
||||
|
|
@ -61,8 +59,7 @@ class Sanitizer final : public nsISupports, public nsWrapperCache {
|
|||
* @return DocumentFragment of the sanitized HTML
|
||||
*/
|
||||
already_AddRefed<DocumentFragment> Sanitize(
|
||||
const mozilla::dom::StringOrDocumentFragmentOrDocument& aInput,
|
||||
ErrorResult& aRv);
|
||||
const mozilla::dom::DocumentFragmentOrDocument& aInput, ErrorResult& aRv);
|
||||
|
||||
/**
|
||||
* sanitizeFor method.
|
||||
|
|
@ -97,8 +94,7 @@ class Sanitizer final : public nsISupports, public nsWrapperCache {
|
|||
private:
|
||||
~Sanitizer() = default;
|
||||
already_AddRefed<DocumentFragment> InputToNewFragment(
|
||||
const mozilla::dom::StringOrDocumentFragmentOrDocument& aInput,
|
||||
ErrorResult& aRv);
|
||||
const mozilla::dom::DocumentFragmentOrDocument& aInput, ErrorResult& aRv);
|
||||
/**
|
||||
* Logs localized message to either content console or browser console
|
||||
* @param aMessage Message to log
|
||||
|
|
|
|||
|
|
@ -108,33 +108,36 @@ SimpleTest.waitForExplicitFinish();
|
|||
for (let testInputAndType of possibleInputTypes(testString)) {
|
||||
const {testInput, testType} = testInputAndType;
|
||||
|
||||
// test sanitize(documentfragment)
|
||||
try {
|
||||
div.innerHTML = "";
|
||||
const docFragment = testSanitizer.sanitize(testInput);
|
||||
div.append(docFragment);
|
||||
is(div.innerHTML, testExpected, `Sanitizer.sanitize() should turn (${testType}) '${testInput}' into '${testExpected}'`);
|
||||
}
|
||||
catch (e) {
|
||||
ok(false, 'Error in sanitize() test: ' + e)
|
||||
if (testType != "String") {
|
||||
// test sanitize(document/fragment)
|
||||
try {
|
||||
div.innerHTML = "";
|
||||
const docFragment = testSanitizer.sanitize(testInput);
|
||||
div.append(docFragment);
|
||||
is(div.innerHTML, testExpected, `Sanitizer.sanitize() should turn (${testType}) '${testInput}' into '${testExpected}'`);
|
||||
}
|
||||
catch (e) {
|
||||
ok(false, 'Error in sanitize() test: ' + e)
|
||||
}
|
||||
}
|
||||
else {
|
||||
// test setHTML:
|
||||
try {
|
||||
div.setHTML(testString, { sanitizer: testSanitizer });
|
||||
is(div.innerHTML, testExpected, `div.setHTML() should turn(${testType}) '${testInput}' into '${testExpected}'`);
|
||||
}
|
||||
catch (e) {
|
||||
ok(false, 'Error in setHTML() test: ' + e)
|
||||
}
|
||||
|
||||
// test setHTML:
|
||||
try {
|
||||
div.setHTML(testString, {sanitizer: testSanitizer });
|
||||
is(div.innerHTML, testExpected, `div.setHTML() should turn(${testType}) '${testInput}' into '${testExpected}'`);
|
||||
}
|
||||
catch (e) {
|
||||
ok(false, 'Error in setHTML() test: ' + e)
|
||||
}
|
||||
|
||||
// test sanitizeFor:
|
||||
try {
|
||||
const newDiv = testSanitizer.sanitizeFor('div', testString);
|
||||
is(newDiv.innerHTML, testExpected, `Sanitizer.sanitizeFor('div', input) should turn(${testType}) '${testInput}' into '${testExpected}'`);
|
||||
}
|
||||
catch (e) {
|
||||
ok(false, 'Error in sanitizeFor() test: ' + e)
|
||||
// test sanitizeFor:
|
||||
try {
|
||||
const newDiv = testSanitizer.sanitizeFor('div', testString);
|
||||
is(newDiv.innerHTML, testExpected, `Sanitizer.sanitizeFor('div', input) should turn(${testType}) '${testInput}' into '${testExpected}'`);
|
||||
}
|
||||
catch (e) {
|
||||
ok(false, 'Error in sanitizeFor() test: ' + e)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@
|
|||
*/
|
||||
|
||||
|
||||
typedef (DOMString or DocumentFragment or Document) SanitizerInput;
|
||||
typedef (DocumentFragment or Document) SanitizerInput;
|
||||
typedef record<DOMString, sequence<DOMString>> AttributeMatchList;
|
||||
|
||||
[Exposed=Window, SecureContext, Pref="dom.security.sanitizer.enabled"]
|
||||
|
|
@ -31,4 +31,5 @@ dictionary SanitizerConfig {
|
|||
AttributeMatchList allowAttributes;
|
||||
AttributeMatchList dropAttributes;
|
||||
boolean allowCustomElements;
|
||||
boolean allowComments;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1,12 +1,3 @@
|
|||
[element-set-sanitized-html.https.tentative.html]
|
||||
[Sanitizer: Element.setHTML throws with insufficient arguments.]
|
||||
expected: FAIL
|
||||
|
||||
[script.setHTML should fail.]
|
||||
expected: FAIL
|
||||
|
||||
[iframe.setHTML should fail.]
|
||||
expected: FAIL
|
||||
|
||||
[object.setHTML should fail.]
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -96,47 +96,14 @@
|
|||
[SanitizerAPI with config: empty dropAttributes list with id attribute, sanitize from document fragment function for empty dropAttributes list with id attribute]
|
||||
expected: FAIL
|
||||
|
||||
[Sanitizer.sanitize(null).]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTMLInputElement, sanitize from document function for HTMLInputElement]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTMLButtonElement, sanitize from document function for HTMLButtonElement]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: malformed HTML, sanitize from document function for malformed HTML]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTML with comments; comments not allowed, sanitize from document function for HTML with comments; comments not allowed]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTML with comments; !allowComments, sanitize from document function for HTML with comments; !allowComments]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTML with comments deeper in the tree, sanitize from document function for HTML with comments deeper in the tree]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTML with comments deeper in the tree, !allowComments, sanitize from document function for HTML with comments deeper in the tree, !allowComments]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTMLInputElement, sanitize from document fragment function for HTMLInputElement]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTMLButtonElement, sanitize from document fragment function for HTMLButtonElement]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: malformed HTML, sanitize from document fragment function for malformed HTML]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTML with comments; comments not allowed, sanitize from document fragment function for HTML with comments; comments not allowed]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTML with comments; !allowComments, sanitize from document fragment function for HTML with comments; !allowComments]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTML with comments deeper in the tree, sanitize from document fragment function for HTML with comments deeper in the tree]
|
||||
expected: FAIL
|
||||
|
||||
[SanitizerAPI with config: HTML with comments deeper in the tree, !allowComments, sanitize from document fragment function for HTML with comments deeper in the tree, !allowComments]
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,13 +1,4 @@
|
|||
[sanitizer-sanitizeFor.https.tentative.html]
|
||||
[Sanitizer.sanitizeFor("script", ...) should fail.]
|
||||
expected: FAIL
|
||||
|
||||
[Sanitizer.sanitizeFor("iframe", ...) should fail.]
|
||||
expected: FAIL
|
||||
|
||||
[Sanitizer.sanitizeFor("object", ...) should fail.]
|
||||
expected: FAIL
|
||||
|
||||
[Sanitizer.sanitizeFor(element, ..)]
|
||||
expected: FAIL
|
||||
|
||||
|
|
|
|||
|
|
@ -18,13 +18,6 @@
|
|||
`Node[${node1.innerHTML}] == Node[${node2.innerHTML}]`);
|
||||
}
|
||||
|
||||
test(t => {
|
||||
const div = document.createElement("div");
|
||||
assert_throws_js(TypeError, _ => div.setHTML());
|
||||
assert_throws_js(TypeError, _ => div.setHTML("abc"));
|
||||
assert_throws_js(TypeError, _ => div.setHTML("abc", null));
|
||||
}, "Sanitizer: Element.setHTML throws with insufficient arguments.");
|
||||
|
||||
for (const context of ["script", "iframe", "object", "div"]) {
|
||||
const should_fail = context != "div";
|
||||
test(t => {
|
||||
|
|
@ -55,7 +48,7 @@
|
|||
const element = document.createElement("template");
|
||||
test(t => {
|
||||
let s = new Sanitizer(testcase.config_input);
|
||||
element.setHTML(testcase.value, s);
|
||||
element.setHTML(testcase.value, {sanitizer: s });
|
||||
assert_node_equals(buildNode(element.localName, testcase.result), element);
|
||||
}, "Sanitizer: Element.setHTML with config: " + testcase.message);
|
||||
}
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче