Changeset 190752 in webkit


Ignore:
Timestamp:
Oct 8, 2015 3:43:17 PM (9 years ago)
Author:
akling@apple.com
Message:

Generated frame tree names should be kept reasonably long.
<​https://webkit.org/b/149874>

Reviewed by Darin Adler.

Source/WebCore:

Some clumsy advertising script is going around assigning JavaScript source code
to the "name" attribute of iframes. This is causing WebKit to generate way too huge
names for anonymous descendants of such iframes.

Previously, the generated name of an anonymous subframe would be its slash-separated
path from the root frame, with the "name" attribute of each ancestor between the
slashes, or "<!--frame${index in parent}-->" for anonymous ancestors.

These ad scripts are often over 100kB in size, with multiple subframes, so we'd end
up with frame names looking like this:

"<!--framePath <MONSTER BLOB OF JAVASCRIPT FROM HELL>/<!--frame0--><!--frame0-->-->"

While this is worth fixing for the memory usage alone, we've been making it way
worse by also using these paths when recording the back/forward history parts of
WebKit session state.

This patch makes generated paths always use index-in-parent as the "directory name"
for ancestors of anonymous subframes. The above example path will now instead be:

"<!--framePath <!--frame0-->/<!--frame0-->/<!--frame0-->-->"

Test: fast/frames/long-names-in-nested-subframes.html

  • page/FrameTree.cpp:

(WebCore::FrameTree::indexInParent):
(WebCore::FrameTree::uniqueChildName):

  • page/FrameTree.h:

LayoutTests:

Added a test to document our name generation behavior for subframes with long-named ancestors.
Also rebaselined some tests that exposed the old behavior.

  • fast/forms/form-and-frame-interaction-retains-values-expected.txt:
  • fast/frames/long-names-in-nested-subframes-expected.txt: Added.
  • fast/frames/long-names-in-nested-subframes.html: Added.
  • http/tests/navigation/image-load-in-subframe-unload-handler-expected.txt:
  • http/tests/security/dataURL/xss-DENIED-from-data-url-sub-frame-2-level-expected.txt:
  • http/tests/security/dataURL/xss-DENIED-to-data-url-sub-frame-2-level-expected.txt:
  • http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level-expected.txt:
  • http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-to-javscript-url-expected.txt:
  • http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url-expected.txt:
Location:
trunk
Files:
2 added
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r190735 r190752  
     12015-10-08  Andreas Kling  <akling@apple.com>
     2
     3        Generated frame tree names should be kept reasonably long.
     4        <https://webkit.org/b/149874>
     5
     6        Reviewed by Darin Adler.
     7
     8        Added a test to document our name generation behavior for subframes with long-named ancestors.
     9        Also rebaselined some tests that exposed the old behavior.
     10
     11        * fast/forms/form-and-frame-interaction-retains-values-expected.txt:
     12        * fast/frames/long-names-in-nested-subframes-expected.txt: Added.
     13        * fast/frames/long-names-in-nested-subframes.html: Added.
     14        * http/tests/navigation/image-load-in-subframe-unload-handler-expected.txt:
     15        * http/tests/security/dataURL/xss-DENIED-from-data-url-sub-frame-2-level-expected.txt:
     16        * http/tests/security/dataURL/xss-DENIED-to-data-url-sub-frame-2-level-expected.txt:
     17        * http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level-expected.txt:
     18        * http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-to-javscript-url-expected.txt:
     19        * http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url-expected.txt:
     20
    1212015-10-08  Saam barati  <sbarati@apple.com>
    222

  • trunk/LayoutTests/fast/forms/form-and-frame-interaction-retains-values-expected.txt

    r36652 r190752  
    1414
    1515--------
    16 Frame: '<!--framePath //submitted/<!--frame0-->-->'
     16Frame: '<!--framePath //<!--frame0-->/<!--frame0-->-->'
    1717--------
    1818

  • trunk/LayoutTests/http/tests/navigation/image-load-in-subframe-unload-handler-expected.txt

    r71256 r190752  
    1 frame "<!--framePath //target/<!--frame0-->-->" - has 1 onunload handler(s)
     1frame "<!--framePath //<!--frame0-->/<!--frame0-->-->" - has 1 onunload handler(s)
    22This test triggers an unload handler that starts an image load in a different frame (and deletes both frames), but ensures the main frame is not destroyed. We pass if we don't crash.

  • trunk/LayoutTests/http/tests/security/dataURL/xss-DENIED-from-data-url-sub-frame-2-level-expected.txt

    r178527 r190752  
    1414
    1515--------
    16 Frame: '<!--framePath //aFrame/<!--frame0-->-->'
     16Frame: '<!--framePath //<!--frame0-->/<!--frame0-->-->'
    1717--------
    1818Inner-inner iframe.

  • trunk/LayoutTests/http/tests/security/dataURL/xss-DENIED-to-data-url-sub-frame-2-level-expected.txt

    r178527 r190752  
    1515
    1616--------
    17 Frame: '<!--framePath //aFrame/<!--frame0-->-->'
     17Frame: '<!--framePath //<!--frame0-->/<!--frame0-->-->'
    1818--------
    1919PASS: Cross frame access to a data: URL 2 levels deep was denied.

  • trunk/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level-expected.txt

    r28128 r190752  
    1111
    1212--------
    13 Frame: '<!--framePath //aFrame/<!--frame0-->-->'
     13Frame: '<!--framePath //<!--frame0-->/<!--frame0-->-->'
    1414--------
    1515Inner-inner iframe.

  • trunk/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-to-javscript-url-expected.txt

    r25381 r190752  
    1010
    1111--------
    12 Frame: '<!--framePath //aFrame/<!--frame0-->-->'
     12Frame: '<!--framePath //<!--frame0-->/<!--frame0-->-->'
    1313--------
    1414PASS: Cross frame access from a javascript: URL was allowed!

  • trunk/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url-expected.txt

    r28128 r190752  
    1212
    1313--------
    14 Frame: '<!--framePath //aFrame/<!--frame0-->-->'
     14Frame: '<!--framePath //<!--frame0-->/<!--frame0-->-->'
    1515--------
    1616Inner-inner iframe.

  • trunk/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-sub-frame-2-level-expected.txt

    r28128 r190752  
    1212
    1313--------
    14 Frame: '<!--framePath //aFrame/<!--frame0-->-->'
     14Frame: '<!--framePath //<!--frame0-->/<!--frame0-->-->'
    1515--------
    1616PASS: Cross frame access to a javascript: URL 2 levels deep was allowed!

  • trunk/Source/WebCore/ChangeLog

    r190746 r190752  
     12015-10-08  Andreas Kling  <akling@apple.com>
     2
     3        Generated frame tree names should be kept reasonably long.
     4        <https://webkit.org/b/149874>
     5
     6        Reviewed by Darin Adler.
     7
     8        Some clumsy advertising script is going around assigning JavaScript source code
     9        to the "name" attribute of iframes. This is causing WebKit to generate way too huge
     10        names for anonymous descendants of such iframes.
     11
     12        Previously, the generated name of an anonymous subframe would be its slash-separated
     13        path from the root frame, with the "name" attribute of each ancestor between the
     14        slashes, or "<!--frame${index in parent}-->" for anonymous ancestors.
     15
     16        These ad scripts are often over 100kB in size, with multiple subframes, so we'd end
     17        up with frame names looking like this:
     18
     19        "<!--framePath //<MONSTER BLOB OF JAVASCRIPT FROM HELL>/<!--frame0--><!--frame0-->-->"
     20
     21        While this is worth fixing for the memory usage alone, we've been making it way
     22        worse by also using these paths when recording the back/forward history parts of
     23        WebKit session state.
     24
     25        This patch makes generated paths always use index-in-parent as the "directory name"
     26        for ancestors of anonymous subframes. The above example path will now instead be:
     27
     28        "<!--framePath //<!--frame0-->/<!--frame0-->/<!--frame0-->-->"
     29
     30        Test: fast/frames/long-names-in-nested-subframes.html
     31
     32        * page/FrameTree.cpp:
     33        (WebCore::FrameTree::indexInParent):
     34        (WebCore::FrameTree::uniqueChildName):
     35        * page/FrameTree.h:
     36
    1372015-10-08  Commit Queue  <commit-queue@webkit.org>
    238

  • trunk/Source/WebCore/page/FrameTree.cpp

    r185231 r190752  
    8383}
    8484
     85unsigned FrameTree::indexInParent() const
     86{
     87    if (!m_parent)
     88        return 0;
     89    unsigned index = 0;
     90    for (Frame* frame = m_parent->tree().firstChild(); frame; frame = frame->tree().nextSibling()) {
     91        if (&frame->tree() == this)
     92            return index;
     93        ++index;
     94    }
     95    RELEASE_ASSERT_NOT_REACHED();
     96}
     97
    8598void FrameTree::appendChild(PassRefPtr<Frame> child)
    8699{
    … …  
    129142AtomicString FrameTree::uniqueChildName(const AtomicString& requestedName) const
    130143{
     144    // If the requested name (the frame's "name" attribute) is unique, just use that.
    131145    if (!requestedName.isEmpty() && !child(requestedName) && requestedName != "_blank")
    132146        return requestedName;
    133147
    134     // Create a repeatable name for a child about to be added to us. The name must be
    135     // unique within the frame tree. The string we generate includes a "path" of names
    136     // from the root frame down to us. For this path to be unique, each set of siblings must
    137     // contribute a unique name to the path, which can't collide with any HTML-assigned names.
    138     // We generate this path component by index in the child list along with an unlikely
    139     // frame name that can't be set in HTML because it collides with comment syntax.
     148    // The "name" attribute was not unique or absent. Generate a name based on the
     149    // new frame's location in the frame tree. The name uses HTML comment syntax to
     150    // avoid collisions with author names.
     151
     152    // An example path for the third child of the second child of the root frame:
     153    // <!--framePath //<!--frame1-->/<!--frame2-->-->
    140154
    141155    const char framePathPrefix[] = "<!--framePath ";
    … …  
    160174        frame = chain[i];
    161175        name.append('/');
    162         name.append(frame->tree().uniqueName());
     176        if (frame->tree().parent()) {
     177            name.appendLiteral("<!--frame");
     178            name.appendNumber(frame->tree().indexInParent());
     179            name.appendLiteral("-->");
     180        }
    163181    }
    164182

  • trunk/Source/WebCore/page/FrameTree.h

    r185231 r190752  
    8585        unsigned scopedChildCount() const;
    8686
     87        unsigned indexInParent() const;
     88
    8789    private:
    8890        Frame* deepLastChild() const;
Note: See TracChangeset for help on using the changeset viewer.