[Webkit-unassigned] [Bug 15123] Self-replicating code makes Safari hang and eventually crash

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=15123


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #79006|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #13 from Adam Barth <abarth at webkit.org>  2011-01-14 17:05:32 PST ---
(From update of attachment 79006)
View in context: https://bugs.webkit.org/attachment.cgi?id=79006&action=review

> WebCore/dom/Document.cpp:214
> +#define MAX_WRITE_RECURSION_DEPTH 20

We usually use constants instead of pre-processor macros.

> WebCore/dom/Document.cpp:425
> +    , m_tooDeepWriteRecursion(false)
> +    , m_writeRecursionDepth(0)

Why do we have both of these variables?  One would seem to be sufficient.

> WebCore/dom/Document.cpp:2222
> +    // Prevent DOS attack from self-replicating javascript which causes infinite recursion here

This comment isn't necessary.

> WebCore/dom/Document.cpp:2233
> +    if (m_tooDeepWriteRecursion) {
> +       m_writeRecursionDepth--;
> +       return;
> +    } else {
> +       m_writeRecursionDepth++;
> +    }

This logic doesn't make much sense to me.  Typically for these recursion-depth counts, we use a small class that increments the counter on construction and decrements the counter on destruction.  That way we know that we never leak a count.  There are examples of how to do this in the HTML parser related to the script nesting level.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.