Commit 42956a5b authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Need to propagate initiator origin to fresh RenderFrameHosts.

This CL fixes a problem where the last committed origin of
RenderFrameHost was never set after document.write cancels an initial
navigation in a subframe or in a popup.

In addition to running the new regression tests, I also manually tested
that the repro steps from https://crbug.com/932067 no longer hit the bug
- in particular, the console output included both "onupgradeneeded" and
"onsuccess".

Bug: 932067
Change-Id: If1f031b988005e498a3515abe3137680db35ea0e
Reviewed-on: https://chromium-review.googlesource.com/c/1480670Reviewed-by: default avatarŁukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635181}
parent a07580d3
......@@ -1953,6 +1953,24 @@ void RenderFrameHostImpl::SetLastCommittedOriginForTesting(
SetLastCommittedOrigin(origin);
}
void RenderFrameHostImpl::SetOriginOfNewFrame(
const url::Origin& new_frame_creator) {
// This method should only be called for *new* frames, that haven't committed
// a navigation yet.
DCHECK(!has_committed_any_navigation_);
DCHECK(GetLastCommittedOrigin().opaque());
// Calculate and set |new_frame_origin|.
bool new_frame_should_be_sandboxed =
blink::WebSandboxFlags::kOrigin ==
(frame_tree_node()->active_sandbox_flags() &
blink::WebSandboxFlags::kOrigin);
url::Origin new_frame_origin = new_frame_should_be_sandboxed
? new_frame_creator.DeriveNewOpaqueOrigin()
: new_frame_creator;
SetLastCommittedOrigin(new_frame_origin);
}
FrameTreeNode* RenderFrameHostImpl::AddChild(
std::unique_ptr<FrameTreeNode> child,
int process_id,
......@@ -1973,8 +1991,12 @@ FrameTreeNode* RenderFrameHostImpl::AddChild(
// in a frame tree should have the same set of proxies.
frame_tree_node_->render_manager()->CreateProxiesForChildFrame(child.get());
children_.push_back(std::move(child));
// When the child is added, it hasn't committed any navigation yet - its
// initial empty document should inherit the origin of its parent (the origin
// may change after the first commit). See also https://crbug.com/932067.
child->current_frame_host()->SetOriginOfNewFrame(GetLastCommittedOrigin());
children_.push_back(std::move(child));
return children_.back().get();
}
......@@ -3719,6 +3741,16 @@ void RenderFrameHostImpl::CreateNewWindow(
RenderFrameHostImpl::FromID(GetProcess()->GetID(), main_frame_route_id);
DCHECK(rfh);
// When the popup is created, it hasn't committed any navigation yet - its
// initial empty document should inherit the origin of its opener (the origin
// may change after the first commit). See also https://crbug.com/932067.
//
// Note that that origin of the new frame might depend on sandbox flags.
// Checking sandbox flags of the new frame should be safe at this point,
// because the flags should be already inherited by the CreateNewWindow call
// above.
rfh->SetOriginOfNewFrame(GetLastCommittedOrigin());
if (base::FeatureList::IsEnabled(network::features::kNetworkService) &&
rfh->waiting_for_init_) {
// Need to check |waiting_for_init_| as some paths inside CreateNewWindow
......
......@@ -1377,6 +1377,11 @@ class CONTENT_EXPORT RenderFrameHostImpl
// Update this frame's last committed origin.
void SetLastCommittedOrigin(const url::Origin& origin);
// Set the |last_committed_origin_| of |this| frame, inheriting the origin
// from |new_frame_creator| as appropriate (e.g. depending on whether |this|
// frame should be sandboxed / should have an opaque origin instead).
void SetOriginOfNewFrame(const url::Origin& new_frame_creator);
// Called when a navigation commits succesfully to |url|. This will update
// |last_committed_site_url_| with the site URL corresponding to |url|.
// Note that this will recompute the site URL from |url| rather than using
......
......@@ -2054,4 +2054,226 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest, VisibilityChildInView) {
nested_iframe_node->current_frame_host()->visibility());
}
IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
OriginOfFreshFrame_Subframe_NavCancelledByDocWrite) {
WebContents* web_contents = shell()->web_contents();
NavigationController& controller = web_contents->GetController();
GURL main_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
ASSERT_TRUE(NavigateToURL(shell(), main_url));
EXPECT_EQ(1, controller.GetEntryCount());
url::Origin main_origin = url::Origin::Create(main_url);
// document.open should cancel the cross-origin navigation to '/hung' and the
// subframe should remain on the parent/initiator origin.
const char kScriptTemplate[] = R"(
const frame = document.createElement('iframe');
frame.src = $1;
document.body.appendChild(frame);
const html = '<!DOCTYPE html><html><body>Hello world!</body></html>';
const doc = frame.contentDocument;
doc.open();
doc.write(html);
doc.close();
frame.contentWindow.origin;
)";
GURL cross_site_url(embedded_test_server()->GetURL("bar.com", "/hung"));
std::string script = JsReplace(kScriptTemplate, cross_site_url);
EXPECT_EQ(main_origin.Serialize(), EvalJs(web_contents, script));
// The subframe navigation should be cancelled and therefore shouldn't
// contribute an extra history entry.
EXPECT_EQ(1, controller.GetEntryCount());
// Browser-side origin should match the renderer-side origin.
// See also https://crbug.com/932067.
ASSERT_EQ(2u, web_contents->GetAllFrames().size());
RenderFrameHost* subframe = web_contents->GetAllFrames()[1];
EXPECT_EQ(main_origin, subframe->GetLastCommittedOrigin());
}
class RenderFrameHostCreatedObserver : public WebContentsObserver {
public:
explicit RenderFrameHostCreatedObserver(WebContents* web_contents)
: WebContentsObserver(web_contents) {}
RenderFrameHost* Wait() {
if (!new_frame_)
run_loop_.Run();
return new_frame_;
}
private:
void RenderFrameCreated(RenderFrameHost* render_frame_host) override {
new_frame_ = render_frame_host;
run_loop_.Quit();
}
base::RunLoop run_loop_;
RenderFrameHost* new_frame_ = nullptr;
DISALLOW_COPY_AND_ASSIGN(RenderFrameHostCreatedObserver);
};
IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
OriginOfFreshFrame_SandboxedSubframe) {
WebContents* web_contents = shell()->web_contents();
NavigationController& controller = web_contents->GetController();
GURL main_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
ASSERT_TRUE(NavigateToURL(shell(), main_url));
EXPECT_EQ(1, controller.GetEntryCount());
url::Origin main_origin = url::Origin::Create(main_url);
// Navigate a sandboxed frame to a cross-origin '/hung'.
RenderFrameHostCreatedObserver subframe_observer(web_contents);
const char kScriptTemplate[] = R"(
const frame = document.createElement('iframe');
frame.sandbox = 'allow-scripts';
frame.src = $1;
document.body.appendChild(frame);
)";
GURL cross_site_url(embedded_test_server()->GetURL("bar.com", "/hung"));
std::string script = JsReplace(kScriptTemplate, cross_site_url);
EXPECT_TRUE(ExecJs(web_contents, script));
// Wait for a new subframe, but ignore the frame returned by
// |subframe_observer| (it might be the speculative one, not the current one).
subframe_observer.Wait();
ASSERT_EQ(2u, web_contents->GetAllFrames().size());
RenderFrameHost* subframe = web_contents->GetAllFrames()[1];
// The browser-side origin of the *sandboxed* subframe should be set to an
// *opaque* origin (with the parent's origin as the precursor origin).
EXPECT_TRUE(subframe->GetLastCommittedOrigin().opaque());
EXPECT_EQ(
main_origin.GetTupleOrPrecursorTupleIfOpaque(),
subframe->GetLastCommittedOrigin().GetTupleOrPrecursorTupleIfOpaque());
// Note that the test cannot check the renderer-side origin of the frame:
// - Scripts cannot be executed before the frame commits,
// - The parent cannot document.write into the *sandboxed* frame.
}
IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
OriginOfFreshFrame_Subframe_AboutBlankAndThenDocWrite) {
WebContents* web_contents = shell()->web_contents();
NavigationController& controller = web_contents->GetController();
GURL main_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
ASSERT_TRUE(NavigateToURL(shell(), main_url));
EXPECT_EQ(1, controller.GetEntryCount());
url::Origin main_origin = url::Origin::Create(main_url);
// Create a new about:blank subframe and document.write into it.
TestNavigationObserver load_observer(web_contents);
RenderFrameHostCreatedObserver subframe_observer(web_contents);
const char kScript[] = R"(
const frame = document.createElement('iframe');
// Don't set |frame.src| - have the frame commit an initial about:blank.
document.body.appendChild(frame);
const html = '<!DOCTYPE html><html><body>Hello world!</body></html>';
const doc = frame.contentDocument;
doc.open();
doc.write(html);
doc.close();
)";
ExecuteScriptAsync(web_contents, kScript);
// Wait for the new subframe to be created - this will be still before the
// commit of about:blank.
RenderFrameHost* subframe = subframe_observer.Wait();
EXPECT_EQ(main_origin, subframe->GetLastCommittedOrigin());
// Wait for the about:blank navigation to finish.
load_observer.Wait();
// The subframe commit to about:blank should not contribute an extra history
// entry.
EXPECT_EQ(1, controller.GetEntryCount());
// Browser-side origin should match the renderer-side origin.
// See also https://crbug.com/932067.
ASSERT_EQ(2u, web_contents->GetAllFrames().size());
RenderFrameHost* subframe2 = web_contents->GetAllFrames()[1];
EXPECT_EQ(subframe, subframe2); // No swaps are expected.
EXPECT_EQ(main_origin, subframe2->GetLastCommittedOrigin());
EXPECT_EQ(main_origin.Serialize(), EvalJs(subframe2, "window.origin"));
}
IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
OriginOfFreshFrame_Popup_NavCancelledByDocWrite) {
WebContents* web_contents = shell()->web_contents();
GURL main_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
ASSERT_TRUE(NavigateToURL(shell(), main_url));
url::Origin main_origin = url::Origin::Create(main_url);
// document.open should cancel the cross-origin navigation to '/hung' and the
// popup should remain on the initiator origin.
WebContentsAddedObserver popup_observer;
const char kScriptTemplate[] = R"(
var popup = window.open($1, 'popup');
const html = '<!DOCTYPE html><html><body>Hello world!</body></html>';
const doc = popup.document;
doc.open();
doc.write(html);
doc.close();
popup.origin;
)";
GURL cross_site_url(embedded_test_server()->GetURL("bar.com", "/hung"));
std::string script = JsReplace(kScriptTemplate, cross_site_url);
EXPECT_EQ(main_origin.Serialize(), EvalJs(web_contents, script));
// Browser-side origin should match the renderer-side origin.
// See also https://crbug.com/932067.
WebContents* popup = popup_observer.GetWebContents();
EXPECT_EQ(main_origin, popup->GetMainFrame()->GetLastCommittedOrigin());
// The popup navigation should be cancelled and therefore shouldn't
// contribute an extra history entry.
EXPECT_EQ(0, popup->GetController().GetEntryCount());
}
IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
OriginOfFreshFrame_Popup_AboutBlankAndThenDocWrite) {
WebContents* web_contents = shell()->web_contents();
GURL main_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
ASSERT_TRUE(NavigateToURL(shell(), main_url));
url::Origin main_origin = url::Origin::Create(main_url);
// Create a new about:blank popup and document.write into it.
WebContentsAddedObserver popup_observer;
TestNavigationObserver load_observer(web_contents);
const char kScript[] = R"(
// Empty |url| argument means that the popup will commit an initial
// about:blank.
var popup = window.open('', 'popup');
const html = '<!DOCTYPE html><html><body>Hello world!</body></html>';
const doc = popup.document;
doc.open();
doc.write(html);
doc.close();
)";
ExecuteScriptAsync(web_contents, kScript);
// Wait for the new popup to be created (this will be before the popup commits
// the initial about:blank page).
WebContents* popup = popup_observer.GetWebContents();
EXPECT_EQ(main_origin, popup->GetMainFrame()->GetLastCommittedOrigin());
// A round-trip to the renderer process is an indirect way to wait for
// DidCommitProvisionalLoad IPC for the initial about:blank page.
// WaitForLoadStop cannot be used, because this commit won't raise
// NOTIFICATION_LOAD_STOP.
EXPECT_EQ(123, EvalJs(popup, "123"));
EXPECT_EQ(main_origin, popup->GetMainFrame()->GetLastCommittedOrigin());
// The about:blank navigation shouldn't contribute an extra history entry.
EXPECT_EQ(0, popup->GetController().GetEntryCount());
}
} // namespace content
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment