Skip to content

Commit

Permalink
Remove trusted types enforcement from DOM node APIs
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=276881

Reviewed by NOBODY (OOPS!).

The enforcement of Trusted Types in various node manipulation APIs in DOM
was based on a previous model for protecting script elements.

This patch removes it as its not needed.

See whatwg/dom#1299 and w3c/trusted-types#537

* LayoutTests/imported/w3c/web-platform-tests/trusted-types/Node-multiple-arguments-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/Node-multiple-arguments-tt-enforced-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/Node-multiple-arguments-tt-enforced.html: Renamed from LayoutTests/imported/w3c/web-platform-tests/trusted-types/block-Node-multiple-arguments.html.
* Source/WebCore/dom/ChildNode.idl:
* Source/WebCore/dom/ContainerNode.cpp:
(WebCore::ContainerNode::append):
(WebCore::ContainerNode::prepend):
(WebCore::ContainerNode::replaceChildren):
* Source/WebCore/dom/ContainerNode.h:
* Source/WebCore/dom/Node.cpp:
(WebCore::nodeSetPreTransformedFromNodeOrStringVector):
(WebCore::Node::convertNodesOrStringsIntoNode):
(WebCore::Node::convertNodesOrStringsIntoNodeVector):
(WebCore::Node::before):
(WebCore::Node::after):
(WebCore::Node::replaceWith):
(WebCore::nodeSetPreTransformedFromNodeOrStringOrTrustedScriptVector): Deleted.
(WebCore::Node::convertNodesOrStringsOrTrustedScriptsIntoNode): Deleted.
(WebCore::Node::convertNodesOrStringsOrTrustedScriptsIntoNodeVector): Deleted.
* Source/WebCore/dom/Node.h:
* Source/WebCore/dom/ParentNode.idl:
* Source/WebCore/dom/TrustedType.cpp:
(WebCore::processNodeOrStringAsTrustedType): Deleted.
* Source/WebCore/dom/TrustedType.h:
  • Loading branch information
lukewarlow committed Jul 22, 2024
1 parent 4aa16f2 commit 4aae90b
Show file tree
Hide file tree
Showing 11 changed files with 256 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
'mixed';
'mixed';
'mixed';
'mixed';
'mixed';'script';
'mixed';'script';
'mixed';'script';
Expand All @@ -31,6 +32,7 @@
'script';
'script';
'script';
'script';

PASS replaceWith('createScript';) on <div> should pass
PASS after('createScript';) on <div> should pass
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,21 @@
const pass_args = [
[ policy.createScript("'createScript';") ],
[ policy.createScript("'createScript #1';"), policy.createScript("'#2;'") ],
];
const fail_args = [
[ "'plain text';" ],
[ "'plain text #1';", "'plain text #2';" ],
[ document.createTextNode("'node';") ],
[ document.createTextNode("'node #1';"),
document.createTextNode("'node #2';") ],
document.createTextNode("'node #2';") ],
[ "'mixed';", document.createTextNode("'node';") ],
[ "'mixed';", policy.createScript("'script';") ],
[ document.createTextNode("'node';"),
policy.createScript("'script';") ],
policy.createScript("'script';") ],
];
const all_args = [].concat(pass_args).concat(fail_args);
const all_args = [].concat(pass_args);

for (const target of targets) {
for (const args of all_args) {
var should_fail = target == "script" && fail_args.indexOf(args) != -1;
var should_fail = false;
var fail_string = should_fail ? "fail" : "pass";

for (const setter of [container.replaceWith, container.after, container.before]) {
Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/dom/ChildNode.idl
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@

// https://dom.spec.whatwg.org/#childnode
interface mixin ChildNode {
[CEReactions=Needed, Unscopable] undefined before((Node or DOMString or TrustedScript)... nodes);
[CEReactions=Needed, Unscopable] undefined after((Node or DOMString or TrustedScript)... nodes);
[CEReactions=Needed, Unscopable] undefined replaceWith((Node or DOMString or TrustedScript)... nodes);
[CEReactions=Needed, Unscopable] undefined before((Node or DOMString)... nodes);
[CEReactions=Needed, Unscopable] undefined after((Node or DOMString)... nodes);
[CEReactions=Needed, Unscopable] undefined replaceWith((Node or DOMString)... nodes);
[CEReactions=Needed, Unscopable] undefined remove();
};
12 changes: 6 additions & 6 deletions Source/WebCore/dom/ContainerNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1164,9 +1164,9 @@ unsigned ContainerNode::childElementCount() const
return std::distance(children.begin(), { });
}

ExceptionOr<void> ContainerNode::append(FixedVector<NodeOrStringOrTrustedScript>&& vector)
ExceptionOr<void> ContainerNode::append(FixedVector<NodeOrString>&& vector)
{
auto result = convertNodesOrStringsOrTrustedScriptsIntoNodeVector(this, WTFMove(vector));
auto result = convertNodesOrStringsIntoNodeVector(WTFMove(vector));
if (result.hasException())
return result.releaseException();

Expand All @@ -1185,9 +1185,9 @@ ExceptionOr<void> ContainerNode::append(FixedVector<NodeOrStringOrTrustedScript>
return { };
}

ExceptionOr<void> ContainerNode::prepend(FixedVector<NodeOrStringOrTrustedScript>&& vector)
ExceptionOr<void> ContainerNode::prepend(FixedVector<NodeOrString>&& vector)
{
auto result = convertNodesOrStringsOrTrustedScriptsIntoNodeVector(this, WTFMove(vector));
auto result = convertNodesOrStringsIntoNodeVector(WTFMove(vector));
if (result.hasException())
return result.releaseException();

Expand All @@ -1208,9 +1208,9 @@ ExceptionOr<void> ContainerNode::prepend(FixedVector<NodeOrStringOrTrustedScript
}

// https://dom.spec.whatwg.org/#dom-parentnode-replacechildren
ExceptionOr<void> ContainerNode::replaceChildren(FixedVector<NodeOrStringOrTrustedScript>&& vector)
ExceptionOr<void> ContainerNode::replaceChildren(FixedVector<NodeOrString>&& vector)
{
auto result = convertNodesOrStringsOrTrustedScriptsIntoNodeVector(this, WTFMove(vector));
auto result = convertNodesOrStringsIntoNodeVector(WTFMove(vector));
if (result.hasException())
return result.releaseException();
auto newChildren = result.releaseReturnValue();
Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/dom/ContainerNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,10 @@ class ContainerNode : public Node {
WEBCORE_EXPORT Element* firstElementChild() const;
WEBCORE_EXPORT Element* lastElementChild() const;
WEBCORE_EXPORT unsigned childElementCount() const;
ExceptionOr<void> append(FixedVector<NodeOrStringOrTrustedScript>&&);
ExceptionOr<void> prepend(FixedVector<NodeOrStringOrTrustedScript>&&);
ExceptionOr<void> append(FixedVector<NodeOrString>&&);
ExceptionOr<void> prepend(FixedVector<NodeOrString>&&);

ExceptionOr<void> replaceChildren(FixedVector<NodeOrStringOrTrustedScript>&&);
ExceptionOr<void> replaceChildren(FixedVector<NodeOrString>&&);

ExceptionOr<void> ensurePreInsertionValidity(Node& newChild, Node* refChild);
ExceptionOr<void> ensurePreInsertionValidityForPhantomDocumentFragment(NodeVector& newChildren, Node* refChild = nullptr);
Expand Down
79 changes: 25 additions & 54 deletions Source/WebCore/dom/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
#include "HTMLDialogElement.h"
#include "HTMLElement.h"
#include "HTMLImageElement.h"
#include "HTMLScriptElement.h"
#include "HTMLSlotElement.h"
#include "HTMLStyleElement.h"
#include "InputEvent.h"
Expand Down Expand Up @@ -83,8 +82,7 @@
#include "TemplateContentDocumentFragment.h"
#include "TextEvent.h"
#include "TextManipulationController.h"
#include "TouchEvent.h"
#include "TrustedType.h"
#include "WebCoreOpaqueRoot.h"
#include "WebCoreOpaqueRoot.h"
#include "WheelEvent.h"
#include "XMLNSNames.h"
Expand Down Expand Up @@ -568,14 +566,13 @@ ExceptionOr<void> Node::appendChild(Node& newChild)
return Exception { ExceptionCode::HierarchyRequestError };
}

static HashSet<RefPtr<Node>> nodeSetPreTransformedFromNodeOrStringOrTrustedScriptVector(const FixedVector<NodeOrStringOrTrustedScript>& vector)
static HashSet<RefPtr<Node>> nodeSetPreTransformedFromNodeOrStringVector(const FixedVector<NodeOrString>& vector)
{
HashSet<RefPtr<Node>> nodeSet;
for (const auto& variant : vector) {
WTF::switchOn(variant,
[&] (const RefPtr<Node>& node) { nodeSet.add(const_cast<Node*>(node.get())); },
[] (const String&) { },
[] (const RefPtr<TrustedScript>&) { }
[] (const String&) { }
);
}
return nodeSet;
Expand All @@ -600,34 +597,18 @@ static RefPtr<Node> firstFollowingSiblingNotInNodeSet(Node& context, const HashS
}

// https://dom.spec.whatwg.org/#converting-nodes-into-a-node
ExceptionOr<RefPtr<Node>> Node::convertNodesOrStringsOrTrustedScriptsIntoNode(RefPtr<Node> parent, FixedVector<NodeOrStringOrTrustedScript>&& vector)
ExceptionOr<RefPtr<Node>> Node::convertNodesOrStringsIntoNode(FixedVector<NodeOrString>&& nodeOrStringVector)
{
if (vector.isEmpty())
if (nodeOrStringVector.isEmpty())
return nullptr;

Ref document = this->document();
NodeVector nodes;
auto trustedTypesEnabled = document->scriptExecutionContext()->settingsValues().trustedTypesEnabled;
for (auto& variant : vector) {
if (trustedTypesEnabled) {
auto textNodeHolder = processNodeOrStringAsTrustedType(document, parent, variant);
if (textNodeHolder.hasException())
return textNodeHolder.releaseException();

if (RefPtr textNode = textNodeHolder.releaseReturnValue()) {
nodes.append(textNode.releaseNonNull());
continue;
}
} else if (std::holds_alternative<String>(variant)) {
nodes.append(Text::create(document, WTFMove(std::get<String>(variant))));
continue;
}

ASSERT(std::holds_alternative<RefPtr<Node>>(variant));
RefPtr node = WTFMove(std::get<RefPtr<Node>>(variant));
ASSERT(node);
nodes.append(node.releaseNonNull());
}
auto nodes = WTF::map(WTFMove(nodeOrStringVector), [&](auto&& variant) -> Ref<Node> {
return WTF::switchOn(WTFMove(variant),
[&](RefPtr<Node>&& node) { return node.releaseNonNull(); },
[&](String&& string) -> Ref<Node> { return Text::create(document, WTFMove(string)); }
);
});

if (nodes.size() == 1)
return RefPtr<Node> { WTFMove(nodes.first()) };
Expand All @@ -642,26 +623,16 @@ ExceptionOr<RefPtr<Node>> Node::convertNodesOrStringsOrTrustedScriptsIntoNode(Re
}

// https://dom.spec.whatwg.org/#converting-nodes-into-a-node except this returns a NodeVector
ExceptionOr<NodeVector> Node::convertNodesOrStringsOrTrustedScriptsIntoNodeVector(RefPtr<Node> parent, FixedVector<NodeOrStringOrTrustedScript>&& vector)
ExceptionOr<NodeVector> Node::convertNodesOrStringsIntoNodeVector(FixedVector<NodeOrString>&& nodeOrStringVector)
{
if (vector.isEmpty())
if (nodeOrStringVector.isEmpty())
return NodeVector { };

Ref document = this->document();
NodeVector nodeVector;
nodeVector.reserveInitialCapacity(vector.size());
auto trustedTypesEnabled = document->scriptExecutionContext()->settingsValues().trustedTypesEnabled;
for (auto& variant : vector) {
if (trustedTypesEnabled) {
auto textNodeHolder = processNodeOrStringAsTrustedType(document, parent, variant);
if (textNodeHolder.hasException())
return textNodeHolder.releaseException();

if (RefPtr textNode = textNodeHolder.releaseReturnValue()) {
nodeVector.append(textNode.releaseNonNull());
continue;
}
} else if (std::holds_alternative<String>(variant)) {
nodeVector.reserveInitialCapacity(nodeOrStringVector.size());
for (auto& variant : nodeOrStringVector) {
if (std::holds_alternative<String>(variant)) {
nodeVector.append(Text::create(document, WTFMove(std::get<String>(variant))));
continue;
}
Expand All @@ -688,16 +659,16 @@ ExceptionOr<NodeVector> Node::convertNodesOrStringsOrTrustedScriptsIntoNodeVecto
return nodeVector;
}

ExceptionOr<void> Node::before(FixedVector<NodeOrStringOrTrustedScript>&& vector)
ExceptionOr<void> Node::before(FixedVector<NodeOrString>&& nodeOrStringVector)
{
RefPtr parent = parentNode();
if (!parent)
return { };

auto nodeSet = nodeSetPreTransformedFromNodeOrStringOrTrustedScriptVector(vector);
auto nodeSet = nodeSetPreTransformedFromNodeOrStringVector(nodeOrStringVector);
RefPtr viablePreviousSibling = firstPrecedingSiblingNotInNodeSet(*this, nodeSet);

auto result = convertNodesOrStringsOrTrustedScriptsIntoNodeVector(parent, WTFMove(vector));
auto result = convertNodesOrStringsIntoNodeVector(WTFMove(nodeOrStringVector));
if (result.hasException())
return result.releaseException();

Expand All @@ -709,16 +680,16 @@ ExceptionOr<void> Node::before(FixedVector<NodeOrStringOrTrustedScript>&& vector
return parent->insertChildrenBeforeWithoutPreInsertionValidityCheck(WTFMove(newChildren), viableNextSibling.get());
}

ExceptionOr<void> Node::after(FixedVector<NodeOrStringOrTrustedScript>&& vector)
ExceptionOr<void> Node::after(FixedVector<NodeOrString>&& nodeOrStringVector)
{
RefPtr parent = parentNode();
if (!parent)
return { };

auto nodeSet = nodeSetPreTransformedFromNodeOrStringOrTrustedScriptVector(vector);
auto nodeSet = nodeSetPreTransformedFromNodeOrStringVector(nodeOrStringVector);
RefPtr viableNextSibling = firstFollowingSiblingNotInNodeSet(*this, nodeSet);

auto result = convertNodesOrStringsOrTrustedScriptsIntoNodeVector(parent, WTFMove(vector));
auto result = convertNodesOrStringsIntoNodeVector(WTFMove(nodeOrStringVector));
if (result.hasException())
return result.releaseException();

Expand All @@ -729,16 +700,16 @@ ExceptionOr<void> Node::after(FixedVector<NodeOrStringOrTrustedScript>&& vector)
return parent->insertChildrenBeforeWithoutPreInsertionValidityCheck(WTFMove(newChildren), viableNextSibling.get());
}

ExceptionOr<void> Node::replaceWith(FixedVector<NodeOrStringOrTrustedScript>&& vector)
ExceptionOr<void> Node::replaceWith(FixedVector<NodeOrString>&& nodeOrStringVector)
{
RefPtr parent = parentNode();
if (!parent)
return { };

auto nodeSet = nodeSetPreTransformedFromNodeOrStringOrTrustedScriptVector(vector);
auto nodeSet = nodeSetPreTransformedFromNodeOrStringVector(nodeOrStringVector);
RefPtr viableNextSibling = firstFollowingSiblingNotInNodeSet(*this, nodeSet);

auto result = convertNodesOrStringsOrTrustedScriptsIntoNode(parent, WTFMove(vector));
auto result = convertNodesOrStringsIntoNode(WTFMove(nodeOrStringVector));
if (result.hasException())
return result.releaseException();

Expand Down
15 changes: 7 additions & 8 deletions Source/WebCore/dom/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ class RenderStyle;
class SVGQualifiedName;
class ShadowRoot;
class TouchEvent;
class TrustedScript;
class WebCoreOpaqueRoot;

namespace Style {
Expand All @@ -91,7 +90,7 @@ enum class MutationObserverOptionType : uint8_t;
using MutationObserverOptions = OptionSet<MutationObserverOptionType>;
using MutationRecordDeliveryOptions = OptionSet<MutationObserverOptionType>;

using NodeOrStringOrTrustedScript = std::variant<RefPtr<Node>, String, RefPtr<TrustedScript>>;
using NodeOrString = std::variant<RefPtr<Node>, String>;

const int initialNodeVectorSize = 11; // Covers 99.5%. See webkit.org/b/80706
typedef Vector<Ref<Node>, initialNodeVectorSize> NodeVector;
Expand Down Expand Up @@ -218,9 +217,9 @@ class Node : public EventTarget {
WEBCORE_EXPORT Element* nextElementSibling() const;

// From the ChildNode - https://dom.spec.whatwg.org/#childnode
ExceptionOr<void> before(FixedVector<NodeOrStringOrTrustedScript>&&);
ExceptionOr<void> after(FixedVector<NodeOrStringOrTrustedScript>&&);
ExceptionOr<void> replaceWith(FixedVector<NodeOrStringOrTrustedScript>&&);
ExceptionOr<void> before(FixedVector<NodeOrString>&&);
ExceptionOr<void> after(FixedVector<NodeOrString>&&);
ExceptionOr<void> replaceWith(FixedVector<NodeOrString>&&);
WEBCORE_EXPORT ExceptionOr<void> remove();

// Other methods (not part of DOM)
Expand Down Expand Up @@ -767,9 +766,9 @@ class Node : public EventTarget {
template<typename NodeClass>
static NodeClass& traverseToRootNodeInternal(const NodeClass&);

// FIXME: Replace all uses of convertNodesOrStringsOrTrustedScriptsIntoNode by convertNodesOrStringsOrTrustedScriptsIntoNodeVector.
ExceptionOr<RefPtr<Node>> convertNodesOrStringsOrTrustedScriptsIntoNode(RefPtr<Node>, FixedVector<NodeOrStringOrTrustedScript>&&);
ExceptionOr<NodeVector> convertNodesOrStringsOrTrustedScriptsIntoNodeVector(RefPtr<Node>, FixedVector<NodeOrStringOrTrustedScript>&&);
// FIXME: Replace all uses of convertNodesOrStringsIntoNode by convertNodesOrStringsIntoNodeVector.
ExceptionOr<RefPtr<Node>> convertNodesOrStringsIntoNode(FixedVector<NodeOrString>&&);
ExceptionOr<NodeVector> convertNodesOrStringsIntoNodeVector(FixedVector<NodeOrString>&&);

private:
virtual PseudoId customPseudoId() const
Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/dom/ParentNode.idl
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ interface mixin ParentNode {
readonly attribute Element? lastElementChild;
readonly attribute unsigned long childElementCount;

[CEReactions=Needed, Unscopable] undefined prepend((Node or DOMString or TrustedScript)... nodes);
[CEReactions=Needed, Unscopable] undefined append((Node or DOMString or TrustedScript)... nodes);
[CEReactions=Needed, Unscopable] undefined replaceChildren((Node or DOMString or TrustedScript)... nodes);
[CEReactions=Needed, Unscopable] undefined prepend((Node or DOMString)... nodes);
[CEReactions=Needed, Unscopable] undefined append((Node or DOMString)... nodes);
[CEReactions=Needed, Unscopable] undefined replaceChildren((Node or DOMString)... nodes);

Element? querySelector(DOMString selectors);
[NewObject] NodeList querySelectorAll(DOMString selectors);
Expand Down
28 changes: 1 addition & 27 deletions Source/WebCore/dom/TrustedType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,11 @@

#include "ContentSecurityPolicy.h"
#include "Document.h"
#include "HTMLScriptElement.h"
#include "HTMLElement.h"
#include "JSDOMExceptionHandling.h"
#include "JSTrustedScript.h"
#include "LocalDOMWindow.h"
#include "Node.h"
#include "SVGNames.h"
#include "Text.h"
#include "TrustedTypePolicy.h"
#include "TrustedTypePolicyFactory.h"
#include "WindowOrWorkerGlobalScopeTrustedTypes.h"
Expand Down Expand Up @@ -304,30 +302,6 @@ ExceptionOr<String> requireTrustedTypesForPreNavigationCheckPasses(ScriptExecuti
: nullString());
}

ExceptionOr<RefPtr<Text>> processNodeOrStringAsTrustedType(Ref<Document> document, RefPtr<Node> parent, std::variant<RefPtr<Node>, String, RefPtr<TrustedScript>> variant)
{
RefPtr<Text> text;
if (std::holds_alternative<String>(variant))
text = Text::create(document, WTFMove(std::get<String>(variant)));
else if (std::holds_alternative<RefPtr<Node>>(variant)) {
if (RefPtr textNode = dynamicDowncast<Text>(std::get<RefPtr<Node>>(variant)))
text = textNode;
}

if (text) {
if (UNLIKELY(is<HTMLScriptElement>(parent))) {
auto holder = trustedTypeCompliantString(TrustedType::TrustedScript, *document->scriptExecutionContext(), text->wholeText(), "HTMLScriptElement text"_s);
if (holder.hasException())
return holder.releaseException();

text->replaceWholeText(holder.releaseReturnValue());
}
} else if (std::holds_alternative<RefPtr<TrustedScript>>(variant))
text = Text::create(document, std::get<RefPtr<TrustedScript>>(variant)->toString());

return text;
}

ExceptionOr<bool> canCompile(ScriptExecutionContext& scriptExecutionContext, JSC::CompilationType compilationType, String codeString, JSC::JSValue bodyArgument)
{
VM& vm = scriptExecutionContext.vm();
Expand Down
5 changes: 0 additions & 5 deletions Source/WebCore/dom/TrustedType.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,8 @@ enum class CompilationType;

namespace WebCore {

class Document;
class Exception;
class Node;
class ScriptExecutionContext;
class Text;

enum class TrustedType : int8_t {
TrustedHTML,
Expand Down Expand Up @@ -73,8 +70,6 @@ ExceptionOr<String> trustedTypeCompliantString(ScriptExecutionContext&, std::var

ExceptionOr<String> trustedTypeCompliantString(ScriptExecutionContext&, std::variant<RefPtr<TrustedScriptURL>, String>&&, const String& sink);

ExceptionOr<RefPtr<Text>> processNodeOrStringAsTrustedType(Ref<Document>, RefPtr<Node> parent, std::variant<RefPtr<Node>, String, RefPtr<TrustedScript>>);

WEBCORE_EXPORT AttributeTypeAndSink trustedTypeForAttribute(const String& elementName, const String& attributeName, const String& elementNamespace, const String& attributeNamespace);

ExceptionOr<bool> canCompile(ScriptExecutionContext&, JSC::CompilationType, String codeString, JSC::JSValue bodyArgument);
Expand Down

0 comments on commit 4aae90b

Please sign in to comment.