Title: [295636] trunk
Revision
295636
Author
[email protected]
Date
2022-06-17 08:45:24 -0700 (Fri, 17 Jun 2022)

Log Message

AX ITM: Crash in com.apple.WebKit.WebContent at Recursion :: com.apple.WebCore: WebCore::AXIsolatedTree::collectNodeChangesForSubtree.
https://bugs.webkit.org/show_bug.cgi?id=241571

Test: accessibility/deep-tree.html

Reviewed by Chris Fleizach.

Added a limit for recursive calls to AXIsolatedTree::collectNodeChangesForSubtree. The limit is obtained from the DOM maximum tree depth.
In addition, added a sanity check during parent-child traversal, that none of the children can be equal to the parent. This will not cover all possible circular relations that can cause an infinite recursion, but may catch some pathological cases.

* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::create):
(WebCore::AXIsolatedTree::collectNodeChangesForSubtree):
(WebCore::AXIsolatedTree::updateNode):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:

* LayoutTests/accessibility/deep-tree-expected.txt: Added.
* LayoutTests/accessibility/deep-tree.html: Added.
* LayoutTests/platform/glib/accessibility/deep-tree-expected.txt: Added.

Canonical link: https://commits.webkit.org/251641@main

Modified Paths

Added Paths

Diff

Added: trunk/LayoutTests/accessibility/deep-tree-expected.txt (0 => 295636)


--- trunk/LayoutTests/accessibility/deep-tree-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/accessibility/deep-tree-expected.txt	2022-06-17 15:45:24 UTC (rev 295636)
@@ -0,0 +1,11 @@
+This tests that trying to retrieve AX objects deeper than the 512 level allowed in the DOM tree fails gracefully.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS axChild.childAtIndex(0).stringValue is 'AXValue: deepest child'
+PASS axChild is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+deepest child

Added: trunk/LayoutTests/accessibility/deep-tree.html (0 => 295636)


--- trunk/LayoutTests/accessibility/deep-tree.html	                        (rev 0)
+++ trunk/LayoutTests/accessibility/deep-tree.html	2022-06-17 15:45:24 UTC (rev 295636)
@@ -0,0 +1,45 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+
+<div id="content" role="group" />
+
+<script>
+    if (window.accessibilityController) {
+        description("This tests that trying to retrieve AX objects deeper than the 512 level allowed in the DOM tree fails gracefully.");
+
+        // Add a 500 levels deep hierarchy and enssure that deepest AX object can be retrieved successfully.
+        let parent = document.getElementById("content");
+        let child = null;
+        for (let i = 0; i < 500; ++i) {
+            child = document.createElement("div");
+            child.setAttribute("role", "group");
+            parent.appendChild(child);
+            parent = child;
+        }
+        child.id = "deepest-child";
+        child.innerText = "deepest child";
+        var axChild = accessibilityController.accessibleElementById("deepest-child");
+        if (accessibilityController.platformName == "mac")
+            shouldBe("axChild.childAtIndex(0).stringValue", "'AXValue: deepest child'");
+        else if (accessibilityController.platformName == "atspi")
+            shouldBe("axChild.stringValue", "'AXValue: deepest child'");
+
+        // Add an additional 10 levels to the tree and check that trying to retrieve the deepest AX object fails gracefully.
+        for (let i = 0; i < 10; ++i) {
+            child = document.createElement("div");
+            child.setAttribute("role", "group");
+            parent.insertBefore(child, parent.firstChild);
+            parent = child;
+        }
+        child.id = "deepest-deepest-child";
+        child.innerText = "deepest deepest child";
+        axChild = accessibilityController.accessibleElementById("deepest-deepest-child");
+        shouldBe("axChild", "null");
+    }
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/platform/glib/accessibility/deep-tree-expected.txt (0 => 295636)


--- trunk/LayoutTests/platform/glib/accessibility/deep-tree-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/glib/accessibility/deep-tree-expected.txt	2022-06-17 15:45:24 UTC (rev 295636)
@@ -0,0 +1,11 @@
+This tests that trying to retrieve AX objects deeper than the 512 level allowed in the DOM tree fails gracefully.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS axChild.stringValue is 'AXValue: deepest child'
+PASS axChild is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+deepest child

Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (295635 => 295636)


--- trunk/LayoutTests/platform/mac-wk1/TestExpectations	2022-06-17 15:43:51 UTC (rev 295635)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations	2022-06-17 15:45:24 UTC (rev 295636)
@@ -1262,6 +1262,7 @@
 accessibility/mac/line-index-for-textmarker.html [ Skip ]
 accessibility/mac/mathml-elements.html [ Skip ]
 accessibility/mac/mathml-root.html [ Skip ]
+accessibility/deep-tree.html [ Skip ]
 
 # <rdar://problem/61066929> [ Stress GC ] flaky JSC::ExceptionScope::assertNoException crash under WebCore::ReadableStreamDefaultController
 webkit.org/b/211923 imported/w3c/web-platform-tests/fetch/api/basic/stream-safe-creation.any.html [ Pass Crash ]

Modified: trunk/LayoutTests/platform/win/TestExpectations (295635 => 295636)


--- trunk/LayoutTests/platform/win/TestExpectations	2022-06-17 15:43:51 UTC (rev 295635)
+++ trunk/LayoutTests/platform/win/TestExpectations	2022-06-17 15:45:24 UTC (rev 295636)
@@ -951,6 +951,8 @@
 accessibility/aria-grid-with-aria-owns-rows.html [ Skip ]
 accessibility/grid-with-aria-owned-cells.html [ Skip ]
 
+accessibility/deep-tree.html [ Skip ]
+
 # TODO webkit-font-smoothing is not supported.
 webkit.org/b/149245 imported/w3c/web-platform-tests/css/css-multicol/multicol-basic-001.html [ Skip ]
 webkit.org/b/149245 imported/w3c/web-platform-tests/css/css-multicol/multicol-basic-002.html [ Skip ]

Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp (295635 => 295636)


--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp	2022-06-17 15:43:51 UTC (rev 295635)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp	2022-06-17 15:45:24 UTC (rev 295636)
@@ -102,6 +102,9 @@
     if (!document.view()->layoutContext().isInRenderTreeLayout() && !document.inRenderTreeUpdate() && !document.inStyleRecalc())
         document.updateLayoutIgnorePendingStylesheets();
 
+    tree->m_maxTreeDepth = document.settings().maximumHTMLParserDOMTreeDepth();
+    ASSERT(tree->m_maxTreeDepth);
+
     // Generate the nodes of the tree and set its root and focused objects.
     // For this, we need the root and focused objects of the AXObject tree.
     auto* axRoot = axObjectCache->getOrCreate(axObjectCache->document().view());
@@ -313,14 +316,24 @@
         return;
     }
 
-    SetForScope collectingNodeChanges(m_isCollectingNodeChanges, true);
+    SetForScope collectingAtTreeLevel(m_collectingNodeChangesAtTreeLevel, m_collectingNodeChangesAtTreeLevel + 1);
+    if (m_collectingNodeChangesAtTreeLevel >= m_maxTreeDepth)
+        return;
+
     m_unresolvedPendingAppends.set(axObject.objectID(), AttachWrapper::OnMainThread);
+    auto axChildrenCopy = axObject.children();
+    Vector<AXID> axChildrenIDs;
+    axChildrenIDs.reserveInitialCapacity(axChildrenCopy.size());
+    for (const auto& axChild : axChildrenCopy) {
+        if (!axChild || axChild.get() == &axObject) {
+            ASSERT_NOT_REACHED();
+            continue;
+        }
 
-    auto axChildrenCopy = axObject.children();
-    auto axChildrenIDs = axChildrenCopy.map([&](auto& axChild) {
+        axChildrenIDs.uncheckedAppend(axChild->objectID());
         collectNodeChangesForSubtree(*axChild);
-        return axChild->objectID();
-    });
+    }
+    axChildrenIDs.shrinkToFit();
 
     // Update the m_nodeMap.
     auto* axParent = axObject.parentObjectUnignored();
@@ -336,7 +349,7 @@
     // If we update a node as the result of some side effect while collecting node changes (e.g. a role change from
     // AccessibilityRenderObject::updateRoleAfterChildrenCreation), queue the append up to be resolved with the rest
     // of the collected changes. This prevents us from creating two node changes for the same object.
-    if (m_isCollectingNodeChanges) {
+    if (isCollectingNodeChanges()) {
         m_unresolvedPendingAppends.set(axObject.objectID(), AttachWrapper::OnAXThread);
         return;
     }

Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h (295635 => 295636)


--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h	2022-06-17 15:43:51 UTC (rev 295635)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h	2022-06-17 15:45:24 UTC (rev 295636)
@@ -387,6 +387,7 @@
     enum class AttachWrapper : bool { OnMainThread, OnAXThread };
     std::optional<NodeChange> nodeChangeForObject(AXCoreObject&, AttachWrapper = AttachWrapper::OnMainThread);
     void collectNodeChangesForSubtree(AXCoreObject&);
+    bool isCollectingNodeChanges() const { return m_collectingNodeChangesAtTreeLevel > 0; }
     void queueChange(const NodeChange&) WTF_REQUIRES_LOCK(m_changeLogLock);
     void queueRemovals(const Vector<AXID>&);
     void queueRemovalsLocked(const Vector<AXID>&) WTF_REQUIRES_LOCK(m_changeLogLock);
@@ -393,6 +394,7 @@
     void queueRemovalsAndUnresolvedChanges(const Vector<AXID>&);
 
     AXIsolatedTreeID m_treeID;
+    unsigned m_maxTreeDepth { 0 };
     AXObjectCache* m_axObjectCache { nullptr };
     bool m_usedOnAXThread { true };
 
@@ -411,8 +413,8 @@
     // The key is the ID of the object that will be resolved into an m_pendingAppends NodeChange.
     // The value is whether the wrapper should be attached on the main thread or the AX thread.
     HashMap<AXID, AttachWrapper> m_unresolvedPendingAppends;
-    // Only accessed on the main thread.
-    bool m_isCollectingNodeChanges { false };
+    // 1-based tree level, 0 = not collecting. Only accessed on the main thread.
+    unsigned m_collectingNodeChangesAtTreeLevel { 0 };
 
     // Only accessed on AX thread requesting data.
     HashMap<AXID, Ref<AXIsolatedObject>> m_readerThreadNodeMap;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to