Title: [99228] trunk
Revision
99228
Author
[email protected]
Date
2011-11-03 12:29:20 -0700 (Thu, 03 Nov 2011)

Log Message

Implement allow-popups for iframe@sandbox
https://bugs.webkit.org/show_bug.cgi?id=66505

Reviewed by Eric Seidel.

Source/WebCore: 

There's been some discussion in the HTML working group about adding an
allow-popups directive to the iframe sandbox.  Microsoft has added it
to IE10 platform preview and is fairly adamant about this feature
because it's needed by one or their products that's planning to use
iframe sandbox.  Hixie says he'll add it to the spec once we implement
it, so here's our implementation.  (See discussion in the W3C linked in
the bug for more details.)

This patch lands most of the infrastructure for this feature, but it
doesn't actually enable the feature.  I'll enable it in a follow-up
patch.

Tests: http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html
       http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html
       http/tests/security/popup-allowed-by-sandbox-when-allowed.html

* html/HTMLIFrameElement.cpp:
(WebCore::HTMLIFrameElement::parseMappedAttribute):
* loader/FrameLoader.h:
(WebCore::FrameLoader::forceSandboxFlags):
* loader/FrameLoaderTypes.h:
* page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::parseSandboxPolicy):
* page/SecurityOrigin.h:
(WebCore::SecurityOrigin::sandboxFlags):
* svg/graphics/SVGImage.cpp:
(WebCore::SVGImage::dataChanged):

LayoutTests: 

Test that the allow-popups directive works as expected.  Note:
no-popup-from-sandbox.html verifies that we still block popups without
the directive.

These tests currently have expected.txt results that show failures, but
they will pass once this feature is enabled.

* http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control-expected.txt: Added.
* http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html: Added.
* http/tests/security/popup-allowed-by-sandbox-is-sandboxed-expected.txt: Added.
* http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html: Added.
* http/tests/security/popup-allowed-by-sandbox-when-allowed-expected.txt: Added.
* http/tests/security/popup-allowed-by-sandbox-when-allowed.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (99227 => 99228)


--- trunk/LayoutTests/ChangeLog	2011-11-03 19:25:25 UTC (rev 99227)
+++ trunk/LayoutTests/ChangeLog	2011-11-03 19:29:20 UTC (rev 99228)
@@ -1,3 +1,24 @@
+2011-11-03  Adam Barth  <[email protected]>
+
+        Implement allow-popups for iframe@sandbox
+        https://bugs.webkit.org/show_bug.cgi?id=66505
+
+        Reviewed by Eric Seidel.
+
+        Test that the allow-popups directive works as expected.  Note:
+        no-popup-from-sandbox.html verifies that we still block popups without
+        the directive.
+
+        These tests currently have expected.txt results that show failures, but
+        they will pass once this feature is enabled.
+
+        * http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control-expected.txt: Added.
+        * http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html: Added.
+        * http/tests/security/popup-allowed-by-sandbox-is-sandboxed-expected.txt: Added.
+        * http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html: Added.
+        * http/tests/security/popup-allowed-by-sandbox-when-allowed-expected.txt: Added.
+        * http/tests/security/popup-allowed-by-sandbox-when-allowed.html: Added.
+
 2011-11-03  Julien Chaffraix  <[email protected]>
 
         Unreviewed gardening after r99212.

Added: trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control-expected.txt (0 => 99228)


--- trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control-expected.txt	2011-11-03 19:29:20 UTC (rev 99228)
@@ -0,0 +1,5 @@
+To run this test outside of DumpRenderTree, please disable your popup blocker!
+
+If you change this test, please be sure to change popup-allowed-by-sandbox-is-sandboxed.html as well!
+
+

Added: trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html (0 => 99228)


--- trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html	2011-11-03 19:29:20 UTC (rev 99228)
@@ -0,0 +1,15 @@
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.setCanOpenWindows(true);
+    layoutTestController.setCloseRemainingWindowsWhenComplete(true);
+}
+</script>
+<p>To run this test outside of DumpRenderTree, please disable your popup blocker!</p>
+<p>If you change this test, please be sure to change popup-allowed-by-sandbox-is-sandboxed.html as well!</p>
+<iframe sandbox="allow-scripts allow-popups allow-forms"
+  src=""
+       <script>
+       var win = window.open('data:text/html,<form action="" ><input type=submit></form><script>document.forms[0].submit(); if (window.layoutTestController) layoutTestController.notifyDone();<\/script>', '_blank');
+       </script>"
+  ></iframe>

Added: trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-expected.txt (0 => 99228)


--- trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-expected.txt	2011-11-03 19:29:20 UTC (rev 99228)
@@ -0,0 +1,7 @@
+To run this test outside of DumpRenderTree, please disable your popup blocker!
+
+If you change this test, please be sure to change popup-allowed-by-sandbox-is-sandboxed-control.html as well!
+
+This test passes if it doesn't alert FAIL.
+
+

Added: trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html (0 => 99228)


--- trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html	2011-11-03 19:29:20 UTC (rev 99228)
@@ -0,0 +1,16 @@
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.setCanOpenWindows(true);
+    layoutTestController.setCloseRemainingWindowsWhenComplete(true);
+}
+</script>
+<p>To run this test outside of DumpRenderTree, please disable your popup blocker!</p>
+<p>If you change this test, please be sure to change popup-allowed-by-sandbox-is-sandboxed-control.html as well!</p>
+<p>This test passes if it doesn't alert FAIL.</p>
+<iframe sandbox="allow-scripts allow-popups"
+  src=""
+       <script>
+       var win = window.open('data:text/html,<form action="" ><input type=submit></form><script>document.forms[0].submit(); if (window.layoutTestController) layoutTestController.notifyDone();<\/script>', '_blank');
+       </script>"
+  ></iframe>

Added: trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-when-allowed-expected.txt (0 => 99228)


--- trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-when-allowed-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-when-allowed-expected.txt	2011-11-03 19:29:20 UTC (rev 99228)
@@ -0,0 +1,4 @@
+ALERT: FAIL
+To run this test outside of DumpRenderTree, please disable your popup blocker!
+
+

Added: trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-when-allowed.html (0 => 99228)


--- trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-when-allowed.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/popup-allowed-by-sandbox-when-allowed.html	2011-11-03 19:29:20 UTC (rev 99228)
@@ -0,0 +1,15 @@
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.setCanOpenWindows(true);
+    layoutTestController.setCloseRemainingWindowsWhenComplete(true);
+}
+</script>
+<p>To run this test outside of DumpRenderTree, please disable your popup blocker!</p>
+<iframe sandbox="allow-scripts allow-popups"
+  src=""
+       <script>
+       var win = window.open('data:text/html,<script>if (window.layoutTestController) layoutTestController.notifyDone();<\/script>', '_blank');
+       alert(win ? 'PASS' : 'FAIL');
+       </script>"
+  ></iframe>

Modified: trunk/Source/WebCore/ChangeLog (99227 => 99228)


--- trunk/Source/WebCore/ChangeLog	2011-11-03 19:25:25 UTC (rev 99227)
+++ trunk/Source/WebCore/ChangeLog	2011-11-03 19:29:20 UTC (rev 99228)
@@ -1,3 +1,38 @@
+2011-11-03  Adam Barth  <[email protected]>
+
+        Implement allow-popups for iframe@sandbox
+        https://bugs.webkit.org/show_bug.cgi?id=66505
+
+        Reviewed by Eric Seidel.
+
+        There's been some discussion in the HTML working group about adding an
+        allow-popups directive to the iframe sandbox.  Microsoft has added it
+        to IE10 platform preview and is fairly adamant about this feature
+        because it's needed by one or their products that's planning to use
+        iframe sandbox.  Hixie says he'll add it to the spec once we implement
+        it, so here's our implementation.  (See discussion in the W3C linked in
+        the bug for more details.)
+
+        This patch lands most of the infrastructure for this feature, but it
+        doesn't actually enable the feature.  I'll enable it in a follow-up
+        patch.
+
+        Tests: http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html
+               http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html
+               http/tests/security/popup-allowed-by-sandbox-when-allowed.html
+
+        * html/HTMLIFrameElement.cpp:
+        (WebCore::HTMLIFrameElement::parseMappedAttribute):
+        * loader/FrameLoader.h:
+        (WebCore::FrameLoader::forceSandboxFlags):
+        * loader/FrameLoaderTypes.h:
+        * page/SecurityOrigin.cpp:
+        (WebCore::SecurityOrigin::parseSandboxPolicy):
+        * page/SecurityOrigin.h:
+        (WebCore::SecurityOrigin::sandboxFlags):
+        * svg/graphics/SVGImage.cpp:
+        (WebCore::SVGImage::dataChanged):
+
 2011-11-03  Mark Hahnenberg  <[email protected]>
 
         De-virtualize JSObject::className

Modified: trunk/Source/WebCore/html/HTMLIFrameElement.cpp (99227 => 99228)


--- trunk/Source/WebCore/html/HTMLIFrameElement.cpp	2011-11-03 19:25:25 UTC (rev 99227)
+++ trunk/Source/WebCore/html/HTMLIFrameElement.cpp	2011-11-03 19:29:20 UTC (rev 99228)
@@ -32,6 +32,7 @@
 #include "HTMLNames.h"
 #include "NodeRenderingContext.h"
 #include "RenderIFrame.h"
+#include "SecurityOrigin.h"
 
 namespace WebCore {
 
@@ -68,42 +69,6 @@
     return HTMLFrameElementBase::mapToEntry(attrName, result);
 }
 
-static SandboxFlags parseSandboxAttribute(Attribute* attribute)
-{
-    if (attribute->isNull())
-        return SandboxNone;
-
-    // Parse the unordered set of unique space-separated tokens.
-    SandboxFlags flags = SandboxAll;
-    const UChar* characters = attribute->value().characters();
-    unsigned length = attribute->value().length();
-    unsigned start = 0;
-    while (true) {
-        while (start < length && isASCIISpace(characters[start]))
-            ++start;
-        if (start >= length)
-            break;
-        unsigned end = start + 1;
-        while (end < length && !isASCIISpace(characters[end]))
-            ++end;
-
-        // Turn off the corresponding sandbox flag if it's set as "allowed".
-        String sandboxToken = String(characters + start, end - start);
-        if (equalIgnoringCase(sandboxToken, "allow-same-origin"))
-            flags &= ~SandboxOrigin;
-        else if (equalIgnoringCase(sandboxToken, "allow-forms"))
-            flags &= ~SandboxForms;
-        else if (equalIgnoringCase(sandboxToken, "allow-scripts"))
-            flags &= ~SandboxScripts;
-        else if (equalIgnoringCase(sandboxToken, "allow-top-navigation"))
-            flags &= ~SandboxTopNavigation;
-
-        start = end + 1;
-    }
-
-    return flags;
-}
-
 void HTMLIFrameElement::parseMappedAttribute(Attribute* attr)
 {
     if (attr->name() == widthAttr)
@@ -127,7 +92,7 @@
             // Add a rule that nulls out our border width.
             addCSSLength(attr, CSSPropertyBorderWidth, "0");
     } else if (attr->name() == sandboxAttr)
-        setSandboxFlags(parseSandboxAttribute(attr));
+        setSandboxFlags(attr->isNull() ? SandboxNone : SecurityOrigin::parseSandboxPolicy(attr->value()));
     else
         HTMLFrameElementBase::parseMappedAttribute(attr);
 }

Modified: trunk/Source/WebCore/loader/FrameLoader.h (99227 => 99228)


--- trunk/Source/WebCore/loader/FrameLoader.h	2011-11-03 19:25:25 UTC (rev 99227)
+++ trunk/Source/WebCore/loader/FrameLoader.h	2011-11-03 19:29:20 UTC (rev 99228)
@@ -215,7 +215,7 @@
     SandboxFlags sandboxFlags() const { return m_sandboxFlags; }
     // The following sandbox flags will be forced, regardless of changes to
     // the sandbox attribute of any parent frames.
-    void setForcedSandboxFlags(SandboxFlags flags) { m_forcedSandboxFlags = flags; m_sandboxFlags |= flags; }
+    void forceSandboxFlags(SandboxFlags flags) { m_forcedSandboxFlags |= flags; m_sandboxFlags |= flags; }
 
     // Mixed content related functions.
     static bool isMixedContent(SecurityOrigin* context, const KURL&);

Modified: trunk/Source/WebCore/loader/FrameLoaderTypes.h (99227 => 99228)


--- trunk/Source/WebCore/loader/FrameLoaderTypes.h	2011-11-03 19:25:25 UTC (rev 99227)
+++ trunk/Source/WebCore/loader/FrameLoaderTypes.h	2011-11-03 19:29:20 UTC (rev 99228)
@@ -100,6 +100,7 @@
         SandboxForms = 1 << 3,
         SandboxScripts = 1 << 4,
         SandboxTopNavigation = 1 << 5,
+        SandboxPopups = 1 << 6,
         SandboxAll = -1 // Mask with all bits set to 1.
     };
 

Modified: trunk/Source/WebCore/page/SecurityOrigin.cpp (99227 => 99228)


--- trunk/Source/WebCore/page/SecurityOrigin.cpp	2011-11-03 19:25:25 UTC (rev 99227)
+++ trunk/Source/WebCore/page/SecurityOrigin.cpp	2011-11-03 19:29:20 UTC (rev 99228)
@@ -543,6 +543,41 @@
     return !URLIsSecureURL;
 }
 
+SandboxFlags SecurityOrigin::parseSandboxPolicy(const String& policy)
+{
+    // Parse the unordered set of unique space-separated tokens.
+    SandboxFlags flags = SandboxAll;
+    const UChar* characters = policy.characters();
+    unsigned length = policy.length();
+    unsigned start = 0;
+    while (true) {
+        while (start < length && isASCIISpace(characters[start]))
+            ++start;
+        if (start >= length)
+            break;
+        unsigned end = start + 1;
+        while (end < length && !isASCIISpace(characters[end]))
+            ++end;
+
+        // Turn off the corresponding sandbox flag if it's set as "allowed".
+        String sandboxToken = policy.substring(start, end - start);
+        if (equalIgnoringCase(sandboxToken, "allow-same-origin"))
+            flags &= ~SandboxOrigin;
+        else if (equalIgnoringCase(sandboxToken, "allow-forms"))
+            flags &= ~SandboxForms;
+        else if (equalIgnoringCase(sandboxToken, "allow-scripts"))
+            flags &= ~SandboxScripts;
+        else if (equalIgnoringCase(sandboxToken, "allow-top-navigation"))
+            flags &= ~SandboxTopNavigation;
+        else if (equalIgnoringCase(sandboxToken, "allow-popups"))
+            flags &= ~SandboxPopups;
+
+        start = end + 1;
+    }
+
+    return flags;
+}
+
 void SecurityOrigin::setLocalLoadPolicy(LocalLoadPolicy policy)
 {
     localLoadPolicy = policy;

Modified: trunk/Source/WebCore/page/SecurityOrigin.h (99227 => 99228)


--- trunk/Source/WebCore/page/SecurityOrigin.h	2011-11-03 19:25:25 UTC (rev 99227)
+++ trunk/Source/WebCore/page/SecurityOrigin.h	2011-11-03 19:29:20 UTC (rev 99228)
@@ -56,6 +56,7 @@
     void setDomainFromDOM(const String& newDomain);
     bool domainWasSetInDOM() const { return m_domainWasSetInDOM; }
 
+    // FIXME: This should move to SchemeRegistry.
     static void setDomainRelaxationForbiddenForURLScheme(bool forbidden, const String&);
     static bool isDomainRelaxationForbiddenForURLScheme(const String&);
 
@@ -114,6 +115,7 @@
     void grantUniversalAccess();
 
     bool isSandboxed(SandboxFlags mask) const { return m_sandboxFlags & mask; }
+    SandboxFlags sandboxFlags() const { return m_sandboxFlags; }
 
     bool canAccessDatabase() const { return !isUnique(); }
     bool canAccessLocalStorage() const { return !isUnique(); }
@@ -178,6 +180,8 @@
     // (and whether it was set) but considering the host. It is used for postMessage.
     bool isSameSchemeHostPort(const SecurityOrigin*) const;
 
+    static SandboxFlags parseSandboxPolicy(const String& policy);
+
     static bool shouldHideReferrer(const KURL&, const String& referrer);
 
     enum LocalLoadPolicy {

Modified: trunk/Source/WebCore/svg/graphics/SVGImage.cpp (99227 => 99228)


--- trunk/Source/WebCore/svg/graphics/SVGImage.cpp	2011-11-03 19:25:25 UTC (rev 99227)
+++ trunk/Source/WebCore/svg/graphics/SVGImage.cpp	2011-11-03 19:29:20 UTC (rev 99228)
@@ -313,7 +313,7 @@
         frame->setView(FrameView::create(frame.get()));
         frame->init();
         FrameLoader* loader = frame->loader();
-        loader->setForcedSandboxFlags(SandboxAll);
+        loader->forceSandboxFlags(SandboxAll);
 
         frame->view()->setCanHaveScrollbars(false); // SVG Images will always synthesize a viewBox, if it's not available, and thus never see scrollbars.
         frame->view()->setTransparent(true); // SVG Images are transparent.
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to