Prevent cross-site pages when --site-per-process is passed

content/browser/site_per_process_browsertest.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/command_line.h"
+#include "base/stringprintf.h"
+#include "base/utf_string_conversions.h"
+#include "content/browser/web_contents/web_contents_impl.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/common/content_switches.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_utils.h"
+#include "content/shell/shell.h"
+#include "content/test/content_browser_test.h"
+#include "content/test/content_browser_test_utils.h"
+
+namespace content {
+
+class SitePerProcessWebContentsObserver: public WebContentsObserver {
+ public:
+  explicit SitePerProcessWebContentsObserver(WebContents* web_contents)
+      : WebContentsObserver(web_contents),
+        navigation_succeeded_(true) {}
+  virtual ~SitePerProcessWebContentsObserver() {}
+
+  virtual void DidFailProvisionalLoad(
+      int64 frame_id,
+      bool is_main_frame,
+      const GURL& validated_url,
+      int error_code,
+      const string16& error_description,
+      RenderViewHost* render_view_host) OVERRIDE {
+    navigation_url_ = validated_url;
+    navigation_succeeded_ = false;
+  }
+
+  virtual void DidCommitProvisionalLoadForFrame(
+      int64 frame_id,
+      bool is_main_frame,
+      const GURL& url,
+      PageTransition transition_type,
+      RenderViewHost* render_view_host) OVERRIDE{
+    navigation_url_ = url;
+    navigation_succeeded_ = true;
+  }
+
+  GURL navigation_url() const {

[nasko, 2012/12/06 17:20:15]

Why not return const GURL&? Do we need to make a copy?

[irobert, 2012/12/06 19:10:40]

Done.

+    return navigation_url_;
+  }
+
+  int navigation_result() const { return navigation_succeeded_; }

[Charlie Reis, 2012/12/06 01:42:45]

nit: navigation_succeeded()

[irobert, 2012/12/06 19:10:40]

Done.

+
+ private:
+  GURL navigation_url_;
+  bool navigation_succeeded_;
+
+  DISALLOW_COPY_AND_ASSIGN(SitePerProcessWebContentsObserver);
+};
+
+class SitePerProcessBrowserTest : public ContentBrowserTest {
+ public:
+  SitePerProcessBrowserTest() {}
+
+  bool NavigateIframeToURL(Shell* window,
+                           const GURL& url,
+                           std::string iframe_id) {
+    std::string script = base::StringPrintf(
+        "var iframes = document.getElementById('%s');iframes.src='%s';",
+        iframe_id.c_str(), url.spec().c_str());
+    WindowedNotificationObserver load_observer(
+        NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(
+            &shell()->web_contents()->GetController()));
+     bool result = content::ExecuteJavaScript(
+         window->web_contents()->GetRenderViewHost(),
+         L"", ASCIIToWide(script));
+     load_observer.Wait();
+     return result;
+  }
+
+  void SetUpCommandLine(CommandLine* command_line) {
+    command_line->AppendSwitch(switches::kSitePerProcess);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
+  ASSERT_TRUE(test_server()->Start());
+  net::TestServer https_server(
+      net::TestServer::TYPE_HTTPS,
+      net::TestServer::kLocalhost,
+      FilePath(FILE_PATH_LITERAL("content/test/data")));
+  ASSERT_TRUE(https_server.Start());
+  GURL main_url(test_server()->GetURL("files/site_per_process_main.html"));
+
+  NavigateToURL(shell(), main_url);
+
+  SitePerProcessWebContentsObserver observer(shell()->web_contents());
+  {
+    // Load same-site page into Iframe.
+    GURL http_url(test_server()->GetURL("files/title1.html"));
+    EXPECT_TRUE(NavigateIframeToURL(shell(), http_url, "test"));
+    EXPECT_EQ(observer.navigation_url(), http_url);
+    EXPECT_TRUE(observer.navigation_result());
+  }
+
+  {
+    // Load cross-site page into Iframe.
+    GURL https_url(https_server.GetURL("files/title1.html"));
+    EXPECT_TRUE(NavigateIframeToURL(shell(), https_url, "test"));
+    EXPECT_EQ(observer.navigation_url(), https_url);
+    EXPECT_FALSE(observer.navigation_result());
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframeRedirectOnce) {
+  ASSERT_TRUE(test_server()->Start());
+  net::TestServer https_server(
+      net::TestServer::TYPE_HTTPS,
+      net::TestServer::kLocalhost,
+      FilePath(FILE_PATH_LITERAL("content/test/data")));
+  ASSERT_TRUE(https_server.Start());
+
+  GURL main_url(test_server()->GetURL("files/site_per_process_main.html"));
+  GURL http_url(test_server()->GetURL("files/title1.html"));
+  GURL https_url(https_server.GetURL("files/title1.html"));
+
+  NavigateToURL(shell(), main_url);
+
+  SitePerProcessWebContentsObserver observer(shell()->web_contents());
+  {
+    // Load cross-site client-redirect page into Iframe.
+    // Should be blocked.
+    GURL client_redirect_https_url(https_server.GetURL(
+        "client-redirect?files/title1.html"));
+    EXPECT_TRUE(NavigateIframeToURL(shell(),

[nasko, 2012/12/06 17:20:15]

This function always takes "test" as the last parameter, why not eliminate it?

[irobert, 2012/12/06 19:10:40]

When I designed the NavigateIframeToURL API, I want it to be generic since I
believe the future tests may have multiple iframes in the page. Therefore, we
should allow the test to choose which iframe should be navigated.

On 2012/12/06 17:20:15, nasko wrote:

+                                    client_redirect_https_url, "test"));
+    // DidFailProvisionalLoad when navigating to client_redirect_https_url.
+    EXPECT_EQ(observer.navigation_url(), client_redirect_https_url);
+    EXPECT_FALSE(observer.navigation_result());
+  }
+
+  {
+    // Load cross-site server-redirect page into Iframe,
+    // which redirects to same-site page.
+    GURL server_redirect_http_url(https_server.GetURL(
+        "server-redirect?" + http_url.spec()));
+    EXPECT_TRUE(NavigateIframeToURL(shell(),
+                                    server_redirect_http_url, "test"));
+    EXPECT_EQ(observer.navigation_url(), http_url);
+    EXPECT_TRUE(observer.navigation_result());
+  }
+
+  {
+    // Load cross-site server-redirect page into Iframe,
+    // which redirects to cross-site page.
+    GURL server_redirect_http_url(https_server.GetURL(
+        "server-redirect?files/title1.html"));
+    EXPECT_TRUE(NavigateIframeToURL(shell(),
+                                    server_redirect_http_url, "test"));
+    // DidFailProvisionalLoad when navigating to https_url.
+    EXPECT_EQ(observer.navigation_url(), https_url);
+    EXPECT_FALSE(observer.navigation_result());
+  }
+
+  {
+    // Load same-site server-redirect page into Iframe.

[nasko, 2012/12/06 17:20:15]

nit: , instead of . at the end of the line.

[irobert, 2012/12/06 19:10:40]

Done.

+    // which redirects to same-site page.
+    GURL server_redirect_http_url(test_server()->GetURL(
+        "server-redirect?files/title1.html"));
+    EXPECT_TRUE(NavigateIframeToURL(shell(),
+                                    server_redirect_http_url, "test"));
+    EXPECT_EQ(observer.navigation_url(), http_url);
+    EXPECT_TRUE(observer.navigation_result());
+   }
+
+  {
+    // Load same-site client-redirect page into Iframe,
+    // which redirects to same-site page.
+    GURL client_redirect_http_url(test_server()->GetURL(
+        "client-redirect?files/title1.html"));
+    EXPECT_TRUE(NavigateIframeToURL(shell(),
+                                    client_redirect_http_url, "test"));
+    EXPECT_EQ(observer.navigation_url(), client_redirect_http_url);
+    EXPECT_TRUE(observer.navigation_result());
+  }

[Charlie Reis, 2012/12/06 01:42:45]

Might as well toss in the other two cases:
same-site server redirect to cross-site page
same-site client redirect to cross-site page

[irobert, 2012/12/06 19:10:40]

Done.

[irobert, 2012/12/06 19:10:40]

Done.

+}
+
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       CrossSiteIframeRedirectTwice) {
+  ASSERT_TRUE(test_server()->Start());
+  net::TestServer https_server(
+      net::TestServer::TYPE_HTTPS,
+      net::TestServer::kLocalhost,
+      FilePath(FILE_PATH_LITERAL("content/test/data")));
+  ASSERT_TRUE(https_server.Start());
+
+  GURL main_url(test_server()->GetURL("files/site_per_process_main.html"));
+  GURL http_url(test_server()->GetURL("files/title1.html"));
+  GURL https_url(https_server.GetURL("files/title1.html"));
+
+  NavigateToURL(shell(), main_url);
+
+  SitePerProcessWebContentsObserver observer(shell()->web_contents());
+  {
+    // Load client-redirect page pointed to a cross-site client-redirect page,

[nasko, 2012/12/06 17:20:15]

nit: s/pointed/pointing/?

[irobert, 2012/12/06 19:10:40]

Done.

+    // which eventually redirects back to same-site page.
+    GURL client_redirect_https_url(https_server.GetURL(
+        "client-redirect?" + http_url.spec()));
+    GURL client_redirect_http_url(test_server()->GetURL(
+        "client-redirect?" + client_redirect_https_url.spec()));
+    EXPECT_TRUE(NavigateIframeToURL(shell(), client_redirect_http_url, "test"));
+
+    // We should check until second client redirect get cancelled.

[Charlie Reis, 2012/12/06 01:42:45]

nit: s/check/wait/

[irobert, 2012/12/06 19:10:40]

Done.

+    WindowedNotificationObserver load_observer2(
+        NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(
+            &shell()->web_contents()->GetController()));

[Charlie Reis, 2012/12/06 01:42:45]

The NavigateIFrameToURL line should be between the load_observer2 declaration
and the load_observer2.Wait() line.

[irobert, 2012/12/06 19:10:40]

I think NavigateIFrameToURL line cannot be between the load_observer2
declaration. This is because load_observer2 is aim at capturing the second load
commit/fail event. If put between the load_observer2 declaration, load_observer2
will get the same event as load_observer.

On 2012/12/06 01:42:45, creis wrote:
Done.

[Charlie Reis, 2012/12/06 20:20:22]

Ah.  This approach won't work either, though.  It will be flaky, because the
second LOAD_STOP could occur before we create load_observer2, and then the
Wait() call will wait forever.

Is there a way to wait for something else, like the DidFailProvisionalLoad?  I'm
not sure.

[irobert, 2012/12/06 22:37:02]

I wrote a new NotificationObeserver which wait until the second given signal
happened.

On 2012/12/06 20:20:22, creis wrote:

+    load_observer2.Wait();
+
+    // DidFailProvisionalLoad when navigating to client_redirect_https_url.
+    EXPECT_EQ(observer.navigation_url(), client_redirect_https_url);
+    EXPECT_FALSE(observer.navigation_result());
+  }
+
+  {
+    // Load server-redirect page pointed to a cross-site server-redirect page,
+    // which eventually redirect back to same-site page.
+    GURL server_redirect_https_url(https_server.GetURL(
+        "server-redirect?" + http_url.spec()));
+    GURL server_redirect_http_url(test_server()->GetURL(
+        "server-redirect?" + server_redirect_https_url.spec()));
+    EXPECT_TRUE(NavigateIframeToURL(shell(),
+                                    server_redirect_http_url, "test"));
+    EXPECT_EQ(observer.navigation_url(), http_url);
+    EXPECT_TRUE(observer.navigation_result());
+  }
+
+  {
+    // Load server-redirect page pointed to a cross-site server-redirect page,
+    // which eventually redirects back to cross-site page.
+    GURL server_redirect_https_url(https_server.GetURL(
+        "server-redirect?" + https_url.spec()));
+    GURL server_redirect_http_url(test_server()->GetURL(
+        "server-redirect?" + server_redirect_https_url.spec()));
+    EXPECT_TRUE(NavigateIframeToURL(shell(), server_redirect_http_url, "test"));
+    // DidFailProvisionalLoad when navigating to https_url.
+    EXPECT_EQ(observer.navigation_url(), https_url);
+    EXPECT_FALSE(observer.navigation_result());
+  }
+
+  {
+    // Load server-redirect page pointed to a cross-site server-redirect page,

[Charlie Reis, 2012/12/06 01:42:45]

It's pointed to a cross-site client-redirect page, right?

[irobert, 2012/12/06 19:10:40]

Done.

+    // which eventually redirects back to same-site page.
+    GURL client_redirect_http_url(https_server.GetURL(
+        "client-redirect?" + http_url.spec()));
+    GURL server_redirect_http_url(test_server()->GetURL(
+        "server-redirect?" + client_redirect_http_url.spec()));
+    EXPECT_TRUE(NavigateIframeToURL(shell(), server_redirect_http_url, "test"));
+    // DidFailProvisionalLoad when navigating to client_redirect_http_url.
+    EXPECT_EQ(observer.navigation_url(), client_redirect_http_url);
+    EXPECT_FALSE(observer.navigation_result());
+  }
+}
+
+}