Navigations failing with "this element is not ready"

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

Problem

DAST may encounter the error "element is not ready" while crawling the target site. This error indicates that the scanner has received a DOM node ID from the browser, but has not received the corresponding information (such as tag name) about that node; and that after waiting for the configured timeout (DAST_PAGE_ELEMENT_READY_TIMEOUT) that information still did not arrive. When this error happens it causes the process which encountered it - typically a search for interactable elements on a page - to terminate, resulting in decreased crawler coverage and scan quality.

Analysis

This error is most likely to occur when node information was sent from the browser but was not captured by the scanner. This can happen when node information is sent in a location that the scanner doesn't know to look for it. We have previously identified one such location as the pseudoElements property of the Node object (internal link).

If the node information was already missed, why do we bother waiting for it? Under normal circumstances, if the browser sends a reference to a node id, then it has also already sent the information about that node (this is not formally documented but is mentioned by the DevTools Protocol developers in several places, such as here). However, the scanner can process some messages coming from the browser out of order, for reasons that are outside of the scope of this issue. This out-of-order processing can result in the scanner being aware of a node id prior to receiving all of its information; waiting for the element to become "ready" is meant to make up for this out-of-order processing by giving time for the original message with the node information to be processed.

Processing of browser messages is extremely fast. If the timeout value is actually reached, it is overwhelmingly likely that the node information was already missed, and not that we simply didn't quite wait long enough for the out-of-order message processing to catch up.

Waiting for an element to be ready is also invoked before processing incremental update messages for an element. This usage is understood to be vestigial; if the scanner is processing an event which updates a node, the event which describes that node should already have been processed. An unready element will only get an incremental update if the original information was missed, in which case waiting will not accomplish anything.

An additional complication is that the behavior of DAST_PAGE_ELEMENT_READY_TIMEOUT is somewhat arbitrary. The duration of the timeout is counted not from when the "unready" element was encountered, but rather from the first time a scanner process needed to wait for the element to become ready. This arbitrary usage means that choosing a value for the timeout is also somewhat arbitrary, and also that changing behavior around the usage of the timeout is unpredictable.

Proposal

  1. Add Metrics
    • How often are elements created in an "unready" state?
    • How often are unready elements later made ready?
    • How often does the scanner wait on an unready element?
    • How often does the timeout expire while waiting for an unready element to become ready?
    • What is the avg/max time it takes for an unready element to become ready?
  2. Continue when element is not ready: when searching for matching elements and one is not ready, treat it as not matching and continue
    • See below
  3. Convert "unready" to "unknown" and remove the wait-for-ready timer (and DAST_PAGE_ELEMENT_READY_TIMEOUT)
    • Requires Process DevTools messages in strict FIFO order

Implementation plan

There are two non-vestigial usages of InternalElement.WaitForReady: ToFormElement and ToHTMLElement. These are used directly or indirectly by various scanner processes which search for elements matching specific criteria. Current behavior for all of these processes when encountering an unready element is to abort the entire process. Change each process to treat the unready element as not matching and continue searching.

  • FindFormsService.Find
  • FindFormsService.getFormChildElements
  • FindImplicitFormsService.findChildElements
  • MatchElementService.FindMatchingElement
  • Tab.FindInteractables

Feature Flag

The change in each of these locations to continue instead of aborting should be guarded by a feature flag. This will allow us to enable this behavior selectively for customers whom it would benefit, and to disable it for specific customers after making it the default if it causes unwanted behavior.

Risks

If there are many elements that are unready, a process which used to abort after waiting for a single unready element (DAST_PAGE_ELEMENT_READY_TIMEOUT x 1) will now wait for each unready element (DAST_PAGE_ELEMENT_READY_TIMEOUT x N) before continuing. This may increase total crawl time, and cause the scan to encounter some timeouts (such as DAST_CRAWL_TIMEOUT) that were not previously encountered.

This risk could be mitigated by changing the definition of DAST_PAGE_ELEMENT_READY_TIMEOUT to count from when the unready element is initially encountered, instead of from the time is is needed by the matching process. This would result in more consistent behavior for a given scan. However it would likely require a change in the default value of the variable, and the general impact on scans would be hard to predict. We would almost certainly have to treat it as a breaking change.

Edited by 🤖 GitLab Bot 🤖